[U-Boot] [PATCH] regmap: Separate memory-based operations

[Simon Glass]

Hi Alexey,

On 29 May 2018 at 13:16, Alexey Brodkin <Alexey.Brodkin at synopsys.com> wrote:
> Hi Simon,
>
> On Fri, 2018-05-25 at 20:07 -0600, Simon Glass wrote:
>> Hi Alexey,
>>
>> On 25 May 2018 at 03:45, Alexey Brodkin <Alexey.Brodkin at synopsys.com> wrote:
>> > Hi Simon,
>> >
>> > On Thu, 2018-05-24 at 20:42 -0600, Simon Glass wrote:
>> > > Hi Alexey,
>
> [snip]
>
>> > > When we want to use a particular I2C bus via regmap, we could bind the
>> > > I2C -> regmap driver as a child of the I2C bus driver.
>> >
>> > I guess that will require us to do corresponding changes in Device Tree
>> > descriptions, right? And that will deviate those .dts-es from their kernel
>> > counterparts [if they exist for a particular board].
>>
>> Actually I don't think so. You can have a helper function that you
>> call to bind the regmap to an I2C device, for example. That can be
>> called from code, without needing any DT changes.
>
> So this helper function will be regmap_init_i2c(), right?
>
>> My objection is mostly to using function pointers to call what look
>> like driver functions. The design of DM in U-Boot is supposed to
>> support child drivers for exactly this purpose. So we should try to
>> use it, unless we hit some terrible design flaw.
>
> Ok that makes sense. As you proposed we'll have new UCLASS like
> UCLASS_SIMPLE_REG and within that class we'll have drivers that access
> registers via memory, i2c, spi etc... that's quite clear.
>
> But what is not very obvious for me is that part about binding with
> "master" devices. Could you please sketch something so I may get a better
> picture?

Yes that sounds right. Basically this interface defines the operations
in your UCLASS_SIMPLE_REG uclass:

+       int (*reg_read)(struct regmap *map, uint reg, uint *val);
+       int (*reg_write)(struct regmap *map, uint reg, uint val);

Then you have different drivers that implement that uclass as you say
(i2c, spi).

The I2C simple_reg driver will require an I2C device to work with.
This can be its parent device. To bind the device you'll need to call
a function like:

int simple_reg_i2c_bind(struct udevice *i2c_dev, struct udevice **devp)

which calls device_bind(), or similar, to bind a new device as a child
of i2c_dev, with uclass UCLASS_SIMPLE_REG. Then the device can be
probed,

The I2C simple_reg driver can implement the reg_read() method by
calling dm_i2c_read() on its parent.

You can put simple_reg_i2c_bind() in the regmap code perhaps, or maybe
there is a better place for it.

You should make sure there is a sandbox test which creates a device
and checks that it can read and write a few registers.

Does that explain it?

Regards,
Simon