[PATCH v3 0/5] Add MAX77541/MAX77540 PMIC Support

[PATCH v3 0/5] Add MAX77541/MAX77540 PMIC Support

[Okan Sahin]

MFD, regulator and ADC driver and related bindings for MAX77540/MAX77541.
The patches are requeired to be applied in sequence.

Changes in v3:
* Patch 1: "drivers: mfd: Add ADI MAX77541/MAX77540 PMIC Support"
  * Change struct name from max77541_dev to max77541
  * Adjust max-line-length lower than 80
* Patch 2: "dt-bindings: mfd: Add ADI MAX77541/MAX77540"
  * Remove adc object as we do not need
  * Remove adc node from example
* Patch 3: "drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support"
  * Change node name from "BUCK#_id" to "buck#_id" in regulator desc
* Patch 4: "dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator"
  * Change node name from "BUCK" to "buck" in regulators
* Patch 5: "drivers: iio: adc: Add ADI MAX77541 ADC Support"
  * Convert voltage values from V to mV for scaling.
  * Convert temperature values from C to miliC for scale and offset
  * Do not set offset bit in info_mask_separate for voltage that does not need offset
  * Remove unnecessary dev_get_drvdata() instead of it use dev_get_regmap to have regmap.
  * Assing hard coded name for adc dev name

Changes in v2:
* Patch 1: "drivers: mfd: Add MAX77541/MAX77540 PMIC Support"
  * Drop "this patch adds" from commit message.
  * Drop redundant blank lines.
  * Drop module version
  * Use definition for parameter of devm_mfd_add_devices(.., -1,..)
  * Use desc in chip_info to adding desc for different devices.
  * Add missing headers and forward declarations.
  * Drop unused elements from max77541_dev struct
  * Add chip_info into max77541_dev struct to identify different devices.
* Patch 2: "dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings"
  * Drop "this patch adds" from commit message.
  * Fix $ref path
  * Drop adc part under allOf
  * Keep only one example (more complex one)
  * Fix make dt_binding_check errors.(trailing space, No newline)
* Patch 3: "drivers: regulator: Add MAX77541 Regulator Support"
  * Drop "this patch adds" from commit message.
  * Add trailing comma for required structs.
  * Fix wrong indentation.
  * Drop redundant blank lines.
  * Drop max77541_regulator_dev struct.
  * Use "regulator_desc *desc" for both regulator
    regarding to "max77541->id"
* Patch 4: "dt-bindings: regulator: max77541-regulator.yaml Add MAX77541
            Regulator bindings"
  * Drop "this patch adds" from commit message.
  * Chance filename (matching compatible), so adi,max77541-regulator.yaml
  * Fix make dt_binding_check errors.(trailing space, No newline)
* Patch 5: "drivers: iio: adc: Add MAX77541 ADC Support"
  * Drop "this patch adds" from commit message.
  * Drop redundant blank lines.
  * Fix wrong include path.
  * Use switch instead of if-else for range setting in max77541_adc_scale
  * Move max77541_adc_range enum from max77541.h to here.
  * Use definition from units.h
  * Drop unused elements from max77541_adc_iio struct
  * Drop the .data from platform_device_id

Okan Sahin (5):
  drivers: mfd: Add ADI MAX77541/MAX77540 PMIC Support
  dt-bindings: mfd: Add ADI MAX77541/MAX77540
  drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support
  dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator
  drivers: iio: adc: Add ADI MAX77541 ADC Support

 .../devicetree/bindings/mfd/adi,max77541.yaml |  87 ++++++
 .../regulator/adi,max77541-regulator.yaml     |  44 +++
 MAINTAINERS                                   |  11 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/max77541-adc.c                | 200 ++++++++++++++
 drivers/mfd/Kconfig                           |  13 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/max77541.c                        | 250 ++++++++++++++++++
 drivers/regulator/Kconfig                     |   9 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/max77541-regulator.c        | 160 +++++++++++
 include/linux/mfd/max77541.h                  | 108 ++++++++
 13 files changed, 896 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77541.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77541-regulator.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 v3 1/5] drivers: mfd: Add ADI MAX77541/MAX77540 PMIC Support

[Okan Sahin]

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       | 250 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/max77541.h | 108 +++++++++++++++
 5 files changed, 379 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..e6bf621cbc8e 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..0ccb96df5db0
--- /dev/null
+++ b/drivers/mfd/max77541.c
@@ -0,0 +1,250 @@
+// 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 const struct chip_info chip[] = {
+	[MAX77540] = {
+		.n_devs = ARRAY_SIZE(max77540_devs),
+		.devs = max77540_devs,
+	},
+	[MAX77541] = {
+		.n_devs = ARRAY_SIZE(max77541_devs),
+		.devs = max77541_devs,
+	},
+};
+
+static int max77541_pmic_irq_init(struct device *dev)
+{
+	struct max77541 *max77541 = dev_get_drvdata(dev);
+	int irq = max77541->i2c->irq;
+	int ret;
+
+	ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77541_src_irq_chip,
+				       &max77541->irq_data);
+	if (ret)
+		return ret;
+
+	ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77541_topsys_irq_chip,
+				       &max77541->irq_topsys);
+	if (ret)
+		return ret;
+
+	ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77541_buck_irq_chip,
+				       &max77541->irq_buck);
+	if (ret)
+		return ret;
+
+	if (max77541->id == MAX77541) {
+		ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
+					       IRQF_ONESHOT | IRQF_SHARED, 0,
+					       &max77541_adc_irq_chip,
+					       &max77541->irq_adc);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+
+static int max77541_pmic_setup(struct device *dev)
+{
+	struct max77541 *max77541 = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = max77541_pmic_irq_init(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize IRQ\n");
+
+	ret = regmap_read(max77541->regmap, MAX77541_REG_INT_SRC, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(max77541->regmap, MAX77541_REG_TOPSYS_INT, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(max77541->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 (max77541->id) {
+	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 *id)
+{
+	struct device *dev = &client->dev;
+	struct max77541 *max77541;
+	const void *match;
+
+	max77541 = devm_kzalloc(&client->dev, sizeof(*max77541), GFP_KERNEL);
+	if (!max77541)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, max77541);
+	max77541->i2c = client;
+
+	match = device_get_match_data(dev);
+	if (match)
+		max77541->id = (enum max7754x_ids)match;
+	else if (id)
+		max77541->id = id->driver_data;
+	else
+		return -ENODEV;
+
+	max77541->chip = &chip[max77541->id];
+
+	max77541->regmap = devm_regmap_init_i2c(client,
+						&max77541_regmap_config);
+	if (IS_ERR(max77541->regmap))
+		return dev_err_probe(dev, PTR_ERR(max77541->regmap),
+				     "Failed to allocate register map\n");
+
+	return max77541_pmic_setup(dev);
+}
+
+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 = 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..44c3c0c2aa97
--- /dev/null
+++ b/include/linux/mfd/max77541.h
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __MAX77541_MFD_H__
+#define __MAX77541_MFD_H__
+
+#include <linux/bits.h>
+#include <linux/types.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_EN_CTRL                    0x0B
+
+/*      BUCK CONFIG       */
+#define MAX77541_REG_BUCK_INT                   0x20
+#define MAX77541_REG_BUCK_INT_M                 0x21
+
+#define MAX77541_REG_M1_VOUT                    0x23
+#define MAX77541_REG_M1_CFG1                    0x25
+
+#define MAX77541_REG_M2_VOUT                    0x33
+#define MAX77541_REG_M2_CFG1                    0x35
+
+/* 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_BIT_M1_EN                      BIT(0)
+#define MAX77541_BIT_M2_EN                      BIT(1)
+
+#define MAX77541_BITS_MX_VOUT                   GENMASK(7, 0)
+#define MAX77541_BITS_MX_CFG1_RNG               GENMASK(7, 6)
+
+/*      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_MAX_REGULATORS 2
+
+#define MAX77541_REGMAP_IRQ_REG(_mask)	\
+	{ .mask = (_mask) }
+
+enum max7754x_ids {
+	MAX77540 = 1,
+	MAX77541 = 2,
+};
+
+enum max77541_regulators {
+	MAX77541_BUCK1 = 1,
+	MAX77541_BUCK2,
+};
+
+struct chip_info {
+	int n_devs;
+	const struct mfd_cell *devs;
+};
+
+struct regmap;
+struct regmap_irq_chip_data;
+struct i2c_client;
+
+struct max77541 {
+	enum max7754x_ids id;
+	const struct chip_info *chip;
+
+	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;
+};
+
+#endif /* __MAX77541_MFD_H__ */
-- 
2.30.2

[PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Okan Sahin]

Add ADI MAX77541/MAX77540 devicetree document.

Signed-off-by: Okan Sahin <okan.sahin@analog.com>
---
 .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 88 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..91d15e9ca2e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
@@ -0,0 +1,87 @@
+# 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-regulator.yaml#
+
+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
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@69 {
+            compatible = "adi,max77541";
+            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;
+                };
+            };
+        };
+    };
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 v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Okan Sahin]

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 | 160 +++++++++++++++++++++++++
 4 files changed, 171 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..342d4f59405c
--- /dev/null
+++ b/drivers/regulator/max77541-regulator.c
@@ -0,0 +1,160 @@
+// 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),
+};
+
+static int max77541_regulator_probe(struct platform_device *pdev)
+{
+	struct max77541 *max77541 = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config config = {};
+	const struct regulator_desc *desc;
+	struct device *dev = &pdev->dev;
+	struct regulator_dev *rdev;
+	int i;
+
+	config.dev = pdev->dev.parent;
+
+	if (max77541->id == MAX77540)
+		desc = max77540_regulators_desc;
+	else if (max77541->id == MAX77541)
+		desc = max77541_regulators_desc;
+	else
+		return -EINVAL;
+
+	for (i = 0; i < MAX77541_MAX_REGULATORS; i++) {
+		rdev = devm_regulator_register(dev,
+					       &desc[i], &config);
+		if (IS_ERR(rdev))
+			return dev_err_probe(dev, PTR_ERR(rdev),
+					     "Failed to register regulator\n");
+	}
+
+	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 v3 4/5] dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator

[Okan Sahin]

Add ADI MAX77541/MAX77540 Regulator devicetree document.

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

diff --git a/Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,max77541-regulator.yaml
new file mode 100644
index 000000000000..fff463d5e79d
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,max77541-regulator.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-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Buck Converter 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 converter 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
diff --git a/MAINTAINERS b/MAINTAINERS
index 5704ed5afce3..fcb846f7250f 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-regulator.yaml
 F:	drivers/mfd/max77541.c
 F:	drivers/regulator/max77541-regulator.c
 F:	include/linux/mfd/max77541.h
-- 
2.30.2

[PATCH v3 5/5] drivers: iio: adc: Add ADI MAX77541 ADC Support

[Okan Sahin]

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 | 200 +++++++++++++++++++++++++++++++++
 4 files changed, 213 insertions(+)
 create mode 100644 drivers/iio/adc/max77541-adc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index fcb846f7250f..763f2eec4bb7 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-regulator.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..9716225b50da 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..8c459661940f
--- /dev/null
+++ b/drivers/iio/adc/max77541-adc.c
@@ -0,0 +1,200 @@
+// 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 <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/units.h>
+#include <linux/mfd/max77541.h>
+
+enum max77541_adc_range {
+	LOW_RANGE,
+	MID_RANGE,
+	HIGH_RANGE,
+};
+
+struct max77541_adc_iio {
+	struct regmap	*regmap;
+};
+
+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_TEMP:
+		*val = DIV_ROUND_CLOSEST(ABSOLUTE_ZERO_MILLICELSIUS,
+					 1725);
+		return IIO_VAL_INT;
+	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 = 25;
+		return IIO_VAL_INT;
+	case MAX77541_ADC_VOUT1_V:
+	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);
+
+		switch (reg_val) {
+		case LOW_RANGE:
+			*val = 6;
+			*val2 = 250000;
+			break;
+		case MID_RANGE:
+			*val = 12;
+			*val2 = 500000;
+			break;
+		case HIGH_RANGE:
+			*val = 25;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+
+		return IIO_VAL_INT_PLUS_MICRO;
+	case MAX77541_ADC_TEMP:
+		*val = 1725;
+		return IIO_VAL_INT;
+	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;
+}
+
+#define MAX77541_ADC_CHANNEL_V(_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), \
+		.datasheet_name = _name,			\
+	}
+
+#define MAX77541_ADC_CHANNEL_TEMP(_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,			\
+	}
+
+static const struct iio_chan_spec max77541_adc_channels[] = {
+	MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VSYS_V, "vsys_v", IIO_VOLTAGE,
+			       MAX77541_REG_ADC_DATA_CH1),
+	MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VOUT1_V, "vout1_v", IIO_VOLTAGE,
+			       MAX77541_REG_ADC_DATA_CH2),
+	MAX77541_ADC_CHANNEL_V(MAX77541_ADC_VOUT2_V, "vout2_v", IIO_VOLTAGE,
+			       MAX77541_REG_ADC_DATA_CH3),
+	MAX77541_ADC_CHANNEL_TEMP(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 device *dev = &pdev->dev;
+	struct max77541_adc_iio *info;
+	struct iio_dev *indio_dev;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	info = iio_priv(indio_dev);
+
+	info->regmap = dev_get_regmap(dev->parent, NULL);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	indio_dev->name = "max77541";
+	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(dev, indio_dev);
+}
+
+static const struct platform_device_id max77541_adc_platform_id[] = {
+	{ "max77541-adc", },
+	{  /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(platform, max77541_adc_platform_id);
+
+static struct platform_driver max77541_adc_driver = {
+	.driver = {
+		.name = "max77541-adc",
+	},
+	.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 v3 1/5] drivers: mfd: Add ADI MAX77541/MAX77540 PMIC Support

[Andy Shevchenko]

On Wed, Jan 18, 2023 at 09:38:08AM +0300, Okan Sahin wrote:
> 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.

...

> +/*
> + * Copyright (c) 2022 Analog Devices, Inc.

Happy New Year!

> + * Mfd core driver for the MAX77540 and MAX77541

MFD

> + */

...

> +	ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> +				       IRQF_ONESHOT | IRQF_SHARED, 0,
> +				       &max77541_buck_irq_chip,
> +				       &max77541->irq_buck);
> +	if (ret)
> +		return ret;
> +
> +	if (max77541->id == MAX77541) {
> +		ret = devm_regmap_add_irq_chip(dev, max77541->regmap, irq,
> +					       IRQF_ONESHOT | IRQF_SHARED, 0,
> +					       &max77541_adc_irq_chip,
> +					       &max77541->irq_adc);
> +		if (ret)
> +			return ret;
> +	}

> +	return ret;

return 0;

...

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

PLATFORM_DEVID_NONE ?

> +					    ARRAY_SIZE(max77540_devs),
> +					    NULL, 0, NULL);

...

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

Ditto.

> +					    ARRAY_SIZE(max77541_devs),
> +					    NULL, 0, NULL);

...

> +static int max77541_i2c_probe(struct i2c_client *client,
> +			      const struct i2c_device_id *id)

No id, please. I.o.w. you should use ->probe_new().

> +{
> +	struct device *dev = &client->dev;
> +	struct max77541 *max77541;
> +	const void *match;
> +
> +	max77541 = devm_kzalloc(&client->dev, sizeof(*max77541), GFP_KERNEL);
> +	if (!max77541)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, max77541);
> +	max77541->i2c = client;
> +
> +	match = device_get_match_data(dev);
> +	if (match)
> +		max77541->id = (enum max7754x_ids)match;

This is dangerous if your enum has 0 as a valid value.
Instead, use pointers in the driver_data, like

	&chip_info chip[MAX77540]

> +	else if (id)
> +		max77541->id = id->driver_data;

> +	else

It's better to check the ID range here.
Or since the change recommended above, check for NULL.

> +		return -ENODEV;
> +
> +	max77541->chip = &chip[max77541->id];
> +
> +	max77541->regmap = devm_regmap_init_i2c(client,
> +						&max77541_regmap_config);
> +	if (IS_ERR(max77541->regmap))
> +		return dev_err_probe(dev, PTR_ERR(max77541->regmap),
> +				     "Failed to allocate register map\n");
> +
> +	return max77541_pmic_setup(dev);
> +}

...

> +/*BITS OF REGISTERS*/

Missing spaces.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Wed, Jan 18, 2023 at 09:38:10AM +0300, Okan Sahin wrote:
> 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.

...

> + * Copyright (c) 2022 Analog Devices, Inc.

Happy New Year!

...

> +static int max77541_regulator_probe(struct platform_device *pdev)
> +{
> +	struct max77541 *max77541 = dev_get_drvdata(pdev->dev.parent);
> +	struct regulator_config config = {};
> +	const struct regulator_desc *desc;
> +	struct device *dev = &pdev->dev;

You may rearrange this a bit

	struct max77541 *max77541 = dev_get_drvdata(dev->parent);

> +	struct regulator_dev *rdev;
> +	int i;

> +	config.dev = pdev->dev.parent;

dev->parent

> +
> +	if (max77541->id == MAX77540)
> +		desc = max77540_regulators_desc;
> +	else if (max77541->id == MAX77541)
> +		desc = max77541_regulators_desc;
> +	else
> +		return -EINVAL;
> +
> +	for (i = 0; i < MAX77541_MAX_REGULATORS; i++) {

> +		rdev = devm_regulator_register(dev,
> +					       &desc[i], &config);

This is perfectly one line.

> +		if (IS_ERR(rdev))
> +			return dev_err_probe(dev, PTR_ERR(rdev),
> +					     "Failed to register regulator\n");
> +	}
> +
> +	return 0;
> +}

...

> +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 */  }

As pointed out, better to use pointers directly.

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Krzysztof Kozlowski]

On 18/01/2023 07:38, Okan Sahin wrote:
> Add ADI MAX77541/MAX77540 devicetree document.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 88 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..91d15e9ca2e3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
> @@ -0,0 +1,87 @@
> +# 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-regulator.yaml#

No improvements regarding bisectability - this patch fails. If you
tested this patch, you would see it.

Instead of ignoring comments, either implement them or ask for
clarification.


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

You do not have 'regulator' property.

> +          properties:
> +            compatible:
> +              const: adi,max77540-regulator
> +    else:
> +      properties:
> +        regulator:

Same problem.

> +          properties:
> +            compatible:
> +              const: adi,max77541-regulator
> +
> +additionalProperties: false
> +

Best regards,
Krzysztof

Re: [PATCH v3 5/5] drivers: iio: adc: Add ADI MAX77541 ADC Support

[Andy Shevchenko]

On Wed, Jan 18, 2023 at 09:38:12AM +0300, Okan Sahin wrote:
> The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
> with four multiplexers for supporting the telemetry feature.

...

> + * Copyright (c) 2022 Analog Devices, Inc.

HNY!

...

> +#include <linux/bitfield.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>

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

What for these two?
Make sure the list of inclusions is not semi-random.

> +#include <linux/units.h>

+ Blank line.

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

...

> +		*val = DIV_ROUND_CLOSEST(ABSOLUTE_ZERO_MILLICELSIUS,
> +					 1725);

This is perfectly one line, please, reduce number of LoCs by unwrapping
such cases.

...

> +		if (ret)
> +			return ret;

+ Blank line.

> +		reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/5] dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator

[Krzysztof Kozlowski]

On 18/01/2023 07:38, Okan Sahin wrote:
> Add ADI MAX77541/MAX77540 Regulator devicetree document.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Rob Herring]

On Wed, 18 Jan 2023 09:38:09 +0300, Okan Sahin wrote:
> Add ADI MAX77541/MAX77540 devicetree document.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 88 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:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/mfd/adi,max77541.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/regulator/adi,max77541-regulator.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77541.example.dtb: pmic@69: regulators: False schema does not allow {'buck1': {'regulator-min-microvolt': [[500000]], 'regulator-max-microvolt': [[5200000]], 'regulator-boot-on': True, 'regulator-always-on': True}, 'buck2': {'regulator-min-microvolt': [[500000]], 'regulator-max-microvolt': [[5200000]], 'regulator-boot-on': True, 'regulator-always-on': True}}
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77541.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230118063822.14521-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 v3 1/5] drivers: mfd: Add ADI MAX77541/MAX77540 PMIC Support

[Lee Jones]

On Wed, 18 Jan 2023, Andy Shevchenko wrote:

> On Wed, Jan 18, 2023 at 09:38:08AM +0300, Okan Sahin wrote:
> > 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.
> 
> ...
> 
> > +/*
> > + * Copyright (c) 2022 Analog Devices, Inc.
> 
> Happy New Year!

If the code hasn't changed greatly since the Copyright, there is no
requirement to update the date.

> > + * Mfd core driver for the MAX77540 and MAX77541
> 
> MFD

Please remove all mention of MFD - it's not a thing - we made it up!

-- 
Lee Jones [李琼斯]

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

[Andy Shevchenko]

On Thu, Jan 19, 2023 at 02:27:29PM +0000, Lee Jones wrote:
> On Wed, 18 Jan 2023, Andy Shevchenko wrote:
> > On Wed, Jan 18, 2023 at 09:38:08AM +0300, Okan Sahin wrote:

...

> > > +/*
> > > + * Copyright (c) 2022 Analog Devices, Inc.
> > 
> > Happy New Year!
> 
> If the code hasn't changed greatly since the Copyright, there is no
> requirement to update the date.

It would have made sense if we have/had seen that code in the past.
And since it's a v3 I think this is the case.

-- 
With Best Regards,
Andy Shevchenko

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

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On Thu, Jan 19, 2023 at 02:27:29PM +0000, Lee Jones wrote:
> On Wed, 18 Jan 2023, Andy Shevchenko wrote:
> 
> > On Wed, Jan 18, 2023 at 09:38:08AM +0300, Okan Sahin wrote:
> > > 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.
> > 
> > ...
> > 
> > > +/*
> > > + * Copyright (c) 2022 Analog Devices, Inc.
> > 
> > Happy New Year!
> 
> If the code hasn't changed greatly since the Copyright, there is no
> requirement to update the date.

Actually a date isn't necessary at all. In fact, by the Berne Convention
copyright protection exists the moment a work is completed, so you could
even argue a copyright notice is unnecessary and the SPDX line alone is
sufficient. However, I would say there is a practical benefit in a
copyright notice identifying the primary authors to make ownership
clear.

For the curious, this LF blog post gives some reasons about why
copyright notices may be better left simple:
https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 5/5] drivers: iio: adc: Add ADI MAX77541 ADC Support

[Jonathan Cameron]

On Wed, 18 Jan 2023 09:38:12 +0300
Okan Sahin <okan.sahin@analog.com> wrote:

> 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>
With Andy's comments resolved, I'm fine with this.
Note it will need to go via mfd because of the included header.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

RE: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Sahin, Okan]

Hi Andy,

Thank you for your feedback and efforts. I have a question below.

On Wed, 18 Jan 2022 11:20 AM
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

>On Wed, Jan 18, 2023 at 09:38:10AM +0300, Okan Sahin wrote:
>> 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.
>
>...
>
>> + * Copyright (c) 2022 Analog Devices, Inc.
>
>Happy New Year!
>
>...
>
>> +static int max77541_regulator_probe(struct platform_device *pdev) {
>> +	struct max77541 *max77541 = dev_get_drvdata(pdev->dev.parent);
>> +	struct regulator_config config = {};
>> +	const struct regulator_desc *desc;
>> +	struct device *dev = &pdev->dev;
>
>You may rearrange this a bit
>
>	struct max77541 *max77541 = dev_get_drvdata(dev->parent);
>
>> +	struct regulator_dev *rdev;
>> +	int i;
>
>> +	config.dev = pdev->dev.parent;
>
>dev->parent
>
>> +
>> +	if (max77541->id == MAX77540)
>> +		desc = max77540_regulators_desc;
>> +	else if (max77541->id == MAX77541)
>> +		desc = max77541_regulators_desc;
>> +	else
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < MAX77541_MAX_REGULATORS; i++) {
>
>> +		rdev = devm_regulator_register(dev,
>> +					       &desc[i], &config);
>
>This is perfectly one line.
Thank you, I will arrange it.
>
>> +		if (IS_ERR(rdev))
>> +			return dev_err_probe(dev, PTR_ERR(rdev),
>> +					     "Failed to register regulator\n");
>> +	}
>> +
>> +	return 0;
>> +}
>
>...
However, this one is not fit when I set max-line-length argument as 80 in checkpatch script. What do you suggest? This line has 99 characters.
>
>> +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 */  }
>
>As pointed out, better to use pointers directly.
>
>> +};
>
>--
>With Best Regards,
>Andy Shevchenko
>

Regards,
Okan Sahin

RE: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Sahin, Okan]

Hi Andy,

Sorry for second question. I do not want to bother you, but I realized that I need to be sure about driver_data before sending new patch. You said that you need to use pointers directly for driver_data then I fixed that part in mfd, but I do not need or  use driver_data in regulator since chip_id comes from mfd device so I think using like below should be enough for my implementation.

static const struct platform_device_id max77541_regulator_platform_id[] = {
	{ "max77540-regulator", },
	{ "max77541-regulator", },
	{  /* sentinel */  }
};
MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);

static const struct of_device_id max77541_regulator_of_id[] = {
	{ .compatible = "adi,max77540-regulator", },
	{ .compatible = "adi,max77541-regulator", },
	{ /* sentinel */  }
};
MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);

What do you think?

On Tue, Jan 31, 2023 at 10:21 AM +0300, Okan Sahin wrote:

>Hi Andy,
>
>Thank you for your feedback and efforts. I have a question below.
>
>On Wed, 18 Jan 2022 11:20 AM
>Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>>On Wed, Jan 18, 2023 at 09:38:10AM +0300, Okan Sahin wrote:
>>> 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.
>>
>>...
>>
>>> + * Copyright (c) 2022 Analog Devices, Inc.
>>
>>Happy New Year!
>>
>>...
>>
>>> +static int max77541_regulator_probe(struct platform_device *pdev) {
>>> +	struct max77541 *max77541 = dev_get_drvdata(pdev->dev.parent);
>>> +	struct regulator_config config = {};
>>> +	const struct regulator_desc *desc;
>>> +	struct device *dev = &pdev->dev;
>>
>>You may rearrange this a bit
>>
>>	struct max77541 *max77541 = dev_get_drvdata(dev->parent);
>>
>>> +	struct regulator_dev *rdev;
>>> +	int i;
>>
>>> +	config.dev = pdev->dev.parent;
>>
>>dev->parent
>>
>>> +
>>> +	if (max77541->id == MAX77540)
>>> +		desc = max77540_regulators_desc;
>>> +	else if (max77541->id == MAX77541)
>>> +		desc = max77541_regulators_desc;
>>> +	else
>>> +		return -EINVAL;
>>> +
>>> +	for (i = 0; i < MAX77541_MAX_REGULATORS; i++) {
>>
>>> +		rdev = devm_regulator_register(dev,
>>> +					       &desc[i], &config);
>>
>>This is perfectly one line.
>Thank you, I will arrange it.
>>
>>> +		if (IS_ERR(rdev))
>>> +			return dev_err_probe(dev, PTR_ERR(rdev),
>>> +					     "Failed to register regulator\n");
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>
>>...
>However, this one is not fit when I set max-line-length argument as 80 in
>checkpatch script. What do you suggest? This line has 99 characters.
>>
>>> +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 */  }
>>
>>As pointed out, better to use pointers directly.
>>
>>> +};
>>
>>--
>>With Best Regards,
>>Andy Shevchenko
>>
>
>Regards,
>Okan Sahin

Regards,
Okan

RE: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Sahin, Okan]

Hi Krzysztof,

Thank you for your feedback and efforts. There is something that confuses me so I would like to ask below

On Wed, 18 Jan 2023 11:23 AM
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

>On 18/01/2023 07:38, Okan Sahin wrote:
>> Add ADI MAX77541/MAX77540 devicetree document.
>>
>> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
>> ---
>>  .../devicetree/bindings/mfd/adi,max77541.yaml | 87 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 88 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..91d15e9ca2e3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/adi,max77541.yaml
>> @@ -0,0 +1,87 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
>> +---
>> +$id:
>> +https://urldefense.com/v3/__http://devicetree.org/schemas/mfd/adi,max
>>
>+77541.yaml*__;Iw!!A3Ni8CS0y2Y!8996jdM8k1vZNHCTUma_rPZJgJFp5YspnfHn9
>Az
>> +1iRZER2Won_Nt-egtD6XnaFy2I2YTAHhCTJA0DcP6xCbYS_SPe8iy$
>> +$schema:
>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>>
>+aml*__;Iw!!A3Ni8CS0y2Y!8996jdM8k1vZNHCTUma_rPZJgJFp5YspnfHn9Az1iRZE
>R2
>> +Won_Nt-egtD6XnaFy2I2YTAHhCTJA0DcP6xCbYS-wr9IcZ$
>> +
>> +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-regulator.yaml#
>
>No improvements regarding bisectability - this patch fails. If you tested this patch,
>you would see it.
>
>Instead of ignoring comments, either implement them or ask for clarification.
>
>
Sorry for misunderstanding, I checked patchset as a whole not one by one this is why I did not get failure after "make dt_binding_check " . Right now, I understand why you are saying this patch fails, but what is your suggestion?  what is the correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5. In the light of discussion, should I remove all the parts related to regulator in patch 2/5, then add adi,max77541-regulator.yaml and update adi,max77541.yaml in patch 4/5? or should I add new patch to update adi,max77541.yaml?

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: adi,max77540
>> +    then:
>> +      properties:
>> +        regulator:
>
>You do not have 'regulator' property.
>
>> +          properties:
>> +            compatible:
>> +              const: adi,max77540-regulator
>> +    else:
>> +      properties:
>> +        regulator:
>
>Same problem.
>
>> +          properties:
>> +            compatible:
>> +              const: adi,max77541-regulator
>> +
>> +additionalProperties: false
>> +
>
>Best regards,
>Krzysztof

Regards,
Okan Sahin

Re: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Tue, Jan 31, 2023 at 07:20:52AM +0000, Sahin, Okan wrote:
> On Wed, 18 Jan 2022 11:20 AM
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >On Wed, Jan 18, 2023 at 09:38:10AM +0300, Okan Sahin wrote:

...

> >> +		rdev = devm_regulator_register(dev,
> >> +					       &desc[i], &config);
> >
> >This is perfectly one line.
> Thank you, I will arrange it.
> >
> >> +		if (IS_ERR(rdev))
> >> +			return dev_err_probe(dev, PTR_ERR(rdev),
> >> +					     "Failed to register regulator\n");
> >> +	}
> >> +
> >> +	return 0;
> >> +}

> However, this one is not fit when I set max-line-length argument as 80 in
> checkpatch script. What do you suggest? This line has 99 characters.

Which line do you refer to?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Tue, Jan 31, 2023 at 09:27:48AM +0000, Sahin, Okan wrote:

First of all, please do avoid top-posting.

> Sorry for second question. I do not want to bother you, but I realized that I
> need to be sure about driver_data before sending new patch. You said that you
> need to use pointers directly for driver_data then I fixed that part in mfd,
> but I do not need or  use driver_data in regulator since chip_id comes from
> mfd device so I think using like below should be enough for my
> implementation.
> 
> static const struct platform_device_id max77541_regulator_platform_id[] = {
> 	{ "max77540-regulator", },
> 	{ "max77541-regulator", },
> 	{  /* sentinel */  }
> };
> MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
> 
> static const struct of_device_id max77541_regulator_of_id[] = {
> 	{ .compatible = "adi,max77540-regulator", },
> 	{ .compatible = "adi,max77541-regulator", },
> 	{ /* sentinel */  }
> };
> MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);
> 
> What do you think?

If you have got all necessary data from the upper layer, why do you need to
have an ID table here? I'm not sure I understand how this OF ID table works
in this case.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Sahin, Okan]

On Tue, 31 Jan 2022 3:24 PM
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

>On Tue, Jan 31, 2023 at 07:20:52AM +0000, Sahin, Okan wrote:
>> On Wed, 18 Jan 2022 11:20 AM
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>> >On Wed, Jan 18, 2023 at 09:38:10AM +0300, Okan Sahin wrote:
>
>...
>
>> >> +		rdev = devm_regulator_register(dev,
>> >> +					       &desc[i], &config);
>> >
>> >This is perfectly one line.
>> Thank you, I will arrange it.
>> >
>> >> +		if (IS_ERR(rdev))
>> >> +			return dev_err_probe(dev, PTR_ERR(rdev),
>> >> +					     "Failed to register regulator\n");
>> >> +	}
>> >> +
>> >> +	return 0;
>> >> +}
>
>> However, this one is not fit when I set max-line-length argument as 80
>> in checkpatch script. What do you suggest? This line has 99 characters.
>
>Which line do you refer to?
I am referring "return dev_err_probe(dev, PTR_ERR(rdev), "Failed to register regulator\n");"
>
>--
>With Best Regards,
>Andy Shevchenko
>

Best Regards,
Okan Sahin

RE: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Sahin, Okan]

On Tue, 31 Jan 2022 3:27 PM
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

>On Tue, Jan 31, 2023 at 09:27:48AM +0000, Sahin, Okan wrote:
>
>First of all, please do avoid top-posting.
>
>> Sorry for second question. I do not want to bother you, but I realized
>> that I need to be sure about driver_data before sending new patch. You
>> said that you need to use pointers directly for driver_data then I
>> fixed that part in mfd, but I do not need or  use driver_data in
>> regulator since chip_id comes from mfd device so I think using like
>> below should be enough for my implementation.
>>
>> static const struct platform_device_id max77541_regulator_platform_id[] = {
>> 	{ "max77540-regulator", },
>> 	{ "max77541-regulator", },
>> 	{  /* sentinel */  }
>> };
>> MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
>>
>> static const struct of_device_id max77541_regulator_of_id[] = {
>> 	{ .compatible = "adi,max77540-regulator", },
>> 	{ .compatible = "adi,max77541-regulator", },
>> 	{ /* sentinel */  }
>> };
>> MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);
>>
>> What do you think?
>
>If you have got all necessary data from the upper layer, why do you need to have
>an ID table here? I'm not sure I understand how this OF ID table works in this
>case.
I added it since there is regulator node in device tree. With the help of devm_regulator_register(..), driver takes parameters of regulator node. I also used id to select and to initialize regulator descriptors which are chip specific. So far there is no comment about OF ID table so I kept it. I thought I need to add both of id table and platform id table as name matching is required to initialize platform device from mfd.
>
>--
>With Best Regards,
>Andy Shevchenko
>

Best Regards,
Okan Sahin

Re: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Tue, Jan 31, 2023 at 01:12:26PM +0000, Sahin, Okan wrote:
> On Tue, 31 Jan 2022 3:24 PM
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >On Tue, Jan 31, 2023 at 07:20:52AM +0000, Sahin, Okan wrote:
> >> On Wed, 18 Jan 2022 11:20 AM
> >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >> >On Wed, Jan 18, 2023 at 09:38:10AM +0300, Okan Sahin wrote:

...

> >> >> +		rdev = devm_regulator_register(dev,
> >> >> +					       &desc[i], &config);
> >> >
> >> >This is perfectly one line.
> >> Thank you, I will arrange it.
> >> >
> >> >> +		if (IS_ERR(rdev))
> >> >> +			return dev_err_probe(dev, PTR_ERR(rdev),
> >> >> +					     "Failed to register regulator\n");
> >> >> +	}
> >> >> +
> >> >> +	return 0;
> >> >> +}
> >
> >> However, this one is not fit when I set max-line-length argument as 80
> >> in checkpatch script. What do you suggest? This line has 99 characters.
> >
> >Which line do you refer to?
> I am referring "return dev_err_probe(dev, PTR_ERR(rdev), "Failed to register
> regulator\n");"

I have had no comments on that line.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Tue, Jan 31, 2023 at 01:23:33PM +0000, Sahin, Okan wrote:
> On Tue, 31 Jan 2022 3:27 PM
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >On Tue, Jan 31, 2023 at 09:27:48AM +0000, Sahin, Okan wrote:

...

> >> Sorry for second question. I do not want to bother you, but I realized
> >> that I need to be sure about driver_data before sending new patch. You
> >> said that you need to use pointers directly for driver_data then I
> >> fixed that part in mfd, but I do not need or  use driver_data in
> >> regulator since chip_id comes from mfd device so I think using like
> >> below should be enough for my implementation.
> >>
> >> static const struct platform_device_id max77541_regulator_platform_id[] = {
> >> 	{ "max77540-regulator", },
> >> 	{ "max77541-regulator", },
> >> 	{  /* sentinel */  }
> >> };
> >> MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
> >>
> >> static const struct of_device_id max77541_regulator_of_id[] = {
> >> 	{ .compatible = "adi,max77540-regulator", },
> >> 	{ .compatible = "adi,max77541-regulator", },
> >> 	{ /* sentinel */  }
> >> };
> >> MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);
> >>
> >> What do you think?
> >
> >If you have got all necessary data from the upper layer, why do you need to have
> >an ID table here? I'm not sure I understand how this OF ID table works in this
> >case.

> I added it since there is regulator node in device tree. With the help of
> devm_regulator_register(..), driver takes parameters of regulator node. I
> also used id to select and to initialize regulator descriptors which are chip
> specific. So far there is no comment about OF ID table so I kept it. I
> thought I need to add both of id table and platform id table as name matching
> is required to initialize platform device from mfd.

For platform device is one mechanism how to enumerate device, and bind it to
the driver. The OF ID table needs to be present in case you are using it for
direct DT enumeration (there is also something related to MFD child nodes, but
you need to check and explain how your device is enumerated by this driver).

I.o.w. please clarify how the OF ID table is being used.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

On Tue, Jan 31, 2023 at 01:23:33PM +0000, Sahin, Okan wrote:
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> >> static const struct of_device_id max77541_regulator_of_id[] = {
> >> 	{ .compatible = "adi,max77540-regulator", },
> >> 	{ .compatible = "adi,max77541-regulator", },
> >> 	{ /* sentinel */  }
> >> };
> >> MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);

> >> What do you think?

> >If you have got all necessary data from the upper layer, why do you need to have
> >an ID table here? I'm not sure I understand how this OF ID table works in this
> >case.

> I added it since there is regulator node in device tree. With the help
> of devm_regulator_register(..), driver takes parameters of regulator
> node. I also used id to select and to initialize regulator descriptors
> which are chip specific. So far there is no comment about OF ID table
> so I kept it. I thought I need to add both of id table and platform id
> table as name matching is required to initialize platform device from
> mfd.

You probably shouldn't have compatibles for subdevices of a MFD unless
it's some reusable IP that'll appear elsewhere, especially for something
like the regulators where grouping them all into a single device is very
Linux specific.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Sahin, Okan]

On Tue, 31 Jan 2022 4:30 PM
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

>On Tue, Jan 31, 2023 at 01:23:33PM +0000, Sahin, Okan wrote:
>> On Tue, 31 Jan 2022 3:27 PM
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>> >On Tue, Jan 31, 2023 at 09:27:48AM +0000, Sahin, Okan wrote:
>
>...
>
>> >> Sorry for second question. I do not want to bother you, but I
>> >> realized that I need to be sure about driver_data before sending
>> >> new patch. You said that you need to use pointers directly for
>> >> driver_data then I fixed that part in mfd, but I do not need or
>> >> use driver_data in regulator since chip_id comes from mfd device so
>> >> I think using like below should be enough for my implementation.
>> >>
>> >> static const struct platform_device_id max77541_regulator_platform_id[] =
>{
>> >> 	{ "max77540-regulator", },
>> >> 	{ "max77541-regulator", },
>> >> 	{  /* sentinel */  }
>> >> };
>> >> MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
>> >>
>> >> static const struct of_device_id max77541_regulator_of_id[] = {
>> >> 	{ .compatible = "adi,max77540-regulator", },
>> >> 	{ .compatible = "adi,max77541-regulator", },
>> >> 	{ /* sentinel */  }
>> >> };
>> >> MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);
>> >>
>> >> What do you think?
>> >
>> >If you have got all necessary data from the upper layer, why do you
>> >need to have an ID table here? I'm not sure I understand how this OF
>> >ID table works in this case.
>
>> I added it since there is regulator node in device tree. With the help
>> of devm_regulator_register(..), driver takes parameters of regulator
>> node. I also used id to select and to initialize regulator descriptors
>> which are chip specific. So far there is no comment about OF ID table
>> so I kept it. I thought I need to add both of id table and platform id
>> table as name matching is required to initialize platform device from mfd.
>
>For platform device is one mechanism how to enumerate device, and bind it to
>the driver. The OF ID table needs to be present in case you are using it for direct
>DT enumeration (there is also something related to MFD child nodes, but you
>need to check and explain how your device is enumerated by this driver).
>
>I.o.w. please clarify how the OF ID table is being used.
>
>--
>With Best Regards,
>Andy Shevchenko
>

Hi Andy,

I do not use "of id table" directly in max77541-regulator.c so do I need to exclude it?
However, devm_regulator_register(..) method initialize each regulator with the nodes under "regulators node". If of_match in desc and name of node matches, then regulator will be initialized with parameters in the node under the regulators node in the device tree. Since I am using device tree to initialize regulators, I added of id table. I hope I explained the situation clearly. 

Regards,
Okan Sahin

Re: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Tue, Jan 31, 2023 at 01:59:45PM +0000, Sahin, Okan wrote:
> On Tue, 31 Jan 2022 4:30 PM
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >On Tue, Jan 31, 2023 at 01:23:33PM +0000, Sahin, Okan wrote:
> >> On Tue, 31 Jan 2022 3:27 PM
> >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >> >On Tue, Jan 31, 2023 at 09:27:48AM +0000, Sahin, Okan wrote:

...

> >> >> Sorry for second question. I do not want to bother you, but I
> >> >> realized that I need to be sure about driver_data before sending
> >> >> new patch. You said that you need to use pointers directly for
> >> >> driver_data then I fixed that part in mfd, but I do not need or
> >> >> use driver_data in regulator since chip_id comes from mfd device so
> >> >> I think using like below should be enough for my implementation.
> >> >>
> >> >> static const struct platform_device_id max77541_regulator_platform_id[] =
> >{
> >> >> 	{ "max77540-regulator", },
> >> >> 	{ "max77541-regulator", },
> >> >> 	{  /* sentinel */  }
> >> >> };
> >> >> MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
> >> >>
> >> >> static const struct of_device_id max77541_regulator_of_id[] = {
> >> >> 	{ .compatible = "adi,max77540-regulator", },
> >> >> 	{ .compatible = "adi,max77541-regulator", },
> >> >> 	{ /* sentinel */  }
> >> >> };
> >> >> MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);
> >> >>
> >> >> What do you think?
> >> >
> >> >If you have got all necessary data from the upper layer, why do you
> >> >need to have an ID table here? I'm not sure I understand how this OF
> >> >ID table works in this case.
> >
> >> I added it since there is regulator node in device tree. With the help
> >> of devm_regulator_register(..), driver takes parameters of regulator
> >> node. I also used id to select and to initialize regulator descriptors
> >> which are chip specific. So far there is no comment about OF ID table
> >> so I kept it. I thought I need to add both of id table and platform id
> >> table as name matching is required to initialize platform device from mfd.
> >
> >For platform device is one mechanism how to enumerate device, and bind it to
> >the driver. The OF ID table needs to be present in case you are using it for direct
> >DT enumeration (there is also something related to MFD child nodes, but you
> >need to check and explain how your device is enumerated by this driver).
> >
> >I.o.w. please clarify how the OF ID table is being used.
> 
> I do not use "of id table" directly in max77541-regulator.c so do I need to exclude it?

Exactly my point. How does this OF ID table affect the device enumeration?

> However, devm_regulator_register(..) method initialize each regulator with
> the nodes under "regulators node". If of_match in desc and name of node
> matches, then regulator will be initialized with parameters in the node under
> the regulators node in the device tree. Since I am using device tree to
> initialize regulators, I added of id table. I hope I explained the situation
> clearly.

This is confusing. If your regulator is enumerated via DT, why do you need MFD?

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v3 3/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Sahin, Okan]

On Tue, 31 Jan 2022 4:30 PM
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

>On Tue, Jan 31, 2023 at 01:59:45PM +0000, Sahin, Okan wrote:
>> On Tue, 31 Jan 2022 4:30 PM
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>> >On Tue, Jan 31, 2023 at 01:23:33PM +0000, Sahin, Okan wrote:
>> >> On Tue, 31 Jan 2022 3:27 PM
>> >> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>> >> >On Tue, Jan 31, 2023 at 09:27:48AM +0000, Sahin, Okan wrote:
>
>...
>
>> >> >> Sorry for second question. I do not want to bother you, but I
>> >> >> realized that I need to be sure about driver_data before sending
>> >> >> new patch. You said that you need to use pointers directly for
>> >> >> driver_data then I fixed that part in mfd, but I do not need or
>> >> >> use driver_data in regulator since chip_id comes from mfd device
>> >> >> so I think using like below should be enough for my implementation.
>> >> >>
>> >> >> static const struct platform_device_id
>> >> >> max77541_regulator_platform_id[] =
>> >{
>> >> >> 	{ "max77540-regulator", },
>> >> >> 	{ "max77541-regulator", },
>> >> >> 	{  /* sentinel */  }
>> >> >> };
>> >> >> MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
>> >> >>
>> >> >> static const struct of_device_id max77541_regulator_of_id[] = {
>> >> >> 	{ .compatible = "adi,max77540-regulator", },
>> >> >> 	{ .compatible = "adi,max77541-regulator", },
>> >> >> 	{ /* sentinel */  }
>> >> >> };
>> >> >> MODULE_DEVICE_TABLE(of, max77541_regulator_of_id);
>> >> >>
>> >> >> What do you think?
>> >> >
>> >> >If you have got all necessary data from the upper layer, why do
>> >> >you need to have an ID table here? I'm not sure I understand how
>> >> >this OF ID table works in this case.
>> >
>> >> I added it since there is regulator node in device tree. With the
>> >> help of devm_regulator_register(..), driver takes parameters of
>> >> regulator node. I also used id to select and to initialize
>> >> regulator descriptors which are chip specific. So far there is no
>> >> comment about OF ID table so I kept it. I thought I need to add
>> >> both of id table and platform id table as name matching is required to
>initialize platform device from mfd.
>> >
>> >For platform device is one mechanism how to enumerate device, and
>> >bind it to the driver. The OF ID table needs to be present in case
>> >you are using it for direct DT enumeration (there is also something
>> >related to MFD child nodes, but you need to check and explain how your
>device is enumerated by this driver).
>> >
>> >I.o.w. please clarify how the OF ID table is being used.
>>
>> I do not use "of id table" directly in max77541-regulator.c so do I need to
>exclude it?
>
>Exactly my point. How does this OF ID table affect the device enumeration?
>
Since this is sub-device, it seems OF ID table does not affect the device enumarion, but as I stated before, I thought I need to add OF ID table because regulator's parameters are initialized via DT with the help of devm_regulator_register(..). It scans all the nodes under regulators node.
>> However, devm_regulator_register(..) method initialize each regulator
>> with the nodes under "regulators node". If of_match in desc and name
>> of node matches, then regulator will be initialized with parameters in
>> the node under the regulators node in the device tree. Since I am
>> using device tree to initialize regulators, I added of id table. I
>> hope I explained the situation clearly.
>
>This is confusing. If your regulator is enumerated via DT, why do you need MFD?
MAX77541 has also adc that is why I added MAX77541 as mfd device
>
>--
>With Best Regards,
>Andy Shevchenko
>
Hi Andy,

Thank you for your support and your time.

Regards,
Okan Sahin

Re: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Krzysztof Kozlowski]

On 31/01/2023 13:02, Sahin, Okan wrote:
>>> +  regulators:
>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>
>> No improvements regarding bisectability - this patch fails. If you tested this patch,
>> you would see it.
>>
>> Instead of ignoring comments, either implement them or ask for clarification.
>>
>>
> Sorry for misunderstanding, I checked patchset as a whole not one by one this is why I did not get failure after "make dt_binding_check " . Right now, I understand why you are saying this patch fails, but what is your suggestion?  what is the correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5. In the light of discussion, should I remove all the parts related to regulator in patch 2/5, then add adi,max77541-regulator.yaml and update adi,max77541.yaml in patch 4/5? or should I add new patch to update adi,max77541.yaml?

Regulator binding patch should be first in the series (bindings are
before usage), then the MFD binding should come. Your cover letter
should clearly at the top mention the dependency. You can also mention
dependency in MFD patch after ---, because many of us do not really read
cover letters...


Best regards,
Krzysztof

RE: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Sahin, Okan]

On Tue, 31 Jan 2023 7:44 PM
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

>On 31/01/2023 13:02, Sahin, Okan wrote:
>>>> +  regulators:
>>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>>
>>> No improvements regarding bisectability - this patch fails. If you
>>> tested this patch, you would see it.
>>>
>>> Instead of ignoring comments, either implement them or ask for clarification.
>>>
>>>
>> Sorry for misunderstanding, I checked patchset as a whole not one by one this is
>why I did not get failure after "make dt_binding_check " . Right now, I understand
>why you are saying this patch fails, but what is your suggestion?  what is the
>correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5.
>In the light of discussion, should I remove all the parts related to regulator in
>patch 2/5, then add adi,max77541-regulator.yaml and update
>adi,max77541.yaml in patch 4/5? or should I add new patch to update
>adi,max77541.yaml?
>
>Regulator binding patch should be first in the series (bindings are before usage),
>then the MFD binding should come. Your cover letter should clearly at the top
>mention the dependency. You can also mention dependency in MFD patch after --
>-, because many of us do not really read cover letters...
>
>
>Best regards,
>Krzysztof
Hi Krzysztof,

Thank you for your feedback. I tried to explain in cover letter .However, I understand that it was not clear enough. I do not want to take your time, but let me ask one thing to understand the case completely. Right now, my order is like below
[cover letter]->[mfd driver]->[mfd binding]->[regulator driver]->[regulator binding]->[adc]. 
Should I completely change the ordering  e.g. starting with regulator ending with mfd or is it sufficient to just get the regulator binding just before the mfd bindings?

Regards,
Okan

Re: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Krzysztof Kozlowski]

On 31/01/2023 22:28, Sahin, Okan wrote:
> On Tue, 31 Jan 2023 7:44 PM
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 31/01/2023 13:02, Sahin, Okan wrote:
>>>>> +  regulators:
>>>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>>>
>>>> No improvements regarding bisectability - this patch fails. If you
>>>> tested this patch, you would see it.
>>>>
>>>> Instead of ignoring comments, either implement them or ask for clarification.
>>>>
>>>>
>>> Sorry for misunderstanding, I checked patchset as a whole not one by one this is
>> why I did not get failure after "make dt_binding_check " . Right now, I understand
>> why you are saying this patch fails, but what is your suggestion?  what is the
>> correct order for this patchset? I sent adi,max77541-regulator.yaml in path 4/5.
>> In the light of discussion, should I remove all the parts related to regulator in
>> patch 2/5, then add adi,max77541-regulator.yaml and update
>> adi,max77541.yaml in patch 4/5? or should I add new patch to update
>> adi,max77541.yaml?
>>
>> Regulator binding patch should be first in the series (bindings are before usage),
>> then the MFD binding should come. Your cover letter should clearly at the top
>> mention the dependency. You can also mention dependency in MFD patch after --
>> -, because many of us do not really read cover letters...
>>
>>
>> Best regards,
>> Krzysztof
> Hi Krzysztof,
> 
> Thank you for your feedback. I tried to explain in cover letter .However, I understand that it was not clear enough. I do not want to take your time, but let me ask one thing to understand the case completely. Right now, my order is like below
> [cover letter]->[mfd driver]->[mfd binding]->[regulator driver]->[regulator binding]->[adc]. 
> Should I completely change the ordering  e.g. starting with regulator ending with mfd or is it sufficient to just get the regulator binding just before the mfd bindings?

"bindings are before usage" - what's unclear?

How can you use binding before defining it?

Best regards,
Krzysztof

RE: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Sahin, Okan]

>> On Wed, 1 Feb 2023 10:26 AM
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

>On 31/01/2023 22:28, Sahin, Okan wrote:
>> On Tue, 31 Jan 2023 7:44 PM
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 31/01/2023 13:02, Sahin, Okan wrote:
>>>>>> +  regulators:
>>>>>> +    $ref: /schemas/regulator/adi,max77541-regulator.yaml#
>>>>>
>>>>> No improvements regarding bisectability - this patch fails. If you
>>>>> tested this patch, you would see it.
>>>>>
>>>>> Instead of ignoring comments, either implement them or ask for
>clarification.
>>>>>
>>>>>
>>>> Sorry for misunderstanding, I checked patchset as a whole not one by
>>>> one this is
>>> why I did not get failure after "make dt_binding_check " . Right now,
>>> I understand why you are saying this patch fails, but what is your
>>> suggestion?  what is the correct order for this patchset? I sent adi,max77541-
>regulator.yaml in path 4/5.
>>> In the light of discussion, should I remove all the parts related to
>>> regulator in patch 2/5, then add adi,max77541-regulator.yaml and
>>> update adi,max77541.yaml in patch 4/5? or should I add new patch to
>>> update adi,max77541.yaml?
>>>
>>> Regulator binding patch should be first in the series (bindings are
>>> before usage), then the MFD binding should come. Your cover letter
>>> should clearly at the top mention the dependency. You can also
>>> mention dependency in MFD patch after -- -, because many of us do not really
>read cover letters...
>>>
>>>
>>> Best regards,
>>> Krzysztof
>> Hi Krzysztof,
>>
>> Thank you for your feedback. I tried to explain in cover letter
>> .However, I understand that it was not clear enough. I do not want to take your
>time, but let me ask one thing to understand the case completely. Right now, my
>order is like below [cover letter]->[mfd driver]->[mfd binding]->[regulator driver]-
>>[regulator binding]->[adc].
>> Should I completely change the ordering  e.g. starting with regulator ending
>with mfd or is it sufficient to just get the regulator binding just before the mfd
>bindings?
>
>"bindings are before usage" - what's unclear?
>
>How can you use binding before defining it?
>
>Best regards,
>Krzysztof

Hi Krysztof,

It is crystal clear that "bindings are before usage", but what I want to know is what is the correct order of patchset for mfd device? Max77541 is multi-functional device. It has both adc and regulator subdevices. I thought I need to put mfd driver first, yet it looks wrong to me right now? 

Regards,
Okan

Re: [PATCH v3 2/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Krzysztof Kozlowski]

On 01/02/2023 08:46, Sahin, Okan wrote:
>> "bindings are before usage" - what's unclear?
>>
>> How can you use binding before defining it?
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krysztof,
> 
> It is crystal clear that "bindings are before usage", but what I want to know is what is the correct order of patchset for mfd device? Max77541 is multi-functional device. It has both adc and regulator subdevices. I thought I need to put mfd driver first, yet it looks wrong to me right now?

MFD driver is also usually the last.

Best regards,
Krzysztof