On Thu, 18 Jun 2020, Arnd Bergmann wrote: > On Thu, Jun 18, 2020 at 10:03 AM Lee Jones <lee.jones@xxxxxxxxxx> wrote: > > > > The existing SYSCON implementation only supports MMIO (memory mapped) > > accesses, facilitated by Regmap. This extends support for registers > > held behind I2C busses. > > > > Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx> > > The implementation looks fine to me, but can you explain how this is going to > be used, and what the advantage is over open-coding the devm_regmap_init_i2c() > in each driver that would use this? Does Regmap let you register/initialise an I2C address more than once? When I attempt it, I get: [ 0.522988] i2c i2c-0: Failed to register i2c client tmp105 at 0x32 (-16) [ 0.523341] i2c i2c-0: of_i2c: Failure registering /bus@4000000/motherboard/iofpga@7,00000000/i2c@16000/temp@32 [ 0.523691] i2c i2c-0: Failed to create I2C device for /bus@4000000/motherboard/iofpga@7,00000000/i2c@16000/temp@32 > Is this about using proper locking through the regmap framework for > shared i2c clients, or to reduce memory consumption when lots of drivers > access the same regmap? All of those things are valid. My use-case is regarding MFDs sharing an I2C interfaced address space with their children. > My impression of the existing syscon code is that the main value-add over > other ways of doing the same is the syscon_regmap_lookup_by_phandle() > interface that gives other drivers a much simpler way of getting the > regmap just based on the DT node. Are you planning to add something > like that here as well? An ideal driver interface might allow > syscon_regmap_lookup_by_phandle() to work for both mmio and i2c > based syscons, or additional ones as well, but implementing this would > be rather tricky when the i2c core is a loadable module. I expect the API would be expanded to cover other use-cases. This is a bare bones implementation which has been kept as atomic as possible for ease of review. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog