Add core driver for the ROHM BD79124 ADC / GPO.

The core driver launches the sub-drivers for the pinmux/GPO and for the
IIO ADC. It also provides the regmap, and forwards the IRQ resource to
the ADC.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
 drivers/mfd/Kconfig              |  12 +++
 drivers/mfd/Makefile             |   1 +
 drivers/mfd/rohm-bd79124.c       | 165 +++++++++++++++++++++++++++++++
 include/linux/mfd/rohm-bd79124.h |  32 ++++++
 4 files changed, 210 insertions(+)
 create mode 100644 drivers/mfd/rohm-bd79124.c
 create mode 100644 include/linux/mfd/rohm-bd79124.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index ae23b317a64e..f024256fb180 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2113,6 +2113,18 @@ config MFD_ROHM_BD71828
 	  also a single-cell linear charger, a Coulomb counter, a real-time
 	  clock (RTC), GPIOs and a 32.768 kHz clock gate.
 
+config MFD_ROHM_BD79124
+	tristate "Rohm BD79124 core driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say yes here to build support for the ROHM BD79124 ADC core. The
+	  ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports
+	  also an automatic measurement mode, with an alarm interrupt for
+	  out-of-window measurements. The window is configurable for each
+	  channel. The ADC inputs can optionally be used as general purpose
+	  outputs.
+
 config MFD_ROHM_BD957XMUF
 	tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs"
 	depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index e057d6d6faef..c7d64e933a7d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -270,6 +270,7 @@ obj-$(CONFIG_MFD_SC27XX_PMIC)	+= sprd-sc27xx-spi.o
 obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
 obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
 obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
+obj-$(CONFIG_MFD_ROHM_BD79124)	+= rohm-bd79124.o
 obj-$(CONFIG_MFD_ROHM_BD957XMUF)	+= rohm-bd9576.o
 obj-$(CONFIG_MFD_ROHM_BD96801)	+= rohm-bd96801.o
 obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
diff --git a/drivers/mfd/rohm-bd79124.c b/drivers/mfd/rohm-bd79124.c
new file mode 100644
index 000000000000..c35ab0e03b0b
--- /dev/null
+++ b/drivers/mfd/rohm-bd79124.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright (C) 2025 ROHM Semiconductors
+//
+// ROHM BD79124 ADC / GPO driver
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+#include <linux/mfd/core.h>
+#include <linux/mfd/rohm-bd79124.h>
+
+static struct resource adc_alert;
+
+enum {
+	CELL_PINMUX,
+	CELL_ADC,
+};
+
+static struct mfd_cell bd79124_cells[] = {
+	[CELL_PINMUX]	= { .name = "bd79124-pinmux", },
+	[CELL_ADC]	= { .name = "bd79124-adc", },
+};
+
+/* Read-only regs */
+static const struct regmap_range bd79124_ro_ranges[] = {
+	{
+		.range_min = BD79124_REG_EVENT_FLAG,
+		.range_max = BD79124_REG_EVENT_FLAG,
+	}, {
+		.range_min = BD79124_REG_RECENT_CH0_LSB,
+		.range_max = BD79124_REG_RECENT_CH7_MSB,
+	},
+};
+
+static const struct regmap_access_table bd79124_ro_regs = {
+	.no_ranges	= &bd79124_ro_ranges[0],
+	.n_no_ranges	= ARRAY_SIZE(bd79124_ro_ranges),
+};
+
+static const struct regmap_range bd79124_volatile_ranges[] = {
+	{
+		.range_min = BD79124_REG_RECENT_CH0_LSB,
+		.range_max = BD79124_REG_RECENT_CH7_MSB,
+	}, {
+		.range_min = BD79124_REG_EVENT_FLAG,
+		.range_max = BD79124_REG_EVENT_FLAG,
+	}, {
+		.range_min = BD79124_REG_EVENT_FLAG_HI,
+		.range_max = BD79124_REG_EVENT_FLAG_HI,
+	}, {
+		.range_min = BD79124_REG_EVENT_FLAG_LO,
+		.range_max = BD79124_REG_EVENT_FLAG_LO,
+	}, {
+		.range_min = BD79124_REG_SYSTEM_STATUS,
+		.range_max = BD79124_REG_SYSTEM_STATUS,
+	},
+};
+
+static const struct regmap_access_table bd79124_volatile_regs = {
+	.yes_ranges	= &bd79124_volatile_ranges[0],
+	.n_yes_ranges	= ARRAY_SIZE(bd79124_volatile_ranges),
+};
+
+static const struct regmap_range bd79124_precious_ranges[] = {
+	{
+		.range_min = BD79124_REG_EVENT_FLAG_HI,
+		.range_max = BD79124_REG_EVENT_FLAG_HI,
+	}, {
+		.range_min = BD79124_REG_EVENT_FLAG_LO,
+		.range_max = BD79124_REG_EVENT_FLAG_LO,
+	},
+};
+
+static const struct regmap_access_table bd79124_precious_regs = {
+	.yes_ranges	= &bd79124_precious_ranges[0],
+	.n_yes_ranges	= ARRAY_SIZE(bd79124_precious_ranges),
+};
+
+static const struct regmap_config bd79124_regmap = {
+	.reg_bits		= 16,
+	.val_bits		= 8,
+	.read_flag_mask		= BD79124_I2C_MULTI_READ,
+	.write_flag_mask	= BD79124_I2C_MULTI_WRITE,
+	.max_register		= BD79124_REG_MAX,
+	.cache_type		= REGCACHE_MAPLE,
+	.volatile_table		= &bd79124_volatile_regs,
+	.wr_table		= &bd79124_ro_regs,
+	.precious_table		= &bd79124_precious_regs,
+};
+
+static int bd79124_probe(struct i2c_client *i2c)
+{
+	int ret;
+	struct regmap *map;
+	struct device *dev = &i2c->dev;
+	int *adc_vref;
+
+	adc_vref = devm_kzalloc(dev, sizeof(*adc_vref), GFP_KERNEL);
+	if (!adc_vref)
+		return -ENOMEM;
+
+	/*
+	 * Better to enable regulators here so we don't need to worry about the
+	 * order of sub-device instantiation. We also need to deliver the
+	 * reference voltage value to the ADC driver. This is done via
+	 * the MFD driver's drvdata.
+	 */
+	*adc_vref = devm_regulator_get_enable_read_voltage(dev, "vdd");
+	if (*adc_vref < 0)
+		return dev_err_probe(dev, ret, "Failed to get the Vdd\n");
+
+	dev_set_drvdata(dev, adc_vref);
+
+	ret = devm_regulator_get_enable(dev, "iovdd");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to enable I/O voltage\n");
+
+	map = devm_regmap_init_i2c(i2c, &bd79124_regmap);
+	if (IS_ERR(map))
+		return dev_err_probe(dev, PTR_ERR(map),
+				     "Failed to initialize Regmap\n");
+
+	if (i2c->irq) {
+		adc_alert = DEFINE_RES_IRQ_NAMED(i2c->irq, "thresh-alert");
+		bd79124_cells[CELL_ADC].resources = &adc_alert;
+		bd79124_cells[CELL_ADC].num_resources = 1;
+	}
+
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, bd79124_cells,
+				   ARRAY_SIZE(bd79124_cells), NULL, 0, NULL);
+	if (ret)
+		dev_err_probe(dev, ret, "Failed to create subdevices\n");
+
+	return ret;
+}
+
+static const struct of_device_id bd79124_of_match[] = {
+	{ .compatible = "rohm,bd79124" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bd79124_of_match);
+
+static const struct i2c_device_id bd79124_id[] = {
+	{ "bd79124", },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, bd79124_id);
+
+static struct i2c_driver bd79124_driver = {
+	.driver = {
+		.name = "bd79124",
+		.of_match_table = bd79124_of_match,
+	},
+	.probe = bd79124_probe,
+	.id_table = bd79124_id,
+};
+module_i2c_driver(bd79124_driver);
+
+MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@gmail.com>");
+MODULE_DESCRIPTION("Core Driver for ROHM BD79124");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/rohm-bd79124.h b/include/linux/mfd/rohm-bd79124.h
new file mode 100644
index 000000000000..505faeb6f135
--- /dev/null
+++ b/include/linux/mfd/rohm-bd79124.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2021 ROHM Semiconductors.
+ *
+ * Author: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+ */
+
+#ifndef _MFD_BD79124_H
+#define _MFD_BD79124_H
+
+#define BD79124_I2C_MULTI_READ		0x30
+#define BD79124_I2C_MULTI_WRITE		0x28
+#define BD79124_REG_MAX			0xaf
+
+#define BD79124_REG_SYSTEM_STATUS	0x0
+#define BD79124_REG_GEN_CFG		0x01
+#define BD79124_REG_OPMODE_CFG		0x04
+#define BD79124_REG_PINCFG		0x05
+#define BD79124_REG_GPO_VAL		0x06
+#define BD79124_REG_SEQUENCE_CFG	0x10
+#define BD79124_REG_MANUAL_CHANNELS	0x11
+#define BD79124_REG_AUTO_CHANNELS	0x12
+#define BD79124_REG_ALERT_CH_SEL	0x14
+#define BD79124_REG_EVENT_FLAG		0x18
+#define BD79124_REG_EVENT_FLAG_HI	0x1a
+#define BD79124_REG_EVENT_FLAG_LO	0x1c
+#define BD79124_REG_HYSTERESIS_CH0	0x20
+#define BD79124_REG_EVENTCOUNT_CH0	0x22
+#define BD79124_REG_RECENT_CH0_LSB	0xa0
+#define BD79124_REG_RECENT_CH7_MSB	0xaf
+
+#endif
-- 
2.48.1

On 31/01/2025 15:37, Matti Vaittinen wrote:
> Add core driver for the ROHM BD79124 ADC / GPO.
> 
> The core driver launches the sub-drivers for the pinmux/GPO and for the
> IIO ADC. It also provides the regmap, and forwards the IRQ resource to
> the ADC.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

Just a note to reviewers - I dropped the MFD from v2. No need to review 
this any further.

Thanks!

Yours,
	-- Matti

On Fri, 31 Jan 2025 15:37:06 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Add core driver for the ROHM BD79124 ADC / GPO.
> 
> The core driver launches the sub-drivers for the pinmux/GPO and for the
> IIO ADC. It also provides the regmap, and forwards the IRQ resource to
> the ADC.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
As per response in cover letter. This is a common device combination and so
far I don't think we ever bothered with an MFD. Lots of ADCs provide
GPIO chips as well so I'd just squash it into the ADC driver.

> ---
>  drivers/mfd/Kconfig              |  12 +++
>  drivers/mfd/Makefile             |   1 +
>  drivers/mfd/rohm-bd79124.c       | 165 +++++++++++++++++++++++++++++++
>  include/linux/mfd/rohm-bd79124.h |  32 ++++++
>  4 files changed, 210 insertions(+)
>  create mode 100644 drivers/mfd/rohm-bd79124.c
>  create mode 100644 include/linux/mfd/rohm-bd79124.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ae23b317a64e..f024256fb180 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2113,6 +2113,18 @@ config MFD_ROHM_BD71828
>  	  also a single-cell linear charger, a Coulomb counter, a real-time
>  	  clock (RTC), GPIOs and a 32.768 kHz clock gate.
>  
> +config MFD_ROHM_BD79124
> +	tristate "Rohm BD79124 core driver"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to build support for the ROHM BD79124 ADC core. The
> +	  ROHM BD79124 is a 12-bit, 8-channel, SAR ADC. The ADC supports
> +	  also an automatic measurement mode, with an alarm interrupt for
> +	  out-of-window measurements. The window is configurable for each
> +	  channel. The ADC inputs can optionally be used as general purpose
> +	  outputs.
> +
>  config MFD_ROHM_BD957XMUF
>  	tristate "ROHM BD9576MUF and BD9573MUF Power Management ICs"
>  	depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index e057d6d6faef..c7d64e933a7d 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -270,6 +270,7 @@ obj-$(CONFIG_MFD_SC27XX_PMIC)	+= sprd-sc27xx-spi.o
>  obj-$(CONFIG_RAVE_SP_CORE)	+= rave-sp.o
>  obj-$(CONFIG_MFD_ROHM_BD71828)	+= rohm-bd71828.o
>  obj-$(CONFIG_MFD_ROHM_BD718XX)	+= rohm-bd718x7.o
> +obj-$(CONFIG_MFD_ROHM_BD79124)	+= rohm-bd79124.o
>  obj-$(CONFIG_MFD_ROHM_BD957XMUF)	+= rohm-bd9576.o
>  obj-$(CONFIG_MFD_ROHM_BD96801)	+= rohm-bd96801.o
>  obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
> diff --git a/drivers/mfd/rohm-bd79124.c b/drivers/mfd/rohm-bd79124.c
> new file mode 100644
> index 000000000000..c35ab0e03b0b
> --- /dev/null
> +++ b/drivers/mfd/rohm-bd79124.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (C) 2025 ROHM Semiconductors
> +//
> +// ROHM BD79124 ADC / GPO driver
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>

mod_devicetable.h

> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/rohm-bd79124.h>
> +
> +static struct resource adc_alert;

What if we have two of these?

> +
> +enum {
> +	CELL_PINMUX,
> +	CELL_ADC,
> +};
> +
> +static struct mfd_cell bd79124_cells[] = {
> +	[CELL_PINMUX]	= { .name = "bd79124-pinmux", },
> +	[CELL_ADC]	= { .name = "bd79124-adc", },
> +};
> +
> +/* Read-only regs */
> +static const struct regmap_range bd79124_ro_ranges[] = {
> +	{
> +		.range_min = BD79124_REG_EVENT_FLAG,
> +		.range_max = BD79124_REG_EVENT_FLAG,
> +	}, {
> +		.range_min = BD79124_REG_RECENT_CH0_LSB,
> +		.range_max = BD79124_REG_RECENT_CH7_MSB,
> +	},
> +};
> +
> +static const struct regmap_access_table bd79124_ro_regs = {
> +	.no_ranges	= &bd79124_ro_ranges[0],
> +	.n_no_ranges	= ARRAY_SIZE(bd79124_ro_ranges),
> +};
> +
> +static const struct regmap_range bd79124_volatile_ranges[] = {
> +	{
> +		.range_min = BD79124_REG_RECENT_CH0_LSB,
> +		.range_max = BD79124_REG_RECENT_CH7_MSB,
> +	}, {
> +		.range_min = BD79124_REG_EVENT_FLAG,
> +		.range_max = BD79124_REG_EVENT_FLAG,
> +	}, {
> +		.range_min = BD79124_REG_EVENT_FLAG_HI,
> +		.range_max = BD79124_REG_EVENT_FLAG_HI,
> +	}, {
> +		.range_min = BD79124_REG_EVENT_FLAG_LO,
> +		.range_max = BD79124_REG_EVENT_FLAG_LO,
> +	}, {
> +		.range_min = BD79124_REG_SYSTEM_STATUS,
> +		.range_max = BD79124_REG_SYSTEM_STATUS,
> +	},
> +};
> +
> +static const struct regmap_access_table bd79124_volatile_regs = {
> +	.yes_ranges	= &bd79124_volatile_ranges[0],
> +	.n_yes_ranges	= ARRAY_SIZE(bd79124_volatile_ranges),
> +};
> +
> +static const struct regmap_range bd79124_precious_ranges[] = {
> +	{
> +		.range_min = BD79124_REG_EVENT_FLAG_HI,
> +		.range_max = BD79124_REG_EVENT_FLAG_HI,
> +	}, {
> +		.range_min = BD79124_REG_EVENT_FLAG_LO,
> +		.range_max = BD79124_REG_EVENT_FLAG_LO,
> +	},
> +};
> +
> +static const struct regmap_access_table bd79124_precious_regs = {
> +	.yes_ranges	= &bd79124_precious_ranges[0],
> +	.n_yes_ranges	= ARRAY_SIZE(bd79124_precious_ranges),
> +};
> +
> +static const struct regmap_config bd79124_regmap = {
> +	.reg_bits		= 16,
> +	.val_bits		= 8,
> +	.read_flag_mask		= BD79124_I2C_MULTI_READ,
> +	.write_flag_mask	= BD79124_I2C_MULTI_WRITE,
> +	.max_register		= BD79124_REG_MAX,
> +	.cache_type		= REGCACHE_MAPLE,
> +	.volatile_table		= &bd79124_volatile_regs,
> +	.wr_table		= &bd79124_ro_regs,
> +	.precious_table		= &bd79124_precious_regs,
> +};
> +
> +static int bd79124_probe(struct i2c_client *i2c)
> +{
> +	int ret;
> +	struct regmap *map;
> +	struct device *dev = &i2c->dev;
> +	int *adc_vref;

Wrap that in a structure.  It's just a bit too odd to have just
one integer!

> +
> +	adc_vref = devm_kzalloc(dev, sizeof(*adc_vref), GFP_KERNEL);
> +	if (!adc_vref)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Better to enable regulators here so we don't need to worry about the
> +	 * order of sub-device instantiation. We also need to deliver the
> +	 * reference voltage value to the ADC driver. This is done via
> +	 * the MFD driver's drvdata.
> +	 */
> +	*adc_vref = devm_regulator_get_enable_read_voltage(dev, "vdd");
> +	if (*adc_vref < 0)
> +		return dev_err_probe(dev, ret, "Failed to get the Vdd\n");
> +
> +	dev_set_drvdata(dev, adc_vref);
> +
> +	ret = devm_regulator_get_enable(dev, "iovdd");
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to enable I/O voltage\n");
> +
> +	map = devm_regmap_init_i2c(i2c, &bd79124_regmap);
> +	if (IS_ERR(map))
> +		return dev_err_probe(dev, PTR_ERR(map),
> +				     "Failed to initialize Regmap\n");
> +
> +	if (i2c->irq) {
> +		adc_alert = DEFINE_RES_IRQ_NAMED(i2c->irq, "thresh-alert");
> +		bd79124_cells[CELL_ADC].resources = &adc_alert;
> +		bd79124_cells[CELL_ADC].num_resources = 1;
> +	}
> +
> +	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, bd79124_cells,
> +				   ARRAY_SIZE(bd79124_cells), NULL, 0, NULL);
> +	if (ret)
> +		dev_err_probe(dev, ret, "Failed to create subdevices\n");
return dev_err_probe();

Then return 0 in other path.

> +
> +	return ret;
> +}
>
> diff --git a/include/linux/mfd/rohm-bd79124.h b/include/linux/mfd/rohm-bd79124.h
> new file mode 100644
> index 000000000000..505faeb6f135
> --- /dev/null
> +++ b/include/linux/mfd/rohm-bd79124.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2021 ROHM Semiconductors.

