[PATCH] regmap: slimbus: add support to raw read/writes

[PATCH] regmap: slimbus: add support to raw read/writes

[Srinivas Kandagatla]

SLIMbus supports upto 16 bytes in value management messages,
so add support to raw read/writes upto 16 bytes.

Also useful for paged register access on SLIMbus interfaced codecs.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/base/regmap/regmap-slimbus.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/base/regmap/regmap-slimbus.c b/drivers/base/regmap/regmap-slimbus.c
index 91d501eda8a9..fc07518666b2 100644
--- a/drivers/base/regmap/regmap-slimbus.c
+++ b/drivers/base/regmap/regmap-slimbus.c
@@ -31,11 +31,30 @@ static int regmap_slimbus_byte_reg_write(void *context, unsigned int reg,
 	return slim_writeb(sdev, reg, val);
 }
 
+static int regmap_slimbus_write(void *context, const void *data, size_t count)
+{
+	struct slim_device *sdev = context;
+
+	return slim_write(sdev, *(u16 *)data, count - 2, (u8 *)data + 2);
+}
+
+static int regmap_slimbus_read(void *context, const void *reg, size_t reg_size,
+			       void *val, size_t val_size)
+{
+	struct slim_device *sdev = context;
+
+	return slim_read(sdev, *(u16 *)reg, val_size, val);
+}
+
 static struct regmap_bus regmap_slimbus_bus = {
 	.reg_write = regmap_slimbus_byte_reg_write,
 	.reg_read = regmap_slimbus_byte_reg_read,
-	.reg_format_endian_default = REGMAP_ENDIAN_LITTLE,
-	.val_format_endian_default = REGMAP_ENDIAN_LITTLE,
+	.max_raw_write	= 16,
+	.max_raw_read	= 16,
+	.write = regmap_slimbus_write,
+	.read = regmap_slimbus_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
+	.val_format_endian_default = REGMAP_ENDIAN_NATIVE,
 };
 
 static const struct regmap_bus *regmap_get_slimbus(struct slim_device *slim,
-- 
2.16.2

Re: [PATCH] regmap: slimbus: add support to raw read/writes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

On Thu, Jul 05, 2018 at 12:37:00PM +0100, Srinivas Kandagatla wrote:

> SLIMbus supports upto 16 bytes in value management messages,
> so add support to raw read/writes upto 16 bytes.

That's not what raw access is.  It's unmediated access to existing
registers, not some other thing with completely different register sizes
which you can't see through the register map.  The intended use is for
things like fast firmware downloads.  If it's not part of the register
map we shouldn't be going through regmap to get to it.

> Also useful for paged register access on SLIMbus interfaced codecs.

That needs a bunch more explanation...  in what way does raw access
relate to paged register maps?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap: slimbus: add support to raw read/writes

[Srinivas Kandagatla]

On 05/07/18 16:08, Mark Brown wrote:
> On Thu, Jul 05, 2018 at 12:37:00PM +0100, Srinivas Kandagatla wrote:
> 
>> SLIMbus supports upto 16 bytes in value management messages,
>> so add support to raw read/writes upto 16 bytes.
> That's not what raw access is.  It's unmediated access to existing
> registers, not some other thing with completely different register sizes
> which you can't see through the register map.  The intended use is for
> things like fast firmware downloads.  If it's not part of the register
> map we shouldn't be going through regmap to get to it.

Thanks for explaining this in detail.

I think I got it wrong by looking at _regmap_bus_read() implementation 
which calls _regmap_raw_read(), this made me think that read/writes 
callbacks imply raw read/writes. Looks like that is not the case.

These bus read/write callbacks are just simple multiple register 
read/writes.

All am trying to do is support paged registers, which seems to be only 
possible via read/write support at bus level.

Does reg_read/reg_write become redundant if we implement read/write 
callbacks at bus regmap level?

> 
>> Also useful for paged register access on SLIMbus interfaced codecs.
> That needs a bunch more explanation...  in what way does raw access
> relate to paged register maps?
Yes, this is nothing to do with raw access! I got it wrong!
WCD9335 codec has paged registers which is what I need!

I will fix the patch and commit message removing the raw related parts 
and resend.

Does that sound okay?

thanks,
srini

Re: [PATCH] regmap: slimbus: add support to raw read/writes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

On Thu, Jul 05, 2018 at 05:13:21PM +0100, Srinivas Kandagatla wrote:

> I think I got it wrong by looking at _regmap_bus_read() implementation which
> calls _regmap_raw_read(), this made me think that read/writes callbacks
> imply raw read/writes. Looks like that is not the case.

They are reads and writes, the thing is that they're reads and writes of
the register map that's also accessible for other operations rather than
something separate.

> All am trying to do is support paged registers, which seems to be only
> possible via read/write support at bus level.

Could you explain what you mean by paged registers?  There's framework
support for what's normally called pages but perhaps this is some
slimbus specific thing?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap: slimbus: add support to raw read/writes

[Srinivas Kandagatla]

On 05/07/18 17:50, Mark Brown wrote:
> On Thu, Jul 05, 2018 at 05:13:21PM +0100, Srinivas Kandagatla wrote:
> 
> 
>> All am trying to do is support paged registers, which seems to be only
>> possible via read/write support at bus level.
> 
> Could you explain what you mean by paged registers?  There's framework
> support for what's normally called pages but perhaps this is some
> slimbus specific thing?

AFAIU, this is not specific to SLIMbus. The way registers are organized 
in WCD9335 codec is in 256 bytes of 6 Pages. Its possible that the 
reason to do this is because SLIMbus has max of 1024 bytes for "User 
Value elements" per device, according to Value Map from SLIMbus Specs.

Access to codec registers is done in 2 steps.

1> Write page number in page register.
2> access register within the page.

This seems to exactly what struct regmap_range_cfg gives.

Adding ranges to regmap_config along with read/write bus callbacks works 
for me.

thanks,
srini
> 

Re: [PATCH] regmap: slimbus: add support to raw read/writes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

On Thu, Jul 05, 2018 at 06:20:50PM +0100, Srinivas Kandagatla wrote:

> Access to codec registers is done in 2 steps.

> 1> Write page number in page register.
> 2> access register within the page.

> This seems to exactly what struct regmap_range_cfg gives.

> Adding ranges to regmap_config along with read/write bus callbacks works for
> me.

OK, great - that's what I was expecting.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regmap: slimbus: add support to raw read/writes

[Srinivas Kandagatla]

On 05/07/18 18:30, Mark Brown wrote:
> On Thu, Jul 05, 2018 at 06:20:50PM +0100, Srinivas Kandagatla wrote:
> 
>> Access to codec registers is done in 2 steps.
> 
>> 1> Write page number in page register.
>> 2> access register within the page.
> 
>> This seems to exactly what struct regmap_range_cfg gives.
> 
>> Adding ranges to regmap_config along with read/write bus callbacks works for
>> me.
> 
> OK, great - that's what I was expecting.
> 
Thanks, I will respin the patch removing reference to raw in commit log 
and patch.

Are reg_read/reg_write callbacks in bus regmap redundant to read/write 
callbacks?

By the looks of it, it seems redundant, reg_read/reg_write will never be 
used if bus regmap has read/write callbacks. Just wanted reconfirm 
before replacing them with just read/write callbacks.

thanks,
srini

Re: [PATCH] regmap: slimbus: add support to raw read/writes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

On Thu, Jul 05, 2018 at 06:39:14PM +0100, Srinivas Kandagatla wrote:

> Are reg_read/reg_write callbacks in bus regmap redundant to read/write
> callbacks?

> By the looks of it, it seems redundant, reg_read/reg_write will never be
> used if bus regmap has read/write callbacks. Just wanted reconfirm before
> replacing them with just read/write callbacks.

Yes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]