[PATCH v2 0/8] Add MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support

[PATCH v2 0/8] Add MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support

[Zeynep Arslanbenzer]

Changes in v2:
* Add MAX77643 MAX77654 MAX77658 support

Zeynep Arslanbenzer (8):
  dt-bindings: regulator: max77658: Add ADI MAX77643/54/58/59 Regulator
  regulator: max77658: Add ADI MAX77643/54/58/59 Regulator Support
  dt-bindings: power: supply: max77658: Add ADI MAX77654/58/59 Charger
  power: supply: max77658: Add ADI MAX77654/58/59 Charger Support
  dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery
  power: supply: max77658: Add ADI MAX77658 Battery Support
  dt-bindings: mfd: max77658: Add ADI MAX77658
  mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

 .../devicetree/bindings/mfd/adi,max77658.yaml | 199 +++++
 .../power/supply/adi,max77658-battery.yaml    |  58 ++
 .../power/supply/adi,max77658-charger.yaml    |  65 ++
 .../regulator/adi,max77658-regulator.yaml     |  32 +
 drivers/mfd/Kconfig                           |  15 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/max77658.c                        | 448 +++++++++++
 drivers/power/supply/Kconfig                  |  14 +
 drivers/power/supply/Makefile                 |   2 +
 drivers/power/supply/max77658-battery.c       | 646 +++++++++++++++
 drivers/power/supply/max77658-charger.c       | 745 ++++++++++++++++++
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/max77658-regulator.c        | 171 ++++
 include/linux/mfd/max77658.h                  |  88 +++
 15 files changed, 2493 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77658.yaml
 create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml
 create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml
 create mode 100644 drivers/mfd/max77658.c
 create mode 100644 drivers/power/supply/max77658-battery.c
 create mode 100644 drivers/power/supply/max77658-charger.c
 create mode 100644 drivers/regulator/max77658-regulator.c
 create mode 100644 include/linux/mfd/max77658.h

-- 
2.25.1

[PATCH v2 1/8] dt-bindings: regulator: max77658: Add ADI MAX77643/54/58/59 Regulator

[Zeynep Arslanbenzer]

Add ADI MAX77643/MAX77654/MAX77658/MAX77659 Regulator devicetree document.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 .../regulator/adi,max77658-regulator.yaml     | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml

diff --git a/Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml
new file mode 100644
index 000000000000..1d097eddcd98
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/adi,max77658-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulator for MAX77658 PMICs family from ADI
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+description: |
+  This is part of the MAX77658 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/adi,max77658.yaml.
+
+  The regulators is represented as a sub-node of the PMIC node on the device tree.
+
+patternProperties:
+  "^LDO[01]$":
+    type: object
+    $ref: regulator.yaml#
+    additionalProperties: false
+    description: |
+      LDO regulator.
+
+    properties:
+      regulator-always-on: true
+      regulator-boot-on: true
+
+additionalProperties: false
+...
-- 
2.25.1

[PATCH v2 2/8] regulator: max77658: Add ADI MAX77643/54/58/59 Regulator Support

[Zeynep Arslanbenzer]

Regulator driver for ADI MAX77643/MAX77654/MAX77658/MAX77659.

MAX77643/MAX77659 has 1 LDO regulator.
MAX77654/MAX77658 has two LDO regulators.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 drivers/regulator/Kconfig              |   8 ++
 drivers/regulator/Makefile             |   1 +
 drivers/regulator/max77658-regulator.c | 171 +++++++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 drivers/regulator/max77658-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 820c9a0788e5..98591d637d0c 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -573,6 +573,14 @@ config REGULATOR_MAX77650
 	  Semiconductor. This device has a SIMO with three independent
 	  power rails and an LDO.
 
+config REGULATOR_MAX77658
+	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 Regulator"
+	depends on MFD_MAX77658
+	help
+	  Regulator driver for MAX77643/MAX77654/MAX77658/MAX77659
+	  PMIC from Analog Devices. This driver supports LDO regulators.
+	  Say Y here to enable the regulator driver.
+
 config REGULATOR_MAX8649
 	tristate "Maxim 8649 voltage regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index b9f5eb35bf5f..14b5f9fd75bf 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
 obj-$(CONFIG_REGULATOR_MAX597X) += max597x-regulator.o
 obj-$(CONFIG_REGULATOR_MAX77620) += max77620-regulator.o
 obj-$(CONFIG_REGULATOR_MAX77650) += max77650-regulator.o
+obj-$(CONFIG_REGULATOR_MAX77658) += max77658-regulator.o
 obj-$(CONFIG_REGULATOR_MAX8649)	+= max8649.o
 obj-$(CONFIG_REGULATOR_MAX8660) += max8660.o
 obj-$(CONFIG_REGULATOR_MAX8893) += max8893.o
diff --git a/drivers/regulator/max77658-regulator.c b/drivers/regulator/max77658-regulator.c
new file mode 100644
index 000000000000..09ef72b979e4
--- /dev/null
+++ b/drivers/regulator/max77658-regulator.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 Analog Devices, Inc.
+ * ADI regulator driver for the MAX77643/MAX77654/MAX77658/MAX77659
+ */
+
+#include <linux/mfd/max77658.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#define MAX77658_LDO_VOLT_REG_MAX	0x7F
+#define MAX77658_LDO_VOLT_N_RANGE	0x80
+#define MAX77658_LDO_VOLT_STEP		25000
+#define MAX77658_LDO_VOLT_BASE		500000
+#define MAX77654_LDO_VOLT_BASE		800000
+
+#define MAX77658_REG_CNFG_LDO0_A	0x48
+#define MAX77658_REG_CNFG_LDO0_B	0x49
+
+#define MAX77654_REG_CNFG_LDO0_A	0x38
+#define MAX77654_REG_CNFG_LDO0_B	0x39
+
+#define MAX77658_REG_CNFG_LDO1_A	0x4A
+#define MAX77658_REG_CNFG_LDO1_B	0x4B
+
+#define MAX77654_REG_CNFG_LDO1_A	0x3A
+#define MAX77654_REG_CNFG_LDO1_B	0x3B
+
+#define MAX77658_BITS_CONFIG_LDOX_A_TV_LDO	GENMASK(6, 0)
+#define MAX77658_BITS_CONFIG_LDOX_B_EN_LDO	GENMASK(2, 0)
+
+#define MAX77658_REG_MAX	2
+
+/*
+ * 0.500 to 3.675V (25mV step)
+ */
+static const struct linear_range MAX77658_LDO_volts[] = {
+	REGULATOR_LINEAR_RANGE(MAX77658_LDO_VOLT_BASE, 0x00,
+			       MAX77658_LDO_VOLT_REG_MAX,
+			       MAX77658_LDO_VOLT_STEP),
+};
+
+static const struct linear_range MAX77654_LDO_volts[] = {
+	REGULATOR_LINEAR_RANGE(MAX77654_LDO_VOLT_BASE, 0x00,
+			       MAX77658_LDO_VOLT_REG_MAX,
+			       MAX77658_LDO_VOLT_STEP),
+};
+
+static const struct regulator_ops max77658_LDO_ops = {
+	.list_voltage	 = regulator_list_voltage_linear_range,
+	.map_voltage	 = regulator_map_voltage_ascend,
+	.is_enabled	 = regulator_is_enabled_regmap,
+	.enable		 = regulator_enable_regmap,
+	.disable	 = regulator_disable_regmap,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_voltage_sel = regulator_set_voltage_sel_regmap,
+};
+
+#define REGULATOR_DESC_LDO(num, volts, vsel_r, vsel_m, enable_r, enable_m) { \
+	.name			= "LDO"#num,				\
+	.id			= num,					\
+	.of_match		= of_match_ptr("LDO"#num),		\
+	.regulators_node	= of_match_ptr("regulators"),		\
+	.ops			= &max77658_LDO_ops,			\
+	.type			= REGULATOR_VOLTAGE,			\
+	.owner			= THIS_MODULE,				\
+	.linear_ranges		= volts,				\
+	.n_linear_ranges	= MAX77658_LDO_VOLT_N_RANGE,		\
+	.vsel_reg		= vsel_r,				\
+	.vsel_mask		= vsel_m,				\
+	.enable_reg		= enable_r,				\
+	.enable_mask		= enable_m,				\
+	.enable_val		= 0x06,					\
+	.disable_val		= 0x04,					\
+}
+
+static const struct regulator_desc max77643_ldo_desc[] = {
+	REGULATOR_DESC_LDO(0, MAX77658_LDO_volts, MAX77654_REG_CNFG_LDO0_A,
+			   MAX77658_BITS_CONFIG_LDOX_A_TV_LDO,
+			   MAX77654_REG_CNFG_LDO0_B,
+			   MAX77658_BITS_CONFIG_LDOX_B_EN_LDO),
+};
+
+static const struct regulator_desc max77654_ldo_desc[] = {
+	REGULATOR_DESC_LDO(0, MAX77654_LDO_volts, MAX77654_REG_CNFG_LDO0_A,
+			   MAX77658_BITS_CONFIG_LDOX_A_TV_LDO,
+			   MAX77654_REG_CNFG_LDO0_B,
+			   MAX77658_BITS_CONFIG_LDOX_B_EN_LDO),
+	REGULATOR_DESC_LDO(1, MAX77654_LDO_volts, MAX77654_REG_CNFG_LDO1_A,
+			   MAX77658_BITS_CONFIG_LDOX_A_TV_LDO,
+			   MAX77654_REG_CNFG_LDO1_B,
+			   MAX77658_BITS_CONFIG_LDOX_B_EN_LDO),
+};
+
+static const struct regulator_desc max77658_ldo_desc[] = {
+	REGULATOR_DESC_LDO(0, MAX77658_LDO_volts, MAX77658_REG_CNFG_LDO0_A,
+			   MAX77658_BITS_CONFIG_LDOX_A_TV_LDO,
+			   MAX77658_REG_CNFG_LDO0_B,
+			   MAX77658_BITS_CONFIG_LDOX_B_EN_LDO),
+	REGULATOR_DESC_LDO(1, MAX77658_LDO_volts, MAX77658_REG_CNFG_LDO1_A,
+			   MAX77658_BITS_CONFIG_LDOX_A_TV_LDO,
+			   MAX77658_REG_CNFG_LDO1_B,
+			   MAX77658_BITS_CONFIG_LDOX_B_EN_LDO),
+};
+
+static int max77658_regulator_probe(struct platform_device *pdev)
+{
+	struct max77658_dev *max77658 = dev_get_drvdata(pdev->dev.parent);
+	const struct regulator_desc *regulators;
+	struct regulator_config config = {};
+	struct regulator_dev *rdev;
+	int n_regulators = 0;
+	int i;
+
+	switch (max77658->chip->id) {
+	case ID_MAX77643:
+	case ID_MAX77659:
+		regulators = max77643_ldo_desc;
+		n_regulators = ARRAY_SIZE(max77643_ldo_desc);
+		break;
+	case ID_MAX77654:
+		regulators = max77654_ldo_desc;
+		n_regulators = ARRAY_SIZE(max77654_ldo_desc);
+		break;
+	case ID_MAX77658:
+		regulators = max77658_ldo_desc;
+		n_regulators = ARRAY_SIZE(max77658_ldo_desc);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	config.dev = pdev->dev.parent;
+
+	for (i = 0; i < n_regulators; i++) {
+		rdev = devm_regulator_register(&pdev->dev, &regulators[i],
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "Failed to register %s regulator\n",
+				regulators[i].name);
+			return PTR_ERR(rdev);
+		}
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id max77658_regulator_id[] = {
+	{ "max77643-regulator" },
+	{ "max77654-regulator" },
+	{ "max77658-regulator" },
+	{ "max77659-regulator" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, max77658_regulator_id);
+
+static struct platform_driver max77658_regulator_driver = {
+	.driver = {
+		.name = "max77658-regulator",
+	},
+	.probe = max77658_regulator_probe,
+	.id_table = max77658_regulator_id,
+};
+
+module_platform_driver(max77658_regulator_driver);
+
+MODULE_DESCRIPTION("MAX77658 Regulator Driver");
+MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v2 3/8] dt-bindings: power: supply: max77658: Add ADI MAX77654/58/59 Charger

[Zeynep Arslanbenzer]

Add ADI MAX77654/MAX77658/MAX77659 power supply devicetree document.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 .../power/supply/adi,max77658-charger.yaml    | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml b/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml
new file mode 100644
index 000000000000..f140abab969c
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/adi,max77658-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Battery charger for MAX77658 PMICs family from ADI.
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+description: |
+  This module is part of the MAX77658 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/adi,max77658.yaml.
+
+  The charger is represented as a sub-node of the PMIC node on the device tree.
+
+properties:
+  compatible:
+    enum:
+      - adi,max77654-charger
+      - adi,max77658-charger
+      - adi,max77659-charger
+
+  adi,fast-charge-timer:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Fast-charge safety timer value (in hours).
+    enum: [3, 5, 7]
+
+  adi,topoff-timer:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Top-Off timer value (in minutes).
+    enum: [0, 5, 10, 15, 20, 25, 30, 35]
+
+  adi,input-current-limit-microamp:
+    description: Input current limit value.
+
+  monitored-battery:
+    description: >
+      phandle to a "simple-battery" compatible node.
+
+      This property must be a phandle to a node using the format described
+      in battery.yaml, with the following properties being required:
+      - constant-charge-current-max-microamp
+
+allOf:
+  - $ref: power-supply.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max77659-charger
+
+    then:
+      properties:
+        adi,input-current-limit-microamp: false
+
+required:
+  - compatible
+
+additionalProperties: false
+
+...
-- 
2.25.1

[PATCH v2 4/8] power: supply: max77658: Add ADI MAX77654/58/59 Charger Support

[Zeynep Arslanbenzer]

Charger driver for ADI MAX77654/MAX77658/MAX77659.

The MAX77654/MAX77658/MAX77659 charger is Smart Power Selector Li+/Li-Poly Charger.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 drivers/power/supply/Kconfig            |   7 +
 drivers/power/supply/Makefile           |   1 +
 drivers/power/supply/max77658-charger.c | 745 ++++++++++++++++++++++++
 3 files changed, 753 insertions(+)
 create mode 100644 drivers/power/supply/max77658-charger.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 0bbfe6a7ce4d..5787a4e90c98 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -565,6 +565,13 @@ config CHARGER_MAX77650
 	  Say Y to enable support for the battery charger control of MAX77650
 	  PMICs.
 
+config CHARGER_MAX77658
+	tristate "Analog Devices MAX77654/MAX77658/MAX77659 battery charger driver"
+	depends on MFD_MAX77658
+	help
+	  Say Y to enable support for the battery charger control of
+	  MAX77654/MAX77658/MAX77659 PMIC.
+
 config CHARGER_MAX77693
 	tristate "Maxim MAX77693 battery charger driver"
 	depends on MFD_MAX77693
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 0ee8653e882e..af4bd6e5969f 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_CHARGER_LTC4162L)	+= ltc4162-l-charger.o
 obj-$(CONFIG_CHARGER_MAX14577)	+= max14577_charger.o
 obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)	+= max14656_charger_detector.o
 obj-$(CONFIG_CHARGER_MAX77650)	+= max77650-charger.o
+obj-$(CONFIG_CHARGER_MAX77658)	+= max77658-charger.o
 obj-$(CONFIG_CHARGER_MAX77693)	+= max77693_charger.o
 obj-$(CONFIG_CHARGER_MAX77976)	+= max77976_charger.o
 obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
diff --git a/drivers/power/supply/max77658-charger.c b/drivers/power/supply/max77658-charger.c
new file mode 100644
index 000000000000..0d42d50ebf40
--- /dev/null
+++ b/drivers/power/supply/max77658-charger.c
@@ -0,0 +1,745 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 Analog Devices, Inc.
+ * ADI charger driver for the MAX77654/MAX77658/MAX77659
+ */
+
+#include <linux/bitfield.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max77658.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+#define MAX77659_CHG_IRQ_MAX			5
+
+#define MAX77658_CHARGER_CURRENT_MAX		300000
+#define MAX77658_CHARGER_CURRENT_MIN		7500
+#define MAX77658_CHARGER_CURRENT_STEP		7500
+#define MAX77658_CNFG_B_ICHGIN_LIM_MAX		475000
+#define MAX77658_CNFG_B_ICHGIN_LIM_MIN		95000
+#define MAX77658_CNFG_B_ICHGIN_LIM_STEP		95000
+#define MAX77658_CNFG_B_ICHGIN_LIM_REG_MAX	4
+#define MAX77658_CHARGER_TOPOFF_TIMER_STEP	5
+
+#define MAX77658_REG_STAT_CHG_A	0x02
+#define MAX77658_REG_STAT_CHG_B	0x03
+#define MAX77658_REG_CNFG_CHG_A	0x20
+#define MAX77658_REG_CNFG_CHG_B	0x21
+#define MAX77658_REG_CNFG_CHG_C	0x22
+#define MAX77658_REG_CNFG_CHG_D	0x23
+#define MAX77658_REG_CNFG_CHG_E	0x24
+#define MAX77658_REG_CNFG_CHG_F	0x25
+#define MAX77658_REG_CNFG_CHG_G	0x26
+#define MAX77658_REG_CNFG_CHG_H	0x27
+#define MAX77658_REG_CNFG_CHG_I	0x28
+
+#define MAX77658_BIT_STAT_A_THM_DTLS		GENMASK(2, 0)
+#define MAX77658_BIT_STAT_A_TJ_REG_STAT		BIT(3)
+#define MAX77658_BIT_STAT_A_VSYSY_MIN_STAT	BIT(4)
+#define MAX77658_BIT_STAT_A_ICHGIN_LIM_STAT	BIT(5)
+#define MAX77658_BIT_STAT_A_VCHGIN_MIN_STAT	BIT(6)
+#define MAX77658_BIT_STAT_B_CHG			BIT(1)
+#define MAX77658_BIT_STAT_B_CHGIN_DTSL		GENMASK(3, 2)
+#define MAX77658_BIT_STAT_B_CHG_DTLS		GENMASK(7, 4)
+#define MAX77658_BIT_CNFG_B_CHG_EN		BIT(0)
+#define MAX77658_BIT_CNFG_B_ICHGIN_LIM		GENMASK(4, 2)
+#define MAX77658_BIT_CNFG_C_TOPOFFTIMER		GENMASK(2, 0)
+#define MAX77658_BIT_CNFG_E_TFASTCHG		GENMASK(1, 0)
+#define MAX77658_BIT_CNFG_E_CC			GENMASK(7, 2)
+#define MAX77658_BIT_CNFG_G_CHG_CV		GENMASK(7, 2)
+
+enum {
+	MAX77658_CHG_DTLS_OFF,
+	MAX77658_CHG_DTLS_PREQUAL,
+	MAX77658_CHG_DTLS_FASTCHARGE_CC,
+	MAX77658_CHG_DTLS_JEITA_FASTCHARGE_CC,
+	MAX77658_CHG_DTLS_FASTCHARGE_CV,
+	MAX77658_CHG_DTLS_JEITA_FASTCHARGE_CV,
+	MAX77658_CHG_DTLS_TOPOFF,
+	MAX77658_CHG_DTLS_JEITA_TOPOFF,
+	MAX77658_CHG_DTLS_DONE,
+	MAX77658_CHG_DTLS_JEITA_DONE,
+	MAX77658_CHG_DTLS_OFF_TIMER_FAULT,
+	MAX77658_CHG_DTLS_OFF_CHARGE_TIMER_FAULT,
+	MAX77658_CHG_DTLS_OFF_BATTERY_TEMP_FAULT,
+	MAX77658_CHG_DTLS_RESERVED_13,
+};
+
+enum {
+	MAX77658_CHG_IRQ_THM_I,
+	MAX77658_CHG_IRQ_CHG_I,
+	MAX77658_CHG_IRQ_CHGIN_I,
+	MAX77658_CHG_IRQ_TJ_REG_I,
+	MAX77658_CHG_IRQ_SYS_CTRL_I,
+	MAX77658_CHG_IRQ_CHGIN_CTRL_I,
+	MAX77658_CHG_IRQ_SYS_CNFG_I,
+	MAX77658_CHG_IRQ_MAX,
+};
+
+struct max77658_charger {
+	struct device *dev;
+	struct max77658_dev *max77658;
+	struct regmap *regmap;
+
+	struct power_supply *psy_chg;
+	struct power_supply_desc psy_chg_d;
+
+	int irq;
+	int irq_arr[MAX77658_CHG_IRQ_MAX];
+	int irq_mask;
+
+	struct delayed_work irq_work;
+
+	int present;
+	int health;
+	int status;
+	int charge_type;
+
+	unsigned int fast_charge_current_ua;
+	unsigned int fast_charge_timer_hr;
+	unsigned int input_current_limit_ua;
+	unsigned int topoff_timer_min;
+};
+
+static int max77658_set_input_current_limit(struct max77658_charger *charger,
+					    int input_current)
+{
+	u8 reg_data = 0;
+
+	input_current = clamp_val(input_current, MAX77658_CNFG_B_ICHGIN_LIM_MIN,
+				  MAX77658_CNFG_B_ICHGIN_LIM_MAX);
+	if (charger->max77658->chip->id == ID_MAX77658) {
+		reg_data = MAX77658_CNFG_B_ICHGIN_LIM_REG_MAX + 1
+			   - DIV_ROUND_UP(input_current,
+					  MAX77658_CNFG_B_ICHGIN_LIM_STEP);
+	} else {
+		reg_data = DIV_ROUND_UP(input_current,
+					MAX77658_CNFG_B_ICHGIN_LIM_STEP) - 1;
+	}
+
+	reg_data = FIELD_PREP(MAX77658_BIT_CNFG_B_ICHGIN_LIM, reg_data);
+	return regmap_update_bits(charger->regmap, MAX77658_REG_CNFG_CHG_B,
+				  MAX77658_BIT_CNFG_B_ICHGIN_LIM, reg_data);
+}
+
+static int max77658_get_input_current_limit(struct max77658_charger *charger,
+					    int *get_current)
+{
+	unsigned int reg_data = 0;
+	int ret;
+
+	ret = regmap_read(charger->regmap, MAX77658_REG_CNFG_CHG_B, &reg_data);
+	if (ret)
+		return ret;
+
+	reg_data = FIELD_GET(MAX77658_BIT_CNFG_B_ICHGIN_LIM, reg_data);
+
+	if (charger->max77658->chip->id == ID_MAX77658) {
+		*get_current = (MAX77658_CNFG_B_ICHGIN_LIM_REG_MAX + 1
+				- reg_data) * MAX77658_CNFG_B_ICHGIN_LIM_STEP;
+	} else {
+		*get_current = (reg_data + 1) * MAX77658_CNFG_B_ICHGIN_LIM_STEP;
+	}
+
+	return 0;
+}
+
+static int max77658_set_charge_current(struct max77658_charger *charger,
+				       int fast_charge_current_ua)
+{
+	unsigned int reg_data;
+
+	fast_charge_current_ua = clamp_val(fast_charge_current_ua,
+					   MAX77658_CHARGER_CURRENT_MIN,
+					   MAX77658_CHARGER_CURRENT_MAX);
+	reg_data = fast_charge_current_ua / MAX77658_CHARGER_CURRENT_STEP - 1;
+	reg_data = FIELD_PREP(MAX77658_BIT_CNFG_E_CC, reg_data);
+
+	return regmap_update_bits(charger->regmap, MAX77658_REG_CNFG_CHG_E,
+				  MAX77658_BIT_CNFG_E_CC, reg_data);
+}
+
+static int max77658_get_charge_current(struct max77658_charger *charger,
+				       int *get_current)
+{
+	unsigned int reg_data, current_val;
+	int ret;
+
+	ret = regmap_read(charger->regmap, MAX77658_REG_CNFG_CHG_E, &reg_data);
+	if (ret)
+		return ret;
+
+	reg_data = FIELD_GET(MAX77658_BIT_CNFG_E_CC, reg_data);
+	current_val = (reg_data + 1) * MAX77658_CHARGER_CURRENT_STEP;
+
+	*get_current = clamp_val(current_val, MAX77658_CHARGER_CURRENT_MIN,
+				 MAX77658_CHARGER_CURRENT_MAX);
+
+	return 0;
+}
+
+static int max77658_set_topoff_timer(struct max77658_charger *charger,
+				     int termination_time_min)
+{
+	unsigned int reg_data;
+
+	termination_time_min = clamp_val(termination_time_min, 0,
+					 MAX77658_BIT_CNFG_C_TOPOFFTIMER
+					 * MAX77658_CHARGER_TOPOFF_TIMER_STEP);
+	reg_data = FIELD_PREP(MAX77658_BIT_CNFG_C_TOPOFFTIMER,
+			      termination_time_min
+			      / MAX77658_CHARGER_TOPOFF_TIMER_STEP);
+
+	return regmap_update_bits(charger->regmap, MAX77658_REG_CNFG_CHG_C,
+				  MAX77658_BIT_CNFG_C_TOPOFFTIMER, reg_data);
+}
+
+static int max77658_charger_initialize(struct max77658_charger *charger)
+{
+	unsigned int val;
+	int ret;
+
+	ret = max77658_set_charge_current(charger,
+					  charger->fast_charge_current_ua);
+	if (ret)
+		return dev_err_probe(charger->dev, ret,
+				     "Error in writing register CNFG_CHG_C\n");
+
+	if (charger->fast_charge_timer_hr == 0 ||
+	    charger->fast_charge_timer_hr > 7)
+		val = 0x00;
+	else if (charger->fast_charge_timer_hr < 3)
+		val = 0x01;
+	else
+		val = DIV_ROUND_UP(charger->fast_charge_timer_hr - 1, 2);
+
+	ret = regmap_update_bits(charger->regmap, MAX77658_REG_CNFG_CHG_E,
+				 MAX77658_BIT_CNFG_E_TFASTCHG, val);
+	if (ret)
+		return dev_err_probe(charger->dev, ret,
+				     "Error in writing register CNFG_CHG_E\n");
+
+	ret = max77658_set_input_current_limit(charger,
+					       charger->input_current_limit_ua);
+	if (ret)
+		return dev_err_probe(charger->dev, ret,
+				     "Error in writing register CNFG_CHG_B\n");
+
+	return max77658_set_topoff_timer(charger, charger->topoff_timer_min);
+}
+
+static void max77658_charger_parse_dt(struct max77658_charger *charger)
+{
+	int ret;
+
+	ret = device_property_read_u32(charger->dev, "adi,fast-charge-timer",
+				       &charger->fast_charge_timer_hr);
+	if (ret) {
+		dev_dbg(charger->dev,
+			"Could not read fast-charge-timer DT property\n");
+		charger->fast_charge_timer_hr = 5;
+	}
+
+	ret = device_property_read_u32(charger->dev, "adi,topoff-timer",
+				       &charger->topoff_timer_min);
+	if (ret) {
+		dev_dbg(charger->dev,
+			"Could not read topoff-timer DT property\n");
+		charger->topoff_timer_min = 30;
+	}
+
+	ret = device_property_read_u32(charger->dev,
+				       "adi,input-current-limit-microamp",
+		&charger->input_current_limit_ua);
+	if (ret) {
+		dev_dbg(charger->dev,
+			"Could not read input-current-limit DT property\n");
+		charger->input_current_limit_ua = 475000;
+	}
+}
+
+struct max77658_charger_status_map {
+	int health;
+	int status;
+	int charge_type;
+};
+
+#define STATUS_MAP(_MAX77658_CHG_DTLS, _health, _status, _charge_type) \
+	[MAX77658_CHG_DTLS_##_MAX77658_CHG_DTLS] = {\
+		.health = POWER_SUPPLY_HEALTH_##_health,\
+		.status = POWER_SUPPLY_STATUS_##_status,\
+		.charge_type = POWER_SUPPLY_CHARGE_TYPE_##_charge_type,\
+	}
+
+static struct max77658_charger_status_map max77658_charger_status_map[] = {
+	/* chg_details_xx, health, status, charge_type */
+	STATUS_MAP(OFF, UNKNOWN, NOT_CHARGING, NONE),
+	STATUS_MAP(PREQUAL, GOOD, CHARGING, TRICKLE),
+	STATUS_MAP(FASTCHARGE_CC, GOOD, CHARGING, FAST),
+	STATUS_MAP(JEITA_FASTCHARGE_CC, GOOD, CHARGING, FAST),
+	STATUS_MAP(FASTCHARGE_CV, GOOD, CHARGING, FAST),
+	STATUS_MAP(JEITA_FASTCHARGE_CV, GOOD, CHARGING, FAST),
+	STATUS_MAP(TOPOFF, GOOD, CHARGING, FAST),
+	STATUS_MAP(JEITA_TOPOFF, GOOD, CHARGING, FAST),
+	STATUS_MAP(DONE, GOOD, FULL, NONE),
+	STATUS_MAP(JEITA_DONE, GOOD, FULL, NONE),
+	STATUS_MAP(OFF_TIMER_FAULT, SAFETY_TIMER_EXPIRE, NOT_CHARGING, NONE),
+	STATUS_MAP(OFF_CHARGE_TIMER_FAULT, UNKNOWN, NOT_CHARGING, NONE),
+	STATUS_MAP(OFF_BATTERY_TEMP_FAULT, HOT, NOT_CHARGING, NONE),
+};
+
+static int max77658_charger_update(struct max77658_charger *charger)
+{
+	unsigned int stat_chg_b, chg_dtls;
+	int ret;
+
+	ret = regmap_read(charger->regmap, MAX77658_REG_STAT_CHG_B,
+			  &stat_chg_b);
+	if (ret) {
+		charger->health = POWER_SUPPLY_HEALTH_UNKNOWN;
+		charger->status = POWER_SUPPLY_STATUS_UNKNOWN;
+		charger->charge_type = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+		return ret;
+	}
+
+	charger->present = stat_chg_b & MAX77658_BIT_STAT_B_CHG;
+	if (!charger->present) {
+		charger->health = POWER_SUPPLY_HEALTH_UNKNOWN;
+		charger->status = POWER_SUPPLY_STATUS_DISCHARGING;
+		charger->charge_type = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+		return 0;
+	}
+
+	chg_dtls = FIELD_GET(MAX77658_BIT_STAT_B_CHG_DTLS, stat_chg_b);
+
+	charger->health = max77658_charger_status_map[chg_dtls].health;
+	charger->status = max77658_charger_status_map[chg_dtls].status;
+	charger->charge_type =
+			max77658_charger_status_map[chg_dtls].charge_type;
+
+	return 0;
+}
+
+static enum power_supply_property max77658_charger_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CURRENT_MAX
+};
+
+static enum power_supply_property max77659_charger_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_CURRENT_NOW
+};
+
+static int max77658_property_is_writeable(struct power_supply *psy,
+					  enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static int max77659_property_is_writeable(struct power_supply *psy,
+					  enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+static int max77658_charger_get_property(struct power_supply *psy,
+					 enum power_supply_property psp,
+					 union power_supply_propval *val)
+{
+	struct max77658_charger *charger = power_supply_get_drvdata(psy);
+	int ret;
+
+	ret = max77658_charger_update(charger);
+	if (ret)
+		return ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = charger->present;
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		val->intval = charger->health;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = charger->status;
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		val->intval = charger->charge_type;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = max77658_get_charge_current(charger, &val->intval);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		if (charger->max77658->chip->id != ID_MAX77659) {
+			ret = max77658_get_input_current_limit(charger,
+							       &val->intval);
+		} else {
+			ret = -EINVAL;
+		}
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static int max77658_charger_set_property(struct power_supply *psy,
+					 enum power_supply_property psp,
+					 const union power_supply_propval *val)
+{
+	struct max77658_charger *charger = power_supply_get_drvdata(psy);
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = regmap_update_bits(charger->regmap,
+					 MAX77658_REG_CNFG_CHG_B,
+					 MAX77658_BIT_CNFG_B_CHG_EN,
+					 !!val->intval);
+		if (ret)
+			return ret;
+
+		ret =
+		max77658_set_charge_current(charger,
+					    charger->fast_charge_current_ua);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		/* val->intval - uA */
+		ret = max77658_set_charge_current(charger, val->intval);
+		if (ret)
+			return ret;
+
+		charger->fast_charge_current_ua = val->intval;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		if (charger->max77658->chip->id == ID_MAX77659) {
+			ret = -EINVAL;
+			break;
+		}
+		ret = max77658_set_input_current_limit(charger, val->intval);
+		if (ret)
+			return ret;
+
+		charger->input_current_limit_ua = val->intval;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static void max77658_charger_irq_handler(struct max77658_charger *charger,
+					 int irq)
+{
+	int chg_present, vchgin, ichgin;
+	unsigned int val, stat_chg_b;
+	int ret;
+
+	switch (irq) {
+	case MAX77658_CHG_IRQ_THM_I:
+		ret = regmap_read(charger->regmap, MAX77658_REG_STAT_CHG_A,
+				  &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77658_BIT_STAT_A_THM_DTLS, val);
+		dev_dbg(charger->dev, "CHG_INT_THM: thm_dtls = %02Xh\n", val);
+		break;
+
+	case MAX77658_CHG_IRQ_CHG_I:
+		ret = regmap_read(charger->regmap, MAX77658_REG_STAT_CHG_B,
+				  &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_B\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77658_BIT_STAT_B_CHG_DTLS, val);
+		dev_dbg(charger->dev,
+			"CHG_INT_CHG: MAX77658_CHG_DTLS = %02Xh\n", val);
+		break;
+
+	case MAX77658_CHG_IRQ_CHGIN_I:
+		ret = regmap_read(charger->regmap,
+				  MAX77658_REG_STAT_CHG_B, &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_B\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77658_BIT_STAT_B_CHG_DTLS, val);
+		dev_dbg(charger->dev,
+			"CHG_INT_CHG: MAX77658_CHG_DTLS = %02Xh\n", val);
+
+		ret = regmap_read(charger->regmap, MAX77658_REG_STAT_CHG_B,
+				  &stat_chg_b);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_B\n");
+			return;
+		}
+
+		chg_present = stat_chg_b & MAX77658_BIT_STAT_B_CHG;
+
+		regmap_update_bits(charger->regmap, MAX77658_REG_CNFG_CHG_B,
+				   MAX77658_BIT_CNFG_B_CHG_EN, !!chg_present);
+		if (chg_present)
+			break;
+
+		max77658_set_charge_current(charger,
+					    charger->fast_charge_current_ua);
+		break;
+
+	case MAX77658_CHG_IRQ_TJ_REG_I:
+		ret = regmap_read(charger->regmap, MAX77658_REG_STAT_CHG_A,
+				  &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77658_BIT_STAT_A_TJ_REG_STAT, val);
+		dev_dbg(charger->dev,
+			"CHG_INT_TJ_REG: tj_reg_stat = %02Xh\n", val);
+		dev_dbg(charger->dev,
+			"CHG_INT_TJ_REG: Die temperature %s Tj-reg\n",
+			 val ? "has exceeded" : "has not exceeded");
+		break;
+
+	case MAX77658_CHG_IRQ_SYS_CTRL_I:
+		ret = regmap_read(charger->regmap, MAX77658_REG_STAT_CHG_A,
+				  &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77658_BIT_STAT_A_VSYSY_MIN_STAT, val);
+		dev_dbg(charger->dev,
+			"CHG_INT_SYS_CTRL: The minimum system voltage regulation loop %s\n",
+			 val ? "has engaged" : "has not engaged");
+		break;
+
+	case MAX77658_CHG_IRQ_CHGIN_CTRL_I:
+		ret = regmap_read(charger->regmap, MAX77658_REG_STAT_CHG_A,
+				  &val);
+		if (ret) {
+			dev_dbg(charger->dev, "Failed to read STAT_CHG_A\n");
+			return;
+		}
+
+		vchgin = FIELD_GET(MAX77658_BIT_STAT_A_VCHGIN_MIN_STAT, val);
+		dev_dbg(charger->dev, "CHG_INT_CHGIN: VCHGIN_MIN_STAT %s\n",
+			vchgin ? "has changed" : "has not changed");
+
+		ichgin = FIELD_GET(MAX77658_BIT_STAT_A_ICHGIN_LIM_STAT, val);
+		dev_dbg(charger->dev, "CHG_INT_CHGIN: ICHGIN_LIM_STAT %s\n",
+			ichgin ? "has changed" : "has not changed");
+		break;
+
+	case MAX77658_CHG_IRQ_SYS_CNFG_I:
+		ret = regmap_read(charger->regmap, MAX77658_REG_CNFG_CHG_G,
+				  &val);
+		if (ret) {
+			dev_dbg(charger->dev, "Failed to read CNFG_CHG_G\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77658_BIT_CNFG_G_CHG_CV, val);
+		dev_dbg(charger->dev, "CHG_INT_SYS_CNFG: CHG_VC = %02Xh\n",
+			val);
+		break;
+
+	default:
+		break;
+	}
+
+	power_supply_changed(charger->psy_chg);
+}
+
+static irqreturn_t max77658_charger_isr(int irq, void *data)
+{
+	struct max77658_charger *charger = data;
+
+	charger->irq = irq;
+	max77658_charger_update(charger);
+	max77658_charger_irq_handler(charger,
+				     charger->irq - charger->irq_arr[0]);
+
+	return IRQ_HANDLED;
+}
+
+static const char * const max77658_irq_descs[] = {
+	"charger-thm",
+	"charger-chg",
+	"charger-chgin",
+	"charger-tj-reg",
+	"charger-sys-ctrl",
+	"charger-chgin-ctrl",
+	"charger-sys-cnfg",
+};
+
+static int max77658_charger_probe(struct platform_device *pdev)
+{
+	struct max77658_dev *max77658 = dev_get_drvdata(pdev->dev.parent);
+	struct power_supply_config charger_cfg = {};
+	struct power_supply_battery_info *info;
+	struct max77658_charger *charger;
+	struct device *dev = &pdev->dev;
+	int i, n_irq, ret;
+
+	charger = devm_kzalloc(dev, sizeof(*charger), GFP_KERNEL);
+	if (!charger)
+		return -ENOMEM;
+
+	charger->dev = dev;
+	charger->max77658 = max77658;
+	charger->regmap = dev_get_regmap(charger->dev->parent, NULL);
+
+	charger->psy_chg_d.get_property	= max77658_charger_get_property;
+	charger->psy_chg_d.set_property	= max77658_charger_set_property;
+	charger->psy_chg_d.type = POWER_SUPPLY_TYPE_USB;
+	charger_cfg.of_node = dev->of_node;
+	charger_cfg.drv_data = charger;
+
+	switch (max77658->chip->id) {
+	case ID_MAX77654:
+		charger->psy_chg_d.properties = max77658_charger_props;
+		charger->psy_chg_d.num_properties =
+					ARRAY_SIZE(max77658_charger_props);
+		charger->psy_chg_d.name = "max77654-charger";
+		charger->psy_chg_d.property_is_writeable =
+				max77658_property_is_writeable;
+		n_irq = MAX77658_CHG_IRQ_MAX;
+		break;
+	case ID_MAX77658:
+		charger->psy_chg_d.properties = max77658_charger_props;
+		charger->psy_chg_d.num_properties =
+					ARRAY_SIZE(max77658_charger_props);
+		charger->psy_chg_d.name = "max77658-charger";
+		charger->psy_chg_d.property_is_writeable =
+				max77658_property_is_writeable;
+		n_irq = MAX77658_CHG_IRQ_MAX;
+		break;
+	case ID_MAX77659:
+		charger->psy_chg_d.properties = max77659_charger_props;
+		charger->psy_chg_d.num_properties =
+					ARRAY_SIZE(max77659_charger_props);
+		charger->psy_chg_d.name = "max77659-charger";
+		charger->psy_chg_d.property_is_writeable =
+				max77659_property_is_writeable;
+		n_irq = MAX77659_CHG_IRQ_MAX;
+		break;
+	default:
+		break;
+	}
+
+	charger->psy_chg = devm_power_supply_register(dev, &charger->psy_chg_d,
+						      &charger_cfg);
+	if (IS_ERR(charger->psy_chg))
+		return dev_err_probe(dev, PTR_ERR(charger->psy_chg),
+				     "Failed to register power supply\n");
+
+	charger->psy_chg->of_node = of_get_child_by_name(dev->parent->of_node,
+							 "charger");
+
+	if (!charger->psy_chg->of_node)
+		dev_err(charger->dev,
+			"of_get_child_by_name\n");
+
+	ret = power_supply_get_battery_info(charger->psy_chg, &info);
+	if (ret) {
+		dev_err(charger->dev, "Unable to get charger info\n");
+		charger->fast_charge_current_ua = 15000;
+	} else {
+		charger->fast_charge_current_ua =
+			info->constant_charge_current_max_ua;
+	}
+
+	max77658_charger_parse_dt(charger);
+
+	ret = max77658_charger_initialize(charger);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to initialize charger\n");
+
+	for (i = 0; i < n_irq; i++) {
+		charger->irq_arr[i] =
+				regmap_irq_get_virq(max77658->irqc_chg, i);
+
+		if (charger->irq_arr[i] < 0)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid IRQ for MAX77658_CHG_IRQ %d\n",
+					     i);
+
+		ret = devm_request_threaded_irq(dev, charger->irq_arr[i],
+						NULL, max77658_charger_isr,
+						IRQF_TRIGGER_FALLING,
+						max77658_irq_descs[i], charger);
+		if (ret)
+			return dev_err_probe(dev, ret,
+						 "Failed to request irq: %d\n",
+						 charger->irq_arr[i]);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id max77658_charger_of_id[] = {
+	{ .compatible = "adi,max77654-charger" },
+	{ .compatible = "adi,max77658-charger" },
+	{ .compatible = "adi,max77659-charger" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max77658_charger_of_id);
+
+static const struct platform_device_id max77658_charger_id[] = {
+	{ "max77654-charger" },
+	{ "max77658-charger" },
+	{ "max77659-charger" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, max77658_charger_id);
+
+static struct platform_driver max77658_charger_driver = {
+	.driver = {
+		.name = "max77658-charger",
+		.of_match_table = max77658_charger_of_id,
+	},
+	.probe = max77658_charger_probe,
+	.id_table = max77658_charger_id,
+};
+
+module_platform_driver(max77658_charger_driver);
+
+MODULE_DESCRIPTION("MAX77658 Charger Driver");
+MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v2 5/8] dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery

[Zeynep Arslanbenzer]

Add ADI MAX77658 power supply devicetree document.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 .../power/supply/adi,max77658-battery.yaml    | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml b/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml
new file mode 100644
index 000000000000..0b696f7c4d1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/adi,max77658-battery.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Battery for MAX77658 PMIC from ADI.
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+description: |
+  This module is part of the MAX77658 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/adi,max77658.yaml.
+
+  The fuel gauge is represented as a sub-node of the PMIC node on the device tree.
+
+properties:
+  compatible:
+    const:
+      adi,max77658-battery
+
+  reg:
+    maxItems: 1
+
+  adi,valrt-min-microvolt:
+    description: Minimum voltage value that triggers the alarm.
+
+  adi,valrt-max-microvolt:
+    description: Maximum voltage value that triggers the alarm.
+
+  adi,salrt-min-percent:
+    description: Minimum percentage of battery that triggers the alarm.
+
+  adi,salrt-max-percent:
+    description: Maximum percentage of battery that triggers the alarm.
+
+  adi,ialrt-min-microamp:
+    description: Minimum current value that triggers the alarm.
+
+  adi,ialrt-max-microamp:
+    description: Maximum current value that triggers the alarm.
+
+  monitored-battery:
+    description: >
+      phandle to a "simple-battery" compatible node.
+
+      This property must be a phandle to a node using the format described
+      in battery.yaml, with the following properties being required:
+      - alert-celsius
+
+required:
+  - compatible
+
+additionalProperties: false
+
+...
-- 
2.25.1

[PATCH v2 6/8] power: supply: max77658: Add ADI MAX77658 Battery Support

[Zeynep Arslanbenzer]

Battery driver for ADI MAX77658.

The MAX77658 is an ultra-low power fuel gauge which implements the Maxim ModelGauge m5 EZ algorithm.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 drivers/power/supply/Kconfig            |   7 +
 drivers/power/supply/Makefile           |   1 +
 drivers/power/supply/max77658-battery.c | 646 ++++++++++++++++++++++++
 3 files changed, 654 insertions(+)
 create mode 100644 drivers/power/supply/max77658-battery.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 5787a4e90c98..0e6855501070 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -572,6 +572,13 @@ config CHARGER_MAX77658
 	  Say Y to enable support for the battery charger control of
 	  MAX77654/MAX77658/MAX77659 PMIC.
 
+config BATTERY_MAX77658
+	tristate "Analog Devices MAX77658 battery driver"
+	depends on MFD_MAX77658
+	help
+	  Say Y to enable support for the battery control of
+	  MAX77658 PMIC.
+
 config CHARGER_MAX77693
 	tristate "Maxim MAX77693 battery charger driver"
 	depends on MFD_MAX77693
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index af4bd6e5969f..e5a425d333a7 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_CHARGER_MAX14577)	+= max14577_charger.o
 obj-$(CONFIG_CHARGER_DETECTOR_MAX14656)	+= max14656_charger_detector.o
 obj-$(CONFIG_CHARGER_MAX77650)	+= max77650-charger.o
 obj-$(CONFIG_CHARGER_MAX77658)	+= max77658-charger.o
+obj-$(CONFIG_BATTERY_MAX77658)	+= max77658-battery.o
 obj-$(CONFIG_CHARGER_MAX77693)	+= max77693_charger.o
 obj-$(CONFIG_CHARGER_MAX77976)	+= max77976_charger.o
 obj-$(CONFIG_CHARGER_MAX8997)	+= max8997_charger.o
diff --git a/drivers/power/supply/max77658-battery.c b/drivers/power/supply/max77658-battery.c
new file mode 100644
index 000000000000..23af6c97f734
--- /dev/null
+++ b/drivers/power/supply/max77658-battery.c
@@ -0,0 +1,646 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 Analog Devices, Inc.
+ * ADI battery driver for the MAX77658
+ */
+
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/mfd/max77658.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+#define MAX77658_IALRTTH_RESOLUTION	8567
+#define MAX77658_CURRENT_RESOLUTION	33487
+#define MAX77658_VOLTAGE_RESOLUTION	78125
+#define MAX77658_FG_DELAY		1000
+#define MAX77658_BATTERY_FULL		100
+#define MAX77658_BATTERY_LOW		40
+#define MAX77658_BATTERY_CRITICAL	10
+#define MAX77658_MAXMINVOLT_STEP	20000
+#define MAX77658_VALRTTH_STEP		20000
+#define MAX77658_VEMPTY_VE_STEP		10000
+#define MAX77658_POWER_STEP		17100
+
+#define MAX77658_REG_STATUS	0x00
+#define MAX77658_REG_VALRTTH	0x01
+#define MAX77658_REG_TALRTTH	0x02
+#define MAX77658_REG_SALRTTH	0x03
+#define MAX77658_REG_CONFIG	0x1D
+#define MAX77658_REG_DEVNAME	0x21
+#define MAX77658_REG_VEMPTY	0x3A
+#define MAX77658_REG_AVGPOWER	0xB3
+#define MAX77658_REG_IALRTTH	0xB4
+#define MAX77658_REG_CONFIG2	0xBB
+#define MAX77658_REG_TEMP	0x08
+#define MAX77658_REG_VCELL	0x09
+#define MAX77658_REG_CURRENT	0x0A
+#define MAX77658_REG_AVGCURRENT	0x0B
+#define MAX77658_REG_AVGVCELL	0x19
+#define MAX77658_REG_MAXMINTEMP	0x1A
+#define MAX77658_REG_MAXMINVOLT	0x1B
+#define MAX77658_REG_MAXMINCURR	0x1C
+#define MAX77658_REG_REPSOC	0x06
+#define MAX77658_REG_TTE	0x11
+#define MAX77658_REG_TTF	0x20
+
+#define MAX77658_BIT_STATUS_BR		BIT(15)
+#define MAX77658_BIT_STATUS_SMX		BIT(14)
+#define MAX77658_BIT_STATUS_TMX		BIT(13)
+#define MAX77658_BIT_STATUS_VMX		BIT(12)
+#define MAX77658_BIT_STATUS_BI		BIT(11)
+#define MAX77658_BIT_STATUS_SMN		BIT(10)
+#define MAX77658_BIT_STATUS_TMN		BIT(9)
+#define MAX77658_BIT_STATUS_VMN		BIT(8)
+#define MAX77658_BIT_STATUS_POR		BIT(2)
+#define	MAX77658_BIT_CONFIG_AEN		BIT(2)
+#define MAX77658_BIT_SALRTTH_SMIN	GENMASK(7, 0)
+#define MAX77658_BIT_SALRTTH_SMAX	GENMASK(15, 8)
+#define MAX77658_BIT_TALRTTH_TMIN	GENMASK(7, 0)
+#define MAX77658_BIT_TALRTTH_TMAX	GENMASK(15, 8)
+#define MAX77658_BIT_IALRTTH_IMIN	GENMASK(7, 0)
+#define MAX77658_BIT_IALRTTH_IMAX	GENMASK(15, 8)
+#define MAX77658_BIT_MAXMINTEMP_MIN	GENMASK(7, 0)
+#define MAX77658_BIT_MAXMINTEMP_MAX	GENMASK(15, 8)
+#define MAX77658_BIT_MAXMINVOLT_MIN	GENMASK(7, 0)
+#define MAX77658_BIT_MAXMINVOLT_MAX	GENMASK(15, 8)
+#define MAX77658_BIT_VEMPTY_VE		GENMASK(15, 7)
+
+struct max77658_fg {
+	struct device *dev;
+	struct max77658_dev *max77658;
+	struct regmap *regmap;
+
+	struct delayed_work work;
+	struct power_supply *battery;
+	struct power_supply_desc psy_batt_d;
+
+	int vcell;
+	int soc;
+	int health;
+	int capacity_level;
+
+	int lasttime_vcell;
+	int lasttime_soc;
+
+	int volt_min_uv;
+	int volt_max_uv;
+	int temp_alert_min;
+	int temp_alert_max;
+	int soc_max;
+	int soc_min;
+	int curr_max_ma;
+	int curr_min_ma;
+};
+
+static int max77658_fg_get_soc(struct max77658_fg *max77658_fg)
+{
+	unsigned int soc;
+	int ret;
+
+	ret = regmap_read(max77658_fg->regmap, MAX77658_REG_REPSOC, &soc);
+	if (ret < 0) {
+		dev_dbg(max77658_fg->dev, "Error in reading register REPSOC\n");
+		return ret;
+	}
+
+	max77658_fg->soc = (u16)(soc >> 8);
+
+	if (max77658_fg->soc > MAX77658_BATTERY_FULL) {
+		max77658_fg->soc = MAX77658_BATTERY_FULL;
+		max77658_fg->capacity_level = POWER_SUPPLY_CAPACITY_LEVEL_FULL;
+		max77658_fg->health = POWER_SUPPLY_HEALTH_GOOD;
+	} else if (max77658_fg->soc < MAX77658_BATTERY_CRITICAL) {
+		max77658_fg->health = POWER_SUPPLY_HEALTH_DEAD;
+		max77658_fg->capacity_level =
+					POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
+	} else if (max77658_fg->soc < MAX77658_BATTERY_LOW) {
+		max77658_fg->health = POWER_SUPPLY_HEALTH_DEAD;
+		max77658_fg->capacity_level = POWER_SUPPLY_CAPACITY_LEVEL_LOW;
+	} else {
+		max77658_fg->health = POWER_SUPPLY_HEALTH_GOOD;
+		max77658_fg->capacity_level =
+					POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;
+	}
+
+	return 0;
+}
+
+static void max77658_stop_work(void *data)
+{
+	struct max77658_fg *max77658_fg = data;
+
+	cancel_delayed_work_sync(&max77658_fg->work);
+}
+
+static u16 max77658_raw_voltage_to_uv(unsigned int val)
+{
+	return (u16)val * MAX77658_VOLTAGE_RESOLUTION;
+}
+
+static void max77658_fg_work(struct work_struct *work)
+{
+	struct max77658_fg *max77658_fg;
+	unsigned int val;
+	int ret;
+
+	max77658_fg = container_of(work, struct max77658_fg, work.work);
+
+	ret = regmap_read(max77658_fg->regmap, MAX77658_REG_VCELL, &val);
+	if (ret < 0)
+		dev_err(max77658_fg->dev, "Error in reading register\n");
+	else
+		max77658_fg->vcell = max77658_raw_voltage_to_uv(val);
+
+	ret = max77658_fg_get_soc(max77658_fg);
+	if (ret < 0)
+		goto out;
+
+	if (max77658_fg->vcell != max77658_fg->lasttime_vcell ||
+	    max77658_fg->soc != max77658_fg->lasttime_soc) {
+		max77658_fg->lasttime_vcell = max77658_fg->vcell;
+		max77658_fg->lasttime_soc = max77658_fg->soc;
+
+		power_supply_changed(max77658_fg->battery);
+	}
+
+out:
+	schedule_delayed_work(&max77658_fg->work, MAX77658_FG_DELAY);
+}
+
+static enum power_supply_property max77658_fg_battery_props[] = {
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_CAPACITY_LEVEL,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_AVG,
+	POWER_SUPPLY_PROP_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN,
+	POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
+	POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN,
+	POWER_SUPPLY_PROP_CAPACITY_ALERT_MAX,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
+	POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CURRENT_MAX,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
+	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+	POWER_SUPPLY_PROP_TEMP_MAX,
+	POWER_SUPPLY_PROP_TEMP_MIN,
+	POWER_SUPPLY_PROP_POWER_AVG
+};
+
+static int max77658_property_is_writeable(struct power_supply *psy,
+					  enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN:
+	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MAX:
+	case POWER_SUPPLY_PROP_TEMP_MAX:
+	case POWER_SUPPLY_PROP_TEMP_MIN:
+		return 1;
+	default:
+		return 0;
+	}
+
+	return 0;
+}
+
+static u16 max77658_raw_current_to_ua(unsigned int val)
+{
+	return sign_extend32(val, 15) * MAX77658_CURRENT_RESOLUTION / 1000000;
+}
+
+static int max77658_fg_get_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    union power_supply_propval *val)
+{
+	struct max77658_fg *max77658_fg = power_supply_get_drvdata(psy);
+	unsigned int reg_val;
+	u8 field_val;
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = max77658_fg->soc;
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		val->intval = max77658_fg->health;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY_LEVEL:
+		val->intval = max77658_fg->capacity_level;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = max77658_fg->vcell;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_AVG:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_AVGVCELL,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = max77658_raw_voltage_to_uv(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MAX:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_MAXMINVOLT,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = FIELD_GET(MAX77658_BIT_MAXMINVOLT_MAX, reg_val)
+			      * MAX77658_MAXMINVOLT_STEP;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_MAXMINVOLT,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = FIELD_GET(MAX77658_BIT_MAXMINVOLT_MIN, reg_val)
+			      * MAX77658_MAXMINVOLT_STEP;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_VEMPTY,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = FIELD_GET(MAX77658_BIT_VEMPTY_VE, reg_val)
+					* MAX77658_VEMPTY_VE_STEP;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_SALRTTH,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = FIELD_GET(MAX77658_BIT_SALRTTH_SMIN, reg_val);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MAX:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_SALRTTH,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = FIELD_GET(MAX77658_BIT_SALRTTH_SMAX, reg_val);
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_TEMP,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = sign_extend32(reg_val, 15);
+		val->intval = val->intval >> 8;
+		break;
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_TALRTTH,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = sign_extend32(reg_val & 0xff, 7);
+		break;
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_TALRTTH,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = sign_extend32(reg_val >> 8, 7);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_CURRENT,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = max77658_raw_current_to_ua(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_AVGCURRENT,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = max77658_raw_current_to_ua(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_TTE,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = (reg_val * 45) >> 3;
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_TTF,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = (reg_val * 45) >> 3;
+		break;
+	case POWER_SUPPLY_PROP_TEMP_MIN:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_MAXMINTEMP,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		field_val = FIELD_GET(MAX77658_BIT_MAXMINTEMP_MIN, reg_val);
+		val->intval = sign_extend32(field_val, 7);
+		break;
+	case POWER_SUPPLY_PROP_TEMP_MAX:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_MAXMINTEMP,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		field_val = FIELD_GET(MAX77658_BIT_MAXMINTEMP_MAX, reg_val);
+		val->intval = sign_extend32(field_val, 7);
+
+		break;
+	case POWER_SUPPLY_PROP_POWER_AVG:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_AVGPOWER,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = reg_val * MAX77658_POWER_STEP;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		ret = regmap_read(max77658_fg->regmap, MAX77658_REG_MAXMINCURR,
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		val->intval = FIELD_GET(MAX77658_BIT_MAXMINTEMP_MAX, reg_val)
+			      * MAX77658_IALRTTH_RESOLUTION;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int max77658_fg_set_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    const union power_supply_propval *val)
+{
+	struct max77658_fg *max77658_fg = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
+		ret = regmap_update_bits(max77658_fg->regmap,
+					 MAX77658_REG_TALRTTH,
+					 MAX77658_BIT_TALRTTH_TMIN,
+					 val->intval);
+		break;
+	case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
+		ret = regmap_update_bits(max77658_fg->regmap,
+					 MAX77658_REG_TALRTTH,
+					 MAX77658_BIT_TALRTTH_TMAX,
+					 val->intval);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MIN:
+		ret = regmap_update_bits(max77658_fg->regmap,
+					 MAX77658_REG_SALRTTH,
+					 MAX77658_BIT_SALRTTH_SMIN,
+					 val->intval);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY_ALERT_MAX:
+		ret = regmap_update_bits(max77658_fg->regmap,
+					 MAX77658_REG_SALRTTH,
+					 MAX77658_BIT_SALRTTH_SMAX,
+					 val->intval);
+		break;
+	case POWER_SUPPLY_PROP_TEMP_MIN:
+		ret = regmap_update_bits(max77658_fg->regmap,
+					 MAX77658_REG_MAXMINTEMP,
+					 MAX77658_BIT_MAXMINTEMP_MIN,
+					 val->intval);
+		break;
+	case POWER_SUPPLY_PROP_TEMP_MAX:
+		ret = regmap_update_bits(max77658_fg->regmap,
+					 MAX77658_REG_MAXMINTEMP,
+					 MAX77658_BIT_MAXMINTEMP_MAX,
+					 val->intval);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int max77658_set_alert_thresholds(struct max77658_fg *max77658_fg)
+{
+	unsigned int val;
+	int ret;
+
+	val = (max77658_fg->volt_min_uv / MAX77658_VALRTTH_STEP);
+	val |= ((max77658_fg->volt_max_uv / MAX77658_VALRTTH_STEP) << 8);
+	ret = regmap_write(max77658_fg->regmap, MAX77658_REG_VALRTTH, val);
+	if (ret)
+		return dev_err_probe(max77658_fg->dev, ret,
+				     "Error in writing register VALRTTH\n");
+
+	val = FIELD_PREP(MAX77658_BIT_TALRTTH_TMAX, max77658_fg->temp_alert_max)
+	   | FIELD_PREP(MAX77658_BIT_TALRTTH_TMIN, max77658_fg->temp_alert_min);
+	ret = regmap_write(max77658_fg->regmap, MAX77658_REG_TALRTTH, val);
+	if (ret)
+		return dev_err_probe(max77658_fg->dev, ret,
+				     "Error in writing register TALRTTH\n");
+
+	val = FIELD_PREP(MAX77658_BIT_SALRTTH_SMAX, max77658_fg->soc_max) |
+		  FIELD_PREP(MAX77658_BIT_SALRTTH_SMIN, max77658_fg->soc_min);
+	ret = regmap_write(max77658_fg->regmap, MAX77658_REG_SALRTTH, val);
+	if (ret)
+		return dev_err_probe(max77658_fg->dev, ret,
+				     "Error in writing register SALRTTH\n");
+
+	val = FIELD_PREP(MAX77658_BIT_IALRTTH_IMAX,
+			 max77658_fg->curr_max_ma / MAX77658_IALRTTH_RESOLUTION)
+	      | FIELD_PREP(MAX77658_BIT_IALRTTH_IMIN,
+	      (max77658_fg->curr_min_ma / MAX77658_IALRTTH_RESOLUTION));
+
+	return regmap_write(max77658_fg->regmap, MAX77658_REG_IALRTTH, val);
+}
+
+static int max77658_fg_initialize(struct max77658_fg *max77658_fg)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = max77658_set_alert_thresholds(max77658_fg);
+	if (ret)
+		return dev_err_probe(max77658_fg->dev, ret,
+				     "Error in setting alert thresholds\n");
+
+	ret = regmap_read(max77658_fg->regmap, MAX77658_REG_STATUS, &reg);
+	if (ret)
+		return dev_err_probe(max77658_fg->dev, ret,
+				     "Error in reading register STATUS\n");
+
+	ret = regmap_write(max77658_fg->regmap, MAX77658_REG_STATUS,
+			   reg & ~MAX77658_BIT_STATUS_POR);
+	if (ret)
+		return dev_err_probe(max77658_fg->dev, ret,
+				     "Error in writing register STATUS\n");
+
+	schedule_delayed_work(&max77658_fg->work, MAX77658_FG_DELAY);
+	return 0;
+}
+
+static void max77658_fg_parse_dt(struct max77658_fg *max77658_fg)
+{
+	struct device *dev = max77658_fg->dev;
+	int ret;
+
+	ret = device_property_read_u32(dev, "adi,valrt-min-microvolt",
+				       &max77658_fg->volt_min_uv);
+	if (ret) {
+		dev_dbg(dev,
+			"Could not read adi,valrt-min-microvolt DT property\n");
+		max77658_fg->volt_min_uv = 0;
+	}
+
+	ret = device_property_read_u32(dev, "adi,valrt-max-microvolt",
+				       &max77658_fg->volt_max_uv);
+	if (ret) {
+		dev_dbg(dev,
+			"Could not read adi,valrt-max-microvolt DT property\n");
+		max77658_fg->volt_max_uv = 5100000;
+	}
+
+	ret = device_property_read_u32(dev, "adi,salrt-min-percent",
+				       &max77658_fg->soc_min);
+	if (ret) {
+		dev_dbg(dev,
+			"Could not read adi,salrt-min-percent DT property\n");
+		max77658_fg->soc_min = 0;
+	}
+
+	ret = device_property_read_u32(dev, "adi,salrt-max-percent",
+				       &max77658_fg->soc_max);
+	if (ret) {
+		dev_dbg(dev,
+			"Could not read adi,salrt-max-percent DT property\n");
+		max77658_fg->soc_max = 255;
+	}
+
+	ret = device_property_read_u32(dev, "adi,ialrt-min-microamp",
+				       &max77658_fg->curr_min_ma);
+	if (ret) {
+		dev_dbg(dev,
+			"Could not read adi,ialrt-min-microamp DT property\n");
+		max77658_fg->curr_min_ma = MAX77658_IALRTTH_RESOLUTION * (-128);
+	}
+
+	ret = device_property_read_u32(dev, "adi,ialrt-max-microamp",
+				       &max77658_fg->curr_max_ma);
+	if (ret) {
+		dev_dbg(dev,
+			"Could not read adi,ialrt-max-microamp DT property\n");
+		max77658_fg->curr_max_ma = MAX77658_IALRTTH_RESOLUTION * 127;
+	}
+}
+
+static int max77658_fg_probe(struct platform_device *pdev)
+{
+	struct max77658_dev *max77658 = dev_get_drvdata(pdev->dev.parent);
+	struct power_supply_battery_info *info;
+	struct power_supply_config fg_cfg = {};
+	struct device *dev = &pdev->dev;
+	struct max77658_fg *fg;
+	int ret = 0;
+
+	fg = devm_kzalloc(&pdev->dev, sizeof(*fg), GFP_KERNEL);
+	if (!fg)
+		return -ENOMEM;
+
+	fg->dev = &pdev->dev;
+	fg->regmap = max77658->regmap_fg;
+
+	fg->psy_batt_d.name = "max77658-battery";
+	fg->psy_batt_d.type = POWER_SUPPLY_TYPE_BATTERY;
+	fg->psy_batt_d.get_property = max77658_fg_get_property;
+	fg->psy_batt_d.set_property = max77658_fg_set_property;
+	fg->psy_batt_d.properties = max77658_fg_battery_props;
+	fg->psy_batt_d.property_is_writeable = max77658_property_is_writeable;
+	fg->psy_batt_d.num_properties = ARRAY_SIZE(max77658_fg_battery_props);
+	fg_cfg.drv_data = fg;
+
+	INIT_DELAYED_WORK(&fg->work, max77658_fg_work);
+	ret = devm_add_action(&pdev->dev, max77658_stop_work, fg);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Error in setting delayed work\n");
+
+	fg->battery = devm_power_supply_register(dev, &fg->psy_batt_d,
+						 &fg_cfg);
+	if (IS_ERR(fg->battery))
+		return dev_err_probe(&pdev->dev, PTR_ERR(fg->battery),
+				     "Failed to register battery\n");
+
+	fg->battery->of_node = of_get_child_by_name(dev->parent->of_node,
+						    "fuel-gauge");
+
+	if (!fg->battery->of_node)
+		dev_err(dev,
+			"of_get_child_by_name\n");
+
+	ret = power_supply_get_battery_info(fg->battery, &info);
+	if (ret) {
+		dev_err(fg->dev, "Unable to get battery info\n");
+		fg->temp_alert_min = info->temp_alert_min;
+		fg->temp_alert_max = info->temp_alert_max;
+	} else {
+		fg->temp_alert_min = -128;
+		fg->temp_alert_max = 127;
+	}
+
+	max77658_fg_parse_dt(fg);
+
+	ret =  max77658_fg_initialize(fg);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Error in initializing fuel gauge\n");
+	return 0;
+}
+
+static const struct of_device_id max77658_battery_of_id[] = {
+	{ .compatible = "adi,max77658-battery" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max77658_battery_of_id);
+
+static const struct platform_device_id max77658_fg_id[] = {
+	{ "max77658-battery", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, max77658_fg_id);
+
+static struct platform_driver max77658_fg_driver = {
+	.driver = {
+		.name = "max77658-battery",
+		.of_match_table = max77658_battery_of_id,
+	},
+	.probe = max77658_fg_probe,
+	.id_table = max77658_fg_id,
+};
+
+module_platform_driver(max77658_fg_driver);
+
+MODULE_DESCRIPTION("MAX77658 Fuel Gauge Driver");
+MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v2 7/8] dt-bindings: mfd: max77658: Add ADI MAX77658

[Zeynep Arslanbenzer]

Add ADI MAX77658 devicetree document.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 .../devicetree/bindings/mfd/adi,max77658.yaml | 199 ++++++++++++++++++
 1 file changed, 199 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77658.yaml

diff --git a/Documentation/devicetree/bindings/mfd/adi,max77658.yaml b/Documentation/devicetree/bindings/mfd/adi,max77658.yaml
new file mode 100644
index 000000000000..6edb59e8f446
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,max77658.yaml
@@ -0,0 +1,199 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/adi,max77658.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MAX77643/MAX77654/MAX77658/MAX77659 PMIC from ADI
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+description: |
+  MAX77643, MAX77654, MAX77658 and MAX77659 devices are a family of ADI PMICs
+  providing battery charging and power supply solutions for
+  low-power applications.
+
+  MAX77643 is a Power Management IC with 1 LDO regulator.
+
+  MAX77654 is a Power Management IC with 2 LDO regulators and 1 charger.
+
+  MAX77658 is a Power Management IC with 2 LDO regulators, 1 charger
+  and 1 fuel gauge.
+
+  MAX77659 is a Power Management IC with 1 LDO regulator and 1 charger.
+
+properties:
+  compatible:
+    enum:
+      - adi,max77643
+      - adi,max77654
+      - adi,max77658
+      - adi,max77659
+
+  reg:
+    description: I2C address of the PMIC
+    items:
+      - enum: [0x40, 0x48]
+
+  interrupts:
+    maxItems: 1
+
+  charger:
+    $ref: /schemas/power/supply/adi,max77658-charger.yaml
+
+  fuel-gauge:
+    $ref: /schemas/power/supply/adi,max77658-battery.yaml
+
+  regulators:
+    $ref: /schemas/regulator/adi,max77658-regulator.yaml
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max77643
+              - adi,max77654
+              - adi,max77658
+
+    then:
+      properties:
+        reg:
+          items:
+            - const: 0x48
+
+    else:
+      properties:
+        reg:
+          items:
+            - const: 0x40
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    battery: battery-cell {
+      compatible = "simple-battery";
+      alert-celsius = <0 100>;
+      constant-charge-current-max-microamp = <15000>;
+    };
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@48 {
+        compatible = "adi,max77643";
+        reg = <0x48>;
+        interrupt-parent = <&gpio>;
+        interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+        regulators {
+          LDO0 {
+            regulator-boot-on;
+            regulator-always-on;
+          };
+        };
+      };
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@48 {
+        compatible = "adi,max77654";
+        reg = <0x48>;
+        interrupt-parent = <&gpio>;
+        interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+        charger {
+          compatible = "adi,max77654-charger";
+          monitored-battery = <&battery>;
+          adi,fast-charge-timer = <5>;
+          adi,topoff-timer = <30>;
+          adi,input-current-limit-microamp = <475000>;
+        };
+        regulators {
+          LDO0 {
+            regulator-boot-on;
+            regulator-always-on;
+          };
+          LDO1 {
+            regulator-boot-on;
+            regulator-always-on;
+          };
+        };
+      };
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@48 {
+        compatible = "adi,max77658";
+        reg = <0x48>;
+        interrupt-parent = <&gpio>;
+        interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+        charger {
+          compatible = "adi,max77658-charger";
+          monitored-battery = <&battery>;
+          adi,fast-charge-timer = <5>;
+          adi,topoff-timer = <30>;
+          adi,input-current-limit-microamp = <475000>;
+        };
+        regulators {
+          LDO0 {
+            regulator-boot-on;
+            regulator-always-on;
+          };
+          LDO1 {
+            regulator-boot-on;
+            regulator-always-on;
+          };
+        };
+        fuel-gauge {
+          compatible = "adi,max77658-battery";
+          monitored-battery = <&battery>;
+          adi,valrt-min-microvolt = <0>;
+          adi,valrt-max-microvolt = <5100000>;
+          adi,salrt-min-percent = <1>;
+          adi,salrt-max-percent = <99>;
+          adi,ialrt-min-microamp = <(-5000)>;
+          adi,ialrt-max-microamp = <5000>;
+        };
+      };
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@40 {
+        compatible = "adi,max77659";
+        reg = <0x40>;
+        interrupt-parent = <&gpio>;
+        interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+        charger {
+          compatible = "adi,max77659-charger";
+          monitored-battery = <&battery>;
+          adi,fast-charge-timer = <5>;
+          adi,topoff-timer = <30>;
+        };
+        regulators {
+          LDO0 {
+            regulator-boot-on;
+            regulator-always-on;
+          };
+        };
+      };
+    };
-- 
2.25.1

[PATCH v2 8/8] mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

[Zeynep Arslanbenzer]

MFD driver for MAX77643/MAX77654/MAX77658/MAX77659 to enable its sub
devices.

The MAX77643 is a multi-function devices. It includes
regulator.

The MAX77654 is a multi-function devices. It includes
regulator and charger.

The MAX77658 is a multi-function devices. It includes
regulator, charger and battery.

The MAX77659 is a multi-function devices. It includes
regulator and charger.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 drivers/mfd/Kconfig          |  15 ++
 drivers/mfd/Makefile         |   1 +
 drivers/mfd/max77658.c       | 448 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/max77658.h |  88 +++++++
 4 files changed, 552 insertions(+)
 create mode 100644 drivers/mfd/max77658.c
 create mode 100644 include/linux/mfd/max77658.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b93856de432..7b4be7fb8662 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -821,6 +821,21 @@ config MFD_MAX77650
 	  the following functionalities of the device: GPIO, regulator,
 	  charger, LED, onkey.
 
+config MFD_MAX77658
+	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support"
+	depends on I2C
+	depends on OF
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  Say Y here to add support for Analog Devices
+	  MAX77643/MAX77654/MAX77658/MAX77659 Power Management IC.
+	  This is the core multifunction
+	  driver for interacting with the device. Additional drivers can be
+	  enabled in order to use the following functionalities of the device:
+	  regulator, charger.
+
 config MFD_MAX77686
 	tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
 	depends on I2C
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 7ed3ef4a698c..f52aff45878f 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -163,6 +163,7 @@ obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
 obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
 obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
 obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
+obj-$(CONFIG_MFD_MAX77658)	+= max77658.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
 obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
 obj-$(CONFIG_MFD_MAX77714)	+= max77714.o
diff --git a/drivers/mfd/max77658.c b/drivers/mfd/max77658.c
new file mode 100644
index 000000000000..a1c6db48eb08
--- /dev/null
+++ b/drivers/mfd/max77658.c
@@ -0,0 +1,448 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 Analog Devices, Inc.
+ * ADI driver for the MAX77643/MAX77654/MAX77658/MAX77659
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max77658.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+
+#define I2C_ADDR_FUEL_GAUGE (0x6C >> 1)
+
+static const struct regmap_config max77658_regmap_config = {
+	.reg_bits   = 8,
+	.val_bits   = 8,
+};
+
+static const struct regmap_config max77658_regmap_config_fg = {
+	.reg_bits   = 8,
+	.val_bits   = 16,
+	.cache_type = REGCACHE_NONE,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+static const struct regmap_irq max77643_glbl0_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
+	{ .mask = MAX77643_BIT_INT_GLBL0_DOD0_R, },
+};
+
+static const struct regmap_irq_chip max77643_glbl0_irq_chip = {
+	.name           = "max77643_glbl0",
+	.status_base    = MAX77658_REG_INT_GLBL0,
+	.mask_base      = MAX77643_REG_INTM_GLBL0,
+	.num_regs       = 1,
+	.irqs           = max77643_glbl0_irqs,
+	.num_irqs       = ARRAY_SIZE(max77643_glbl0_irqs),
+};
+
+static const struct regmap_irq max77658_glbl0_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_DOD1_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_DOD0_R, },
+};
+
+static const struct regmap_irq_chip max77654_glbl0_irq_chip = {
+	.name           = "max77654_glbl0",
+	.status_base    = MAX77658_REG_INT_GLBL0,
+	.mask_base      = MAX77654_REG_INTM_GLBL0,
+	.num_regs       = 1,
+	.irqs           = max77658_glbl0_irqs,
+	.num_irqs       = ARRAY_SIZE(max77658_glbl0_irqs),
+};
+
+static const struct regmap_irq_chip max77658_glbl0_irq_chip = {
+	.name           = "max77658_glbl0",
+	.status_base    = MAX77658_REG_INT_GLBL0,
+	.mask_base      = MAX77658_REG_INTM_GLBL0,
+	.num_regs       = 1,
+	.irqs           = max77658_glbl0_irqs,
+	.num_irqs       = ARRAY_SIZE(max77658_glbl0_irqs),
+};
+
+static const struct regmap_irq max77659_glbl0_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL0_DOD0_R, },
+};
+
+static const struct regmap_irq_chip max77659_glbl0_irq_chip = {
+	.name           = "max77659_glbl0",
+	.status_base    = MAX77658_REG_INT_GLBL0,
+	.mask_base      = MAX77654_REG_INTM_GLBL0,
+	.num_regs       = 1,
+	.irqs           = max77659_glbl0_irqs,
+	.num_irqs       = ARRAY_SIZE(max77659_glbl0_irqs),
+};
+
+static const struct regmap_irq max77643_glbl1_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_SBB0_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_SBB1_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_SBB2_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
+};
+
+static const struct regmap_irq_chip max77643_glbl1_irq_chip = {
+	.name           = "max77643_glbl1",
+	.status_base    = MAX77658_REG_INT_GLBL1,
+	.mask_base      = MAX77643_REG_INTM_GLBL1,
+	.num_regs       = 1,
+	.irqs           = max77643_glbl1_irqs,
+	.num_irqs       = ARRAY_SIZE(max77643_glbl1_irqs),
+};
+
+static const struct regmap_irq max77654_glbl1_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
+	{ .mask = MAX77654_BIT_INT_GLBL1_GPI2_F, },
+	{ .mask = MAX77654_BIT_INT_GLBL1_GPI2_R, },
+	{ .mask = MAX77654_BIT_INT_GLBL1_SBB_TO, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_LDO1_F, },
+};
+
+static const struct regmap_irq_chip max77654_glbl1_irq_chip = {
+	.name           = "max77654_glbl1",
+	.status_base    = MAX77658_REG_INT_GLBL1,
+	.mask_base      = MAX77654_REG_INTM_GLBL1,
+	.num_regs       = 1,
+	.irqs           = max77654_glbl1_irqs,
+	.num_irqs       = ARRAY_SIZE(max77654_glbl1_irqs),
+};
+
+static const struct regmap_irq max77658_glbl1_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_SBB0_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_SBB1_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_SBB2_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_LDO1_F, },
+};
+
+static const struct regmap_irq_chip max77658_glbl1_irq_chip = {
+	.name           = "max77658_glbl1",
+	.status_base    = MAX77658_REG_INT_GLBL1,
+	.mask_base      = MAX77658_REG_INTM_GLBL1,
+	.num_regs       = 1,
+	.irqs           = max77658_glbl1_irqs,
+	.num_irqs       = ARRAY_SIZE(max77658_glbl1_irqs),
+};
+
+static const struct regmap_irq max77659_glbl1_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
+	{ .mask = MAX77659_BIT_INT_GLBL1_SBB_TO, },
+	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
+};
+
+static const struct regmap_irq_chip max77659_glbl1_irq_chip = {
+	.name           = "max77659_glbl1",
+	.status_base    = MAX77658_REG_INT_GLBL1,
+	.mask_base      = MAX77658_REG_INTM_GLBL1,
+	.num_regs       = 1,
+	.irqs           = max77659_glbl1_irqs,
+	.num_irqs       = ARRAY_SIZE(max77659_glbl1_irqs),
+};
+
+static const struct regmap_irq max77658_chg_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_THM, },
+	{ .mask = MAX77658_BIT_INT_CHG, },
+	{ .mask = MAX77658_BIT_INT_CHGIN, },
+	{ .mask = MAX77658_BIT_INT_TJ_REG, },
+	{ .mask = MAX77658_BIT_INT_CHGIN_CTRL, },
+	{ .mask = MAX77658_BIT_INT_SYS_CTRL, },
+	{ .mask = MAX77658_BIT_INT_SYS_CNFG, },
+};
+
+static const struct regmap_irq_chip max77654_chg_irq_chip = {
+	.name           = "max77654_chg",
+	.status_base    = MAX77658_REG_INT_CHG,
+	.mask_base      = MAX77658_REG_INTM_CHG,
+	.num_regs       = 1,
+	.irqs           = max77658_chg_irqs,
+	.num_irqs       = ARRAY_SIZE(max77658_chg_irqs),
+};
+
+static const struct regmap_irq_chip max77658_chg_irq_chip = {
+	.name           = "max77658_chg",
+	.status_base    = MAX77658_REG_INT_CHG,
+	.mask_base      = MAX77658_REG_INTM_CHG,
+	.num_regs       = 1,
+	.irqs           = max77658_chg_irqs,
+	.num_irqs       = ARRAY_SIZE(max77658_chg_irqs),
+};
+
+static const struct regmap_irq max77659_chg_irqs[] = {
+	{ .mask = MAX77658_BIT_INT_THM, },
+	{ .mask = MAX77658_BIT_INT_CHG, },
+	{ .mask = MAX77658_BIT_INT_CHGIN, },
+	{ .mask = MAX77658_BIT_INT_TJ_REG, },
+	{ .mask = MAX776569_BIT_INT_SYS_CTRL, },
+};
+
+static const struct regmap_irq_chip max77659_chg_irq_chip = {
+	.name           = "max77659_chg",
+	.status_base    = MAX77658_REG_INT_CHG,
+	.mask_base      = MAX77658_REG_INTM_CHG,
+	.num_regs       = 1,
+	.irqs           = max77659_chg_irqs,
+	.num_irqs       = ARRAY_SIZE(max77659_chg_irqs),
+};
+
+static const struct mfd_cell max77643_devs[] = {
+	MFD_CELL_OF("max77643-regulator", NULL, NULL, 0, 0,
+		    "adi,max77643-regulator"),
+};
+
+static const struct mfd_cell max77654_devs[] = {
+	MFD_CELL_OF("max77654-regulator", NULL, NULL, 0, 0,
+		    "adi,max77654-regulator"),
+	MFD_CELL_OF("max77654-charger", NULL, NULL, 0, 0,
+		    "adi,max77654-charger"),
+};
+
+static const struct mfd_cell max77658_devs[] = {
+	MFD_CELL_OF("max77658-regulator", NULL, NULL, 0, 0,
+		    "adi,max77658-regulator"),
+	MFD_CELL_OF("max77658-charger", NULL, NULL, 0, 0,
+		    "adi,max77658-charger"),
+	MFD_CELL_OF("max77658-battery", NULL, NULL, 0, 0,
+		    "adi,max77658-battery"),
+};
+
+static const struct mfd_cell max77659_devs[] = {
+	MFD_CELL_OF("max77659-regulator", NULL, NULL, 0, 0,
+		    "adi,max77659-regulator"),
+	MFD_CELL_OF("max77659-charger", NULL, NULL, 0, 0,
+		    "adi,max77659-charger"),
+};
+
+static const struct chip_info chip[] = {
+	[ID_MAX77643] = {
+		.id = ID_MAX77643,
+		.n_devs = ARRAY_SIZE(max77643_devs),
+		.devs = max77643_devs,
+	},
+	[ID_MAX77654] = {
+		.id = ID_MAX77654,
+		.n_devs = ARRAY_SIZE(max77654_devs),
+		.devs = max77654_devs,
+	},
+	[ID_MAX77658] = {
+		.id = ID_MAX77658,
+		.n_devs = ARRAY_SIZE(max77658_devs),
+		.devs = max77658_devs,
+	},
+	[ID_MAX77659] = {
+		.id = ID_MAX77659,
+		.n_devs = ARRAY_SIZE(max77659_devs),
+		.devs = max77659_devs,
+	},
+};
+
+static int max77658_pmic_irq_init(struct device *dev)
+{
+	const struct regmap_irq_chip *glbl0_chip, *glbl1_chip, *chg_chip;
+	struct max77658_dev *max77658 = dev_get_drvdata(dev);
+	int ret, i;
+
+	switch (max77658->chip->id) {
+	case ID_MAX77643:
+		glbl0_chip = &max77643_glbl0_irq_chip;
+		glbl1_chip = &max77643_glbl1_irq_chip;
+		break;
+	case ID_MAX77654:
+		glbl0_chip = &max77654_glbl0_irq_chip;
+		glbl1_chip = &max77654_glbl1_irq_chip;
+		chg_chip = &max77654_chg_irq_chip;
+		break;
+	case ID_MAX77658:
+		glbl0_chip = &max77658_glbl0_irq_chip;
+		glbl1_chip = &max77658_glbl1_irq_chip;
+		chg_chip = &max77658_chg_irq_chip;
+		break;
+	case ID_MAX77659:
+		glbl0_chip = &max77659_glbl0_irq_chip;
+		glbl1_chip = &max77659_glbl1_irq_chip;
+		chg_chip = &max77659_chg_irq_chip;
+		break;
+	default:
+		break;
+	}
+
+	for (i = 0; i < glbl0_chip->num_regs; i++) {
+		ret = regmap_update_bits(max77658->regmap,
+					 glbl0_chip->mask_base,
+					 (1 << glbl0_chip->irqs[i].reg_offset),
+					 1);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Unable to write Global0 Interrupt Masking register\n");
+	}
+
+	for (i = 0; i < glbl1_chip->num_regs; i++) {
+		ret = regmap_update_bits(max77658->regmap,
+					 glbl1_chip->mask_base,
+					 (1 << glbl1_chip->irqs[i].reg_offset),
+					 1);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Unable to write Global1 Interrupt Masking register\n");
+	}
+
+	if (max77658->chip->id != ID_MAX77643) {
+		for (i = 0; i < chg_chip->num_regs; i++) {
+			ret = regmap_update_bits(max77658->regmap,
+						 chg_chip->mask_base,
+						 (1 <<
+						 chg_chip->irqs[i].reg_offset),
+						 1);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Unable to write Charger Interrupt Masking register\n");
+		}
+
+		ret = devm_regmap_add_irq_chip(dev, max77658->regmap,
+					       max77658->irq,
+					       IRQF_ONESHOT | IRQF_SHARED, 0,
+					       chg_chip, &max77658->irqc_chg);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to add charger IRQ chip\n");
+	}
+
+	ret = devm_regmap_add_irq_chip(dev, max77658->regmap, max77658->irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       glbl0_chip, &max77658->irqc_glbl0);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to add global0 IRQ chip\n");
+
+	return devm_regmap_add_irq_chip(dev, max77658->regmap, max77658->irq,
+					IRQF_ONESHOT | IRQF_SHARED, 0,
+					glbl1_chip, &max77658->irqc_glbl1);
+}
+
+static int max77658_pmic_setup(struct device *dev)
+{
+	struct max77658_dev *max77658 = dev_get_drvdata(dev);
+	int ret;
+
+	ret = max77658_pmic_irq_init(max77658->dev);
+	if (ret)
+		return ret;
+
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
+				   max77658->chip->devs, max77658->chip->n_devs,
+				   NULL, 0, NULL);
+
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add sub-devices\n");
+
+	ret = device_init_wakeup(dev, true);
+	if (ret)
+		return dev_err_probe(dev, ret, "Unable to init wakeup\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id max77658_i2c_id[];
+
+static int max77658_i2c_probe(struct i2c_client *client)
+{
+	struct max77658_dev *max77658;
+	struct i2c_client *fuel;
+
+	max77658 = devm_kzalloc(&client->dev, sizeof(*max77658), GFP_KERNEL);
+	if (!max77658)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, max77658);
+	max77658->dev = &client->dev;
+	max77658->irq = client->irq;
+
+	if (max77658->dev->of_node)
+		max77658->chip  = of_device_get_match_data(max77658->dev);
+	else
+		max77658->chip  = (struct chip_info *)
+					i2c_match_id(max77658_i2c_id,
+						     client)->driver_data;
+	if (!max77658->chip)
+		return -EINVAL;
+
+	max77658->regmap = devm_regmap_init_i2c(client,
+						&max77658_regmap_config);
+	if (IS_ERR(max77658->regmap))
+		return dev_err_probe(max77658->dev, PTR_ERR(max77658->regmap),
+				     "Failed to allocate register map\n");
+
+	fuel = i2c_new_dummy_device(client->adapter, I2C_ADDR_FUEL_GAUGE);
+	if (IS_ERR(fuel))
+		return dev_err_probe(max77658->dev, PTR_ERR(fuel),
+				     "failed add i2c device[0x%Xh]\n",
+				      I2C_ADDR_FUEL_GAUGE);
+
+	i2c_set_clientdata(fuel, max77658);
+
+	max77658->regmap_fg = devm_regmap_init_i2c(fuel,
+						   &max77658_regmap_config_fg);
+	if (IS_ERR(max77658->regmap_fg))
+		return dev_err_probe(max77658->dev,
+				     PTR_ERR(max77658->regmap_fg),
+				     "failed to initialize i2c device[0x%Xh]\n",
+				     I2C_ADDR_FUEL_GAUGE);
+
+	return max77658_pmic_setup(max77658->dev);
+}
+
+static const struct of_device_id max77658_of_id[] = {
+	{ .compatible = "adi,max77643", .data = &chip[ID_MAX77643] },
+	{ .compatible = "adi,max77654", .data = &chip[ID_MAX77654] },
+	{ .compatible = "adi,max77658", .data = &chip[ID_MAX77658] },
+	{ .compatible = "adi,max77659", .data = &chip[ID_MAX77659] },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max77658_of_id);
+
+static const struct i2c_device_id max77658_i2c_id[] = {
+	{ "max77643", (kernel_ulong_t)&chip[ID_MAX77643] },
+	{ "max77654", (kernel_ulong_t)&chip[ID_MAX77654] },
+	{ "max77658", (kernel_ulong_t)&chip[ID_MAX77658] },
+	{ "max77659", (kernel_ulong_t)&chip[ID_MAX77659] },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, max77658_i2c_id);
+
+static struct i2c_driver max77658_driver = {
+	.driver = {
+		.name = "max77658",
+		.of_match_table = max77658_of_id,
+	},
+	.probe_new = max77658_i2c_probe,
+	.id_table = max77658_i2c_id,
+};
+module_i2c_driver(max77658_driver);
+
+MODULE_DESCRIPTION("MAX77658 MFD Driver");
+MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/max77658.h b/include/linux/mfd/max77658.h
new file mode 100644
index 000000000000..471a8474513e
--- /dev/null
+++ b/include/linux/mfd/max77658.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __MAX77658_MFD_H__
+#define __MAX77658_MFD_H__
+
+#include <linux/bits.h>
+#include <linux/types.h>
+
+#define MAX77658_REG_INT_GLBL0	0x00
+#define MAX77658_REG_INT_CHG	0x01
+#define MAX77658_REG_INT_GLBL1	0x04
+#define MAX77658_REG_INTM_CHG	0x07
+#define MAX77658_REG_INTM_GLBL0	0x08
+#define MAX77658_REG_INTM_GLBL1	0x09
+
+#define MAX77654_REG_INTM_GLBL1	0x08
+#define MAX77654_REG_INTM_GLBL0	0x09
+
+#define MAX77643_REG_INT_GLBL1	0x01
+#define MAX77643_REG_INTM_GLBL0	0x04
+#define MAX77643_REG_INTM_GLBL1	0x05
+
+#define MAX77658_BIT_INT_GLBL0_GPIO0_F	BIT(0)
+#define MAX77658_BIT_INT_GLBL0_GPIO0_R	BIT(1)
+#define MAX77658_BIT_INT_GLBL0_EN_F	BIT(2)
+#define MAX77658_BIT_INT_GLBL0_EN_R	BIT(3)
+#define MAX77658_BIT_INT_GLBL0_TJAL1_R	BIT(4)
+#define MAX77658_BIT_INT_GLBL0_TJAL2_R	BIT(5)
+#define MAX77658_BIT_INT_GLBL0_DOD1_R	BIT(6)
+#define MAX77658_BIT_INT_GLBL0_DOD0_R	BIT(7)
+
+#define MAX77643_BIT_INT_GLBL0_DOD0_R	BIT(6)
+
+#define MAX77658_BIT_INT_GLBL1_GPI1_F	BIT(0)
+#define MAX77658_BIT_INT_GLBL1_GPI1_R	BIT(1)
+#define MAX77658_BIT_INT_GLBL1_SBB0_F	BIT(2)
+#define MAX77658_BIT_INT_GLBL1_SBB1_F	BIT(3)
+#define MAX77658_BIT_INT_GLBL1_SBB2_F	BIT(4)
+#define MAX77658_BIT_INT_GLBL1_LDO0_F	BIT(5)
+#define MAX77658_BIT_INT_GLBL1_LDO1_F	BIT(6)
+
+#define MAX77659_BIT_INT_GLBL1_SBB_TO	BIT(4)
+
+#define MAX77654_BIT_INT_GLBL1_GPI2_F	BIT(2)
+#define MAX77654_BIT_INT_GLBL1_GPI2_R	BIT(3)
+#define MAX77654_BIT_INT_GLBL1_SBB_TO	BIT(4)
+
+#define MAX77658_BIT_INT_THM		BIT(0)
+#define MAX77658_BIT_INT_CHG		BIT(1)
+#define MAX77658_BIT_INT_CHGIN		BIT(2)
+#define MAX77658_BIT_INT_TJ_REG		BIT(3)
+#define MAX77658_BIT_INT_CHGIN_CTRL	BIT(4)
+#define MAX77658_BIT_INT_SYS_CTRL	BIT(5)
+#define MAX77658_BIT_INT_SYS_CNFG	BIT(6)
+
+#define MAX776569_BIT_INT_SYS_CTRL	BIT(4)
+
+enum max77658_ids {
+	ID_MAX77643,
+	ID_MAX77654,
+	ID_MAX77658,
+	ID_MAX77659
+};
+
+struct chip_info {
+	enum max77658_ids id;
+	int n_devs;
+	const struct mfd_cell *devs;
+};
+
+struct device;
+struct regmap;
+struct regmap_irq_chip_data;
+
+struct max77658_dev {
+	struct device *dev;
+	const struct chip_info *chip;
+
+	int irq;
+	struct regmap_irq_chip_data *irqc_glbl0;
+	struct regmap_irq_chip_data *irqc_glbl1;
+	struct regmap_irq_chip_data *irqc_chg;
+
+	struct regmap *regmap;
+	struct regmap *regmap_fg;
+};
+
+#endif /* __MAX77658_MFD_H__ */
-- 
2.25.1

Re: [PATCH v2 1/8] dt-bindings: regulator: max77658: Add ADI MAX77643/54/58/59 Regulator

[Krzysztof Kozlowski]

On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
> Add ADI MAX77643/MAX77654/MAX77658/MAX77659 Regulator devicetree document.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../regulator/adi,max77658-regulator.yaml     | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml
> new file mode 100644
> index 000000000000..1d097eddcd98
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/adi,max77658-regulator.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/adi,max77658-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Regulator for MAX77658 PMICs family from ADI
> +
> +maintainers:
> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> +
> +description: |
> +  This is part of the MAX77658 MFD device. For more details
> +  see Documentation/devicetree/bindings/mfd/adi,max77658.yaml.
> +
> +  The regulators is represented as a sub-node of the PMIC node on the device tree.

There is no select/compatible here, so this makes little sense as
separate binding. Make it part of parent schema instead.

Best regards,
Krzysztof

Re: [PATCH v2 2/8] regulator: max77658: Add ADI MAX77643/54/58/59 Regulator Support

[Krzysztof Kozlowski]

On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
> Regulator driver for ADI MAX77643/MAX77654/MAX77658/MAX77659.
> 
> MAX77643/MAX77659 has 1 LDO regulator.
> MAX77654/MAX77658 has two LDO regulators.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>



> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id max77658_regulator_id[] = {
> +	{ "max77643-regulator" },
> +	{ "max77654-regulator" },
> +	{ "max77658-regulator" },
> +	{ "max77659-regulator" },

Why do you need so many entries? They do not differ.

Best regards,
Krzysztof

Re: [PATCH v2 3/8] dt-bindings: power: supply: max77658: Add ADI MAX77654/58/59 Charger

[Krzysztof Kozlowski]

On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
> Add ADI MAX77654/MAX77658/MAX77659 power supply devicetree document.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../power/supply/adi,max77658-charger.yaml    | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml b/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml
> new file mode 100644
> index 000000000000..f140abab969c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/adi,max77658-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Battery charger for MAX77658 PMICs family from ADI.

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Actually you
ignored all of them. Maybe my feedback got lost between the quotes,
maybe you just forgot to apply it. Please go back to the previous
discussion and either implement all requested changes or keep discussing
them.

Thank you.

Best regards,
Krzysztof

Re: [PATCH v2 5/8] dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery

[Krzysztof Kozlowski]

On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
> Add ADI MAX77658 power supply devicetree document.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../power/supply/adi,max77658-battery.yaml    | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml b/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml
> new file mode 100644
> index 000000000000..0b696f7c4d1b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/adi,max77658-battery.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Battery for MAX77658 PMIC from ADI.

Implement all previous comments, not just some.


> +
> +maintainers:
> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> +
> +description: |
> +  This module is part of the MAX77658 MFD device. For more details
> +  see Documentation/devicetree/bindings/mfd/adi,max77658.yaml.
> +
> +  The fuel gauge is represented as a sub-node of the PMIC node on the device tree.
> +
> +properties:
> +  compatible:
> +    const:
> +      adi,max77658-battery

It's one line.

> +
> +  reg:
> +    maxItems: 1
> +
> +  adi,valrt-min-microvolt:
> +    description: Minimum voltage value that triggers the alarm.
> +
> +  adi,valrt-max-microvolt:
> +    description: Maximum voltage value that triggers the alarm.

Use the same syntax as battery.yaml

> +
> +  adi,salrt-min-percent:
> +    description: Minimum percentage of battery that triggers the alarm.
> +
> +  adi,salrt-max-percent:
> +    description: Maximum percentage of battery that triggers the alarm.

That's not suitable for DT. Do not encode policies into DT.

> +
> +  adi,ialrt-min-microamp:
> +    description: Minimum current value that triggers the alarm.
> +
> +  adi,ialrt-max-microamp:
> +    description: Maximum current value that triggers the alarm.
> +
> +  monitored-battery:
> +    description: >
> +      phandle to a "simple-battery" compatible node.
> +
> +      This property must be a phandle to a node using the format described

You already said it above.

> +      in battery.yaml, with the following properties being required:
> +      - alert-celsius
> +
> +required:
> +  - compatible

Why reg and monitored-batter are not required?


Best regards,
Krzysztof

Re: [PATCH v2 7/8] dt-bindings: mfd: max77658: Add ADI MAX77658

[Krzysztof Kozlowski]

On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
> Add ADI MAX77658 devicetree document.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77658.yaml | 199 ++++++++++++++++++
>  1 file changed, 199 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77658.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/adi,max77658.yaml b/Documentation/devicetree/bindings/mfd/adi,max77658.yaml
> new file mode 100644
> index 000000000000..6edb59e8f446
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,max77658.yaml
> @@ -0,0 +1,199 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/adi,max77658.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MAX77643/MAX77654/MAX77658/MAX77659 PMIC from ADI
> +
> +maintainers:
> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> +
> +description: |
> +  MAX77643, MAX77654, MAX77658 and MAX77659 devices are a family of ADI PMICs
> +  providing battery charging and power supply solutions for
> +  low-power applications.
> +
> +  MAX77643 is a Power Management IC with 1 LDO regulator.
> +
> +  MAX77654 is a Power Management IC with 2 LDO regulators and 1 charger.
> +
> +  MAX77658 is a Power Management IC with 2 LDO regulators, 1 charger
> +  and 1 fuel gauge.
> +
> +  MAX77659 is a Power Management IC with 1 LDO regulator and 1 charger.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,max77643
> +      - adi,max77654
> +      - adi,max77658
> +      - adi,max77659
> +
> +  reg:
> +    description: I2C address of the PMIC
> +    items:
> +      - enum: [0x40, 0x48]
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  charger:
> +    $ref: /schemas/power/supply/adi,max77658-charger.yaml
> +
> +  fuel-gauge:
> +    $ref: /schemas/power/supply/adi,max77658-battery.yaml
> +
> +  regulators:
> +    $ref: /schemas/regulator/adi,max77658-regulator.yaml
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,max77643
> +              - adi,max77654
> +              - adi,max77658
> +
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - const: 0x48
> +
> +    else:
> +      properties:
> +        reg:
> +          items:
> +            - const: 0x40
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    battery: battery-cell {
> +      compatible = "simple-battery";
> +      alert-celsius = <0 100>;
> +      constant-charge-current-max-microamp = <15000>;
> +    };
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      pmic@48 {
> +        compatible = "adi,max77643";
> +        reg = <0x48>;
> +        interrupt-parent = <&gpio>;
> +        interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
> +        regulators {
> +          LDO0 {
> +            regulator-boot-on;
> +            regulator-always-on;
> +          };
> +        };
> +      };
> +    };

Four examples is too much. Keep only some relevant one or two.

Best regards,
Krzysztof

Re: [PATCH v2 8/8] mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

[Lee Jones]

On Wed, 22 Mar 2023, Zeynep Arslanbenzer wrote:

> MFD driver for MAX77643/MAX77654/MAX77658/MAX77659 to enable its sub

Please drop all references to 'MFD'.

What are these devices, really?  I suspect they are PMICs?

> devices.
>
> The MAX77643 is a multi-function devices. It includes
> regulator.
>
> The MAX77654 is a multi-function devices. It includes
> regulator and charger.
>
> The MAX77658 is a multi-function devices. It includes
> regulator, charger and battery.
>
> The MAX77659 is a multi-function devices. It includes
> regulator and charger.
>
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  drivers/mfd/Kconfig          |  15 ++
>  drivers/mfd/Makefile         |   1 +
>  drivers/mfd/max77658.c       | 448 +++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max77658.h |  88 +++++++
>  4 files changed, 552 insertions(+)
>  create mode 100644 drivers/mfd/max77658.c
>  create mode 100644 include/linux/mfd/max77658.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8b93856de432..7b4be7fb8662 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -821,6 +821,21 @@ config MFD_MAX77650
>  	  the following functionalities of the device: GPIO, regulator,
>  	  charger, LED, onkey.
>
> +config MFD_MAX77658
> +	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support"
> +	depends on I2C
> +	depends on OF
> +	select MFD_CORE
> +	select REGMAP_I2C
> +	select REGMAP_IRQ
> +	help
> +	  Say Y here to add support for Analog Devices
> +	  MAX77643/MAX77654/MAX77658/MAX77659 Power Management IC.

"MAX776xx series"?

> +	  This is the core multifunction

Just "core driver" is fine.

Odd place to line wrap?

> +	  driver for interacting with the device. Additional drivers can be

"can be"?  It's probably pretty useless if you don't, no?

> +	  enabled in order to use the following functionalities of the device:
> +	  regulator, charger.

"... in order to use the regular and charger functionality of the device".

>  config MFD_MAX77686
>  	tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
>  	depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 7ed3ef4a698c..f52aff45878f 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -163,6 +163,7 @@ obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
>  obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>  obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
> +obj-$(CONFIG_MFD_MAX77658)	+= max77658.o
>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
>  obj-$(CONFIG_MFD_MAX77714)	+= max77714.o
> diff --git a/drivers/mfd/max77658.c b/drivers/mfd/max77658.c
> new file mode 100644
> index 000000000000..a1c6db48eb08
> --- /dev/null
> +++ b/drivers/mfd/max77658.c
> @@ -0,0 +1,448 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 Analog Devices, Inc.
> + * ADI driver for the MAX77643/MAX77654/MAX77658/MAX77659
> + */

No need to list every device.

"MAX776xx series"?

> +#include <linux/i2c.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/max77658.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +#define I2C_ADDR_FUEL_GAUGE (0x6C >> 1)
> +
> +static const struct regmap_config max77658_regmap_config = {
> +	.reg_bits   = 8,
> +	.val_bits   = 8,
> +};
> +
> +static const struct regmap_config max77658_regmap_config_fg = {
> +	.reg_bits   = 8,
> +	.val_bits   = 16,
> +	.cache_type = REGCACHE_NONE,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +static const struct regmap_irq max77643_glbl0_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
> +	{ .mask = MAX77643_BIT_INT_GLBL0_DOD0_R, },
> +};
> +
> +static const struct regmap_irq_chip max77643_glbl0_irq_chip = {
> +	.name           = "max77643_glbl0",
> +	.status_base    = MAX77658_REG_INT_GLBL0,
> +	.mask_base      = MAX77643_REG_INTM_GLBL0,
> +	.num_regs       = 1,
> +	.irqs           = max77643_glbl0_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77643_glbl0_irqs),
> +};
> +
> +static const struct regmap_irq max77658_glbl0_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_DOD1_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_DOD0_R, },
> +};
> +
> +static const struct regmap_irq_chip max77654_glbl0_irq_chip = {
> +	.name           = "max77654_glbl0",
> +	.status_base    = MAX77658_REG_INT_GLBL0,
> +	.mask_base      = MAX77654_REG_INTM_GLBL0,
> +	.num_regs       = 1,
> +	.irqs           = max77658_glbl0_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77658_glbl0_irqs),
> +};
> +
> +static const struct regmap_irq_chip max77658_glbl0_irq_chip = {
> +	.name           = "max77658_glbl0",
> +	.status_base    = MAX77658_REG_INT_GLBL0,
> +	.mask_base      = MAX77658_REG_INTM_GLBL0,
> +	.num_regs       = 1,
> +	.irqs           = max77658_glbl0_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77658_glbl0_irqs),
> +};
> +
> +static const struct regmap_irq max77659_glbl0_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL0_DOD0_R, },
> +};
> +
> +static const struct regmap_irq_chip max77659_glbl0_irq_chip = {
> +	.name           = "max77659_glbl0",
> +	.status_base    = MAX77658_REG_INT_GLBL0,
> +	.mask_base      = MAX77654_REG_INTM_GLBL0,
> +	.num_regs       = 1,
> +	.irqs           = max77659_glbl0_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77659_glbl0_irqs),
> +};
> +
> +static const struct regmap_irq max77643_glbl1_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB0_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB1_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB2_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
> +};
> +
> +static const struct regmap_irq_chip max77643_glbl1_irq_chip = {
> +	.name           = "max77643_glbl1",
> +	.status_base    = MAX77658_REG_INT_GLBL1,
> +	.mask_base      = MAX77643_REG_INTM_GLBL1,
> +	.num_regs       = 1,
> +	.irqs           = max77643_glbl1_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77643_glbl1_irqs),
> +};
> +
> +static const struct regmap_irq max77654_glbl1_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
> +	{ .mask = MAX77654_BIT_INT_GLBL1_GPI2_F, },
> +	{ .mask = MAX77654_BIT_INT_GLBL1_GPI2_R, },
> +	{ .mask = MAX77654_BIT_INT_GLBL1_SBB_TO, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO1_F, },
> +};
> +
> +static const struct regmap_irq_chip max77654_glbl1_irq_chip = {
> +	.name           = "max77654_glbl1",
> +	.status_base    = MAX77658_REG_INT_GLBL1,
> +	.mask_base      = MAX77654_REG_INTM_GLBL1,
> +	.num_regs       = 1,
> +	.irqs           = max77654_glbl1_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77654_glbl1_irqs),
> +};
> +
> +static const struct regmap_irq max77658_glbl1_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB0_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB1_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB2_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO1_F, },
> +};
> +
> +static const struct regmap_irq_chip max77658_glbl1_irq_chip = {
> +	.name           = "max77658_glbl1",
> +	.status_base    = MAX77658_REG_INT_GLBL1,
> +	.mask_base      = MAX77658_REG_INTM_GLBL1,
> +	.num_regs       = 1,
> +	.irqs           = max77658_glbl1_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77658_glbl1_irqs),
> +};
> +
> +static const struct regmap_irq max77659_glbl1_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
> +	{ .mask = MAX77659_BIT_INT_GLBL1_SBB_TO, },
> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
> +};
> +
> +static const struct regmap_irq_chip max77659_glbl1_irq_chip = {
> +	.name           = "max77659_glbl1",
> +	.status_base    = MAX77658_REG_INT_GLBL1,
> +	.mask_base      = MAX77658_REG_INTM_GLBL1,
> +	.num_regs       = 1,
> +	.irqs           = max77659_glbl1_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77659_glbl1_irqs),
> +};
> +
> +static const struct regmap_irq max77658_chg_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_THM, },
> +	{ .mask = MAX77658_BIT_INT_CHG, },
> +	{ .mask = MAX77658_BIT_INT_CHGIN, },
> +	{ .mask = MAX77658_BIT_INT_TJ_REG, },
> +	{ .mask = MAX77658_BIT_INT_CHGIN_CTRL, },
> +	{ .mask = MAX77658_BIT_INT_SYS_CTRL, },
> +	{ .mask = MAX77658_BIT_INT_SYS_CNFG, },
> +};
> +
> +static const struct regmap_irq_chip max77654_chg_irq_chip = {
> +	.name           = "max77654_chg",
> +	.status_base    = MAX77658_REG_INT_CHG,
> +	.mask_base      = MAX77658_REG_INTM_CHG,
> +	.num_regs       = 1,
> +	.irqs           = max77658_chg_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77658_chg_irqs),
> +};
> +
> +static const struct regmap_irq_chip max77658_chg_irq_chip = {
> +	.name           = "max77658_chg",
> +	.status_base    = MAX77658_REG_INT_CHG,
> +	.mask_base      = MAX77658_REG_INTM_CHG,
> +	.num_regs       = 1,
> +	.irqs           = max77658_chg_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77658_chg_irqs),
> +};
> +
> +static const struct regmap_irq max77659_chg_irqs[] = {
> +	{ .mask = MAX77658_BIT_INT_THM, },
> +	{ .mask = MAX77658_BIT_INT_CHG, },
> +	{ .mask = MAX77658_BIT_INT_CHGIN, },
> +	{ .mask = MAX77658_BIT_INT_TJ_REG, },
> +	{ .mask = MAX776569_BIT_INT_SYS_CTRL, },
> +};
> +
> +static const struct regmap_irq_chip max77659_chg_irq_chip = {
> +	.name           = "max77659_chg",
> +	.status_base    = MAX77658_REG_INT_CHG,
> +	.mask_base      = MAX77658_REG_INTM_CHG,
> +	.num_regs       = 1,
> +	.irqs           = max77659_chg_irqs,
> +	.num_irqs       = ARRAY_SIZE(max77659_chg_irqs),
> +};
> +
> +static const struct mfd_cell max77643_devs[] = {
> +	MFD_CELL_OF("max77643-regulator", NULL, NULL, 0, 0,
> +		    "adi,max77643-regulator"),
> +};
> +
> +static const struct mfd_cell max77654_devs[] = {
> +	MFD_CELL_OF("max77654-regulator", NULL, NULL, 0, 0,
> +		    "adi,max77654-regulator"),
> +	MFD_CELL_OF("max77654-charger", NULL, NULL, 0, 0,
> +		    "adi,max77654-charger"),
> +};
> +
> +static const struct mfd_cell max77658_devs[] = {
> +	MFD_CELL_OF("max77658-regulator", NULL, NULL, 0, 0,
> +		    "adi,max77658-regulator"),
> +	MFD_CELL_OF("max77658-charger", NULL, NULL, 0, 0,
> +		    "adi,max77658-charger"),
> +	MFD_CELL_OF("max77658-battery", NULL, NULL, 0, 0,
> +		    "adi,max77658-battery"),
> +};
> +
> +static const struct mfd_cell max77659_devs[] = {
> +	MFD_CELL_OF("max77659-regulator", NULL, NULL, 0, 0,
> +		    "adi,max77659-regulator"),
> +	MFD_CELL_OF("max77659-charger", NULL, NULL, 0, 0,
> +		    "adi,max77659-charger"),
> +};

You can unwrap all of these, you have 100-chars to play with.

> +static const struct chip_info chip[] = {
> +	[ID_MAX77643] = {
> +		.id = ID_MAX77643,
> +		.n_devs = ARRAY_SIZE(max77643_devs),
> +		.devs = max77643_devs,
> +	},
> +	[ID_MAX77654] = {
> +		.id = ID_MAX77654,
> +		.n_devs = ARRAY_SIZE(max77654_devs),
> +		.devs = max77654_devs,
> +	},
> +	[ID_MAX77658] = {
> +		.id = ID_MAX77658,
> +		.n_devs = ARRAY_SIZE(max77658_devs),
> +		.devs = max77658_devs,
> +	},
> +	[ID_MAX77659] = {
> +		.id = ID_MAX77659,
> +		.n_devs = ARRAY_SIZE(max77659_devs),
> +		.devs = max77659_devs,
> +	},
> +};
> +
> +static int max77658_pmic_irq_init(struct device *dev)
> +{
> +	const struct regmap_irq_chip *glbl0_chip, *glbl1_chip, *chg_chip;
> +	struct max77658_dev *max77658 = dev_get_drvdata(dev);
> +	int ret, i;
> +
> +	switch (max77658->chip->id) {
> +	case ID_MAX77643:
> +		glbl0_chip = &max77643_glbl0_irq_chip;
> +		glbl1_chip = &max77643_glbl1_irq_chip;
> +		break;
> +	case ID_MAX77654:
> +		glbl0_chip = &max77654_glbl0_irq_chip;
> +		glbl1_chip = &max77654_glbl1_irq_chip;
> +		chg_chip = &max77654_chg_irq_chip;
> +		break;
> +	case ID_MAX77658:
> +		glbl0_chip = &max77658_glbl0_irq_chip;
> +		glbl1_chip = &max77658_glbl1_irq_chip;
> +		chg_chip = &max77658_chg_irq_chip;
> +		break;
> +	case ID_MAX77659:
> +		glbl0_chip = &max77659_glbl0_irq_chip;
> +		glbl1_chip = &max77659_glbl1_irq_chip;
> +		chg_chip = &max77659_chg_irq_chip;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	for (i = 0; i < glbl0_chip->num_regs; i++) {
> +		ret = regmap_update_bits(max77658->regmap,
> +					 glbl0_chip->mask_base,
> +					 (1 << glbl0_chip->irqs[i].reg_offset),
> +					 1);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Unable to write Global0 Interrupt Masking register\n");
> +	}
> +
> +	for (i = 0; i < glbl1_chip->num_regs; i++) {
> +		ret = regmap_update_bits(max77658->regmap,
> +					 glbl1_chip->mask_base,
> +					 (1 << glbl1_chip->irqs[i].reg_offset),
> +					 1);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Unable to write Global1 Interrupt Masking register\n");
> +	}
> +
> +	if (max77658->chip->id != ID_MAX77643) {
> +		for (i = 0; i < chg_chip->num_regs; i++) {
> +			ret = regmap_update_bits(max77658->regmap,
> +						 chg_chip->mask_base,
> +						 (1 <<
> +						 chg_chip->irqs[i].reg_offset),
> +						 1);
> +			if (ret)
> +				return dev_err_probe(dev, ret,
> +						     "Unable to write Charger Interrupt Masking register\n");
> +		}
> +
> +		ret = devm_regmap_add_irq_chip(dev, max77658->regmap,
> +					       max77658->irq,
> +					       IRQF_ONESHOT | IRQF_SHARED, 0,
> +					       chg_chip, &max77658->irqc_chg);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to add charger IRQ chip\n");
> +	}
> +
> +	ret = devm_regmap_add_irq_chip(dev, max77658->regmap, max77658->irq,
> +				       IRQF_ONESHOT | IRQF_SHARED, 0,
> +				       glbl0_chip, &max77658->irqc_glbl0);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to add global0 IRQ chip\n");
> +
> +	return devm_regmap_add_irq_chip(dev, max77658->regmap, max77658->irq,
> +					IRQF_ONESHOT | IRQF_SHARED, 0,
> +					glbl1_chip, &max77658->irqc_glbl1);

This function is hectic.

What exactly are you doing here?

> +}
> +
> +static int max77658_pmic_setup(struct device *dev)
> +{
> +	struct max77658_dev *max77658 = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = max77658_pmic_irq_init(max77658->dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
> +				   max77658->chip->devs, max77658->chip->n_devs,
> +				   NULL, 0, NULL);
> +

These are usually placed in probe.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add sub-devices\n");
> +
> +	ret = device_init_wakeup(dev, true);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Unable to init wakeup\n");
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id max77658_i2c_id[];

What on earth is this?  A struct forward declaration?

If you need this, just move the original code block to here.

> +static int max77658_i2c_probe(struct i2c_client *client)
> +{
> +	struct max77658_dev *max77658;
> +	struct i2c_client *fuel;
> +
> +	max77658 = devm_kzalloc(&client->dev, sizeof(*max77658), GFP_KERNEL);
> +	if (!max77658)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, max77658);
> +	max77658->dev = &client->dev;

How do you fetch back max77658?

Don't you need access to the device structs?

If so, you already have a reference, no?

> +	max77658->irq = client->irq;
> +
> +	if (max77658->dev->of_node)
> +		max77658->chip  = of_device_get_match_data(max77658->dev);
> +	else
> +		max77658->chip  = (struct chip_info *)

Cast from void * shouldn't be required.

> +					i2c_match_id(max77658_i2c_id,
> +						     client)->driver_data;

100-chars everywhere.

> +	if (!max77658->chip)
> +		return -EINVAL;
> +
> +	max77658->regmap = devm_regmap_init_i2c(client,
> +						&max77658_regmap_config);
> +	if (IS_ERR(max77658->regmap))
> +		return dev_err_probe(max77658->dev, PTR_ERR(max77658->regmap),
> +				     "Failed to allocate register map\n");

s/allocate/initialise/

"regmap" is fine.

> +	fuel = i2c_new_dummy_device(client->adapter, I2C_ADDR_FUEL_GAUGE);
> +	if (IS_ERR(fuel))
> +		return dev_err_probe(max77658->dev, PTR_ERR(fuel),
> +				     "failed add i2c device[0x%Xh]\n",

"Failed to create I2C device"

> +				      I2C_ADDR_FUEL_GAUGE);
> +
> +	i2c_set_clientdata(fuel, max77658);
> +
> +	max77658->regmap_fg = devm_regmap_init_i2c(fuel,
> +						   &max77658_regmap_config_fg);
> +	if (IS_ERR(max77658->regmap_fg))
> +		return dev_err_probe(max77658->dev,
> +				     PTR_ERR(max77658->regmap_fg),
> +				     "failed to initialize i2c device[0x%Xh]\n",

"Failed"

"I2C"

> +				     I2C_ADDR_FUEL_GAUGE);
> +
> +	return max77658_pmic_setup(max77658->dev);
> +}
> +
> +static const struct of_device_id max77658_of_id[] = {
> +	{ .compatible = "adi,max77643", .data = &chip[ID_MAX77643] },
> +	{ .compatible = "adi,max77654", .data = &chip[ID_MAX77654] },
> +	{ .compatible = "adi,max77658", .data = &chip[ID_MAX77658] },
> +	{ .compatible = "adi,max77659", .data = &chip[ID_MAX77659] },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max77658_of_id);
> +
> +static const struct i2c_device_id max77658_i2c_id[] = {
> +	{ "max77643", (kernel_ulong_t)&chip[ID_MAX77643] },
> +	{ "max77654", (kernel_ulong_t)&chip[ID_MAX77654] },
> +	{ "max77658", (kernel_ulong_t)&chip[ID_MAX77658] },
> +	{ "max77659", (kernel_ulong_t)&chip[ID_MAX77659] },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max77658_i2c_id);

Please don't pass MFD data through the DT platform initialisation API.

Pass the Chip ID and match in C-code.

> +static struct i2c_driver max77658_driver = {
> +	.driver = {
> +		.name = "max77658",
> +		.of_match_table = max77658_of_id,
> +	},
> +	.probe_new = max77658_i2c_probe,
> +	.id_table = max77658_i2c_id,
> +};
> +module_i2c_driver(max77658_driver);
> +
> +MODULE_DESCRIPTION("MAX77658 MFD Driver");
> +MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/max77658.h b/include/linux/mfd/max77658.h
> new file mode 100644
> index 000000000000..471a8474513e
> --- /dev/null
> +++ b/include/linux/mfd/max77658.h
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef __MAX77658_MFD_H__
> +#define __MAX77658_MFD_H__
> +
> +#include <linux/bits.h>
> +#include <linux/types.h>
> +
> +#define MAX77658_REG_INT_GLBL0	0x00
> +#define MAX77658_REG_INT_CHG	0x01
> +#define MAX77658_REG_INT_GLBL1	0x04
> +#define MAX77658_REG_INTM_CHG	0x07
> +#define MAX77658_REG_INTM_GLBL0	0x08
> +#define MAX77658_REG_INTM_GLBL1	0x09
> +
> +#define MAX77654_REG_INTM_GLBL1	0x08
> +#define MAX77654_REG_INTM_GLBL0	0x09
> +
> +#define MAX77643_REG_INT_GLBL1	0x01
> +#define MAX77643_REG_INTM_GLBL0	0x04
> +#define MAX77643_REG_INTM_GLBL1	0x05
> +
> +#define MAX77658_BIT_INT_GLBL0_GPIO0_F	BIT(0)
> +#define MAX77658_BIT_INT_GLBL0_GPIO0_R	BIT(1)
> +#define MAX77658_BIT_INT_GLBL0_EN_F	BIT(2)
> +#define MAX77658_BIT_INT_GLBL0_EN_R	BIT(3)
> +#define MAX77658_BIT_INT_GLBL0_TJAL1_R	BIT(4)
> +#define MAX77658_BIT_INT_GLBL0_TJAL2_R	BIT(5)
> +#define MAX77658_BIT_INT_GLBL0_DOD1_R	BIT(6)
> +#define MAX77658_BIT_INT_GLBL0_DOD0_R	BIT(7)
> +
> +#define MAX77643_BIT_INT_GLBL0_DOD0_R	BIT(6)
> +
> +#define MAX77658_BIT_INT_GLBL1_GPI1_F	BIT(0)
> +#define MAX77658_BIT_INT_GLBL1_GPI1_R	BIT(1)
> +#define MAX77658_BIT_INT_GLBL1_SBB0_F	BIT(2)
> +#define MAX77658_BIT_INT_GLBL1_SBB1_F	BIT(3)
> +#define MAX77658_BIT_INT_GLBL1_SBB2_F	BIT(4)
> +#define MAX77658_BIT_INT_GLBL1_LDO0_F	BIT(5)
> +#define MAX77658_BIT_INT_GLBL1_LDO1_F	BIT(6)
> +
> +#define MAX77659_BIT_INT_GLBL1_SBB_TO	BIT(4)
> +
> +#define MAX77654_BIT_INT_GLBL1_GPI2_F	BIT(2)
> +#define MAX77654_BIT_INT_GLBL1_GPI2_R	BIT(3)
> +#define MAX77654_BIT_INT_GLBL1_SBB_TO	BIT(4)
> +
> +#define MAX77658_BIT_INT_THM		BIT(0)
> +#define MAX77658_BIT_INT_CHG		BIT(1)
> +#define MAX77658_BIT_INT_CHGIN		BIT(2)
> +#define MAX77658_BIT_INT_TJ_REG		BIT(3)
> +#define MAX77658_BIT_INT_CHGIN_CTRL	BIT(4)
> +#define MAX77658_BIT_INT_SYS_CTRL	BIT(5)
> +#define MAX77658_BIT_INT_SYS_CNFG	BIT(6)
> +
> +#define MAX776569_BIT_INT_SYS_CTRL	BIT(4)
> +
> +enum max77658_ids {
> +	ID_MAX77643,
> +	ID_MAX77654,
> +	ID_MAX77658,
> +	ID_MAX77659
> +};
> +
> +struct chip_info {
> +	enum max77658_ids id;
> +	int n_devs;
> +	const struct mfd_cell *devs;
> +};
> +
> +struct device;
> +struct regmap;
> +struct regmap_irq_chip_data;
> +
> +struct max77658_dev {
> +	struct device *dev;
> +	const struct chip_info *chip;
> +
> +	int irq;
> +	struct regmap_irq_chip_data *irqc_glbl0;
> +	struct regmap_irq_chip_data *irqc_glbl1;
> +	struct regmap_irq_chip_data *irqc_chg;
> +
> +	struct regmap *regmap;
> +	struct regmap *regmap_fg;
> +};
> +
> +#endif /* __MAX77658_MFD_H__ */
> --
> 2.25.1
>

--
Lee Jones [李琼斯]

RE: [PATCH v2 8/8] mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

[Arslanbenzer, Zeynep]

>On Wed, 22 Mar 2023, Zeynep Arslanbenzer wrote:
>
>> MFD driver for MAX77643/MAX77654/MAX77658/MAX77659 to enable its sub
>
>Please drop all references to 'MFD'.
>
>What are these devices, really?  I suspect they are PMICs?

Hi Lee,

Thank you for your review. Yes, all devices covered by this patch are PMIC.

>
>> devices.
>>
>> The MAX77643 is a multi-function devices. It includes regulator.
>>
>> The MAX77654 is a multi-function devices. It includes regulator and 
>> charger.
>>
>> The MAX77658 is a multi-function devices. It includes regulator, 
>> charger and battery.
>>
>> The MAX77659 is a multi-function devices. It includes regulator and 
>> charger.
>>
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>> ---
>>  drivers/mfd/Kconfig          |  15 ++
>>  drivers/mfd/Makefile         |   1 +
>>  drivers/mfd/max77658.c       | 448 +++++++++++++++++++++++++++++++++++
>>  include/linux/mfd/max77658.h |  88 +++++++
>>  4 files changed, 552 insertions(+)
>>  create mode 100644 drivers/mfd/max77658.c  create mode 100644 
>> include/linux/mfd/max77658.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 
>> 8b93856de432..7b4be7fb8662 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -821,6 +821,21 @@ config MFD_MAX77650
>>  	  the following functionalities of the device: GPIO, regulator,
>>  	  charger, LED, onkey.
>>
>> +config MFD_MAX77658
>> +	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support"
>> +	depends on I2C
>> +	depends on OF
>> +	select MFD_CORE
>> +	select REGMAP_I2C
>> +	select REGMAP_IRQ
>> +	help
>> +	  Say Y here to add support for Analog Devices
>> +	  MAX77643/MAX77654/MAX77658/MAX77659 Power Management IC.
>
>"MAX776xx series"?

I will update the patch to be compatible with the MAX776xx.

>
>> +	  This is the core multifunction
>
>Just "core driver" is fine.
>
>Odd place to line wrap?
>
>> +	  driver for interacting with the device. Additional drivers can be
>
>"can be"?  It's probably pretty useless if you don't, no?
>
>> +	  enabled in order to use the following functionalities of the device:
>> +	  regulator, charger.
>
>"... in order to use the regular and charger functionality of the device".
>
>>  config MFD_MAX77686
>>  	tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
>>  	depends on I2C
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 
>> 7ed3ef4a698c..f52aff45878f 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -163,6 +163,7 @@ obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
>>  obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>>  obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
>> +obj-$(CONFIG_MFD_MAX77658)	+= max77658.o
>>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
>>  obj-$(CONFIG_MFD_MAX77714)	+= max77714.o
>> diff --git a/drivers/mfd/max77658.c b/drivers/mfd/max77658.c new file 
>> mode 100644 index 000000000000..a1c6db48eb08
>> --- /dev/null
>> +++ b/drivers/mfd/max77658.c
>> @@ -0,0 +1,448 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2023 Analog Devices, Inc.
>> + * ADI driver for the MAX77643/MAX77654/MAX77658/MAX77659
>> + */
>
>No need to list every device.
>
>"MAX776xx series"?
>
>> +#include <linux/i2c.h>>
>> +#include <linux/mfd/core.h>>
>> +#include <linux/mfd/max77658.h>>
>> +#include <linux/mod_devicetable.h>>
>> +#include <linux/of_device.h>>
>> +#include <linux/regmap.h>>
>> +
>> +#define I2C_ADDR_FUEL_GAUGE (0x6C >>>> 1)
>> +
>> +static const struct regmap_config max77658_regmap_config = {
>> +	.reg_bits   = 8,
>> +	.val_bits   = 8,
>> +};
>> +
>> +static const struct regmap_config max77658_regmap_config_fg = {
>> +	.reg_bits   = 8,
>> +	.val_bits   = 16,
>> +	.cache_type = REGCACHE_NONE,
>> +	.val_format_endian = REGMAP_ENDIAN_LITTLE, };
>> +
>> +static const struct regmap_irq max77643_glbl0_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
>> +	{ .mask = MAX77643_BIT_INT_GLBL0_DOD0_R, }, };
>> +
>> +static const struct regmap_irq_chip max77643_glbl0_irq_chip = {
>> +	.name           = "max77643_glbl0",
>> +	.status_base    = MAX77658_REG_INT_GLBL0,
>> +	.mask_base      = MAX77643_REG_INTM_GLBL0,
>> +	.num_regs       = 1,
>> +	.irqs           = max77643_glbl0_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77643_glbl0_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77658_glbl0_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_DOD1_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_DOD0_R, }, };
>> +
>> +static const struct regmap_irq_chip max77654_glbl0_irq_chip = {
>> +	.name           = "max77654_glbl0",
>> +	.status_base    = MAX77658_REG_INT_GLBL0,
>> +	.mask_base      = MAX77654_REG_INTM_GLBL0,
>> +	.num_regs       = 1,
>> +	.irqs           = max77658_glbl0_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77658_glbl0_irqs),
>> +};
>> +
>> +static const struct regmap_irq_chip max77658_glbl0_irq_chip = {
>> +	.name           = "max77658_glbl0",
>> +	.status_base    = MAX77658_REG_INT_GLBL0,
>> +	.mask_base      = MAX77658_REG_INTM_GLBL0,
>> +	.num_regs       = 1,
>> +	.irqs           = max77658_glbl0_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77658_glbl0_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77659_glbl0_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_GPIO0_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_EN_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL1_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_TJAL2_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL0_DOD0_R, }, };
>> +
>> +static const struct regmap_irq_chip max77659_glbl0_irq_chip = {
>> +	.name           = "max77659_glbl0",
>> +	.status_base    = MAX77658_REG_INT_GLBL0,
>> +	.mask_base      = MAX77654_REG_INTM_GLBL0,
>> +	.num_regs       = 1,
>> +	.irqs           = max77659_glbl0_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77659_glbl0_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77643_glbl1_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB0_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB1_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB2_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, }, };
>> +
>> +static const struct regmap_irq_chip max77643_glbl1_irq_chip = {
>> +	.name           = "max77643_glbl1",
>> +	.status_base    = MAX77658_REG_INT_GLBL1,
>> +	.mask_base      = MAX77643_REG_INTM_GLBL1,
>> +	.num_regs       = 1,
>> +	.irqs           = max77643_glbl1_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77643_glbl1_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77654_glbl1_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
>> +	{ .mask = MAX77654_BIT_INT_GLBL1_GPI2_F, },
>> +	{ .mask = MAX77654_BIT_INT_GLBL1_GPI2_R, },
>> +	{ .mask = MAX77654_BIT_INT_GLBL1_SBB_TO, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO1_F, }, };
>> +
>> +static const struct regmap_irq_chip max77654_glbl1_irq_chip = {
>> +	.name           = "max77654_glbl1",
>> +	.status_base    = MAX77658_REG_INT_GLBL1,
>> +	.mask_base      = MAX77654_REG_INTM_GLBL1,
>> +	.num_regs       = 1,
>> +	.irqs           = max77654_glbl1_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77654_glbl1_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77658_glbl1_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB0_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB1_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_SBB2_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO1_F, }, };
>> +
>> +static const struct regmap_irq_chip max77658_glbl1_irq_chip = {
>> +	.name           = "max77658_glbl1",
>> +	.status_base    = MAX77658_REG_INT_GLBL1,
>> +	.mask_base      = MAX77658_REG_INTM_GLBL1,
>> +	.num_regs       = 1,
>> +	.irqs           = max77658_glbl1_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77658_glbl1_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77659_glbl1_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_F, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_GPI1_R, },
>> +	{ .mask = MAX77659_BIT_INT_GLBL1_SBB_TO, },
>> +	{ .mask = MAX77658_BIT_INT_GLBL1_LDO0_F, }, };
>> +
>> +static const struct regmap_irq_chip max77659_glbl1_irq_chip = {
>> +	.name           = "max77659_glbl1",
>> +	.status_base    = MAX77658_REG_INT_GLBL1,
>> +	.mask_base      = MAX77658_REG_INTM_GLBL1,
>> +	.num_regs       = 1,
>> +	.irqs           = max77659_glbl1_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77659_glbl1_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77658_chg_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_THM, },
>> +	{ .mask = MAX77658_BIT_INT_CHG, },
>> +	{ .mask = MAX77658_BIT_INT_CHGIN, },
>> +	{ .mask = MAX77658_BIT_INT_TJ_REG, },
>> +	{ .mask = MAX77658_BIT_INT_CHGIN_CTRL, },
>> +	{ .mask = MAX77658_BIT_INT_SYS_CTRL, },
>> +	{ .mask = MAX77658_BIT_INT_SYS_CNFG, }, };
>> +
>> +static const struct regmap_irq_chip max77654_chg_irq_chip = {
>> +	.name           = "max77654_chg",
>> +	.status_base    = MAX77658_REG_INT_CHG,
>> +	.mask_base      = MAX77658_REG_INTM_CHG,
>> +	.num_regs       = 1,
>> +	.irqs           = max77658_chg_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77658_chg_irqs),
>> +};
>> +
>> +static const struct regmap_irq_chip max77658_chg_irq_chip = {
>> +	.name           = "max77658_chg",
>> +	.status_base    = MAX77658_REG_INT_CHG,
>> +	.mask_base      = MAX77658_REG_INTM_CHG,
>> +	.num_regs       = 1,
>> +	.irqs           = max77658_chg_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77658_chg_irqs),
>> +};
>> +
>> +static const struct regmap_irq max77659_chg_irqs[] = {
>> +	{ .mask = MAX77658_BIT_INT_THM, },
>> +	{ .mask = MAX77658_BIT_INT_CHG, },
>> +	{ .mask = MAX77658_BIT_INT_CHGIN, },
>> +	{ .mask = MAX77658_BIT_INT_TJ_REG, },
>> +	{ .mask = MAX776569_BIT_INT_SYS_CTRL, }, };
>> +
>> +static const struct regmap_irq_chip max77659_chg_irq_chip = {
>> +	.name           = "max77659_chg",
>> +	.status_base    = MAX77658_REG_INT_CHG,
>> +	.mask_base      = MAX77658_REG_INTM_CHG,
>> +	.num_regs       = 1,
>> +	.irqs           = max77659_chg_irqs,
>> +	.num_irqs       = ARRAY_SIZE(max77659_chg_irqs),
>> +};
>> +
>> +static const struct mfd_cell max77643_devs[] = {
>> +	MFD_CELL_OF("max77643-regulator", NULL, NULL, 0, 0,
>> +		    "adi,max77643-regulator"),
>> +};
>> +
>> +static const struct mfd_cell max77654_devs[] = {
>> +	MFD_CELL_OF("max77654-regulator", NULL, NULL, 0, 0,
>> +		    "adi,max77654-regulator"),
>> +	MFD_CELL_OF("max77654-charger", NULL, NULL, 0, 0,
>> +		    "adi,max77654-charger"),
>> +};
>> +
>> +static const struct mfd_cell max77658_devs[] = {
>> +	MFD_CELL_OF("max77658-regulator", NULL, NULL, 0, 0,
>> +		    "adi,max77658-regulator"),
>> +	MFD_CELL_OF("max77658-charger", NULL, NULL, 0, 0,
>> +		    "adi,max77658-charger"),
>> +	MFD_CELL_OF("max77658-battery", NULL, NULL, 0, 0,
>> +		    "adi,max77658-battery"),
>> +};
>> +
>> +static const struct mfd_cell max77659_devs[] = {
>> +	MFD_CELL_OF("max77659-regulator", NULL, NULL, 0, 0,
>> +		    "adi,max77659-regulator"),
>> +	MFD_CELL_OF("max77659-charger", NULL, NULL, 0, 0,
>> +		    "adi,max77659-charger"),
>> +};
>
>You can unwrap all of these, you have 100-chars to play with.

Based on other maintainers' reviews, I limited the number of characters per line to 80. 
Here is the link for the related review -> https://lore.kernel.org/lkml/9915e079-c6b5-a8f4-734e-f0325809efd0@linaro.org/
I can continue according to the joint decision.

>
>> +static const struct chip_info chip[] = {
>> +	[ID_MAX77643] = {
>> +		.id = ID_MAX77643,
>> +		.n_devs = ARRAY_SIZE(max77643_devs),
>> +		.devs = max77643_devs,
>> +	},
>> +	[ID_MAX77654] = {
>> +		.id = ID_MAX77654,
>> +		.n_devs = ARRAY_SIZE(max77654_devs),
>> +		.devs = max77654_devs,
>> +	},
>> +	[ID_MAX77658] = {
>> +		.id = ID_MAX77658,
>> +		.n_devs = ARRAY_SIZE(max77658_devs),
>> +		.devs = max77658_devs,
>> +	},
>> +	[ID_MAX77659] = {
>> +		.id = ID_MAX77659,
>> +		.n_devs = ARRAY_SIZE(max77659_devs),
>> +		.devs = max77659_devs,
>> +	},
>> +};
>> +
>> +static int max77658_pmic_irq_init(struct device *dev) {
>> +	const struct regmap_irq_chip *glbl0_chip, *glbl1_chip, *chg_chip;
>> +	struct max77658_dev *max77658 = dev_get_drvdata(dev);
>> +	int ret, i;
>> +
>> +	switch (max77658->>chip->>id) {
>> +	case ID_MAX77643:
>> +		glbl0_chip = &max77643_glbl0_irq_chip;
>> +		glbl1_chip = &max77643_glbl1_irq_chip;
>> +		break;
>> +	case ID_MAX77654:
>> +		glbl0_chip = &max77654_glbl0_irq_chip;
>> +		glbl1_chip = &max77654_glbl1_irq_chip;
>> +		chg_chip = &max77654_chg_irq_chip;
>> +		break;
>> +	case ID_MAX77658:
>> +		glbl0_chip = &max77658_glbl0_irq_chip;
>> +		glbl1_chip = &max77658_glbl1_irq_chip;
>> +		chg_chip = &max77658_chg_irq_chip;
>> +		break;
>> +	case ID_MAX77659:
>> +		glbl0_chip = &max77659_glbl0_irq_chip;
>> +		glbl1_chip = &max77659_glbl1_irq_chip;
>> +		chg_chip = &max77659_chg_irq_chip;
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	for (i = 0; i < glbl0_chip->>num_regs; i++) {
>> +		ret = regmap_update_bits(max77658->>regmap,
>> +					 glbl0_chip->>mask_base,
>> +					 (1 << glbl0_chip->>irqs[i].reg_offset),
>> +					 1);
>> +		if (ret)
>> +			return dev_err_probe(dev, ret,
>> +					     "Unable to write Global0 Interrupt Masking register\n");
>> +	}
>> +
>> +	for (i = 0; i < glbl1_chip->>num_regs; i++) {
>> +		ret = regmap_update_bits(max77658->>regmap,
>> +					 glbl1_chip->>mask_base,
>> +					 (1 << glbl1_chip->>irqs[i].reg_offset),
>> +					 1);
>> +		if (ret)
>> +			return dev_err_probe(dev, ret,
>> +					     "Unable to write Global1 Interrupt Masking register\n");
>> +	}
>> +
>> +	if (max77658->>chip->>id != ID_MAX77643) {
>> +		for (i = 0; i < chg_chip->>num_regs; i++) {
>> +			ret = regmap_update_bits(max77658->>regmap,
>> +						 chg_chip->>mask_base,
>> +						 (1 <<
>> +						 chg_chip->>irqs[i].reg_offset),
>> +						 1);
>> +			if (ret)
>> +				return dev_err_probe(dev, ret,
>> +						     "Unable to write Charger Interrupt Masking register\n");
>> +		}
>> +
>> +		ret = devm_regmap_add_irq_chip(dev, max77658->>regmap,
>> +					       max77658->>irq,
>> +					       IRQF_ONESHOT | IRQF_SHARED, 0,
>> +					       chg_chip, &max77658->>irqc_chg);
>> +		if (ret)
>> +			return dev_err_probe(dev, ret,
>> +					     "Failed to add charger IRQ chip\n");
>> +	}
>> +
>> +	ret = devm_regmap_add_irq_chip(dev, max77658->>regmap, max77658->>irq,
>> +				       IRQF_ONESHOT | IRQF_SHARED, 0,
>> +				       glbl0_chip, &max77658->>irqc_glbl0);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret,
>> +				     "Failed to add global0 IRQ chip\n");
>> +
>> +	return devm_regmap_add_irq_chip(dev, max77658->>regmap, max77658->>irq,
>> +					IRQF_ONESHOT | IRQF_SHARED, 0,
>> +					glbl1_chip, &max77658->>irqc_glbl1);
>
>This function is hectic.
>
>What exactly are you doing here?

I mask all interrupts of global0, global1, and charger inside the loops. 

>
>> +}
>> +
>> +static int max77658_pmic_setup(struct device *dev) {
>> +	struct max77658_dev *max77658 = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = max77658_pmic_irq_init(max77658->>dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE,
>> +				   max77658->>chip->>devs, max77658->>chip->>n_devs,
>> +				   NULL, 0, NULL);
>> +
>
>These are usually placed in probe.
>
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Failed to add sub-devices\n");
>> +
>> +	ret = device_init_wakeup(dev, true);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "Unable to init wakeup\n");
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct i2c_device_id max77658_i2c_id[];
>
>What on earth is this?  A struct forward declaration?
>
>If you need this, just move the original code block to here.
>
>> +static int max77658_i2c_probe(struct i2c_client *client) {
>> +	struct max77658_dev *max77658;
>> +	struct i2c_client *fuel;
>> +
>> +	max77658 = devm_kzalloc(&client->>dev, sizeof(*max77658), GFP_KERNEL);
>> +	if (!max77658)
>> +		return -ENOMEM;
>> +
>> +	i2c_set_clientdata(client, max77658);
>> +	max77658->>dev = &client->>dev;
>
>How do you fetch back max77658?
>
>Don't you need access to the device structs?
>
>If so, you already have a reference, no?
>
>> +	max77658->>irq = client->>irq;
>> +
>> +	if (max77658->>dev->>of_node)
>> +		max77658->>chip  = of_device_get_match_data(max77658->>dev);
>> +	else
>> +		max77658->>chip  = (struct chip_info *)
>
>Cast from void * shouldn't be required.
>
>> +					i2c_match_id(max77658_i2c_id,
>> +						     client)->>driver_data;
>
>100-chars everywhere.
>
>> +	if (!max77658->>chip)
>> +		return -EINVAL;
>> +
>> +	max77658->>regmap = devm_regmap_init_i2c(client,
>> +						&max77658_regmap_config);
>> +	if (IS_ERR(max77658->>regmap))
>> +		return dev_err_probe(max77658->>dev, PTR_ERR(max77658->>regmap),
>> +				     "Failed to allocate register map\n");
>
>s/allocate/initialise/
>
>"regmap" is fine.
>
>> +	fuel = i2c_new_dummy_device(client->>adapter, I2C_ADDR_FUEL_GAUGE);
>> +	if (IS_ERR(fuel))
>> +		return dev_err_probe(max77658->>dev, PTR_ERR(fuel),
>> +				     "failed add i2c device[0x%Xh]\n",
>
>"Failed to create I2C device"
>
>> +				      I2C_ADDR_FUEL_GAUGE);
>> +
>> +	i2c_set_clientdata(fuel, max77658);
>> +
>> +	max77658->>regmap_fg = devm_regmap_init_i2c(fuel,
>> +						   &max77658_regmap_config_fg);
>> +	if (IS_ERR(max77658->>regmap_fg))
>> +		return dev_err_probe(max77658->>dev,
>> +				     PTR_ERR(max77658->>regmap_fg),
>> +				     "failed to initialize i2c device[0x%Xh]\n",
>
>"Failed"
>
>"I2C"
>
>> +				     I2C_ADDR_FUEL_GAUGE);
>> +
>> +	return max77658_pmic_setup(max77658->>dev);
>> +}
>> +
>> +static const struct of_device_id max77658_of_id[] = {
>> +	{ .compatible = "adi,max77643", .data = &chip[ID_MAX77643] },
>> +	{ .compatible = "adi,max77654", .data = &chip[ID_MAX77654] },
>> +	{ .compatible = "adi,max77658", .data = &chip[ID_MAX77658] },
>> +	{ .compatible = "adi,max77659", .data = &chip[ID_MAX77659] },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, max77658_of_id);
>> +
>> +static const struct i2c_device_id max77658_i2c_id[] = {
>> +	{ "max77643", (kernel_ulong_t)&chip[ID_MAX77643] },
>> +	{ "max77654", (kernel_ulong_t)&chip[ID_MAX77654] },
>> +	{ "max77658", (kernel_ulong_t)&chip[ID_MAX77658] },
>> +	{ "max77659", (kernel_ulong_t)&chip[ID_MAX77659] },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, max77658_i2c_id);
>
>Please don't pass MFD data through the DT platform initialisation API.
>
>Pass the Chip ID and match in C-code.
>
>> +static struct i2c_driver max77658_driver = {
>> +	.driver = {
>> +		.name = "max77658",
>> +		.of_match_table = max77658_of_id,
>> +	},
>> +	.probe_new = max77658_i2c_probe,
>> +	.id_table = max77658_i2c_id,
>> +};
>> +module_i2c_driver(max77658_driver);
>> +
>> +MODULE_DESCRIPTION("MAX77658 MFD Driver"); 
>> +MODULE_AUTHOR("Nurettin.Bolucu@analog.com, 
>> +Zeynep.Arslanbenzer@analog.com"); MODULE_LICENSE("GPL");
>> diff --git a/include/linux/mfd/max77658.h 
>> b/include/linux/mfd/max77658.h new file mode 100644 index 
>> 000000000000..471a8474513e
>> --- /dev/null
>> +++ b/include/linux/mfd/max77658.h
>> @@ -0,0 +1,88 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +
>> +#ifndef __MAX77658_MFD_H__
>> +#define __MAX77658_MFD_H__
>> +
>> +#include <linux/bits.h>>
>> +#include <linux/types.h>>
>> +
>> +#define MAX77658_REG_INT_GLBL0	0x00
>> +#define MAX77658_REG_INT_CHG	0x01
>> +#define MAX77658_REG_INT_GLBL1	0x04
>> +#define MAX77658_REG_INTM_CHG	0x07
>> +#define MAX77658_REG_INTM_GLBL0	0x08
>> +#define MAX77658_REG_INTM_GLBL1	0x09
>> +
>> +#define MAX77654_REG_INTM_GLBL1	0x08
>> +#define MAX77654_REG_INTM_GLBL0	0x09
>> +
>> +#define MAX77643_REG_INT_GLBL1	0x01
>> +#define MAX77643_REG_INTM_GLBL0	0x04
>> +#define MAX77643_REG_INTM_GLBL1	0x05
>> +
>> +#define MAX77658_BIT_INT_GLBL0_GPIO0_F	BIT(0)
>> +#define MAX77658_BIT_INT_GLBL0_GPIO0_R	BIT(1)
>> +#define MAX77658_BIT_INT_GLBL0_EN_F	BIT(2)
>> +#define MAX77658_BIT_INT_GLBL0_EN_R	BIT(3)
>> +#define MAX77658_BIT_INT_GLBL0_TJAL1_R	BIT(4)
>> +#define MAX77658_BIT_INT_GLBL0_TJAL2_R	BIT(5)
>> +#define MAX77658_BIT_INT_GLBL0_DOD1_R	BIT(6)
>> +#define MAX77658_BIT_INT_GLBL0_DOD0_R	BIT(7)
>> +
>> +#define MAX77643_BIT_INT_GLBL0_DOD0_R	BIT(6)
>> +
>> +#define MAX77658_BIT_INT_GLBL1_GPI1_F	BIT(0)
>> +#define MAX77658_BIT_INT_GLBL1_GPI1_R	BIT(1)
>> +#define MAX77658_BIT_INT_GLBL1_SBB0_F	BIT(2)
>> +#define MAX77658_BIT_INT_GLBL1_SBB1_F	BIT(3)
>> +#define MAX77658_BIT_INT_GLBL1_SBB2_F	BIT(4)
>> +#define MAX77658_BIT_INT_GLBL1_LDO0_F	BIT(5)
>> +#define MAX77658_BIT_INT_GLBL1_LDO1_F	BIT(6)
>> +
>> +#define MAX77659_BIT_INT_GLBL1_SBB_TO	BIT(4)
>> +
>> +#define MAX77654_BIT_INT_GLBL1_GPI2_F	BIT(2)
>> +#define MAX77654_BIT_INT_GLBL1_GPI2_R	BIT(3)
>> +#define MAX77654_BIT_INT_GLBL1_SBB_TO	BIT(4)
>> +
>> +#define MAX77658_BIT_INT_THM		BIT(0)
>> +#define MAX77658_BIT_INT_CHG		BIT(1)
>> +#define MAX77658_BIT_INT_CHGIN		BIT(2)
>> +#define MAX77658_BIT_INT_TJ_REG		BIT(3)
>> +#define MAX77658_BIT_INT_CHGIN_CTRL	BIT(4)
>> +#define MAX77658_BIT_INT_SYS_CTRL	BIT(5)
>> +#define MAX77658_BIT_INT_SYS_CNFG	BIT(6)
>> +
>> +#define MAX776569_BIT_INT_SYS_CTRL	BIT(4)
>> +
>> +enum max77658_ids {
>> +	ID_MAX77643,
>> +	ID_MAX77654,
>> +	ID_MAX77658,
>> +	ID_MAX77659
>> +};
>> +
>> +struct chip_info {
>> +	enum max77658_ids id;
>> +	int n_devs;
>> +	const struct mfd_cell *devs;
>> +};
>> +
>> +struct device;
>> +struct regmap;
>> +struct regmap_irq_chip_data;
>> +
>> +struct max77658_dev {
>> +	struct device *dev;
>> +	const struct chip_info *chip;
>> +
>> +	int irq;
>> +	struct regmap_irq_chip_data *irqc_glbl0;
>> +	struct regmap_irq_chip_data *irqc_glbl1;
>> +	struct regmap_irq_chip_data *irqc_chg;
>> +
>> +	struct regmap *regmap;
>> +	struct regmap *regmap_fg;
>> +};
>> +
>> +#endif /* __MAX77658_MFD_H__ */
>> --
>> 2.25.1
>>
>
>--
>Lee Jones [李琼斯]

Best regards,
Zeynep

RE: [PATCH v2 3/8] dt-bindings: power: supply: max77658: Add ADI MAX77654/58/59 Charger

[Arslanbenzer, Zeynep]

>On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
>> Add ADI MAX77654/MAX77658/MAX77659 power supply devicetree document.
>> 
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>> ---
>>  .../power/supply/adi,max77658-charger.yaml    | 65 +++++++++++++++++++
>>  1 file changed, 65 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/power/supply/adi,max77658-charger.ya
>> ml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.
>> yaml 
>> b/Documentation/devicetree/bindings/power/supply/adi,max77658-charger.
>> yaml
>> new file mode 100644
>> index 000000000000..f140abab969c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77658-char
>> +++ ger.yaml
>> @@ -0,0 +1,65 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
>> +---
>> +$id: 
>> +https://urldefense.com/v3/__http://devicetree.org/schemas/power/suppl
>> +y/adi,max77658-charger.yaml*__;Iw!!A3Ni8CS0y2Y!6-xzReMH0We2Hv87KO_HBQ
>> +bP0IddfulmvzWOzSsxmA6TqV_V2Mo6KyJ5H6hAqZGFeGOE4UUIy9Dv-ZNUon6VJR3ITTw
>> +OLmNnuOb-$
>> +$schema: 
>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>> +aml*__;Iw!!A3Ni8CS0y2Y!6-xzReMH0We2Hv87KO_HBQbP0IddfulmvzWOzSsxmA6TqV
>> +_V2Mo6KyJ5H6hAqZGFeGOE4UUIy9Dv-ZNUon6VJR3ITTwOLk8moz89$
>> +
>> +title: Battery charger for MAX77658 PMICs family from ADI.
>
>This is a friendly reminder during the review process.
>
>It seems my previous comments were not fully addressed. Actually you ignored all of them. Maybe my feedback got lost between the quotes, maybe you just forgot to >apply it. Please go back to the previous discussion and either implement all requested changes or keep discussing them.
>
>Thank you.
>
>Best regards,
>Krzysztof

Hi Krzysztof,

Thank you for your review. I actually tried to complete most of them but I may have misunderstood some of them. I'm sorry about that. I will be more careful in future versions.

Best regards,
Zeynep

RE: [PATCH v2 8/8] mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

[Arslanbenzer, Zeynep]

On Thu, 30 Mar 2023, Lee Jones wrote:

>On Wed, 22 Mar 2023, Zeynep Arslanbenzer wrote:
>
>> MFD driver for MAX77643/MAX77654/MAX77658/MAX77659 to enable its sub
>
>Please drop all references to 'MFD'.
>
>What are these devices, really?  I suspect they are PMICs?
>
>> devices.
>>
>> The MAX77643 is a multi-function devices. It includes regulator.
>>
>> The MAX77654 is a multi-function devices. It includes regulator and 
>> charger.
>>
>> The MAX77658 is a multi-function devices. It includes regulator, 
>> charger and battery.
>>
>> The MAX77659 is a multi-function devices. It includes regulator and 
>> charger.
>>
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>> ---
>>  drivers/mfd/Kconfig          |  15 ++
>>  drivers/mfd/Makefile         |   1 +
>>  drivers/mfd/max77658.c       | 448 +++++++++++++++++++++++++++++++++++
>>  include/linux/mfd/max77658.h |  88 +++++++
>>  4 files changed, 552 insertions(+)
>>  create mode 100644 drivers/mfd/max77658.c  create mode 100644 
>> include/linux/mfd/max77658.h
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 
>> 8b93856de432..7b4be7fb8662 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -821,6 +821,21 @@ config MFD_MAX77650
>>  	  the following functionalities of the device: GPIO, regulator,
>>  	  charger, LED, onkey.
>>
>> +config MFD_MAX77658
>> +	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support"
>> +	depends on I2C
>> +	depends on OF
>> +	select MFD_CORE
>> +	select REGMAP_I2C
>> +	select REGMAP_IRQ
>> +	help
>> +	  Say Y here to add support for Analog Devices
>> +	  MAX77643/MAX77654/MAX77658/MAX77659 Power Management IC.
>
>"MAX776xx series"?

As I realized later, max77620, max77650, max77686, and max77693 drivers were merged to Linux before our patch. They are also PMIC devices and our patch does not cover them. Therefore, I think it would not be appropriate to use MAX776xx.

>
>> +	  This is the core multifunction
>
>Just "core driver" is fine.
>
>Odd place to line wrap?
>
>> +	  driver for interacting with the device. Additional drivers can be
>
>"can be"?  It's probably pretty useless if you don't, no?
>
>> +	  enabled in order to use the following functionalities of the device:
>> +	  regulator, charger.
>
>"... in order to use the regular and charger functionality of the device".
>
>>  config MFD_MAX77686
>>  	tristate "Maxim Semiconductor MAX77686/802 PMIC Support"
>>  	depends on I2C
>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 
>> 7ed3ef4a698c..f52aff45878f 100644
>> --- a/drivers/mfd/Makefile
>> +++ b/drivers/mfd/Makefile
>> @@ -163,6 +163,7 @@ obj-$(CONFIG_MFD_DA9150)	+= da9150-core.o
>>  obj-$(CONFIG_MFD_MAX14577)	+= max14577.o
>>  obj-$(CONFIG_MFD_MAX77620)	+= max77620.o
>>  obj-$(CONFIG_MFD_MAX77650)	+= max77650.o
>> +obj-$(CONFIG_MFD_MAX77658)	+= max77658.o
>>  obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
>>  obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
>>  obj-$(CONFIG_MFD_MAX77714)	+= max77714.o
>> diff --git a/drivers/mfd/max77658.c b/drivers/mfd/max77658.c new file 
>> mode 100644 index 000000000000..a1c6db48eb08
>> --- /dev/null
>> +++ b/drivers/mfd/max77658.c
>> @@ -0,0 +1,448 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2023 Analog Devices, Inc.
>> + * ADI driver for the MAX77643/MAX77654/MAX77658/MAX77659
>> + */
>
>No need to list every device.
>
>"MAX776xx series"?
>

Best regards,
Zeynep

Re: [PATCH v2 8/8] mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

[Lee Jones]

On Sun, 23 Apr 2023, Arslanbenzer, Zeynep wrote:

> On Thu, 30 Mar 2023, Lee Jones wrote:
> 
> >On Wed, 22 Mar 2023, Zeynep Arslanbenzer wrote:
> >
> >> MFD driver for MAX77643/MAX77654/MAX77658/MAX77659 to enable its sub
> >
> >Please drop all references to 'MFD'.
> >
> >What are these devices, really?  I suspect they are PMICs?
> >
> >> devices.
> >>
> >> The MAX77643 is a multi-function devices. It includes regulator.
> >>
> >> The MAX77654 is a multi-function devices. It includes regulator and 
> >> charger.
> >>
> >> The MAX77658 is a multi-function devices. It includes regulator, 
> >> charger and battery.
> >>
> >> The MAX77659 is a multi-function devices. It includes regulator and 
> >> charger.
> >>
> >> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> >> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> >> ---
> >>  drivers/mfd/Kconfig          |  15 ++
> >>  drivers/mfd/Makefile         |   1 +
> >>  drivers/mfd/max77658.c       | 448 +++++++++++++++++++++++++++++++++++
> >>  include/linux/mfd/max77658.h |  88 +++++++
> >>  4 files changed, 552 insertions(+)
> >>  create mode 100644 drivers/mfd/max77658.c  create mode 100644 
> >> include/linux/mfd/max77658.h
> >>
> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 
> >> 8b93856de432..7b4be7fb8662 100644
> >> --- a/drivers/mfd/Kconfig
> >> +++ b/drivers/mfd/Kconfig
> >> @@ -821,6 +821,21 @@ config MFD_MAX77650
> >>  	  the following functionalities of the device: GPIO, regulator,
> >>  	  charger, LED, onkey.
> >>
> >> +config MFD_MAX77658
> >> +	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support"
> >> +	depends on I2C
> >> +	depends on OF
> >> +	select MFD_CORE
> >> +	select REGMAP_I2C
> >> +	select REGMAP_IRQ
> >> +	help
> >> +	  Say Y here to add support for Analog Devices
> >> +	  MAX77643/MAX77654/MAX77658/MAX77659 Power Management IC.
> >
> >"MAX776xx series"?
> 
> As I realized later, max77620, max77650, max77686, and max77693 drivers were merged to Linux before our patch. They are also PMIC devices and our patch does not cover them. Therefore, I think it would not be appropriate to use MAX776xx.

Perhaps you can come up with something a little more scalable then.

What if you added support for another 10 devices?

-- 
Lee Jones [李琼斯]

RE: [PATCH v2 8/8] mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

[Arslanbenzer, Zeynep]

On Mon, 24 Apr 2023, Lee Jones wrote:
>
>On Sun, 23 Apr 2023, Arslanbenzer, Zeynep wrote:
>
>> On Thu, 30 Mar 2023, Lee Jones wrote:
>> 
>> >On Wed, 22 Mar 2023, Zeynep Arslanbenzer wrote:
>> >
>> >> MFD driver for MAX77643/MAX77654/MAX77658/MAX77659 to enable its 
>> >> sub
>> >
>> >Please drop all references to 'MFD'.
>> >
>> >What are these devices, really?  I suspect they are PMICs?
>> >
>> >> devices.
>> >>
>> >> The MAX77643 is a multi-function devices. It includes regulator.
>> >>
>> >> The MAX77654 is a multi-function devices. It includes regulator and 
>> >> charger.
>> >>
>> >> The MAX77658 is a multi-function devices. It includes regulator, 
>> >> charger and battery.
>> >>
>> >> The MAX77659 is a multi-function devices. It includes regulator and 
>> >> charger.
>> >>
>> >> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>> >> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>> >> ---
>> >>  drivers/mfd/Kconfig          |  15 ++
>> >>  drivers/mfd/Makefile         |   1 +
>> >>  drivers/mfd/max77658.c       | 448 +++++++++++++++++++++++++++++++++++
>> >>  include/linux/mfd/max77658.h |  88 +++++++
>> >>  4 files changed, 552 insertions(+)  create mode 100644 
>> >> drivers/mfd/max77658.c  create mode 100644 
>> >> include/linux/mfd/max77658.h
>> >>
>> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index
>> >> 8b93856de432..7b4be7fb8662 100644
>> >> --- a/drivers/mfd/Kconfig
>> >> +++ b/drivers/mfd/Kconfig
>> >> @@ -821,6 +821,21 @@ config MFD_MAX77650
>> >>  	  the following functionalities of the device: GPIO, regulator,
>> >>  	  charger, LED, onkey.
>> >>
>> >> +config MFD_MAX77658
>> >> +	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support"
>> >> +	depends on I2C
>> >> +	depends on OF
>> >> +	select MFD_CORE
>> >> +	select REGMAP_I2C
>> >> +	select REGMAP_IRQ
>> >> +	help
>> >> +	  Say Y here to add support for Analog Devices
>> >> +	  MAX77643/MAX77654/MAX77658/MAX77659 Power Management IC.
>> >
>> >"MAX776xx series"?
>> 
>> As I realized later, max77620, max77650, max77686, and max77693 drivers were merged to Linux before our patch. They are also PMIC devices and our patch does not cover them. Therefore, I think it would not be appropriate to use MAX776xx.
>
>Perhaps you can come up with something a little more scalable then.
>
>What if you added support for another 10 devices?
>
For now, we have no plans to add any new device support to this driver. We named the driver max77658 because it has the most inclusive feature among the supported devices. We can shorten it to MAX77643/54/58/59 or just type max77658 in Kconfig and specify other supported devices in the device tree. Would one of these be a more suitable solution?

Best regards,
Zeynep

Re: [PATCH v2 8/8] mfd: max77658: Add ADI MAX77643/54/58/59 MFD Support

[Lee Jones]

On Tue, 25 Apr 2023, Arslanbenzer, Zeynep wrote:

> On Mon, 24 Apr 2023, Lee Jones wrote:
> >
> >On Sun, 23 Apr 2023, Arslanbenzer, Zeynep wrote:
> >
> >> On Thu, 30 Mar 2023, Lee Jones wrote:
> >> 
> >> >On Wed, 22 Mar 2023, Zeynep Arslanbenzer wrote:
> >> >
> >> >> MFD driver for MAX77643/MAX77654/MAX77658/MAX77659 to enable its 
> >> >> sub
> >> >
> >> >Please drop all references to 'MFD'.
> >> >
> >> >What are these devices, really?  I suspect they are PMICs?
> >> >
> >> >> devices.
> >> >>
> >> >> The MAX77643 is a multi-function devices. It includes regulator.
> >> >>
> >> >> The MAX77654 is a multi-function devices. It includes regulator and 
> >> >> charger.
> >> >>
> >> >> The MAX77658 is a multi-function devices. It includes regulator, 
> >> >> charger and battery.
> >> >>
> >> >> The MAX77659 is a multi-function devices. It includes regulator and 
> >> >> charger.
> >> >>
> >> >> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> >> >> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> >> >> ---
> >> >>  drivers/mfd/Kconfig          |  15 ++
> >> >>  drivers/mfd/Makefile         |   1 +
> >> >>  drivers/mfd/max77658.c       | 448 +++++++++++++++++++++++++++++++++++
> >> >>  include/linux/mfd/max77658.h |  88 +++++++
> >> >>  4 files changed, 552 insertions(+)  create mode 100644 
> >> >> drivers/mfd/max77658.c  create mode 100644 
> >> >> include/linux/mfd/max77658.h
> >> >>
> >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index
> >> >> 8b93856de432..7b4be7fb8662 100644
> >> >> --- a/drivers/mfd/Kconfig
> >> >> +++ b/drivers/mfd/Kconfig
> >> >> @@ -821,6 +821,21 @@ config MFD_MAX77650
> >> >>  	  the following functionalities of the device: GPIO, regulator,
> >> >>  	  charger, LED, onkey.
> >> >>
> >> >> +config MFD_MAX77658
> >> >> +	tristate "Analog Devices MAX77643/MAX77654/MAX77658/MAX77659 PMIC Support"
> >> >> +	depends on I2C
> >> >> +	depends on OF
> >> >> +	select MFD_CORE
> >> >> +	select REGMAP_I2C
> >> >> +	select REGMAP_IRQ
> >> >> +	help
> >> >> +	  Say Y here to add support for Analog Devices
> >> >> +	  MAX77643/MAX77654/MAX77658/MAX77659 Power Management IC.
> >> >
> >> >"MAX776xx series"?
> >> 
> >> As I realized later, max77620, max77650, max77686, and max77693 drivers were merged to Linux before our patch. They are also PMIC devices and our patch does not cover them. Therefore, I think it would not be appropriate to use MAX776xx.
> >
> >Perhaps you can come up with something a little more scalable then.
> >
> >What if you added support for another 10 devices?
> >
> For now, we have no plans to add any new device support to this driver. We named the driver max77658 because it has the most inclusive feature among the supported devices. We can shorten it to MAX77643/54/58/59 or just type max77658 in Kconfig and specify other supported devices in the device tree. Would one of these be a more suitable solution?

The former looks like a nice middle-ground.

-- 
Lee Jones [李琼斯]

RE: [PATCH v2 5/8] dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery

[Arslanbenzer, Zeynep]

On Wed, 22 Mar 2023, Krzysztof Kozlowski wrote:
>On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
>> Add ADI MAX77658 power supply devicetree document.
>> 
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>> ---
>>  .../power/supply/adi,max77658-battery.yaml    | 58 +++++++++++++++++++
>>  1 file changed, 58 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/power/supply/adi,max77658-battery.ya
>> ml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.
>> yaml 
>> b/Documentation/devicetree/bindings/power/supply/adi,max77658-battery.
>> yaml
>> new file mode 100644
>> index 000000000000..0b696f7c4d1b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77658-batt
>> +++ ery.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
>> +---
>> +$id: 
>> +https://urldefense.com/v3/__http://devicetree.org/schemas/power/suppl
>> +y/adi,max77658-battery.yaml*__;Iw!!A3Ni8CS0y2Y!7jzMr8UalEjjYfmquE6Iqt
>> +SndU7f-c9va789cC2VmSpvstAZ-AokoftF1vX_ZdeLxGuE455k4EMaG0BdyEAEeqCT4rs
>> +zrkvmwS9F$
>> +$schema: 
>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>> +aml*__;Iw!!A3Ni8CS0y2Y!7jzMr8UalEjjYfmquE6IqtSndU7f-c9va789cC2VmSpvst
>> +AZ-AokoftF1vX_ZdeLxGuE455k4EMaG0BdyEAEeqCT4rszromzOD1g$
>> +
>> +title: Battery for MAX77658 PMIC from ADI.
>
>Implement all previous comments, not just some.
>
>
>> +
>> +maintainers:
>> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>> +
>> +description: |
>> +  This module is part of the MAX77658 MFD device. For more details
>> +  see Documentation/devicetree/bindings/mfd/adi,max77658.yaml.
>> +
>> +  The fuel gauge is represented as a sub-node of the PMIC node on the device tree.
>> +
>> +properties:
>> +  compatible:
>> +    const:
>> +      adi,max77658-battery
>
>It's one line.
>
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  adi,valrt-min-microvolt:
>> +    description: Minimum voltage value that triggers the alarm.
>> +
>> +  adi,valrt-max-microvolt:
>> +    description: Maximum voltage value that triggers the alarm.
>
>Use the same syntax as battery.yaml
>
>> +
>> +  adi,salrt-min-percent:
>> +    description: Minimum percentage of battery that triggers the alarm.
>> +
>> +  adi,salrt-max-percent:
>> +    description: Maximum percentage of battery that triggers the alarm.
>
>That's not suitable for DT. Do not encode policies into DT.
>
>> +
>> +  adi,ialrt-min-microamp:
>> +    description: Minimum current value that triggers the alarm.
>> +
>> +  adi,ialrt-max-microamp:
>> +    description: Maximum current value that triggers the alarm.
>> +
>> +  monitored-battery:
>> +    description: >
>> +      phandle to a "simple-battery" compatible node.
>> +
>> +      This property must be a phandle to a node using the format 
>> + described
>
>You already said it above.
>
>> +      in battery.yaml, with the following properties being required:
>> +      - alert-celsius
>> +
>> +required:
>> +  - compatible
>
>Why reg and monitored-batter are not required?
>
If no monitored-battery information is supplied, we set default values for alert-celsius. The reg property is the I2C address of the device and it is part of the parent schema. Therefore, both are not required in this file.

Best regards,
Zeynep

RE: [PATCH v2 2/8] regulator: max77658: Add ADI MAX77643/54/58/59 Regulator Support

[Arslanbenzer, Zeynep]

On Wed, 22 Mar 2023, Krzysztof Kozlowski wrote:
>On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
>> Regulator driver for ADI MAX77643/MAX77654/MAX77658/MAX77659.
>> 
>> MAX77643/MAX77659 has 1 LDO regulator.
>> MAX77654/MAX77658 has two LDO regulators.
>> 
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>
>
>
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct platform_device_id max77658_regulator_id[] = {
>> +	{ "max77643-regulator" },
>> +	{ "max77654-regulator" },
>> +	{ "max77658-regulator" },
>> +	{ "max77659-regulator" },
>
>Why do you need so many entries? They do not differ.

They are slightly different. Just MAX77659 and MAX77643 regulators have exactly the same features. MAX77659 and MAX77643 have 1 LDO regulator but others have 2 and the voltage base of the MAX77654 regulators is different from others. Should I use the same entry for the MAX77643 and MAX77659?

Best regards,
Zeynep

Re: [PATCH v2 5/8] dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery

[Krzysztof Kozlowski]

On 01/05/2023 22:20, Arslanbenzer, Zeynep wrote:

>> You already said it above.
>>
>>> +      in battery.yaml, with the following properties being required:
>>> +      - alert-celsius
>>> +
>>> +required:
>>> +  - compatible
>>
>> Why reg and monitored-batter are not required?
>>
> If no monitored-battery information is supplied, we set default values for alert-celsius.

So the device can operate without battery? Interesting... are you sure,
you can physically remove battery and device will work?

> The reg property is the I2C address of the device and it is part of the parent schema. Therefore, both are not required in this file.

What does it mean "in parent schema"? You have reg here, so how parent
schema is related to this?

Since you did not reply, I assume all other comments will be implemented.

Best regards,
Krzysztof

Re: [PATCH v2 2/8] regulator: max77658: Add ADI MAX77643/54/58/59 Regulator Support

[Krzysztof Kozlowski]

On 02/05/2023 08:32, Arslanbenzer, Zeynep wrote:
> On Wed, 22 Mar 2023, Krzysztof Kozlowski wrote:
>> On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
>>> Regulator driver for ADI MAX77643/MAX77654/MAX77658/MAX77659.
>>>
>>> MAX77643/MAX77659 has 1 LDO regulator.
>>> MAX77654/MAX77658 has two LDO regulators.
>>>
>>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>>
>>
>>
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct platform_device_id max77658_regulator_id[] = {
>>> +	{ "max77643-regulator" },
>>> +	{ "max77654-regulator" },
>>> +	{ "max77658-regulator" },
>>> +	{ "max77659-regulator" },
>>
>> Why do you need so many entries? They do not differ.
> 
> They are slightly different. Just MAX77659 and MAX77643 regulators have exactly the same features. MAX77659 and MAX77643 have 1 LDO regulator but others have 2 and the voltage base of the MAX77654 regulators is different from others. Should I use the same entry for the MAX77643 and MAX77659?

Wrap your email replies, it's difficult to read and reply.

Your driver does not choose regulators based on these compatibles. Your
of_device_id table claims all devices are fully compatible and do not
differ from regulators point of view. If they are different, you should
encode the difference. If not, use only one entry in of_device_id (only
of_device_id, not bindings).

Best regards,
Krzysztof

RE: [PATCH v2 5/8] dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery

[Arslanbenzer, Zeynep]

On Tue, 2 May 2023, Krzysztof Kozlowski wrote:
>On 01/05/2023 22:20, Arslanbenzer, Zeynep wrote:
>
>>> You already said it above.
>>>
>>>> +      in battery.yaml, with the following properties being required:
>>>> +      - alert-celsius
>>>> +
>>>> +required:
>>>> +  - compatible
>>>
>>> Why reg and monitored-batter are not required?
>>>
>> If no monitored-battery information is supplied, we set default values for alert-celsius.
>
>So the device can operate without battery? Interesting... are you sure, you can physically remove battery and device will work?
>

I mean if battery info is not supplied, we use default values.

ret = power_supply_get_battery_info(fg->battery, &info);
if (ret) {
    dev_err(fg->dev, "Unable to get battery info\n");
    fg->temp_alert_min = info->temp_alert_min;
    fg->temp_alert_max = info->temp_alert_max;
} else {
    fg->temp_alert_min = -128;
    fg->temp_alert_max = 127;
}


>> The reg property is the I2C address of the device and it is part of the parent schema. Therefore, both are not required in this file.
>
>What does it mean "in parent schema"? You have reg here, so how parent schema is related to this?

It is my mistake, I will remove reg from here.

Best regards,
Zeynep

Re: [PATCH v2 5/8] dt-bindings: power: supply: max77658: Add ADI MAX77658 Battery

[Krzysztof Kozlowski]

On 02/05/2023 17:05, Arslanbenzer, Zeynep wrote:
> On Tue, 2 May 2023, Krzysztof Kozlowski wrote:
>> On 01/05/2023 22:20, Arslanbenzer, Zeynep wrote:
>>
>>>> You already said it above.
>>>>
>>>>> +      in battery.yaml, with the following properties being required:
>>>>> +      - alert-celsius
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>
>>>> Why reg and monitored-batter are not required?
>>>>
>>> If no monitored-battery information is supplied, we set default values for alert-celsius.
>>
>> So the device can operate without battery? Interesting... are you sure, you can physically remove battery and device will work?
>>
> 
> I mean if battery info is not supplied, we use default values.
> 
> ret = power_supply_get_battery_info(fg->battery, &info);
> if (ret) {
>     dev_err(fg->dev, "Unable to get battery info\n");
>     fg->temp_alert_min = info->temp_alert_min;
>     fg->temp_alert_max = info->temp_alert_max;
> } else {
>     fg->temp_alert_min = -128;
>     fg->temp_alert_max = 127;
> }

You speak about driver, but I speak about hardware. The bindings are for
the latter.

Best regards,
Krzysztof

RE: [PATCH v2 2/8] regulator: max77658: Add ADI MAX77643/54/58/59 Regulator Support

[Arslanbenzer, Zeynep]

On Tue, 2 May 2023, Krzysztof Kozlowski wrote:
>On 02/05/2023 08:32, Arslanbenzer, Zeynep wrote:
>> On Wed, 22 Mar 2023, Krzysztof Kozlowski wrote:
>>> On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
>>>> Regulator driver for ADI MAX77643/MAX77654/MAX77658/MAX77659.
>>>>
>>>> MAX77643/MAX77659 has 1 LDO regulator.
>>>> MAX77654/MAX77658 has two LDO regulators.
>>>>
>>>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>>>
>>>
>>>
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static const struct platform_device_id max77658_regulator_id[] = {
>>>> +	{ "max77643-regulator" },
>>>> +	{ "max77654-regulator" },
>>>> +	{ "max77658-regulator" },
>>>> +	{ "max77659-regulator" },
>>>
>>> Why do you need so many entries? They do not differ.
>> 
>> They are slightly different. Just MAX77659 and MAX77643 regulators have 
>> exactly the same features. MAX77659 and MAX77643 have 1 LDO regulator but 
>> others have 2 and the voltage base of the MAX77654 regulators is different 
>> from others. Should I use the same entry for the MAX77643 and MAX77659?
>
>Your driver does not choose regulators based on these compatibles. Your
>of_device_id table claims all devices are fully compatible and do not
>differ from regulators point of view. If they are different, you should
>encode the difference. If not, use only one entry in of_device_id (only
>of_device_id, not bindings).

I used id table matching and I did not use of_device_id table. Should I use 
OF style match instead?

Best regards,
Zeynep

Re: [PATCH v2 2/8] regulator: max77658: Add ADI MAX77643/54/58/59 Regulator Support

[Krzysztof Kozlowski]

On 04/05/2023 12:36, Arslanbenzer, Zeynep wrote:
> On Tue, 2 May 2023, Krzysztof Kozlowski wrote:
>> On 02/05/2023 08:32, Arslanbenzer, Zeynep wrote:
>>> On Wed, 22 Mar 2023, Krzysztof Kozlowski wrote:
>>>> On 22/03/2023 06:56, Zeynep Arslanbenzer wrote:
>>>>> Regulator driver for ADI MAX77643/MAX77654/MAX77658/MAX77659.
>>>>>
>>>>> MAX77643/MAX77659 has 1 LDO regulator.
>>>>> MAX77654/MAX77658 has two LDO regulators.
>>>>>
>>>>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
>>>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>>>>
>>>>
>>>>
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct platform_device_id max77658_regulator_id[] = {
>>>>> +	{ "max77643-regulator" },
>>>>> +	{ "max77654-regulator" },
>>>>> +	{ "max77658-regulator" },
>>>>> +	{ "max77659-regulator" },
>>>>
>>>> Why do you need so many entries? They do not differ.
>>>
>>> They are slightly different. Just MAX77659 and MAX77643 regulators have 
>>> exactly the same features. MAX77659 and MAX77643 have 1 LDO regulator but 
>>> others have 2 and the voltage base of the MAX77654 regulators is different 
>>> from others. Should I use the same entry for the MAX77643 and MAX77659?
>>
>> Your driver does not choose regulators based on these compatibles. Your
>> of_device_id table claims all devices are fully compatible and do not
>> differ from regulators point of view. If they are different, you should
>> encode the difference. If not, use only one entry in of_device_id (only
>> of_device_id, not bindings).
> 
> I used id table matching and I did not use of_device_id table. Should I use 
> OF style match instead?

My comment stands regardless which device ID table you use. It's the
same mechanism.

Best regards,
Krzysztof