Re: [PATCH 1/5] mfd: add Congatec Board Controller mfd driver

[Lee Jones <lee@kernel.org>]

On Fri, 13 Sep 2024, Thomas Richard wrote:

> On 9/12/24 16:13, Lee Jones wrote:
> > On Tue, 10 Sep 2024, Thomas Richard wrote:
> > 
> >> On 8/22/24 12:38, Lee Jones wrote:
> >>> On Fri, 09 Aug 2024, Thomas Richard wrote:
> >>>
> >>>> Add core MFD driver for the Board Controller found on some Congatec SMARC
> >>>> module. This Board Controller provides functions like watchdog, GPIO, and
> >>>> I2C busses.
> >>>>
> >>>> This commit add support only for the conga-SA7 module.
> >>>>
> >>>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> >>>> ---
> >>>>  drivers/mfd/Kconfig      |  12 ++
> >>>>  drivers/mfd/Makefile     |   1 +
> >>>>  drivers/mfd/cgbc-core.c  | 453 +++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  include/linux/mfd/cgbc.h |  44 +++++
> >>>>  4 files changed, 510 insertions(+)
> >>>>
> >>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >>>> index bc8be2e593b6..3e0530f30267 100644
> >>>> --- a/drivers/mfd/Kconfig
> >>>> +++ b/drivers/mfd/Kconfig
> >>>> @@ -224,6 +224,18 @@ config MFD_AXP20X_RSB
> >>>>  	  components like regulators or the PEK (Power Enable Key) under the
> >>>>  	  corresponding menus.
> >>>>  
> >>>> +config MFD_CGBC
> >>>> +	tristate "Congatec Board Controller"
> >>>> +	select MFD_CORE
> >>>> +	depends on X86
> >>>> +	help
> >>>> +	  This is the core driver of the Board Controller found on some Congatec
> >>>> +	  SMARC modules. The Board Controller provides functions like watchdog,
> >>>> +	  I2C busses, and GPIO controller.
> >>>> +
> >>>> +	  To compile this driver as a module, choose M here: the module will be
> >>>> +	  called cgbc-core.
> >>>> +
> >>>>  config MFD_CROS_EC_DEV
> >>>>  	tristate "ChromeOS Embedded Controller multifunction device"
> >>>>  	select MFD_CORE
> >>>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> >>>> index 02b651cd7535..d5da3fcd691c 100644
> >>>> --- a/drivers/mfd/Makefile
> >>>> +++ b/drivers/mfd/Makefile
> >>>> @@ -13,6 +13,7 @@ obj-$(CONFIG_MFD_SM501)		+= sm501.o
> >>>>  obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835-pm.o
> >>>>  obj-$(CONFIG_MFD_BCM590XX)	+= bcm590xx.o
> >>>>  obj-$(CONFIG_MFD_BD9571MWV)	+= bd9571mwv.o
> >>>> +obj-$(CONFIG_MFD_CGBC)		+= cgbc-core.o
> >>>>  obj-$(CONFIG_MFD_CROS_EC_DEV)	+= cros_ec_dev.o
> >>>>  obj-$(CONFIG_MFD_CS42L43)	+= cs42l43.o
> >>>>  obj-$(CONFIG_MFD_CS42L43_I2C)	+= cs42l43-i2c.o
> >>>> diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
> >>>> new file mode 100644
> >>>> index 000000000000..cca9b1170cc9
> >>>> --- /dev/null
> >>>> +++ b/drivers/mfd/cgbc-core.c
> >>>> @@ -0,0 +1,453 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0-or-later
> >>>> +/*
> >>>> + * Congatec Board Controller MFD core driver.
> >>>
> >>> No such thing as an MFD.
> >>
> >> What should it be if it's not an MFD ?
> > 
> > You should be telling me this. :)
> > 
> > "Board Controller" according to the Kconfig entry.
> > 
> 
> This Congatec Board Controller is an external micro-controller that is
> interfaced with the CPU through a eSPI bus.
> This Board Controller provides multiple functions: an I2C controller, a
> GPIO controller, a watchdog and other not yet implemented functions
> (temp/voltage sensors, backlight).
> 
> Therefore, the MFD subsystem is a very good fit, as it allows to have a
> core driver that implements the communication with the external
> micro-controller, and then individual drivers for each of the functions
> offered by this Board Controller.

Sorry for the ambiguity, that was not the point of the review comment.

Please remove all mentions to MFD in the help/header text.

-- 
Lee Jones [李琼斯]