Message-ID: <20200728071949.GE1850026@dell>
Date:   Tue, 28 Jul 2020 08:19:49 +0100
From:   Lee Jones <lee.jones@...aro.org>
To:     Michael Walle <michael@...le.cc>
Cc:     linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
        linux-pwm@...r.kernel.org, linux-watchdog@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Rob Herring <robh+dt@...nel.org>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Shawn Guo <shawnguo@...nel.org>, Li Yang <leoyang.li@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Marc Zyngier <maz@...nel.org>, Mark Brown <broonie@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Pavel Machek <pavel@....cz>
Subject: Re: [PATCH v6 01/13] mfd: add simple regmap based I2C driver

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@...le.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.

>    driver will have a "select MFD_SIMPLE_MFD_I2C". Instead added
>    a small description to the driver itself.
>  - removed "struct simple_mfd_i2c_config" and use regmap_config
>    directly
>  - changed builtin_i2c_driver() to module_i2c_driver(), added
>    MODULE_ boilerplate
>  - cleaned up the included files
> 
> Changes since v4:
>  - new patch. Lee, please bear with me. I didn't want to delay the
>    new version (where a lot of remarks on the other patches were
>    addressed) even more, just because we haven't figured out how
>    to deal with the MFD part. So for now, I've included this one.
> 
>  drivers/mfd/Kconfig          |  5 ++++
>  drivers/mfd/Makefile         |  1 +
>  drivers/mfd/simple-mfd-i2c.c | 55 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
>  create mode 100644 drivers/mfd/simple-mfd-i2c.c
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 33df0837ab41..c08539c7a166 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1162,6 +1162,11 @@ config MFD_SI476X_CORE
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called si476x-core.
>  
> +config MFD_SIMPLE_MFD_I2C
> +	tristate
> +	depends on I2C
> +	select REGMAP_I2C

Please provide a full help.

>  config MFD_SM501
>  	tristate "Silicon Motion SM501"
>  	depends on HAS_DMA
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index a60e5f835283..78d24a3e7c9e 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -264,3 +264,4 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
>  obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
>  
>  obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
> +obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)	+= simple-mfd-i2c.o
> diff --git a/drivers/mfd/simple-mfd-i2c.c b/drivers/mfd/simple-mfd-i2c.c
> new file mode 100644
> index 000000000000..45090ddad104
> --- /dev/null
> +++ b/drivers/mfd/simple-mfd-i2c.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * A very simple I2C MFD driver

Simple MFD - I2C

> + * The driver enumerates its children and registers a common regmap. Use
> + * dev_get_regmap(pdev->dev.parent, NULL) in the child nodes to fetch that
> + * regmap instance.

This driver creates a single register map with the intention for it to
be shared by all sub-devices.  Children can use their parent's device
structure (dev.parent) in order reference it. 

> + * In the future this driver might be extended to support also other interfaces
> + * like SPI etc.

Remove this please.

> + */

'\n' here.

> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/regmap.h>
> +
> +static const struct regmap_config simple_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +};
> +
> +static int simple_mfd_i2c_probe(struct i2c_client *i2c)
> +{
> +	const struct regmap_config *config;
> +	struct regmap *regmap;
> +
> +	config = device_get_match_data(&i2c->dev);
> +	if (!config)
> +		config = &simple_regmap_config;
> +
> +	regmap = devm_regmap_init_i2c(i2c, config);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	return devm_of_platform_populate(&i2c->dev);
> +}
> +
> +static const struct of_device_id simple_mfd_i2c_of_match[] = {
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, simple_mfd_i2c_of_match);
> +
> +static struct i2c_driver simple_mfd_i2c_driver = {
> +	.probe_new = simple_mfd_i2c_probe,
> +	.driver = {
> +		.name = "simple-mfd-i2c",
> +		.of_match_table = simple_mfd_i2c_of_match,
> +	},
> +};
> +module_i2c_driver(simple_mfd_i2c_driver);
> +
> +MODULE_AUTHOR("Michael Walle <michael@...le.cc>");
> +MODULE_DESCRIPTION("Simple I2C MFD driver");

Simple MFD - I2C driver

> +MODULE_LICENSE("GPL v2");

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

Powered by blists - more mailing lists