[PATCH 0/5] staging: drivers: mfd: Add MAX77541 MFD and related device drivers

[PATCH 0/5] staging: drivers: mfd: Add MAX77541 MFD and related device drivers

[Okan Sahin]

This patchset adds mfd, regulator and adc driver and related bindings.The patches 
are required to be applied in sequence.

Okan Sahin (5):
  staging: drivers: mfd: Add MAX77541/MAX77540 PMIC Support
  staging: dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings
  staging: drivers: regulator: Add MAX77541 Regulator Support
  staging: dt-bindings: regulator: adi,max77541.yaml Add MAX77541
    Regulator bindings
  staging: drivers: iio: adc: Adc MAX77541 ADC Support

 .../devicetree/bindings/mfd/adi,max77541.yaml | 134 ++++++++++
 .../bindings/regulator/adi,max77541.yaml      |  44 ++++
 MAINTAINERS                                   |  11 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/max77541-adc.c                | 224 +++++++++++++++++
 drivers/mfd/Kconfig                           |  13 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/max77541.c                        | 236 ++++++++++++++++++
 drivers/regulator/Kconfig                     |   9 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/max77541-regulator.c        | 181 ++++++++++++++
 include/linux/mfd/max77541.h                  | 150 +++++++++++
 13 files changed, 1016 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541.yaml
 create mode 100644 drivers/iio/adc/max77541-adc.c
 create mode 100644 drivers/mfd/max77541.c
 create mode 100644 drivers/regulator/max77541-regulator.c
 create mode 100644 include/linux/mfd/max77541.h

-- 
2.30.2

[PATCH 1/5] staging: drivers: mfd: Add MAX77541/MAX77540 PMIC Support

[Okan Sahin]

This patch adds MFD driver for MAX77541/MAX77540 to enable its sub
devices.

The MAX77541 is a multi-function devices. It includes
buck converter and ADC.

The MAX77540 is a high-efficiency buck converter
with two 3A switching phases.

They have same regmap except for ADC part of MAX77541.

Signed-off-by: Okan Sahin <okan.sahin@analog.com>
---
 MAINTAINERS                  |   7 ++
 drivers/mfd/Kconfig          |  13 ++
 drivers/mfd/Makefile         |   1 +
 drivers/mfd/max77541.c       | 236 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/max77541.h | 150 ++++++++++++++++++++++
 5 files changed, 407 insertions(+)
 create mode 100644 drivers/mfd/max77541.c
 create mode 100644 include/linux/mfd/max77541.h

diff --git a/MAINTAINERS b/MAINTAINERS
index cf0f18502372..af94d06bb9f0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12497,6 +12497,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/regulator/maxim,max20086.yaml
 F:	drivers/regulator/max20086-regulator.c
 
+MAXIM MAX77541 PMIC MFD DRIVER
+M:	Okan Sahin <okan.sahin@analog.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/mfd/max77541.c
+F:	include/linux/mfd/max77541.h
+
 MAXIM MAX77650 PMIC MFD DRIVER
 M:	Bartosz Golaszewski <brgl@bgdev.pl>
 L:	linux-kernel@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b93856de432..153e8d6757b0 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -791,6 +791,19 @@ config MFD_MAX14577
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MAX77541
+	tristate "Analog Devices MAX77541/77540 PMIC Support"
+	depends on I2C=y
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  Say yes here to add support for Analog Devices
+	  MAX77541 and MAX77540 Power Management ICs.
+	  This driver provides common support for accessing the device;
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
 config MFD_MAX77620
 	bool "Maxim Semiconductor MAX77620 and MAX20024 PMIC Support"
 	depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 7ed3ef4a698c..bf21228f5742 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
 obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