No update on that date?

> + *
> + * Author: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> + */
> +
> +#ifndef _MFD_BD79124_H
> +#define _MFD_BD79124_H
> +
> +#define BD79124_I2C_MULTI_READ		0x30
> +#define BD79124_I2C_MULTI_WRITE		0x28
> +#define BD79124_REG_MAX			0xaf
> +
> +#define BD79124_REG_SYSTEM_STATUS	0x0

Give it two digits. 0x00 for ever so slight readability advantage.

> +#define BD79124_REG_GEN_CFG		0x01
> +#define BD79124_REG_OPMODE_CFG		0x04
> +#define BD79124_REG_PINCFG		0x05
> +#define BD79124_REG_GPO_VAL		0x06
> +#define BD79124_REG_SEQUENCE_CFG	0x10
> +#define BD79124_REG_MANUAL_CHANNELS	0x11
> +#define BD79124_REG_AUTO_CHANNELS	0x12
> +#define BD79124_REG_ALERT_CH_SEL	0x14
> +#define BD79124_REG_EVENT_FLAG		0x18
> +#define BD79124_REG_EVENT_FLAG_HI	0x1a
> +#define BD79124_REG_EVENT_FLAG_LO	0x1c
> +#define BD79124_REG_HYSTERESIS_CH0	0x20
> +#define BD79124_REG_EVENTCOUNT_CH0	0x22
> +#define BD79124_REG_RECENT_CH0_LSB	0xa0
> +#define BD79124_REG_RECENT_CH7_MSB	0xaf
> +
> +#endif

On 31/01/2025 19:14, Jonathan Cameron wrote:
> On Fri, 31 Jan 2025 15:37:06 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> Add core driver for the ROHM BD79124 ADC / GPO.
>>
>> The core driver launches the sub-drivers for the pinmux/GPO and for the
>> IIO ADC. It also provides the regmap, and forwards the IRQ resource to
>> the ADC.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> As per response in cover letter. This is a common device combination and so
> far I don't think we ever bothered with an MFD. Lots of ADCs provide
> GPIO chips as well so I'd just squash it into the ADC driver.

You may be right with this. I still need to digest this a bit as I 
explaned in the cover letter discussion :)

All of your inline comments were valid and I agree with them. Noticing 
the single IC limitation was great!

I'll address all the findings if I keep the MFD approach.

Yours,
	-- Matti