linux-kernel - Re: [PATCH v3 1/1] mfd: Add I2C based System Configuaration (SYSCON) access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 18 Jun 2020 11:07:04 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     michael@...le.cc, Rob Herring <robh+dt@...nel.org>,
        Mark Brown <broonie@...nel.org>,
        gregkh <gregkh@...uxfoundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        DTML <devicetree@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/1] mfd: Add I2C based System Configuaration (SYSCON)
 access

On Thu, 18 Jun 2020, Arnd Bergmann wrote:

> On Thu, Jun 18, 2020 at 10:03 AM Lee Jones <lee.jones@...aro.org> 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@...aro.org>
> 
> 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@...0000/motherboard/iofpga@7,00000000/i2c@...00/temp@32
[    0.523691] i2c i2c-0: Failed to create I2C device for /bus@...0000/motherboard/iofpga@7,00000000/i2c@...00/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