+obj-$(CONFIG_MFD_MAX77541)	+= max77541.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
diff --git a/drivers/mfd/max77541.c b/drivers/mfd/max77541.c
new file mode 100644
index 000000000000..97a2df3ce0b6
--- /dev/null
+++ b/drivers/mfd/max77541.c
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Analog Devices, Inc.
+ * Mfd core driver for the MAX77540 and MAX77541
+ */
+
+ #include <linux/i2c.h>
+ #include <linux/interrupt.h>
+ #include <linux/mfd/core.h>
+ #include <linux/mfd/max77541.h>
+ #include <linux/regmap.h>
+
+static const struct regmap_config max77541_regmap_config = {
+	.reg_bits   = 8,
+	.val_bits   = 8,
+};
+
+static const struct regmap_irq max77541_src_irqs[] = {
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_INT_SRC_TOPSYS),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_INT_SRC_BUCK),
+};
+
+static const struct regmap_irq_chip max77541_src_irq_chip = {
+	.name		= "max77541-src",
+	.status_base	= MAX77541_REG_INT_SRC,
+	.mask_base	= MAX77541_REG_INT_SRC,
+	.num_regs	= 1,
+	.irqs		= max77541_src_irqs,
+	.num_irqs       = ARRAY_SIZE(max77541_src_irqs),
+};
+
+static const struct regmap_irq max77541_topsys_irqs[] = {
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_TJ_120C),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_TJ_140C),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_TSHDN),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_UVLO),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_ALT_SWO),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET),
+};
+
+static const struct regmap_irq_chip max77541_topsys_irq_chip = {
+	.name		= "max77541-topsys",
+	.status_base	= MAX77541_REG_TOPSYS_INT,
+	.mask_base	= MAX77541_REG_TOPSYS_INT_M,
+	.num_regs	= 1,
+	.irqs		= max77541_topsys_irqs,
+	.num_irqs	= ARRAY_SIZE(max77541_topsys_irqs),
+};
+
+static const struct regmap_irq max77541_buck_irqs[] = {
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M1_POK_FLT),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M2_POK_FLT),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M1_SCFLT),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_BUCK_INT_M2_SCFLT),
+};
+
+static const struct regmap_irq_chip max77541_buck_irq_chip = {
+	.name		= "max77541-buck",
+	.status_base	= MAX77541_REG_BUCK_INT,
+	.mask_base	= MAX77541_REG_BUCK_INT_M,
+	.num_regs	= 1,
+	.irqs		= max77541_buck_irqs,
+	.num_irqs	= ARRAY_SIZE(max77541_buck_irqs),
+};
+
+static const struct regmap_irq max77541_adc_irqs[] = {
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH1_I),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH2_I),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH3_I),
+	MAX77541_REGMAP_IRQ_REG(MAX77541_BIT_ADC_INT_CH6_I),
+};
+
+static const struct regmap_irq_chip max77541_adc_irq_chip = {
+	.name		= "max77541-adc",
+	.status_base	= MAX77541_REG_ADC_INT,
+	.mask_base	= MAX77541_REG_ADC_MSK,
+	.num_regs	= 1,
+	.irqs		= max77541_adc_irqs,
+	.num_irqs	= ARRAY_SIZE(max77541_adc_irqs),
+};
+
+static const struct mfd_cell max77540_devs[] = {
+	MFD_CELL_OF("max77540-regulator", NULL, NULL, 0, 0,
+		    "adi,max77540-regulator"),
+};
+
+static const struct mfd_cell max77541_devs[] = {
+	MFD_CELL_OF("max77541-regulator", NULL, NULL, 0, 0,
+		    "adi,max77541-regulator"),
+	MFD_CELL_OF("max77541-adc", NULL, NULL, 0, 0,
+		    NULL),
+};
+
+static int max77541_pmic_irq_init(struct max77541_dev *me)
+{
+	struct regmap *regmap = me->regmap;
+	struct device *dev = me->dev;
+	int irq = me->i2c->irq;
+	int ret;
+
+	ret = devm_regmap_add_irq_chip(dev, regmap, irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77541_src_irq_chip, &me->irq_data);
+	if (ret)
+		return ret;
+
+	ret = devm_regmap_add_irq_chip(dev, regmap, irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77541_topsys_irq_chip, &me->irq_topsys);
+	if (ret)
+		return ret;
+
+	ret = devm_regmap_add_irq_chip(dev, regmap, irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77541_buck_irq_chip, &me->irq_buck);
+	if (ret)
+		return ret;
+
+	if (me->type == MAX77541) {
+		ret = devm_regmap_add_irq_chip(dev, regmap, irq,
+					       IRQF_ONESHOT | IRQF_SHARED, 0,
+					       &max77541_adc_irq_chip,
+					       &me->irq_adc);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+
+static int max77541_pmic_setup(struct max77541_dev *me)
+{
+	struct regmap *regmap = me->regmap;
+	struct device *dev = me->dev;
+	unsigned int val;
+	int ret;
+
+	ret = max77541_pmic_irq_init(me);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize IRQ\n");
+
+	ret = regmap_read(regmap, MAX77541_REG_INT_SRC, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(regmap, MAX77541_REG_TOPSYS_INT, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(regmap, MAX77541_REG_BUCK_INT, &val);
+	if (ret)
+		return ret;
+
+	ret = device_init_wakeup(dev, true);
+	if (ret)
+		return dev_err_probe(dev, ret, "Unable to init wakeup\n");
+
+	switch (me->type) {
+	case MAX77540:
+		return devm_mfd_add_devices(dev, -1, max77540_devs, ARRAY_SIZE(max77540_devs),
+					    NULL, 0, NULL);
+	case MAX77541:
+		return devm_mfd_add_devices(dev, -1, max77541_devs, ARRAY_SIZE(max77541_devs),
+					    NULL, 0, NULL);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max77541_i2c_probe(struct i2c_client *client)
+{
+	const struct i2c_device_id *chip_id;
+	struct max77541_dev *me;
+	const void *chip_data;
+
+	me = devm_kzalloc(&client->dev, sizeof(*me), GFP_KERNEL);
+	if (!me)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, me);
+	me->dev = &client->dev;
+	me->i2c = client;
+
+	chip_id = to_i2c_driver(client->dev.driver)->id_table;
+
+	chip_data = device_get_match_data(me->dev);
+	if (!chip_data) {
+		chip_data = (void *)i2c_match_id(chip_id, client)->driver_data;
+		if (!chip_data)
+			return dev_err_probe(me->dev, -ENODEV, "Unable to find device\n");
+	}
+
+	me->type = (enum dev_type)chip_data;
+	me->regmap = devm_regmap_init_i2c(client, &max77541_regmap_config);
+	if (IS_ERR(me->regmap))
+		return dev_err_probe(me->dev,  PTR_ERR(me->regmap),
+					"Failed to allocate register map\n");
+
+	return max77541_pmic_setup(me);
+}
+
+static const struct of_device_id max77541_of_id[] = {
+	{
+		.compatible = "adi,max77540",
+		.data = (void *)MAX77540,
+	},
+	{
+		.compatible = "adi,max77541",
+		.data = (void *)MAX77541,
+	},
+	{ /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(of, max77541_of_id);
+
+static const struct i2c_device_id max77541_i2c_id[] = {
+	{ "max77540", MAX77540 },
+	{ "max77541", MAX77541 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, max77541_i2c_id);
+
+static struct i2c_driver max77541_i2c_driver = {
+	.driver = {
+		.name = "max77541",
+		.of_match_table = max77541_of_id,
+	},
+	.probe_new = max77541_i2c_probe,
+	.id_table = max77541_i2c_id,
+};
+
+module_i2c_driver(max77541_i2c_driver);
+
+MODULE_DESCRIPTION("MAX7740/MAX7741 MFD Driver");
+MODULE_AUTHOR("Okan Sahin <okan.sahin@analog.com>");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
diff --git a/include/linux/mfd/max77541.h b/include/linux/mfd/max77541.h
new file mode 100644
index 000000000000..6f2753300227
--- /dev/null
+++ b/include/linux/mfd/max77541.h
@@ -0,0 +1,150 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __MAX77541_MFD_H__
+#define __MAX77541_MFD_H__
+
+/*      REGISTERS       */
+
+/*      GLOBAL CONFIG1       */
+#define MAX77541_REG_INT_SRC                    0x00
+#define MAX77541_REG_INT_SRC_M                  0x01
+#define MAX77541_REG_TOPSYS_INT                 0x02
+#define MAX77541_REG_TOPSYS_INT_M               0x03
+#define MAX77541_REG_TOPSYS_STAT                0x04
+#define MAX77541_REG_DEVICE_CFG1                0x06
+#define MAX77541_REG_DEVICE_CFG2                0x07
+#define MAX77541_REG_TOPSYS_CFG                 0x08
+#define MAX77541_REG_PROT_CFG                   0x09
+#define MAX77541_REG_EN_CTRL                    0x0B
+
+/*      GLOBAL CONFIG2       */
+#define MAX77541_REG_GLB_CFG1                   0x11
+
+/*      BUCK CONFIG       */
+#define MAX77541_REG_BUCK_INT                   0x20
+#define MAX77541_REG_BUCK_INT_M                 0x21
+#define MAX77541_REG_BUCK_STAT                  0x22
+
+/*      BUCK1       */
+#define MAX77541_REG_M1_VOUT                    0x23
+#define MAX77541_REG_M1_CFG1                    0x25
+#define MAX77541_REG_M1_CFG2                    0x26
+#define MAX77541_REG_M1_CFG3                    0x27
+
+/*      BUCK2       */
+#define MAX77541_REG_M2_VOUT                    0x33
+#define MAX77541_REG_M2_CFG1                    0x35
+#define MAX77541_REG_M2_CFG2                    0x36
+#define MAX77541_REG_M2_CFG3                    0x37
+
+#define MAX77541_REG_NOT_AVAILABLE              0xFF
+
+/* INTERRUPT MASKS*/
+#define MAX77541_REG_INT_SRC_MASK               0x00
+#define MAX77541_REG_TOPSYS_INT_MASK            0x00
+#define MAX77541_REG_BUCK_INT_MASK              0x00
+
+/*BITS OF REGISTERS*/
+
+#define MAX77541_BIT_INT_SRC_TOPSYS             BIT(0)
+#define MAX77541_BIT_INT_SRC_BUCK               BIT(1)
+
+#define MAX77541_BIT_TOPSYS_INT_TJ_120C         BIT(0)
+#define MAX77541_BIT_TOPSYS_INT_TJ_140C         BIT(1)
+#define MAX77541_BIT_TOPSYS_INT_TSHDN           BIT(2)
+#define MAX77541_BIT_TOPSYS_INT_UVLO            BIT(3)
+#define MAX77541_BIT_TOPSYS_INT_ALT_SWO         BIT(4)
+#define MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET    BIT(5)
+
+#define MAX77541_BIT_BUCK_INT_M1_POK_FLT        BIT(0)
+#define MAX77541_BIT_BUCK_INT_M2_POK_FLT        BIT(1)
+#define MAX77541_BIT_BUCK_INT_M1_SCFLT          BIT(4)
+#define MAX77541_BIT_BUCK_INT_M2_SCFLT          BIT(5)
+
+#define MAX77541_BITS_DEVICE_CFG1_SEL1_LATCH    GENMASK(4, 0)
+#define MAX77541_BITS_DEVICE_CFG2_SEL2_LATCH    GENMASK(4, 0)
+
+#define MAX77541_BIT_TOPSYS_CFG_DIS_ALT_IN      BIT(0)
+
+#define MAX77541_BITS_PROT_CFG_POK_TO           GENMASK(1, 0)
+#define MAX77541_BIT_PROT_CFG_EN_FTMON          BIT(2)
+
+#define MAX77541_BIT_M1_EN                      BIT(0)
+#define MAX77541_BIT_M2_EN                      BIT(1)
+#define MAX77541_BIT_M1_LPM                     BIT(4)
+#define MAX77541_BIT_M2_LPM                     BIT(5)
+
+#define MAX77541_BITS_GLB_CFG1_SSTOP_SR         GENMASK(2, 0)
+#define MAX77541_BITS_GLB_CFG1_SSTRT_SR         GENMASK(5, 3)
+
+#define MAX77541_BITS_MX_VOUT                   GENMASK(7, 0)
+
+#define MAX77541_BITS_MX_CFG1_RU_SR             GENMASK(2, 0)
+#define MAX77541_BITS_MX_CFG1_RD_SR             GENMASK(5, 3)
+#define MAX77541_BITS_MX_CFG1_RNG               GENMASK(7, 6)
+
+#define MAX77541_BIT_MX_CFG2_FPWM               BIT(0)
+#define MAX77541_BIT_MX_CFG2_FSREN              BIT(1)
+#define MAX77541_BITS_MX_CFG2_SS_PAT            GENMASK(3, 2)
+#define MAX77541_BITS_MX_CFG2_SS_FREQ           GENMASK(5, 4)
+#define MAX77541_BITS_MX_CFG2_SS_ENV            GENMASK(7, 6)
+
+#define MAX77541_BITS_MX_CFG3_ILIM              GENMASK(1, 0)
+#define MAX77541_BITS_MX_CFG3_FREQ              GENMASK(3, 2)
+#define MAX77541_BIT_MX_CFG3_FTRAK              BIT(4)
+#define MAX77541_BIT_MX_CFG3_RESRESH            BIT(5)
+#define MAX77541_BIT_MX_CFG3_ADIS1              BIT(6)
+#define MAX77541_BIT_MX_CFG3_ADIS100            BIT(7)
+
+#define MAX77541_MAX_REGULATORS 2
+
+/*      ADC       */
+#define MAX77541_REG_ADC_INT                    0x70
+#define MAX77541_REG_ADC_MSK                    0x71
+
+#define MAX77541_REG_ADC_DATA_CH1               0x72
+#define MAX77541_REG_ADC_DATA_CH2               0x73
+#define MAX77541_REG_ADC_DATA_CH3               0x74
+#define MAX77541_REG_ADC_DATA_CH6               0x77
+
+#define MAX77541_BIT_ADC_INT_CH1_I              BIT(0)
+#define MAX77541_BIT_ADC_INT_CH2_I              BIT(1)
+#define MAX77541_BIT_ADC_INT_CH3_I              BIT(2)
+#define MAX77541_BIT_ADC_INT_CH6_I              BIT(5)
+
+#define MAX77541_REGMAP_IRQ_REG(_mask)	\
+	{ .mask = (_mask),  }
+
+enum dev_type {
+	MAX77540 = 1,
+	MAX77541 = 2,
+};
+
+enum max77541_regulators {
+	MAX77541_BUCK1 = 1,
+	MAX77541_BUCK2,
+};
+
+enum mx_range {
+	LOW_RANGE,
+	MID_RANGE,
+	HIGH_RANGE,
+	RESERVED
+};
+
+struct max77541_dev {
+	void *pdata;
+	struct device *dev;
+
+	struct regmap_irq_chip_data *irq_data;
+	struct regmap_irq_chip_data *irq_buck;
+	struct regmap_irq_chip_data *irq_topsys;
+	struct regmap_irq_chip_data *irq_adc;
+
+	struct i2c_client *i2c;
+	struct regmap *regmap;
+
+	u8 type;
+};
+
+#endif /* __MAX77541_MFD_H__ */
-- 
2.30.2

[PATCH 2/5] staging: dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings

[Okan Sahin]

This patch adds document the bindings for MAX77541 MFD driver. It also
includes MAX77540 driver whose regmap is covered by MAX77541.

Signed-off-by: Okan Sahin <okan.sahin@analog.com>
---
 .../devicetree/bindings/mfd/adi,max77541.yaml | 134 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 135 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml

diff --git a/Documentation/devicetree/bindings/mfd/adi,max77541.yaml b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
new file mode 100644
index 000000000000..205953e6dd15
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/adi,max77541.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MAX77540/MAX77541 PMIC from ADI.
+
+maintainers:
+  - Okan Sahin <okan.sahin@analog.com>
+
+description: |
+  MAX77540 is a Power Management IC with 2 buck regulators.
+
+  MAX77541 is a Power Management IC with 2 buck regulators and 1 ADC.
+
+properties:
+  compatible:
+    enum:
+      - adi,max77540
+      - adi,max77541
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  regulators:
+    $ref: /schemas/regulator/adi,max77541.yaml#
+
+  adc:
+    type: object
+    additionalProperties: false
+    properties:
+      compatible:
+        const: adi,max77541-adc
+    required:
+      -compatible
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: adi,max77540
+    then:
+      properties:
+        regulator:
+          properties:
+            compatible:
+              const: adi,max77540-regulator
+    else:
+      properties:
+        regulator:
+          properties:
+            compatible:
+              const: adi,max77541-regulator
+        adc:
+          properties:
+            compatible:
+              const: adi,max77541-adc
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@69 {
+            compatible = "adi,max77540";
+            reg = <0x69>;
+            interrupt-parent = <&gpio>;
+            interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+
+            regulators {
+                buck1 {
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <5200000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+                buck2 {
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <5200000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+            };
+        };
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@69 {
+            compatible = "adi,max77541";
+            reg = <0x63>;
+            interrupt-parent = <&gpio>;
+            interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+
+            regulators {
+                buck1 {
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <5200000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+                buck2 {
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <5200000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+            };
+
+            adc {
+                compatible = "adi,max77541-adc";
+            }
+        };
+    };
\ No newline at end of file
diff --git a/MAINTAINERS b/MAINTAINERS
index af94d06bb9f0..22f5a9c490e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12501,6 +12501,7 @@ MAXIM MAX77541 PMIC MFD DRIVER
 M:	Okan Sahin <okan.sahin@analog.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/adi,max77541.yaml
 F:	drivers/mfd/max77541.c
 F:	include/linux/mfd/max77541.h
 
-- 
2.30.2

[PATCH 3/5] staging: drivers: regulator: Add MAX77541 Regulator Support

[Okan Sahin]

This patch adds regulator driver for both MAX77541 and MAX77540.
The MAX77541 is a high-efficiency step-down converter
with two 3A switching phases for single-cell Li+ battery and 5VDC systems.

The MAX77540 is a high-efficiency step-down converter
with two 3A switching phases.

Signed-off-by: Okan Sahin <okan.sahin@analog.com>
---
 MAINTAINERS                            |   1 +
 drivers/regulator/Kconfig              |   9 ++
 drivers/regulator/Makefile             |   1 +
 drivers/regulator/max77541-regulator.c | 181 +++++++++++++++++++++++++
 4 files changed, 192 insertions(+)
 create mode 100644 drivers/regulator/max77541-regulator.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 22f5a9c490e3..5704ed5afce3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12503,6 +12503,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/adi,max77541.yaml
 F:	drivers/mfd/max77541.c
+F:	drivers/regulator/max77541-regulator.c
 F:	include/linux/mfd/max77541.h
 
 MAXIM MAX77650 PMIC MFD DRIVER
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 070e4403c6c2..1e416c195af9 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -556,6 +556,15 @@ config REGULATOR_MAX597X
 	  The MAX5970/5978 is a smart switch with no output regulation, but
 	  fault protection and voltage and current monitoring capabilities.
 
+config REGULATOR_MAX77541
+	tristate "Analog Devices MAX77541/77540 Regulator"
+	depends on MFD_MAX77541
+	help
+	  This driver controls a Analog Devices MAX77541/77540 regulators
+	  via I2C bus. Both MAX77540 and MAX77541 are dual-phase
+	  high-efficiency buck converter. Say Y here to
+	  enable the regulator driver.
+
 config REGULATOR_MAX77620
 	tristate "Maxim 77620/MAX20024 voltage regulator"
 	depends on MFD_MAX77620 || COMPILE_TEST
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 5962307e1130..c19efc7cfbef 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_REGULATOR_LTC3676) += ltc3676.o
 obj-$(CONFIG_REGULATOR_MAX14577) += max14577-regulator.o
 obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
 obj-$(CONFIG_REGULATOR_MAX597X) += max597x-regulator.o
+obj-$(CONFIG_REGULATOR_MAX77541) += max77541-regulator.o
 obj-$(CONFIG_REGULATOR_MAX77620) += max77620-regulator.o
 obj-$(CONFIG_REGULATOR_MAX77650) += max77650-regulator.o
 obj-$(CONFIG_REGULATOR_MAX8649)	+= max8649.o
diff --git a/drivers/regulator/max77541-regulator.c b/drivers/regulator/max77541-regulator.c
new file mode 100644
index 000000000000..9204b15f8eed
--- /dev/null
+++ b/drivers/regulator/max77541-regulator.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Analog Devices, Inc.
+ * ADI Regulator driver for the MAX77540 and MAX77541
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/max77541.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+static const struct regulator_ops max77541_buck_ops = {
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.list_voltage		= regulator_list_voltage_pickable_linear_range,
+	.get_voltage_sel	= regulator_get_voltage_sel_pickable_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_pickable_regmap,
+};
+
+static const struct linear_range max77540_buck_ranges[] = {
+	/* Ranges when VOLT_SEL bits are 0x00 */
+	REGULATOR_LINEAR_RANGE(500000, 0x00, 0x8B, 5000),
+	REGULATOR_LINEAR_RANGE(1200000, 0x8C, 0xFF, 0),
+	/* Ranges when VOLT_SEL bits are 0x40 */
+	REGULATOR_LINEAR_RANGE(1200000, 0x00, 0x8B, 10000),
+	REGULATOR_LINEAR_RANGE(2400000, 0x8C, 0xFF, 0),
+	/* Ranges when VOLT_SEL bits are  0x80 */
+	REGULATOR_LINEAR_RANGE(2000000, 0x00, 0x9F, 20000),
+	REGULATOR_LINEAR_RANGE(5200000, 0xA0, 0xFF, 0),
+};
+
+static const struct linear_range max77541_buck_ranges[] = {
+	/* Ranges when VOLT_SEL bits are 0x00 */
+	REGULATOR_LINEAR_RANGE(300000, 0x00, 0xB3, 5000),
+	REGULATOR_LINEAR_RANGE(1200000, 0xB4, 0xFF, 0),
+	/* Ranges when VOLT_SEL bits are 0x40 */
+	REGULATOR_LINEAR_RANGE(1200000, 0x00, 0x8B, 10000),
+	REGULATOR_LINEAR_RANGE(2400000, 0x8C, 0xFF, 0),
+	/* Ranges when VOLT_SEL bits are  0x80 */
+	REGULATOR_LINEAR_RANGE(2000000, 0x00, 0x9F, 20000),
+	REGULATOR_LINEAR_RANGE(5200000, 0xA0, 0xFF, 0),
+};
+
+static const unsigned int max77541_buck_volt_range_sel[] = {
+	0x00, 0x00, 0x40, 0x40, 0x80, 0x80
+};
+
+#define MAX77540_BUCK(_id, _ops)					\
+	{	.id = MAX77541_BUCK ## _id,				\
+		.name = "BUCK"#_id,					\
+		.of_match = "BUCK"#_id,					\
+		.regulators_node = "regulators",			\
+		.enable_reg = MAX77541_REG_EN_CTRL,			\
+		.enable_mask = MAX77541_BIT_M ## _id ## _EN,		\
+		.ops = &(_ops),						\
+		.type = REGULATOR_VOLTAGE,				\
+		.linear_ranges = max77540_buck_ranges,			\
+		.n_linear_ranges = ARRAY_SIZE(max77540_buck_ranges),	\
+		.vsel_reg = MAX77541_REG_M ## _id ## _VOUT,		\
+		.vsel_mask = MAX77541_BITS_MX_VOUT,			\
+		.vsel_range_reg = MAX77541_REG_M ## _id ## _CFG1,	\
+		.vsel_range_mask = MAX77541_BITS_MX_CFG1_RNG,		\
+		.linear_range_selectors = max77541_buck_volt_range_sel, \
+		.owner = THIS_MODULE,					\
+	}
+
+#define MAX77541_BUCK(_id, _ops)					\
+	{	.id = MAX77541_BUCK ## _id,				\
+		.name = "BUCK"#_id,					\
+		.of_match = "BUCK"#_id,					\
+		.regulators_node = "regulators",			\
+		.enable_reg = MAX77541_REG_EN_CTRL,			\
+		.enable_mask = MAX77541_BIT_M ## _id ## _EN,		\
+		.ops = &(_ops),						\
+		.type = REGULATOR_VOLTAGE,				\
+		.linear_ranges = max77541_buck_ranges,			\
+		.n_linear_ranges = ARRAY_SIZE(max77541_buck_ranges),	\
+		.vsel_reg = MAX77541_REG_M ## _id ## _VOUT,		\
+		.vsel_mask = MAX77541_BITS_MX_VOUT,			\
+		.vsel_range_reg = MAX77541_REG_M ## _id ## _CFG1,	\
+		.vsel_range_mask = MAX77541_BITS_MX_CFG1_RNG,		\
+		.linear_range_selectors = max77541_buck_volt_range_sel, \
+		.owner = THIS_MODULE,					\
+	}
+
+static const struct regulator_desc max77540_regulators_desc[] = {
+	MAX77540_BUCK(1, max77541_buck_ops),
+	MAX77540_BUCK(2, max77541_buck_ops)
+};
+
+static const struct regulator_desc max77541_regulators_desc[] = {
+	MAX77541_BUCK(1, max77541_buck_ops),
+	MAX77541_BUCK(2, max77541_buck_ops)
+};
+
+struct max77541_regulator_dev {
+	struct device *dev;
+	struct max77541_dev *max77541;
+};
+
+static int max77541_regulator_probe(struct platform_device *pdev)
+{
+	struct max77541_dev *max77541 = dev_get_drvdata(pdev->dev.parent);
+	struct max77541_regulator_dev *regulator;
+	struct regulator_config config = {};
+	struct regulator_dev *rdev;
+	int i;
+
+	regulator = devm_kzalloc(&pdev->dev, sizeof(*regulator), GFP_KERNEL);
+	if (!regulator)
+		return -ENOMEM;
+
+	regulator->dev = &pdev->dev;
+	regulator->max77541 = max77541;
+
+	config.dev = pdev->dev.parent;
+	config.driver_data = regulator;
+	config.regmap = regulator->max77541->regmap;
+
+	for (i = 0; i < MAX77541_MAX_REGULATORS; i++) {
+		switch (regulator->max77541->type) {
+		case MAX77540:
+			rdev = devm_regulator_register(&pdev->dev,
+						       &max77540_regulators_desc[i], &config);
+			if (IS_ERR(rdev))
+				return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
+							"Failed to register regulator\n");
+			break;
+		case MAX77541:
+			rdev = devm_regulator_register(&pdev->dev,
+						       &max77541_regulators_desc[i], &config);
+			if (IS_ERR(rdev))
+				return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
+						"Failed to register regulator\n");
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id max77541_regulator_platform_id[] = {
+	{ "max77540-regulator", MAX77540 },
+	{ "max77541-regulator", MAX77541 },
+	{  /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
+
+static const struct of_device_id max77541_regulator_of_id[] = {
+	{
+		.compatible = "adi,max77540-regulator",
+		.data = (void *)MAX77540,
+	},
+	{
+		.compatible = "adi,max77541-regulator",
+		.data = (void *)MAX77541,
+	},
+	{ /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);
+
+static struct platform_driver max77541_regulator_driver = {
+	.driver = {
+		.name = "max77541-regulator",
+		.of_match_table = max77541_regulator_of_id,
+	},
+	.probe = max77541_regulator_probe,
+	.id_table = max77541_regulator_platform_id,
+};
+
+module_platform_driver(max77541_regulator_driver);
+
+MODULE_AUTHOR("Okan Sahin <Okan.Sahin@analog.com>");
+MODULE_DESCRIPTION("MAX77540/MAX77541 regulator driver");
+MODULE_LICENSE("GPL");
-- 
2.30.2

[PATCH 4/5] staging: dt-bindings: regulator: adi,max77541.yaml Add MAX77541 Regulator bindings

[Okan Sahin]

This patch adds document the bindings for MAX77541 and MAX77540
regulator drivers.

Signed-off-by: Okan Sahin <okan.sahin@analog.com>
---
 .../bindings/regulator/adi,max77541.yaml      | 44 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541.yaml

diff --git a/Documentation/devicetree/bindings/regulator/adi,max77541.yaml b/Documentation/devicetree/bindings/regulator/adi,max77541.yaml
new file mode 100644
index 000000000000..1f828895ab3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,max77541.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/adi,max77541.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Buck Converter driver for MAX77540/MAX77541
+
+maintainers:
+  - Okan Sahin <okan.sahin@analog.com>
+
+description: |
+  This is a part of device tree bindings for ADI MAX77540/MAX77541
+
+  The buck convertere is represented as a sub-node of the PMIC node on the device tree.
+
+  The device has two buck regulators.
+  See also Documentation/devicetree/bindings/mfd/adi,max77541.yaml for
+  additional information and example.
+
+properties:
+  compatible:
+    enum:
+      - adi,max77540-regulator
+      - adi,max77541-regulator
+
+patternProperties:
+  "^BUCK[12]$":
+    type: object
+    $ref: regulator.yaml#
+    additionalProperties: false
+    description: |
+      Buck regulator.
+
+    properties:
+      regulator-name: true
+      regulator-always-on: true
+      regulator-boot-on: true
+      regulator-min-microvolt:
+        minimum: 300000
+      regulator-max-microvolt:
+        maximum: 5200000
+
+additionalProperties: false
\ No newline at end of file
diff --git a/MAINTAINERS b/MAINTAINERS
index 5704ed5afce3..8e5572b28a8c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12502,6 +12502,7 @@ M:	Okan Sahin <okan.sahin@analog.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/adi,max77541.yaml
+F:	Documentation/devicetree/bindings/regulator/adi,max77541.yaml
 F:	drivers/mfd/max77541.c
 F:	drivers/regulator/max77541-regulator.c
 F:	include/linux/mfd/max77541.h
-- 
2.30.2

[PATCH 5/5] staging: drivers: iio: adc: Adc MAX77541 ADC Support

[Okan Sahin]

This patch add adc support for MAX77541.

The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
with four multiplexers for supporting the telemetry feature

Signed-off-by: Okan Sahin <okan.sahin@analog.com>
---
 MAINTAINERS                    |   1 +
 drivers/iio/adc/Kconfig        |  11 ++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/max77541-adc.c | 224 +++++++++++++++++++++++++++++++++
 4 files changed, 237 insertions(+)
 create mode 100644 drivers/iio/adc/max77541-adc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e5572b28a8c..18ce4644cc75 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12503,6 +12503,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/adi,max77541.yaml
 F:	Documentation/devicetree/bindings/regulator/adi,max77541.yaml
+F:	drivers/iio/adc/max77541-adc.c
 F:	drivers/mfd/max77541.c
 F:	drivers/regulator/max77541-regulator.c
 F:	include/linux/mfd/max77541.h
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 791612ca6012..2e7833b33f12 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -696,6 +696,17 @@ config MAX1363
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1363.
 
+config MAX77541_ADC
+	tristate "Analog Devices MAX77541 ADC driver"
+	depends on MFD_MAX77541
+	help
+	  This driver controls a Analog Devices MAX77541 adc
+	  via I2C bus. This device has one adc. Say yes here to build
+	  support for Analog Devices MAX77541 ADC interface.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max77541-adc.
+
 config MAX9611
 	tristate "Maxim max9611/max9612 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 46caba7a010c..03774cccbb4b 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_MAX1118) += max1118.o
 obj-$(CONFIG_MAX11205) += max11205.o
 obj-$(CONFIG_MAX1241) += max1241.o
 obj-$(CONFIG_MAX1363) += max1363.o
+obj-$(CONFIG_MAX77541_ADC) += max77541-adc.o
 obj-$(CONFIG_MAX9611) += max9611.o
 obj-$(CONFIG_MCP320X) += mcp320x.o
 obj-$(CONFIG_MCP3422) += mcp3422.o
diff --git a/drivers/iio/adc/max77541-adc.c b/drivers/iio/adc/max77541-adc.c
new file mode 100644
index 000000000000..7ca8576bd4e1
--- /dev/null
+++ b/drivers/iio/adc/max77541-adc.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Analog Devices, Inc.
+ * ADI MAX77541 ADC Driver with IIO interface
+ */
+
+#include <linux/bitfield.h>
+#include <linux/iio/iio.h>
+#include <include/linux/mfd/max77541.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#define MAX77541_ADC_CHANNEL(_channel, _name, _type, _reg) \
+	{							\
+		.type = _type,					\
+		.indexed = 1,					\
+		.channel = _channel,				\
+		.address = _reg,				\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+				      BIT(IIO_CHAN_INFO_SCALE) |\
+				      BIT(IIO_CHAN_INFO_OFFSET),\
+		.datasheet_name = _name,			\
+	}
+
+enum {
+	MAX77541_ADC_CH1_I = 0,
+	MAX77541_ADC_CH2_I,
+	MAX77541_ADC_CH3_I,
+	MAX77541_ADC_CH6_I,
+
+	MAX77541_ADC_IRQMAX_I,
+};
+
+struct max77541_adc_iio {
+	struct regmap	*regmap;
+	int irq;
+	int irq_arr[MAX77541_ADC_IRQMAX_I];
+};
+
+enum max77541_adc_channel {
+	MAX77541_ADC_VSYS_V = 0,
+	MAX77541_ADC_VOUT1_V,
+	MAX77541_ADC_VOUT2_V,
+	MAX77541_ADC_TEMP,
+};
+
+static int max77541_adc_offset(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       int *val, int *val2)
+{
+	switch (chan->channel) {
+	case MAX77541_ADC_VSYS_V:
+	case MAX77541_ADC_VOUT1_V:
+	case MAX77541_ADC_VOUT2_V:
+		*val = 0;
+		*val2 = 0;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case MAX77541_ADC_TEMP:
+		*val = -273;
+		*val2 = 0;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max77541_adc_scale(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int *val, int *val2)
+{
+	struct max77541_adc_iio *info = iio_priv(indio_dev);
+	unsigned int reg_val;
+	int ret;
+
+	switch (chan->channel) {
+	case MAX77541_ADC_VSYS_V:
+		*val = 0;
+		*val2 = 25000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case MAX77541_ADC_VOUT1_V:
+		ret = regmap_read(info->regmap, MAX77541_REG_M2_CFG1, &reg_val);
+		if (ret)
+			return ret;
+
+		reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);
+
+		*val = 0;
+
+		if (reg_val == LOW_RANGE)
+			*val2 = 6250;
+		else if (reg_val == MID_RANGE)
+			*val2 = 12500;
+		else if (reg_val == HIGH_RANGE)
+			*val2 = 25000;
+		else
+			return -EINVAL;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	case MAX77541_ADC_VOUT2_V:
+		ret = regmap_read(info->regmap, MAX77541_REG_M2_CFG1, &reg_val);
+		if (ret)
+			return ret;
+		reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);
+
+		*val = 0;
+
+		if (reg_val == LOW_RANGE)
+			*val2 = 6250;
+		else if (reg_val == MID_RANGE)
+			*val2 = 12500;
+		else if (reg_val == HIGH_RANGE)
+			*val2 = 25000;
+		else
+			return -EINVAL;
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	case MAX77541_ADC_TEMP:
+		*val = 1;
+		*val2 = 725000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max77541_adc_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val)
+{
+	struct max77541_adc_iio *info = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_read(info->regmap, chan->address, val);
+	if (ret)
+		return ret;
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_chan_spec max77541_adc_channels[] = {
+	MAX77541_ADC_CHANNEL(MAX77541_ADC_VSYS_V, "vsys_v", IIO_VOLTAGE,
+			     MAX77541_REG_ADC_DATA_CH1),
+	MAX77541_ADC_CHANNEL(MAX77541_ADC_VOUT1_V, "vout1_v", IIO_VOLTAGE,
+			     MAX77541_REG_ADC_DATA_CH2),
+	MAX77541_ADC_CHANNEL(MAX77541_ADC_VOUT2_V, "vout2_v", IIO_VOLTAGE,
+			     MAX77541_REG_ADC_DATA_CH3),
+	MAX77541_ADC_CHANNEL(MAX77541_ADC_TEMP, "temp", IIO_TEMP,
+			     MAX77541_REG_ADC_DATA_CH6),
+};
+
+static int max77541_adc_read_raw(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan,
+				 int *val, int *val2, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_OFFSET:
+		return max77541_adc_offset(indio_dev, chan, val, val2);
+	case IIO_CHAN_INFO_SCALE:
+		return max77541_adc_scale(indio_dev, chan, val, val2);
+	case IIO_CHAN_INFO_RAW:
+		return max77541_adc_raw(indio_dev, chan, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info max77541_adc_info = {
+	.read_raw = max77541_adc_read_raw,
+};
+
+static int max77541_adc_probe(struct platform_device *pdev)
+{
+	struct max77541_dev *max77541;
+	struct max77541_adc_iio *info;
+	struct iio_dev *indio_dev;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	info = iio_priv(indio_dev);
+
+	info->regmap = max77541->regmap;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	indio_dev->name = platform_get_device_id(pdev)->name;
+	indio_dev->info = &max77541_adc_info;
+	indio_dev->channels = max77541_adc_channels;
+	indio_dev->num_channels = ARRAY_SIZE(max77541_adc_channels);
+
+	return devm_iio_device_register(&pdev->dev, indio_dev);
+}
+
+static const struct platform_device_id max77541_adc_platform_id[] = {
+	{ "max77541-adc", MAX77541, },
+	{  /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(platform, max77541_adc_platform_id);
+
+static const struct of_device_id max77541_adc_of_id[] = {
+	{
+		.compatible = "adi,max77541-adc",
+		.data = (void *)MAX77541,
+	},
+	{  /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(of, max77541_adc_of_id);
+
+static struct platform_driver max77541_adc_driver = {
+	.driver = {
+		.name = "max77541-adc",
+		.of_match_table = max77541_adc_of_id,
+	},
+	.probe = max77541_adc_probe,
+	.id_table = max77541_adc_platform_id,
+};
+
+module_platform_driver(max77541_adc_driver);
+
+MODULE_AUTHOR("Okan Sahin <Okan.Sahin@analog.com>");
+MODULE_DESCRIPTION("MAX77541 ADC driver");
+MODULE_LICENSE("GPL");
-- 
2.30.2

Re: [PATCH 2/5] staging: dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings

[Krzysztof Kozlowski]

On 07/12/2022 10:08, Okan Sahin wrote:
> This patch adds document the bindings for MAX77541 MFD driver. It also

Do not use "This commit/patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> includes MAX77540 driver whose regmap is covered by MAX77541.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77541.yaml | 134 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 135 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/adi,max77541.yaml b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> new file mode 100644
> index 000000000000..205953e6dd15
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/adi,max77541.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MAX77540/MAX77541 PMIC from ADI.

Drop trailing space.

> +
> +maintainers:
> +  - Okan Sahin <okan.sahin@analog.com>
> +
> +description: |
> +  MAX77540 is a Power Management IC with 2 buck regulators.
> +
> +  MAX77541 is a Power Management IC with 2 buck regulators and 1 ADC.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,max77540
> +      - adi,max77541
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  regulators:
> +    $ref: /schemas/regulator/adi,max77541.yaml#

I don't think you tested this patch. There is no such file.

> +
> +  adc:
> +    type: object
> +    additionalProperties: false
> +    properties:
> +      compatible:
> +        const: adi,max77541-adc

Why having a child without any resources? It does not look like needed
and instead your parent driver should instantiate the child device.

> +    required:
> +      -compatible
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: adi,max77540
> +    then:
> +      properties:
> +        regulator:
> +          properties:
> +            compatible:
> +              const: adi,max77540-regulator
> +    else:
> +      properties:
> +        regulator:
> +          properties:
> +            compatible:
> +              const: adi,max77541-regulator
> +        adc:
> +          properties:
> +            compatible:
> +              const: adi,max77541-adc

The adc part is not needed anyway - duplicating what's in top-level
properties.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic@69 {
> +            compatible = "adi,max77540";
> +            reg = <0x69>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
> +
> +            regulators {
> +                buck1 {
> +                    regulator-min-microvolt = <500000>;
> +                    regulator-max-microvolt = <5200000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +                buck2 {
> +                    regulator-min-microvolt = <500000>;
> +                    regulator-max-microvolt = <5200000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +            };
> +        };
> +    };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic@69 {
> +            compatible = "adi,max77541";

Keep only one example (more complex one) - they are almost the same.

> +            reg = <0x63>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
> +
> +            regulators {
> +                buck1 {
> +                    regulator-min-microvolt = <500000>;
> +                    regulator-max-microvolt = <5200000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +                buck2 {
> +                    regulator-min-microvolt = <500000>;
> +                    regulator-max-microvolt = <5200000>;
> +                    regulator-boot-on;
> +                    regulator-always-on;
> +                };
> +            };
> +
> +            adc {
> +                compatible = "adi,max77541-adc";
> +            }
> +        };
> +    };
> \ No newline at end of file

Error here.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index af94d06bb9f0..22f5a9c490e3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12501,6 +12501,7 @@ MAXIM MAX77541 PMIC MFD DRIVER
>  M:	Okan Sahin <okan.sahin@analog.com>
>  L:	linux-kernel@vger.kernel.org
>  S:	Maintained
> +F:	Documentation/devicetree/bindings/mfd/adi,max77541.yaml
>  F:	drivers/mfd/max77541.c
>  F:	include/linux/mfd/max77541.h
>  

Best regards,
Krzysztof

Re: [PATCH 4/5] staging: dt-bindings: regulator: adi,max77541.yaml Add MAX77541 Regulator bindings

[Krzysztof Kozlowski]

On 07/12/2022 10:08, Okan Sahin wrote:
> This patch adds document the bindings for MAX77541 and MAX77540
> regulator drivers.

Do not use "This commit/patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../bindings/regulator/adi,max77541.yaml      | 44 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/adi,max77541.yaml b/Documentation/devicetree/bindings/regulator/adi,max77541.yaml
> new file mode 100644
> index 000000000000..1f828895ab3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/adi,max77541.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/adi,max77541.yaml#

Filename matching compatible, so adi,max77541-regulator.yaml

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Buck Converter driver for MAX77540/MAX77541

Drop "driver" and any other references to Linux drivers.

> +
> +maintainers:
> +  - Okan Sahin <okan.sahin@analog.com>
> +
> +description: |
> +  This is a part of device tree bindings for ADI MAX77540/MAX77541
> +
> +  The buck convertere is represented as a sub-node of the PMIC node on the device tree.

Typo, converter

> +
> +  The device has two buck regulators.
> +  See also Documentation/devicetree/bindings/mfd/adi,max77541.yaml for
> +  additional information and example.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,max77540-regulator
> +      - adi,max77541-regulator
> +
> +patternProperties:
> +  "^BUCK[12]$":

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +    type: object
> +    $ref: regulator.yaml#
> +    additionalProperties: false
> +    description: |
> +      Buck regulator.
> +
> +    properties:
> +      regulator-name: true
> +      regulator-always-on: true
> +      regulator-boot-on: true
> +      regulator-min-microvolt:
> +        minimum: 300000
> +      regulator-max-microvolt:
> +        maximum: 5200000
> +
> +additionalProperties: false
> \ No newline at end of file

Check your patches before sending...

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5704ed5afce3..8e5572b28a8c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12502,6 +12502,7 @@ M:	Okan Sahin <okan.sahin@analog.com>
>  L:	linux-kernel@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> +F:	Documentation/devicetree/bindings/regulator/adi,max77541.yaml
>  F:	drivers/mfd/max77541.c
>  F:	drivers/regulator/max77541-regulator.c
>  F:	include/linux/mfd/max77541.h

Best regards,
Krzysztof

Re: [PATCH 0/5] staging: drivers: mfd: Add MAX77541 MFD and related device drivers

[Andy Shevchenko]

On Wed, Dec 07, 2022 at 12:08:39PM +0300, Okan Sahin wrote:
> This patchset adds mfd, regulator and adc driver and related bindings.The patches 
> are required to be applied in sequence.

You have an indentation / wrapping issues in the above text.

Nevertheless, why staging? What does it mean?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/5] staging: drivers: regulator: Add MAX77541 Regulator Support

[Andy Shevchenko]

On Wed, Dec 07, 2022 at 12:08:42PM +0300, Okan Sahin wrote:
> This patch adds regulator driver for both MAX77541 and MAX77540.

Read Submitting Patches documentation on how to create better commit message.

> The MAX77541 is a high-efficiency step-down converter
> with two 3A switching phases for single-cell Li+ battery and 5VDC systems.
> 
> The MAX77540 is a high-efficiency step-down converter
> with two 3A switching phases.

...

> +/*
> + * Copyright (c) 2022 Analog Devices, Inc.
> + * ADI Regulator driver for the MAX77540 and MAX77541

> + *

Redundant blank line.

> + */

...

> +static const unsigned int max77541_buck_volt_range_sel[] = {
> +	0x00, 0x00, 0x40, 0x40, 0x80, 0x80

You can leave trailing comma.

> +};

...

> +static const struct regulator_desc max77540_regulators_desc[] = {
> +	MAX77540_BUCK(1, max77541_buck_ops),
> +	MAX77540_BUCK(2, max77541_buck_ops)

Ditto.

> +};
> +
> +static const struct regulator_desc max77541_regulators_desc[] = {
> +	MAX77541_BUCK(1, max77541_buck_ops),
> +	MAX77541_BUCK(2, max77541_buck_ops)

Ditto.

> +};

...

> +struct max77541_regulator_dev {
> +	struct device *dev;

Isn't it the same as...

> +	struct max77541_dev *max77541;

...max77541->dev ?

> +};

...

> +static int max77541_regulator_probe(struct platform_device *pdev)
> +{

	truct device *dev = &pdev->dev;

will save you a bit of code below.

> +	struct max77541_dev *max77541 = dev_get_drvdata(pdev->dev.parent);


...


> +				return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
> +							"Failed to register regulator\n");

Wrong indentation.

...

> +				return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
> +						"Failed to register regulator\n");

Ditto.

...

> +

Redundant blank line.

> +module_platform_driver(max77541_regulator_driver);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] staging: drivers: mfd: Add MAX77541/MAX77540 PMIC Support

[Andy Shevchenko]

On Wed, Dec 07, 2022 at 12:08:40PM +0300, Okan Sahin wrote:
> This patch adds MFD driver for MAX77541/MAX77540 to enable its sub
> devices.
> 
> The MAX77541 is a multi-function devices. It includes
> buck converter and ADC.
> 
> The MAX77540 is a high-efficiency buck converter
> with two 3A switching phases.
> 
> They have same regmap except for ADC part of MAX77541.

Same comment as per patch 2.

...

> +	help
> +	  Say yes here to add support for Analog Devices
> +	  MAX77541 and MAX77540 Power Management ICs.
> +	  This driver provides common support for accessing the device;
> +	  additional drivers must be enabled in order to use the functionality
> +	  of the device.

Arbitrary line lengths.

...

> +		return devm_mfd_add_devices(dev, -1, max77540_devs, ARRAY_SIZE(max77540_devs),

There is a definition for -1, use it.

> +					    NULL, 0, NULL);

...

> +		return devm_mfd_add_devices(dev, -1, max77541_devs, ARRAY_SIZE(max77541_devs),

Ditto.

> +					    NULL, 0, NULL);

...

> +	chip_id = to_i2c_driver(client->dev.driver)->id_table;

> +	if (!chip_data) {
> +		chip_data = (void *)i2c_match_id(chip_id, client)->driver_data;


> +	}

We have a new helper for this.

...

> +		return dev_err_probe(me->dev,  PTR_ERR(me->regmap),
> +					"Failed to allocate register map\n");

Wrong indentation.

...

> +

Redundant blank line.

> +module_i2c_driver(max77541_i2c_driver);

...

> +MODULE_VERSION("1.0");

Why?

...

Missing inclusions:
 - bits.h
 - types.h

Missing forward declarations for:
	struct device
	struct regmap
	struct regmap_irq_chip_data
	...

> +/*      REGISTERS       */

...

> +#define MAX77541_REGMAP_IRQ_REG(_mask)	\
> +	{ .mask = (_mask),  }

When {} are on one line, the trailing comma inside is not needed.

...

> +enum mx_range {
> +	LOW_RANGE,
> +	MID_RANGE,
> +	HIGH_RANGE,

> +	RESERVED

Is it terminator?

> +};

...

> +struct max77541_dev {

> +	void *pdata;

Why do you need this?

> +	struct device *dev;

Isn't it the same as dev inside regmap?

> +	struct regmap_irq_chip_data *irq_data;
> +	struct regmap_irq_chip_data *irq_buck;
> +	struct regmap_irq_chip_data *irq_topsys;
> +	struct regmap_irq_chip_data *irq_adc;

> +	struct i2c_client *i2c;

Is this is really needed? Perhaps regmap below provides what you need, no?

> +	struct regmap *regmap;
> +
> +	u8 type;
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/5] staging: drivers: iio: adc: Adc MAX77541 ADC Support

[Andy Shevchenko]

On Wed, Dec 07, 2022 at 12:08:44PM +0300, Okan Sahin wrote:
> This patch add adc support for MAX77541.
> 
> The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
> with four multiplexers for supporting the telemetry feature

Same comment as per patch 2.

...

> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>

> +#include <include/linux/mfd/max77541.h>

Hmm...

> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/of_regulator.h>

...

> +enum {
> +	MAX77541_ADC_CH1_I = 0,
> +	MAX77541_ADC_CH2_I,
> +	MAX77541_ADC_CH3_I,
> +	MAX77541_ADC_CH6_I,
> +
> +	MAX77541_ADC_IRQMAX_I,

If it's a terminator, drop the trailing comma.

> +};

...

> +	case MAX77541_ADC_TEMP:
> +		*val = -273;

I believe we have definition for this in units.h. Can you use it?

> +		*val2 = 0;
> +		return IIO_VAL_INT_PLUS_MICRO;


> +	}
> +}

...

> +		*val = 0;
> +
> +		if (reg_val == LOW_RANGE)
> +			*val2 = 6250;
> +		else if (reg_val == MID_RANGE)
> +			*val2 = 12500;
> +		else if (reg_val == HIGH_RANGE)
> +			*val2 = 25000;
> +		else
> +			return -EINVAL;

Can it be provided as a table?

...

> +		*val = 0;
> +
> +		if (reg_val == LOW_RANGE)
> +			*val2 = 6250;
> +		else if (reg_val == MID_RANGE)
> +			*val2 = 12500;
> +		else if (reg_val == HIGH_RANGE)
> +			*val2 = 25000;
> +		else
> +			return -EINVAL;

Ditto.

...

> +

Redundant blank line.

> +module_platform_driver(max77541_adc_driver);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] staging: dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings

[Rob Herring]

On Wed, 07 Dec 2022 12:08:41 +0300, Okan Sahin wrote:
> This patch adds document the bindings for MAX77541 MFD driver. It also
> includes MAX77540 driver whose regmap is covered by MAX77541.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77541.yaml | 134 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 135 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/mfd/adi,max77541.yaml:134:7: [error] no new line character at the end of file (new-line-at-end-of-file)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77541.yaml: properties:adc:required: '-compatible' is not of type 'array'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77541.yaml: properties:adc:required: '-compatible' is not of type 'array'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
./Documentation/devicetree/bindings/mfd/adi,max77541.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/regulator/adi,max77541.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77541.yaml: ignoring, error in schema: properties: adc: required
Error: Documentation/devicetree/bindings/mfd/adi,max77541.example.dts:99.13-14 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:406: Documentation/devicetree/bindings/mfd/adi,max77541.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1492: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221207090906.5896-3-okan.sahin@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 4/5] staging: dt-bindings: regulator: adi,max77541.yaml Add MAX77541 Regulator bindings

[Rob Herring]

On Wed, 07 Dec 2022 12:08:43 +0300, Okan Sahin wrote:
> This patch adds document the bindings for MAX77541 and MAX77540
> regulator drivers.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../bindings/regulator/adi,max77541.yaml      | 44 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/regulator/adi,max77541.yaml:44:28: [error] no new line character at the end of file (new-line-at-end-of-file)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):
Documentation/devicetree/bindings/regulator/adi,max77541.yaml: Documentation/devicetree/bindings/mfd/adi,max77541.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221207090906.5896-5-okan.sahin@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 0/5] staging: drivers: mfd: Add MAX77541 MFD and related device drivers

[Jonathan Cameron]

On Wed, 7 Dec 2022 13:09:34 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, Dec 07, 2022 at 12:08:39PM +0300, Okan Sahin wrote:
> > This patchset adds mfd, regulator and adc driver and related bindings.The patches 
> > are required to be applied in sequence.  
> 
> You have an indentation / wrapping issues in the above text.
> 
> Nevertheless, why staging? What does it mean?
> 

The main reason to go via staging is because a driver is sitting out
of tree and it is useful to bring it in on the basis that it can then be
cleaned up in tree before moving out of staging.

For a relatively small driver like this, that's hard to argue.  Just
clean it up in response to review feedback and then we can take it
directly into relevant subsystems in the main tree.

Jonathan

Re: [PATCH 1/5] staging: drivers: mfd: Add MAX77541/MAX77540 PMIC Support

[Jonathan Cameron]

On Wed, 7 Dec 2022 12:08:40 +0300
Okan Sahin <okan.sahin@analog.com> wrote:

> This patch adds MFD driver for MAX77541/MAX77540 to enable its sub
> devices.
> 
> The MAX77541 is a multi-function devices. It includes
> buck converter and ADC.
> 
> The MAX77540 is a high-efficiency buck converter
> with two 3A switching phases.
> 
> They have same regmap except for ADC part of MAX77541.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>

I'll try not to duplicate the points Andy has already made, but there
may be some overlap!

> ---
>  MAINTAINERS                  |   7 ++
>  drivers/mfd/Kconfig          |  13 ++
>  drivers/mfd/Makefile         |   1 +
>  drivers/mfd/max77541.c       | 236 +++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max77541.h | 150 ++++++++++++++++++++++

Huh? It's not in staging, so just change the patch titles!

>  5 files changed, 407 insertions(+)
>  create mode 100644 drivers/mfd/max77541.c
>  create mode 100644 include/linux/mfd/max77541.h
> 
or MAX77620 and MAX20024 PMIC Support"
>  	depends on I2C=y
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 7ed3ef4a698c..bf21228f5742 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -161,6 +161,7 @@ obj-$(CONFIG_MFD_DA9063)	+= da9063.o
>  obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>  
>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
> +obj-$(CONFIG_MFD_MAX77541)	+= max77541.o
>  obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>  obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
> diff --git a/drivers/mfd/max77541.c b/drivers/mfd/max77541.c
> new file mode 100644
> index 000000000000..97a2df3ce0b6
> --- /dev/null
> +++ b/drivers/mfd/max77541.c
> @@ -0,0 +1,236 @@

...


> +
> +static int max77541_pmic_setup(struct max77541_dev *me)
> +{
> +	struct regmap *regmap = me->regmap;
> +	struct device *dev = me->dev;
> +	unsigned int val;
> +	int ret;
> +
> +	ret = max77541_pmic_irq_init(me);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to initialize IRQ\n");
> +
> +	ret = regmap_read(regmap, MAX77541_REG_INT_SRC, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(regmap, MAX77541_REG_TOPSYS_INT, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(regmap, MAX77541_REG_BUCK_INT, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = device_init_wakeup(dev, true);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Unable to init wakeup\n");
> +
> +	switch (me->type) {
> +	case MAX77540:
once you've switched to using a chip_info structure instead this block just becomes

	return devm_mfd_add_devices(dev, -1, me->chip_info.devs, me->chip_info.num_devs,
				    NULL, 0, NULL);

> +		return devm_mfd_add_devices(dev, -1, max77540_devs, ARRAY_SIZE(max77540_devs),
> +					    NULL, 0, NULL);
> +	case MAX77541:
> +		return devm_mfd_add_devices(dev, -1, max77541_devs, ARRAY_SIZE(max77541_devs),
> +					    NULL, 0, NULL);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int max77541_i2c_probe(struct i2c_client *client)
> +{
> +	const struct i2c_device_id *chip_id;
> +	struct max77541_dev *me;
> +	const void *chip_data;
> +
> +	me = devm_kzalloc(&client->dev, sizeof(*me), GFP_KERNEL);
> +	if (!me)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, me);
> +	me->dev = &client->dev;
> +	me->i2c = client;
> +
> +	chip_id = to_i2c_driver(client->dev.driver)->id_table;
> +
> +	chip_data = device_get_match_data(me->dev);
> +	if (!chip_data) {
> +		chip_data = (void *)i2c_match_id(chip_id, client)->driver_data;

Ah. This is why your enum doesn't have 0 in it. Solve that by using a poitner
to a chip_info structure rather than an enum value

> +		if (!chip_data)
> +			return dev_err_probe(me->dev, -ENODEV, "Unable to find device\n");

Message is rather misleading. More a case of unknown device.

> +	}
> +
> +	me->type = (enum dev_type)chip_data;
> +	me->regmap = devm_regmap_init_i2c(client, &max77541_regmap_config);
> +	if (IS_ERR(me->regmap))
> +		return dev_err_probe(me->dev,  PTR_ERR(me->regmap),
> +					"Failed to allocate register map\n");
> +
> +	return max77541_pmic_setup(me);
> +}
> +
> +static const struct of_device_id max77541_of_id[] = {
> +	{
> +		.compatible = "adi,max77540",
> +		.data = (void *)MAX77540,
> +	},
> +	{
> +		.compatible = "adi,max77541",
> +		.data = (void *)MAX77541,
> +	},
> +	{ /* sentinel */  }
> +};
> +MODULE_DEVICE_TABLE(of, max77541_of_id);
> +
> +static const struct i2c_device_id max77541_i2c_id[] = {
> +	{ "max77540", MAX77540 },
> +	{ "max77541", MAX77541 },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max77541_i2c_id);
> +
> +static struct i2c_driver max77541_i2c_driver = {
> +	.driver = {
> +		.name = "max77541",
> +		.of_match_table = max77541_of_id,
> +	},
> +	.probe_new = max77541_i2c_probe,
> +	.id_table = max77541_i2c_id,
> +};
> +
> +module_i2c_driver(max77541_i2c_driver);
> +
> +MODULE_DESCRIPTION("MAX7740/MAX7741 MFD Driver");
> +MODULE_AUTHOR("Okan Sahin <okan.sahin@analog.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION("1.0");

We almost never assign module versions any more.  They aren't useful
given we must maintain backwards compatibility of userspace interfaces anyway.

> diff --git a/include/linux/mfd/max77541.h b/include/linux/mfd/max77541.h
> new file mode 100644
> index 000000000000..6f2753300227
> --- /dev/null
> +++ b/include/linux/mfd/max77541.h
> @@ -0,0 +1,150 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef __MAX77541_MFD_H__
> +#define __MAX77541_MFD_H__
> +
> +/*      REGISTERS       */
> +
> +/*      GLOBAL CONFIG1       */
> +#define MAX77541_REG_INT_SRC                    0x00
> +#define MAX77541_REG_INT_SRC_M                  0x01
> +#define MAX77541_REG_TOPSYS_INT                 0x02
> +#define MAX77541_REG_TOPSYS_INT_M               0x03
> +#define MAX77541_REG_TOPSYS_STAT                0x04
> +#define MAX77541_REG_DEVICE_CFG1                0x06
> +#define MAX77541_REG_DEVICE_CFG2                0x07
> +#define MAX77541_REG_TOPSYS_CFG                 0x08
> +#define MAX77541_REG_PROT_CFG                   0x09
> +#define MAX77541_REG_EN_CTRL                    0x0B
> +
> +/*      GLOBAL CONFIG2       */

Hmm.  A register called CFG1 is CONFIG2?
That's novel ;)


> +#define MAX77541_REG_GLB_CFG1                   0x11
> +
> +/*      BUCK CONFIG       */
> +#define MAX77541_REG_BUCK_INT                   0x20
> +#define MAX77541_REG_BUCK_INT_M                 0x21
> +#define MAX77541_REG_BUCK_STAT                  0x22
> +
> +/*      BUCK1       */
> +#define MAX77541_REG_M1_VOUT                    0x23
> +#define MAX77541_REG_M1_CFG1                    0x25
> +#define MAX77541_REG_M1_CFG2                    0x26
> +#define MAX77541_REG_M1_CFG3                    0x27
> +
> +/*      BUCK2       */
> +#define MAX77541_REG_M2_VOUT                    0x33
> +#define MAX77541_REG_M2_CFG1                    0x35
> +#define MAX77541_REG_M2_CFG2                    0x36
> +#define MAX77541_REG_M2_CFG3                    0x37
> +
> +#define MAX77541_REG_NOT_AVAILABLE              0xFF
> +
> +/* INTERRUPT MASKS*/
> +#define MAX77541_REG_INT_SRC_MASK               0x00
> +#define MAX77541_REG_TOPSYS_INT_MASK            0x00
> +#define MAX77541_REG_BUCK_INT_MASK              0x00
> +
> +/*BITS OF REGISTERS*/
> +
> +#define MAX77541_BIT_INT_SRC_TOPSYS             BIT(0)
> +#define MAX77541_BIT_INT_SRC_BUCK               BIT(1)
> +
> +#define MAX77541_BIT_TOPSYS_INT_TJ_120C         BIT(0)
> +#define MAX77541_BIT_TOPSYS_INT_TJ_140C         BIT(1)
> +#define MAX77541_BIT_TOPSYS_INT_TSHDN           BIT(2)
> +#define MAX77541_BIT_TOPSYS_INT_UVLO            BIT(3)
> +#define MAX77541_BIT_TOPSYS_INT_ALT_SWO         BIT(4)
> +#define MAX77541_BIT_TOPSYS_INT_EXT_FREQ_DET    BIT(5)
> +
> +#define MAX77541_BIT_BUCK_INT_M1_POK_FLT        BIT(0)
> +#define MAX77541_BIT_BUCK_INT_M2_POK_FLT        BIT(1)
> +#define MAX77541_BIT_BUCK_INT_M1_SCFLT          BIT(4)
> +#define MAX77541_BIT_BUCK_INT_M2_SCFLT          BIT(5)
> +
> +#define MAX77541_BITS_DEVICE_CFG1_SEL1_LATCH    GENMASK(4, 0)
> +#define MAX77541_BITS_DEVICE_CFG2_SEL2_LATCH    GENMASK(4, 0)
> +
> +#define MAX77541_BIT_TOPSYS_CFG_DIS_ALT_IN      BIT(0)
> +
> +#define MAX77541_BITS_PROT_CFG_POK_TO           GENMASK(1, 0)
> +#define MAX77541_BIT_PROT_CFG_EN_FTMON          BIT(2)
> +
> +#define MAX77541_BIT_M1_EN                      BIT(0)
> +#define MAX77541_BIT_M2_EN                      BIT(1)
> +#define MAX77541_BIT_M1_LPM                     BIT(4)
> +#define MAX77541_BIT_M2_LPM                     BIT(5)
> +
> +#define MAX77541_BITS_GLB_CFG1_SSTOP_SR         GENMASK(2, 0)

Naming of BITS is unusual for what I think is a field mask.
MSK or MASK is more common.  Cn use that for the single bit
fields as well.

> +#define MAX77541_BITS_GLB_CFG1_SSTRT_SR         GENMASK(5, 3)
> +
> +#define MAX77541_BITS_MX_VOUT                   GENMASK(7, 0)
> +
> +#define MAX77541_BITS_MX_CFG1_RU_SR             GENMASK(2, 0)
> +#define MAX77541_BITS_MX_CFG1_RD_SR             GENMASK(5, 3)
> +#define MAX77541_BITS_MX_CFG1_RNG               GENMASK(7, 6)
> +
> +#define MAX77541_BIT_MX_CFG2_FPWM               BIT(0)
> +#define MAX77541_BIT_MX_CFG2_FSREN              BIT(1)
> +#define MAX77541_BITS_MX_CFG2_SS_PAT            GENMASK(3, 2)
> +#define MAX77541_BITS_MX_CFG2_SS_FREQ           GENMASK(5, 4)
> +#define MAX77541_BITS_MX_CFG2_SS_ENV            GENMASK(7, 6)
> +
> +#define MAX77541_BITS_MX_CFG3_ILIM              GENMASK(1, 0)
> +#define MAX77541_BITS_MX_CFG3_FREQ              GENMASK(3, 2)
> +#define MAX77541_BIT_MX_CFG3_FTRAK              BIT(4)
> +#define MAX77541_BIT_MX_CFG3_RESRESH            BIT(5)
> +#define MAX77541_BIT_MX_CFG3_ADIS1              BIT(6)
> +#define MAX77541_BIT_MX_CFG3_ADIS100            BIT(7)
> +
> +#define MAX77541_MAX_REGULATORS 2
> +
> +/*      ADC       */
> +#define MAX77541_REG_ADC_INT                    0x70
> +#define MAX77541_REG_ADC_MSK                    0x71
> +
> +#define MAX77541_REG_ADC_DATA_CH1               0x72
> +#define MAX77541_REG_ADC_DATA_CH2               0x73
> +#define MAX77541_REG_ADC_DATA_CH3               0x74
> +#define MAX77541_REG_ADC_DATA_CH6               0x77
> +
> +#define MAX77541_BIT_ADC_INT_CH1_I              BIT(0)
> +#define MAX77541_BIT_ADC_INT_CH2_I              BIT(1)
> +#define MAX77541_BIT_ADC_INT_CH3_I              BIT(2)
> +#define MAX77541_BIT_ADC_INT_CH6_I              BIT(5)
> +
> +#define MAX77541_REGMAP_IRQ_REG(_mask)	\
> +	{ .mask = (_mask),  }

Define is longer than the code. Just use the code everywhere
you current use this macro.

> +
> +enum dev_type {
> +	MAX77540 = 1,
If you need to index from 1, add a comment on why.

> +	MAX77541 = 2,
> +};
> +
> +enum max77541_regulators {
> +	MAX77541_BUCK1 = 1,
> +	MAX77541_BUCK2,
> +};
> +
> +enum mx_range {

Prefix this with max77541
Also, probably better to introduce it in the patch where it is used.
For now I've no idea why we need a reserved value for example!

> +	LOW_RANGE,
> +	MID_RANGE,
> +	HIGH_RANGE,
> +	RESERVED
> +};
> +
> +struct max77541_dev {

> +	u8 type;

Rather than using an enum (incidentally the type of this should be a
named enum, not a u8) much better to define a
'chip info' structure that describes the different specific features of
each chip.  That approach ends up much more extensible as new devices
are added to the driver than a constant increase in switch statements.

> +};
> +
> +#endif /* __MAX77541_MFD_H__ */

Re: [PATCH 3/5] staging: drivers: regulator: Add MAX77541 Regulator Support

[Jonathan Cameron]

On Wed, 7 Dec 2022 12:08:42 +0300
Okan Sahin <okan.sahin@analog.com> wrote:

> This patch adds regulator driver for both MAX77541 and MAX77540.
> The MAX77541 is a high-efficiency step-down converter
> with two 3A switching phases for single-cell Li+ battery and 5VDC systems.
> 
> The MAX77540 is a high-efficiency step-down converter
> with two 3A switching phases.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>

...

> +static int max77541_regulator_probe(struct platform_device *pdev)
> +{
> +	struct max77541_dev *max77541 = dev_get_drvdata(pdev->dev.parent);
> +	struct max77541_regulator_dev *regulator;
> +	struct regulator_config config = {};
> +	struct regulator_dev *rdev;
> +	int i;
> +
> +	regulator = devm_kzalloc(&pdev->dev, sizeof(*regulator), GFP_KERNEL);
> +	if (!regulator)
> +		return -ENOMEM;
> +
> +	regulator->dev = &pdev->dev;
> +	regulator->max77541 = max77541;
> +
> +	config.dev = pdev->dev.parent;
> +	config.driver_data = regulator;
> +	config.regmap = regulator->max77541->regmap;
> +
> +	for (i = 0; i < MAX77541_MAX_REGULATORS; i++) {
> +		switch (regulator->max77541->type) {
> +		case MAX77540:

As in mfd driver, better to move this from code to constant data
by using a chip_info type structure.  You could just use the
*_regulator_desc array, but probably better to wrap that in a structure
to reduce difficulty of adding other stuff in future.


> +			rdev = devm_regulator_register(&pdev->dev,
> +						       &max77540_regulators_desc[i], &config);
> +			if (IS_ERR(rdev))
> +				return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
> +							"Failed to register regulator\n");
> +			break;
> +		case MAX77541:
> +			rdev = devm_regulator_register(&pdev->dev,
> +						       &max77541_regulators_desc[i], &config);
> +			if (IS_ERR(rdev))
> +				return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
> +						"Failed to register regulator\n");
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}

Re: [PATCH 5/5] staging: drivers: iio: adc: Adc MAX77541 ADC Support

[Jonathan Cameron]

On Wed, 7 Dec 2022 12:08:44 +0300
Okan Sahin <okan.sahin@analog.com> wrote:

> This patch add adc support for MAX77541.
> 
> The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
> with four multiplexers for supporting the telemetry feature

Good to add a little detail on what features the driver currently supports.

> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>

...

> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 791612ca6012..2e7833b33f12 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -696,6 +696,17 @@ config MAX1363
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called max1363.
>  
> +config MAX77541_ADC
> +	tristate "Analog Devices MAX77541 ADC driver"
> +	depends on MFD_MAX77541
> +	help
> +	  This driver controls a Analog Devices MAX77541 adc
ADC

> +	  via I2C bus. This device has one adc. Say yes here to build
> +	  support for Analog Devices MAX77541 ADC interface.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called max77541-adc.
> +

...

> diff --git a/drivers/iio/adc/max77541-adc.c b/drivers/iio/adc/max77541-adc.c
> new file mode 100644
> index 000000000000..7ca8576bd4e1
> --- /dev/null
> +++ b/drivers/iio/adc/max77541-adc.c
> @@ -0,0 +1,224 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 Analog Devices, Inc.
> + * ADI MAX77541 ADC Driver with IIO interface
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <include/linux/mfd/max77541.h>

Move this driver specific header include to below the main
block. We want to make it clear it is special.

> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/of_regulator.h>

Would be very very surprised to see off_regulator.h correctly included
in an IIO driver.  Check all of these for whether they are necessary
(rather than cut and paste from another driver).

> +
> +#define MAX77541_ADC_CHANNEL(_channel, _name, _type, _reg) \
> +	{							\
> +		.type = _type,					\
> +		.indexed = 1,					\
> +		.channel = _channel,				\
> +		.address = _reg,				\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
> +				      BIT(IIO_CHAN_INFO_SCALE) |\
> +				      BIT(IIO_CHAN_INFO_OFFSET),\
> +		.datasheet_name = _name,			\
> +	}
> +
> +enum {
> +	MAX77541_ADC_CH1_I = 0,
> +	MAX77541_ADC_CH2_I,
> +	MAX77541_ADC_CH3_I,
> +	MAX77541_ADC_CH6_I,
> +
> +	MAX77541_ADC_IRQMAX_I,
> +};
> +
> +struct max77541_adc_iio {
> +	struct regmap	*regmap;
> +	int irq;
> +	int irq_arr[MAX77541_ADC_IRQMAX_I];

Clear out unused elements until the follow up patch that actually makes
us of them. For now they are just distracting noise.

> +};
> +
> +enum max77541_adc_channel {
> +	MAX77541_ADC_VSYS_V = 0,
> +	MAX77541_ADC_VOUT1_V,
> +	MAX77541_ADC_VOUT2_V,
> +	MAX77541_ADC_TEMP,
> +};
> +
> +static int max77541_adc_offset(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       int *val, int *val2)
> +{
> +	switch (chan->channel) {
> +	case MAX77541_ADC_VSYS_V:
> +	case MAX77541_ADC_VOUT1_V:
> +	case MAX77541_ADC_VOUT2_V:
> +		*val = 0;
> +		*val2 = 0;
> +		return IIO_VAL_INT_PLUS_MICRO;

If the offset is zero, then don't have the interface.  Default
assumption if OFFSET is not provided is that the offset is 0.


> +	case MAX77541_ADC_TEMP:
> +		*val = -273;
> +		*val2 = 0;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int max77541_adc_scale(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      int *val, int *val2)
> +{
> +	struct max77541_adc_iio *info = iio_priv(indio_dev);
> +	unsigned int reg_val;
> +	int ret;
> +
> +	switch (chan->channel) {
> +	case MAX77541_ADC_VSYS_V:
> +		*val = 0;
> +		*val2 = 25000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case MAX77541_ADC_VOUT1_V:
> +		ret = regmap_read(info->regmap, MAX77541_REG_M2_CFG1, &reg_val);
> +		if (ret)
> +			return ret;
> +
> +		reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);
> +
> +		*val = 0;
> +
> +		if (reg_val == LOW_RANGE)

Ah. Here is the mysterious macro from patch 1.  Bring that definition forwards
to this patch.  Also switch statement would be more readable here.

> +			*val2 = 6250;
> +		else if (reg_val == MID_RANGE)
> +			*val2 = 12500;
> +		else if (reg_val == HIGH_RANGE)
> +			*val2 = 25000;
> +		else
> +			return -EINVAL;
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case MAX77541_ADC_VOUT2_V:
> +		ret = regmap_read(info->regmap, MAX77541_REG_M2_CFG1, &reg_val);

Umm. Is this identical to the above case?  If so, just have one block for
both.
  
> +		if (ret)
> +			return ret;
> +		reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);
> +
> +		*val = 0;
> +
> +		if (reg_val == LOW_RANGE)
> +			*val2 = 6250;
> +		else if (reg_val == MID_RANGE)
> +			*val2 = 12500;
> +		else if (reg_val == HIGH_RANGE)
> +			*val2 = 25000;
> +		else
> +			return -EINVAL;
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case MAX77541_ADC_TEMP:
> +		*val = 1;
> +		*val2 = 725000;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +

> +
> +static const struct iio_chan_spec max77541_adc_channels[] = {

Bring the macro definition down to just above this array. That will
make it easier to review as we can see the right parameters are being
passed in.   For 4 channels, I'm not sure I'd bother wrapping it in
a macro at all (particularly as that macro needs to be more
complex - see above), but I don't mind if you want to keep it that way.


I would also move this down below the iio_info definition because
that will keep all the code relevant to that in one place rather
than throwing this in the middle.

> +	MAX77541_ADC_CHANNEL(MAX77541_ADC_VSYS_V, "vsys_v", IIO_VOLTAGE,
> +			     MAX77541_REG_ADC_DATA_CH1),
> +	MAX77541_ADC_CHANNEL(MAX77541_ADC_VOUT1_V, "vout1_v", IIO_VOLTAGE,
> +			     MAX77541_REG_ADC_DATA_CH2),
> +	MAX77541_ADC_CHANNEL(MAX77541_ADC_VOUT2_V, "vout2_v", IIO_VOLTAGE,
> +			     MAX77541_REG_ADC_DATA_CH3),
> +	MAX77541_ADC_CHANNEL(MAX77541_ADC_TEMP, "temp", IIO_TEMP,
> +			     MAX77541_REG_ADC_DATA_CH6),
> +};
> +
> +static int max77541_adc_read_raw(struct iio_dev *indio_dev,
> +				 struct iio_chan_spec const *chan,
> +				 int *val, int *val2, long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_OFFSET:
> +		return max77541_adc_offset(indio_dev, chan, val, val2);
> +	case IIO_CHAN_INFO_SCALE:
> +		return max77541_adc_scale(indio_dev, chan, val, val2);
> +	case IIO_CHAN_INFO_RAW:
> +		return max77541_adc_raw(indio_dev, chan, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info max77541_adc_info = {
> +	.read_raw = max77541_adc_read_raw,
> +};
> +
> +static int max77541_adc_probe(struct platform_device *pdev)
> +{
> +	struct max77541_dev *max77541;
> +	struct max77541_adc_iio *info;
> +	struct iio_dev *indio_dev;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*info));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	info = iio_priv(indio_dev);
> +
> +	info->regmap = max77541->regmap;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	indio_dev->name = platform_get_device_id(pdev)->name;
> +	indio_dev->info = &max77541_adc_info;
> +	indio_dev->channels = max77541_adc_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max77541_adc_channels);
> +
> +	return devm_iio_device_register(&pdev->dev, indio_dev);
> +}
> +
> +static const struct platform_device_id max77541_adc_platform_id[] = {
> +	{ "max77541-adc", MAX77541, },

Better to introduce the complexity given by the type only when it is needed.
So for now drop the .data values here and below.

If / when that does happen I'll be asking you to use a chip_info structure
anyway - right now that structure would be empty so no point in adding it
yet.

> +	{  /* sentinel */  }
> +};
> +MODULE_DEVICE_TABLE(platform, max77541_adc_platform_id);
> +
> +static const struct of_device_id max77541_adc_of_id[] = {
> +	{
> +		.compatible = "adi,max77541-adc",
> +		.data = (void *)MAX77541,
> +	},
> +	{  /* sentinel */  }
> +};
> +MODULE_DEVICE_TABLE(of, max77541_adc_of_id);
> +
> +static struct platform_driver max77541_adc_driver = {
> +	.driver = {
> +		.name = "max77541-adc",
> +		.of_match_table = max77541_adc_of_id,
> +	},
> +	.probe = max77541_adc_probe,
> +	.id_table = max77541_adc_platform_id,
> +};
> +
Drop blank line.  Macro and structure are very closely associated
so it's good to make that obvious by not separating them.
> +module_platform_driver(max77541_adc_driver);
> +
> +MODULE_AUTHOR("Okan Sahin <Okan.Sahin@analog.com>");
> +MODULE_DESCRIPTION("MAX77541 ADC driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH 0/5] staging: drivers: mfd: Add MAX77541 MFD and related device drivers

[Andy Shevchenko]

On Sun, Dec 11, 2022 at 12:20:43PM +0000, Jonathan Cameron wrote:
> On Wed, 7 Dec 2022 13:09:34 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Dec 07, 2022 at 12:08:39PM +0300, Okan Sahin wrote:
> > > This patchset adds mfd, regulator and adc driver and related bindings.The patches 
> > > are required to be applied in sequence.  
> > 
> > You have an indentation / wrapping issues in the above text.
> > 
> > Nevertheless, why staging? What does it mean?
> 
> The main reason to go via staging is because a driver is sitting out
> of tree and it is useful to bring it in on the basis that it can then be
> cleaned up in tree before moving out of staging.

But files are not in staging. Me being confused.

> For a relatively small driver like this, that's hard to argue.  Just
> clean it up in response to review feedback and then we can take it
> directly into relevant subsystems in the main tree.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/5] staging: drivers: mfd: Add MAX77541 MFD and related device drivers

[Jonathan Cameron]

On Sun, 11 Dec 2022 15:34:10 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sun, Dec 11, 2022 at 12:20:43PM +0000, Jonathan Cameron wrote:
> > On Wed, 7 Dec 2022 13:09:34 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:  
> > > On Wed, Dec 07, 2022 at 12:08:39PM +0300, Okan Sahin wrote:  
> > > > This patchset adds mfd, regulator and adc driver and related bindings.The patches 
> > > > are required to be applied in sequence.    
> > > 
> > > You have an indentation / wrapping issues in the above text.
> > > 
> > > Nevertheless, why staging? What does it mean?  
> > 
> > The main reason to go via staging is because a driver is sitting out
> > of tree and it is useful to bring it in on the basis that it can then be
> > cleaned up in tree before moving out of staging.  
> 
> But files are not in staging. Me being confused.

I noticed that when I got to the patches :)
Odd indeed - I'm guessing some cut and paste gone wrong.

Jonathan

> 
> > For a relatively small driver like this, that's hard to argue.  Just
> > clean it up in response to review feedback and then we can take it
> > directly into relevant subsystems in the main tree.  
>