| Message ID | 20180705113700.32465-1-srinivas.kandagatla@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | regmap: slimbus: add support to raw read/writes | expand |
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?
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
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?
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 >
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.
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
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.
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,
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(-) -- 2.16.2