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

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

[Okan Sahin]

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

Changes in v4:
* Patch 1: "dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator"
  * NO CHANGE
* Patch 2: "drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support"
  * Drop OF ID Table
  * Drop driver_data in platform_device_id
* Patch 3: "drivers: iio: adc: Add ADI MAX77541 ADC Support"
  * Add missing blank line
* Patch 4: "dt-bindings: mfd: adi,max77541.yaml Add MAX77541 bindings"
  * NO CHANGE(Order of patchset changed, and [4/5] has dependency to [1/5])
* Patch 5: "drivers: mfd: Add MAX77541/MAX77540 PMIC Support"
  * Use pointers in the driver_data
  * Use probe_new instead of probe
  * Use PLATFORM_DEVID_NONE macro instead of "-1"

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):
  dt-bindings: regulator: Add ADI MAX77541/MAX77540 Regulator
  drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support
  drivers: iio: adc: Add ADI MAX77541 ADC Support
  dt-bindings: mfd: Add ADI MAX77541/MAX77540
  drivers: mfd: Add ADI MAX77541/MAX77540 PMIC Support

 .../devicetree/bindings/mfd/adi,max77541.yaml |  87 +++++++
 .../regulator/adi,max77541-regulator.yaml     |  44 ++++
 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                        | 244 ++++++++++++++++++
 drivers/regulator/Kconfig                     |   9 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/max77541-regulator.c        | 145 +++++++++++
 include/linux/mfd/max77541.h                  | 108 ++++++++
 12 files changed, 864 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 v4 1/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>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../regulator/adi,max77541-regulator.yaml     | 44 +++++++++++++++++++
 1 file changed, 44 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
-- 
2.30.2

