Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver

From: Lee Jones <lee.jones@linaro.org>
To: Michael Walle <michael@walle.cc>
Cc: linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-pwm@vger.kernel.org, linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Jean Delvare" <jdelvare@suse.com>,
	"Guenter Roeck" <linux@roeck-us.net>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Wim Van Sebroeck" <wim@linux-watchdog.org>,
	"Shawn Guo" <shawnguo@kernel.org>, "Li Yang" <leoyang.li@nxp.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Marc Zyngier" <maz@kernel.org>,
	"Mark Brown" <broonie@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>, "Pavel Machek" <pavel@ucw.cz>
Subject: Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver
Date: Tue, 28 Jul 2020 09:35:18 +0100	[thread overview]
Message-ID: <20200728083518.GC2419169@dell> (raw)
In-Reply-To: <5de219e973e9d3c1455f1a09b4ce4177@walle.cc>

On Tue, 28 Jul 2020, Michael Walle wrote:

> Am 2020-07-28 10:15, schrieb Lee Jones:
> > On Tue, 28 Jul 2020, Michael Walle wrote:
> > 
> > > Am 2020-07-28 09:19, schrieb Lee Jones:
> > > > On Sun, 26 Jul 2020, Michael Walle wrote:
> > > >
> > > > > There are I2C devices which contain several different functions but
> > > > > doesn't require any special access functions. For these kind of
> > > > > drivers
> > > > > an I2C regmap should be enough.
> > > > >
> > > > > Create an I2C driver which creates an I2C regmap and enumerates its
> > > > > children. If a device wants to use this as its MFD core driver, it has
> > > > > to add an individual compatible string. It may provide its own regmap
> > > > > configuration.
> > > > >
> > > > > Subdevices can use dev_get_regmap() on the parent to get their regmap
> > > > > instance.
> > > > >
> > > > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > > > ---
> > > > > Changes since v5:
> > > > >  - removed "select MFD_CORE" in Kconfig
> > > > >  - removed help text in Kconfig, we assume that the users of this
> > > >
> > > > That's the opposite of what I asked for.
> > > 
> > > What is the use to describe the symbol, if it is not user selectable?
> > > I'm not aware this is done anywhere in the kernel, am I missing
> > > something?
> > 
> > You mean in menuconfig?
> > 
> > I find 'help's helpful even outside of menuconfig.
> > 
> > Surely I'm not the only one who reads them 'raw'?
> 
> Its already available in the header of the file. But sure, I can
> copy it.

Thanks.

[...]

> > > Why would you remove information about the intention of this driver?
> > > If
> > > someone
> > > looks for a place to implement its SPI/I3C/Slimbus MFD driver this
> > > might
> > > come
> > > in handy.
> > 
> > By all means put something similar in the commit log, but it has no
> > place in the driver itself.  Besides, if we were to add support for
> > SPI, it is likely to be a completely separate/unrelated driver.
> 
> Why would that be another driver? It would be 90% copy paste with
> regmap_init_i2c() replaced by regmap_init_spi() and i2c_driver replaced
> by spi_driver.

We'll investigate options if/when the time comes.  If 'spi_driver' and
'i2c_driver' can *sensibly* co-exist, then that is certainly an option
we can explore.

> But I don't care. I'll remove it.

Please.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog