Hi Pratyush,
On Tue, 5 May 2020 at 13:47, Pratyush Yadav p.yadav@ti.com wrote:
Hi Simon,
I would be taking up this series and address the comments.
On 10/12/19 03:18PM, Simon Glass wrote:
Hi Jean-Jacques,
On Tue, 5 Nov 2019 at 04:47, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
Some linux drivers provide their own read/write functions to access data from/of the regmap. Adding support for it.
Signed-off-by: Jean-Jacques Hiblot jjhiblot@ti.com
Changes in v2:
- Only use custom accessors if {,SPL,TPL}_REGMAP_ACCESSORS is enabled
drivers/core/Kconfig | 25 +++++++++++++++++++++++++ drivers/core/regmap.c | 22 ++++++++++++++++++++-- include/regmap.h | 28 +++++++++++++++++++++++++--- 3 files changed, 70 insertions(+), 5 deletions(-)
Coming back to the discussion on driver model....
IIUC, you are suggesting that we create a uclass for regmap, and fill in the ops with the driver's custom read/write functions, correct? This would mean we will have to create udevice for each regmap. A driver can have multiple regmaps (up to 18 for example in the Linux Cadence Sierra PHY driver in kernel). Creating a device for each regmap is very inefficient. Is the overhead really worth it? Does it solve any significant problem? Also, a regmap is not a device, it is an abstraction layer to simplify register access. Does it then make sense to model it as a device?
I was actually referring to the access method of the regmap but I suppose the effect is the same. This question has been running for a while.
The issue is that this is getting more and more complicated. We use devices to model complicated things. It is an abstraction. It doesn't have to be a physical device.
With regmap we are creating an ad-hoc structure and associated infrastructure to deal with this one case. It is not possible to see the regmaps with 'dm dump', for example.
How do you specify the fields? I would expect that this would be done in the driver tree? Perhaps in a subnode of the device?
Just to state what I see as the advantages of using a separate device for access:
- Remove the #ifdef in the regmap struct
If you really want to remove the #ifdefs, we can set reg_read to a default, and let drivers override if when creating a regmap. Then in regmap_read, just call reg_read. This gets rid of the #ifdefs with a small change. I suspect this will have a much smaller impact on memory usage than using a udevice.
Yes that sounds good, but you are getting closer and closer to it being a device.
- Easy to specify the behaviour in a device-tree node
Quoting from the kernel's documentation of regmap_config:
reg_read: Optional callback that if filled will be used to perform all the reads from the registers. Should only be provided for devices whose read operation cannot be represented as a simple read operation on a bus such as SPI, I2C, etc. Most of the devices do not need this. reg_write: Same as above for writing.
These custom accessors are meant to be used when the read or write needs more work to be done than the standard regmap reads/writes. And so they are supposed to be driver-specific functions. I don't think it is at all possible to specify something like this in devicetree. Am I missing something?
Can you point to an example of this extra read/write code?
The kernel is a rabbit-warren of custom interfaces. I am trying to keep driver model uniform, so long as the cost is reasonable. I'm not sure that it is worth having a regmap device for simple things, but as things get more complex, I think we should look at it.
Of course you can specify this in the device tree: just use a compatible string to select the appropriate regmap driver.
spi { compatible = "vendor,myspi"; regmaps = <®map_simple 4 ®map_mailbox 0 FLAGS>; }
regmap_mailbox: regmap-mailbox { compatible = "regmap,mailbox"; other props }
struct regmap *regmap = regmap_get_by_index(dev, 1)
regmap_read(regmap, ...)
- Easy to extend as the child device can do what it likes with respect to access
Disadvantage is that it takes a bit more space.
Yes space is a concern. A device takes about 84 bytes I think. But as we add more and more complexity we might as well take the hit.
Regards, Simon