[PATCH v4 2/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>
---
 drivers/regulator/Kconfig              |   9 ++
 drivers/regulator/Makefile             |   1 +
 drivers/regulator/max77541-regulator.c | 145 +++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 drivers/regulator/max77541-regulator.c

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..0cf3b041451f
--- /dev/null
+++ b/drivers/regulator/max77541-regulator.c
@@ -0,0 +1,145 @@
+// 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->chip->id == MAX77540)
+		desc = max77540_regulators_desc;
+	else if (max77541->chip->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", },
+	{ "max77541-regulator", },
+	{  /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(platform, max77541_regulator_platform_id);
+
+static struct platform_driver max77541_regulator_driver = {
+	.driver = {
+		.name = "max77541-regulator",
+	},
+	.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 v4 3/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>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/Kconfig        |  11 ++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/max77541-adc.c | 200 +++++++++++++++++++++++++++++++++
 3 files changed, 212 insertions(+)
 create mode 100644 drivers/iio/adc/max77541-adc.c

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..c236122a9549
--- /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/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.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

[PATCH v4 4/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 +++++++++++++++++++
 1 file changed, 87 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;
+                };
+            };
+        };
+    };
-- 
2.30.2

[PATCH v4 5/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>
---
 drivers/mfd/Kconfig          |  13 ++
 drivers/mfd/Makefile         |   1 +
 drivers/mfd/max77541.c       | 244 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/max77541.h | 108 ++++++++++++++++
 4 files changed, 366 insertions(+)
 create mode 100644 drivers/mfd/max77541.c
 create mode 100644 include/linux/mfd/max77541.h

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..f4d8f9e40ada
--- /dev/null
+++ b/drivers/mfd/max77541.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Analog Devices, Inc.
+ * Driver for the MAX77540 and MAX77541
+ */
+
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/core.h>
+#include <linux/of_device.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] = {
+		.id = MAX77540,
+		.n_devs = ARRAY_SIZE(max77540_devs),
+		.devs = max77540_devs,
+	},
+	[MAX77541] = {
+		.id = 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->chip->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");
+
+	return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+				    max77541->chip->devs,
+				    max77541->chip->n_devs,
+				    NULL, 0, NULL);
+}
+
+static const struct i2c_device_id max77541_i2c_id[];
+
+static int max77541_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct max77541 *max77541;
+
+	max77541 = devm_kzalloc(&client->dev, sizeof(*max77541), GFP_KERNEL);
+	if (!max77541)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, max77541);
+	max77541->i2c = client;
+
+	if (dev->of_node)
+		max77541->chip  = of_device_get_match_data(dev);
+	else
+		max77541->chip  = (struct chip_info *)
+					i2c_match_id(max77541_i2c_id,
+						     client)->driver_data;
+	if (!max77541->chip)
+		return -EINVAL;
+
+	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 = &chip[MAX77540],
+	},
+	{
+		.compatible = "adi,max77541",
+		.data = &chip[MAX77541],
+	},
+	{ /* sentinel */  }
+};
+MODULE_DEVICE_TABLE(of, max77541_of_id);
+
+static const struct i2c_device_id max77541_i2c_id[] = {
+	{ "max77540", (kernel_ulong_t)&chip[MAX77540] },
+	{ "max77541", (kernel_ulong_t)&chip[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..986f0f71c196
--- /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,
+	MAX77541,
+};
+
+enum max77541_regulators {
+	MAX77541_BUCK1 = 1,
+	MAX77541_BUCK2,
+};
+
+struct chip_info {
+	enum max7754x_ids id;
+	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

Re: [PATCH v4 2/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Mark Brown]

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

On Wed, Feb 01, 2023 at 01:35:15PM +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.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

> +	if (max77541->chip->id == MAX77540)
> +		desc = max77540_regulators_desc;
> +	else if (max77541->chip->id == MAX77541)
> +		desc = max77541_regulators_desc;
> +	else
> +		return -EINVAL;

Write this as a switch statement for extensibility.

Otherwise this looks good.

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

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

[Krzysztof Kozlowski]

On 01/02/2023 11:35, Okan Sahin 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>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/adc/Kconfig        |  11 ++
>  drivers/iio/adc/Makefile       |   1 +
>  drivers/iio/adc/max77541-adc.c | 200 +++++++++++++++++++++++++++++++++
>  3 files changed, 212 insertions(+)
>  create mode 100644 drivers/iio/adc/max77541-adc.c
> 

The comment about subject prefixes applies also here and to other patches.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 01/02/2023 11:35, 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>
> ---
>  .../regulator/adi,max77541-regulator.yaml     | 44 +++++++++++++++++++
>  1 file changed, 44 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

Why do need the compatible? I don't see any usage of it in the driver
(empty driver data).

Best regards,
Krzysztof

Re: [PATCH v4 4/5] dt-bindings: mfd: Add ADI MAX77541/MAX77540

[Krzysztof Kozlowski]

On 01/02/2023 11:35, 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 +++++++++++++++++++
>  1 file changed, 87 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

You do not use these compatibles and they do not bring the benefit.
Probably this should be all dropped.

> +
> +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 {

So no compatible?

Best regards,
Krzysztof

Re: [PATCH v4 2/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Wed, Feb 01, 2023 at 01:35:15PM +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.

...

With

> +	struct device *dev = &pdev->dev;

to be here the following can be shortened

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

	struct max77541 *max77541 = dev_get_drvdata(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;

	config.dev = dev->parent;

...

> +static const struct platform_device_id max77541_regulator_platform_id[] = {
> +	{ "max77540-regulator", },
> +	{ "max77541-regulator", },

Inner commas are not required.

> +	{  /* sentinel */  }
> +};

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Wed, Feb 01, 2023 at 01:35:16PM +0300, Okan Sahin wrote:
> The MAX77541 has an 8-bit Successive Approximation Register (SAR) ADC
> with four multiplexers for supporting the telemetry feature.

...

> +struct max77541_adc_iio {
> +	struct regmap	*regmap;
> +};

Is it really needed? See below.

...

> +enum max77541_adc_channel {
> +	MAX77541_ADC_VSYS_V = 0,

0 is the default in C enums, why do you need this explicitly? If these are
HW related, you need to assign them all to be crystal clear.

> +	MAX77541_ADC_VOUT1_V,
> +	MAX77541_ADC_VOUT2_V,
> +	MAX77541_ADC_TEMP,
> +};

...

> +		reg_val = FIELD_GET(MAX77541_BITS_MX_CFG1_RNG, reg_val);

> +

Redundant blank line.

> +		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;
> +		}

...

	struct regmap **regmap;

> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*info));

sizeof(*regmap)

> +	if (!indio_dev)
> +		return -ENOMEM;

> +	info = iio_priv(indio_dev);

	regmap = iio_priv(...);

> +	info->regmap = dev_get_regmap(dev->parent, NULL);

	*regmap = ...;

It won't probably gain anything in the code generation or memory footprint,
so up to you. But in general depending on API it might be a benefit.

...

> +static const struct platform_device_id max77541_adc_platform_id[] = {
> +	{ "max77541-adc", },

Inner comma is not needed.

> +	{  /* sentinel */  }
> +};

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v4 2/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Sahin, Okan]

Wed, 1 Feb 2023 5:30 PM
Mark Brown <broonie@kernel.org> wrote:

>On Wed, Feb 01, 2023 at 01:35:15PM +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.
>
>Please submit patches using subject lines reflecting the style for the subsystem,
>this makes it easier for people to identify relevant patches.
>Look at what existing commits in the area you're changing are doing and make
>sure your subject lines visually resemble what they're doing.
>There's no need to resubmit to fix this alone.
>
>> +	if (max77541->chip->id == MAX77540)
>> +		desc = max77540_regulators_desc;
>> +	else if (max77541->chip->id == MAX77541)
>> +		desc = max77541_regulators_desc;
>> +	else
>> +		return -EINVAL;
>
>Write this as a switch statement for extensibility.
>
>Otherwise this looks good.

Hi Mark,

Thank you for your feedback.  I am not sure that I fully understand your feedback. What do you mean by "reflecting style for the subsystem". For example, this patch includes files modified or added under regulator directory as stated in the subject line. Do I need to say anything about mfd as regulator is subdevice?

Regards,
Okan Sahin

Re: [PATCH v4 2/5] drivers: regulator: Add ADI MAX77541/MAX77540 Regulator Support

[Andy Shevchenko]

On Mon, Feb 20, 2023 at 02:58:47PM +0000, Sahin, Okan wrote:
> Wed, 1 Feb 2023 5:30 PM
> Mark Brown <broonie@kernel.org> wrote:
> >On Wed, Feb 01, 2023 at 01:35:15PM +0300, Okan Sahin wrote:

> >Please submit patches using subject lines reflecting the style for the subsystem,
> >this makes it easier for people to identify relevant patches.
> >Look at what existing commits in the area you're changing are doing and make
> >sure your subject lines visually resemble what they're doing.
> >There's no need to resubmit to fix this alone.
> >
> >> +	if (max77541->chip->id == MAX77540)
> >> +		desc = max77540_regulators_desc;
> >> +	else if (max77541->chip->id == MAX77541)
> >> +		desc = max77541_regulators_desc;
> >> +	else
> >> +		return -EINVAL;
> >
> >Write this as a switch statement for extensibility.
> >
> >Otherwise this looks good.

> Thank you for your feedback.  I am not sure that I fully understand your
> feedback. What do you mean by "reflecting style for the subsystem". For
> example, this patch includes files modified or added under regulator
> directory as stated in the subject line. Do I need to say anything about mfd
> as regulator is subdevice?

It's about Subject.

Just run `git log --online --no-merges -- drivers/regulator/max77*` and
look closely at it.

-- 
With Best Regards,
Andy Shevchenko

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

[Lee Jones]

On Wed, 01 Feb 2023, 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.
> 
> Signed-off-by: Okan Sahin <okan.sahin@analog.com>
> ---
>  drivers/mfd/Kconfig          |  13 ++
>  drivers/mfd/Makefile         |   1 +
>  drivers/mfd/max77541.c       | 244 +++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max77541.h | 108 ++++++++++++++++
>  4 files changed, 366 insertions(+)
>  create mode 100644 drivers/mfd/max77541.c
>  create mode 100644 include/linux/mfd/max77541.h
> 
> 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

60-chars is not a good choice for line wrap.  80 would be better.

> +	  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.

Spaces after '.' and ';'.

Please mention the device functionality i.e. the children.

> +
>  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..f4d8f9e40ada
> --- /dev/null
> +++ b/drivers/mfd/max77541.c
> @@ -0,0 +1,244 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2022 Analog Devices, Inc.
> + * Driver for the MAX77540 and MAX77541
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/max77541.h>
> +#include <linux/regmap.h>

Alphabetical.

> +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,

MAX77541_REG_INT_SRC_M?

> +	.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"),

No line-wrap required here.

Please use 100-chars throughout the C file.

> +};
> +
> +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[] = {

Why do you need this require sub-structure? 

> +	[MAX77540] = {
> +		.id = MAX77540,
> +		.n_devs = ARRAY_SIZE(max77540_devs),
> +		.devs = max77540_devs,
> +	},
> +	[MAX77541] = {
> +		.id = 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->chip->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;

What are you doing with 'val' before over-writing it in the next line?

> +	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");
> +
> +	return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> +				    max77541->chip->devs,
> +				    max77541->chip->n_devs,
> +				    NULL, 0, NULL);

We usually place these at the bottom of .probe()
> +}
> +
> +static const struct i2c_device_id max77541_i2c_id[];
> +
> +static int max77541_i2c_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct max77541 *max77541;
> +
> +	max77541 = devm_kzalloc(&client->dev, sizeof(*max77541), GFP_KERNEL);

"dev"

> +	if (!max77541)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, max77541);
> +	max77541->i2c = client;
> +
> +	if (dev->of_node)
> +		max77541->chip  = of_device_get_match_data(dev);
> +	else
> +		max77541->chip  = (struct chip_info *)
> +					i2c_match_id(max77541_i2c_id,
> +						     client)->driver_data;
> +	if (!max77541->chip)
> +		return -EINVAL;
> +
> +	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 = &chip[MAX77540],
> +	},
> +	{
> +		.compatible = "adi,max77541",
> +		.data = &chip[MAX77541],
> +	},
> +	{ /* sentinel */  }
> +};
> +MODULE_DEVICE_TABLE(of, max77541_of_id);
> +
> +static const struct i2c_device_id max77541_i2c_id[] = {
> +	{ "max77540", (kernel_ulong_t)&chip[MAX77540] },
> +	{ "max77541", (kernel_ulong_t)&chip[MAX77541] },

Just 'MAX77540' is fine.

> +	{ /* sentinel */ }

Remove the comment, we know how terminators work.

Same comments for max77541_of_id.

> +};
> +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,

The "i2c" parts here are superfluous.

> +};
> +module_i2c_driver(max77541_i2c_driver);
> +
> +MODULE_DESCRIPTION("MAX7740/MAX7741 MFD Driver");

Drop all mention of MFD from the code.

> +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..986f0f71c196
> --- /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__

__MFD_<DEVICE>__

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

Inconsistent.

> +#define MAX77541_REG_INT_SRC_MASK               0x00
> +#define MAX77541_REG_TOPSYS_INT_MASK            0x00
> +#define MAX77541_REG_BUCK_INT_MASK              0x00
> +
> +/*BITS OF REGISTERS*/

Spaces.

> +#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)

It's nice when these are listed under the their registers, like:

#define MAX77541_REG_TOPSYS_INT                   0x02
#define MAX77541_REG_TOPSYS_INT_M                 0x03

#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       */

Just one space each side is fine.

> +#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) }

Same line.

Although, I'm not sure this adds to anything in the code.

Use REGMAP_IRQ_REG() or just the .mask line in place.

> +enum max7754x_ids {
> +	MAX77540,
> +	MAX77541,
> +};
> +
> +enum max77541_regulators {
> +	MAX77541_BUCK1 = 1,
> +	MAX77541_BUCK2,
> +};
> +
> +struct chip_info {
> +	enum max7754x_ids id;
> +	int n_devs;
> +	const struct mfd_cell *devs;
> +};
> +
> +struct regmap;
> +struct regmap_irq_chip_data;
> +struct i2c_client;
> +
> +struct max77541 {
> +	enum max7754x_ids id;

Why do you need this at all?

You definitely don't need it twice.

> +	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;

These are the important ones.  Please place them at the top.

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

-- 
Lee Jones [李琼斯]