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

From: Lee Jones <lee.jones@linaro.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: DTML <devicetree@vger.kernel.org>,
	gregkh <gregkh@linuxfoundation.org>,
	Mark Brown <broonie@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	michael@walle.cc, Rob Herring <robh+dt@kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 1/1] mfd: Add I2C based System Configuaration (SYSCON) access
Date: Thu, 18 Jun 2020 11:07:04 +0100	[thread overview]
Message-ID: <20200618100704.GC954398@dell> (raw)
In-Reply-To: <CAK8P3a3iRmXC2jDj92QHKqyD+x_UJ7rWU_KcGt=MFOD9UW38RA@mail.gmail.com>

On Thu, 18 Jun 2020, Arnd Bergmann wrote:

> On Thu, Jun 18, 2020 at 10:03 AM Lee Jones <lee.jones@linaro.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@linaro.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@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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel