[PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers

devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers
@ 2022-12-20 13:22 Zeynep Arslanbenzer
  2022-12-20 13:22 ` [PATCH 1/6] drivers: mfd: add MAX77659 PMIC support Zeynep Arslanbenzer
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Zeynep Arslanbenzer @ 2022-12-20 13:22 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, sre, lgirdwood, broonie
  Cc: Zeynep.Arslanbenzer, Nurettin.Bolucu, linux-kernel, devicetree,
	linux-pm

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

Zeynep Arslanbenzer (6):
  drivers: mfd: add MAX77659 PMIC support
  dt-bindings: mfd: add MAX77659 binding
  drivers: power: supply: add MAX77659 charger support
  dt-bindings: power: supply: add MAX77659 charger binding
  drivers: regulator: add MAX77659 regulator support
  dt-bindings: regulator: add MAX77659 regulator binding

 .../devicetree/bindings/mfd/adi,max77659.yaml |  70 +++
 .../power/supply/adi,max77659-charger.yaml    |  42 ++
 .../regulator/adi,max77659-regulator.yaml     |  31 +
 MAINTAINERS                                   |  13 +
 drivers/mfd/Kconfig                           |  14 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/max77659.c                        | 188 ++++++
 drivers/power/supply/Kconfig                  |   7 +
 drivers/power/supply/Makefile                 |   1 +
 drivers/power/supply/max77659-charger.c       | 547 ++++++++++++++++++
 drivers/regulator/Kconfig                     |   8 +
 drivers/regulator/Makefile                    |   1 +
 drivers/regulator/max77659-regulator.c        |  98 ++++
 include/linux/mfd/max77659.h                  |  60 ++
 14 files changed, 1081 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77659.yaml
 create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
 create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml
 create mode 100644 drivers/mfd/max77659.c
 create mode 100644 drivers/power/supply/max77659-charger.c
 create mode 100644 drivers/regulator/max77659-regulator.c
 create mode 100644 include/linux/mfd/max77659.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/6] drivers: mfd: add MAX77659 PMIC support
  2022-12-20 13:22 [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers Zeynep Arslanbenzer
@ 2022-12-20 13:22 ` Zeynep Arslanbenzer
  2022-12-20 14:47   ` Krzysztof Kozlowski
  2022-12-20 13:22 ` [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding Zeynep Arslanbenzer
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Zeynep Arslanbenzer @ 2022-12-20 13:22 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, sre, lgirdwood, broonie
  Cc: Zeynep.Arslanbenzer, Nurettin.Bolucu, linux-kernel, devicetree,
	linux-pm

This patch adds MFD driver for MAX77659 to enable its sub
devices.

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

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 MAINTAINERS                  |   8 ++
 drivers/mfd/Kconfig          |  14 +++
 drivers/mfd/Makefile         |   1 +
 drivers/mfd/max77659.c       | 188 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/max77659.h |  60 +++++++++++
 5 files changed, 271 insertions(+)
 create mode 100644 drivers/mfd/max77659.c
 create mode 100644 include/linux/mfd/max77659.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a608f19da3a9..45a8a471c7c0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12640,6 +12640,14 @@ F:	drivers/power/supply/max77650-charger.c
 F:	drivers/regulator/max77650-regulator.c
 F:	include/linux/mfd/max77650.h
 
+ANALOG DEVICES MAX77659 PMIC MFD DRIVER
+M:	Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+M:	Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/mfd/max77659.c
+F:	include/linux/mfd/max77659.h
+
 MAXIM MAX77714 PMIC MFD DRIVER
 M:	Luca Ceresoli <luca@lucaceresoli.net>
 S:	Maintained
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b93856de432..4d5e446edfa5 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -821,6 +821,20 @@ config MFD_MAX77650
 	  the following functionalities of the device: GPIO, regulator,
 	  charger, LED, onkey.
 
+config MFD_MAX77659
+	tristate "Analog Devices 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 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..f7b39bf4aacb 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_MAX77659)	+= max77659.o
 obj-$(CONFIG_MFD_MAX77686)	+= max77686.o
 obj-$(CONFIG_MFD_MAX77693)	+= max77693.o
 obj-$(CONFIG_MFD_MAX77714)	+= max77714.o
diff --git a/drivers/mfd/max77659.c b/drivers/mfd/max77659.c
new file mode 100644
index 000000000000..8401aea20e8c
--- /dev/null
+++ b/drivers/mfd/max77659.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max77659.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+
+static const struct regmap_config max77659_regmap_config = {
+	.reg_bits   = 8,
+	.val_bits   = 8,
+};
+
+static const struct regmap_irq max77659_glbl0_irqs[] = {
+	{ .mask = MAX77659_BIT_INT_GLBL0_GPIO0_F, },
+	{ .mask = MAX77659_BIT_INT_GLBL0_GPIO0_R, },
+	{ .mask = MAX77659_BIT_INT_GLBL0_EN_F, },
+	{ .mask = MAX77659_BIT_INT_GLBL0_EN_R, },
+	{ .mask = MAX77659_BIT_INT_GLBL0_TJAL1_R, },
+	{ .mask = MAX77659_BIT_INT_GLBL0_TJAL2_R, },
+	{ .mask = MAX77659_BIT_INT_GLBL0_DOD0_R, },
+};
+
+static const struct regmap_irq_chip max77659_glbl0_irq_chip = {
+	.name           = "max77659_glbl0",
+	.status_base    = MAX77659_REG_INT_GLBL0,
+	.mask_base      = MAX77659_REG_INTM_GLBL0,
+	.num_regs       = 1,
+	.irqs           = max77659_glbl0_irqs,
+	.num_irqs       = ARRAY_SIZE(max77659_glbl0_irqs),
+};
+
+static const struct regmap_irq max77659_glbl1_irqs[] = {
+	{ .mask = MAX77659_BIT_INT_GLBL1_GPI1_F, },
+	{ .mask = MAX77659_BIT_INT_GLBL1_GPI1_R, },
+	{ .mask = MAX77659_BIT_INT_GLBL1_SBB_TO, },
+	{ .mask = MAX77659_BIT_INT_GLBL1_LDO0_F, },
+};
+
+static const struct regmap_irq_chip max77659_glbl1_irq_chip = {
+	.name           = "max77659_glbl1",
+	.status_base    = MAX77659_REG_INT_GLBL1,
+	.mask_base      = MAX77659_REG_INTM_GLBL1,
+	.num_regs       = 1,
+	.irqs           = max77659_glbl1_irqs,
+	.num_irqs       = ARRAY_SIZE(max77659_glbl1_irqs),
+};
+
+static const struct regmap_irq max77659_chg_irqs[] = {
+	{ .mask = MAX77659_BIT_INT_THM, },
+	{ .mask = MAX77659_BIT_INT_CHG, },
+	{ .mask = MAX77659_BIT_INT_CHGIN, },
+	{ .mask = MAX77659_BIT_INT_TJ_REG, },
+	{ .mask = MAX77659_BIT_INT_SYS_CTRL, },
+};
+
+static const struct regmap_irq_chip max77659_chg_irq_chip = {
+	.name = "max77659_chg",
+	.status_base = MAX77659_REG_INT_CHG,
+	.mask_base = MAX77659_REG_INT_M_CHG,
+	.num_regs = 1,
+	.irqs = max77659_chg_irqs,
+	.num_irqs = ARRAY_SIZE(max77659_chg_irqs),
+};
+
+static const struct mfd_cell max77659_devs[] = {
+	MFD_CELL_OF(MAX77659_REGULATOR_NAME, NULL, NULL, 0, 0, "adi,max77659-regulator"),
+	MFD_CELL_OF(MAX77659_CHARGER_NAME, NULL, NULL, 0, 0, "adi,max77659-charger"),
+};
+
+static int max77659_pmic_irq_init(struct max77659_dev *me)
+{
+	struct device *dev = me->dev;
+	int ret;
+
+	ret = regmap_write(me->regmap, MAX77659_REG_INTM_GLBL0, MAX77659_INT_GLBL0_MASK);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Unable to write Global0 Interrupt Masking register\n");
+
+	ret = regmap_write(me->regmap, MAX77659_REG_INTM_GLBL1, MAX77659_INT_GLBL1_MASK);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Unable to write Global1 Interrupt Masking register\n");
+
+	ret = devm_regmap_add_irq_chip(dev, me->regmap, me->irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77659_glbl0_irq_chip, &me->irqc_glbl0);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add global0 IRQ chip\n");
+
+	ret = devm_regmap_add_irq_chip(dev, me->regmap, me->irq,
+				       IRQF_ONESHOT | IRQF_SHARED, 0,
+				       &max77659_glbl1_irq_chip, &me->irqc_glbl1);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add global1 IRQ chip\n");
+
+	return devm_regmap_add_irq_chip(dev, me->regmap, me->irq,
+					IRQF_ONESHOT | IRQF_SHARED, 0,
+					&max77659_chg_irq_chip, &me->irqc_chg);
+}
+
+static int max77659_pmic_setup(struct max77659_dev *me)
+{
+	struct device *dev = me->dev;
+	unsigned int val;
+	int ret;
+
+	ret = max77659_pmic_irq_init(me);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to initialize IRQ\n");
+
+	ret = regmap_read(me->regmap, MAX77659_REG_INT_GLBL0, &val);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Unable to read Global0 Interrupt Status register\n");
+
+	ret = regmap_read(me->regmap, MAX77659_REG_INT_GLBL1, &val);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Unable to read Global1 Interrupt Status register\n");
+
+	ret = regmap_read(me->regmap, MAX77659_REG_INT_CHG, &val);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Unable to read Charger Interrupt Status register\n");
+	ret = device_init_wakeup(dev, true);
+	if (ret)
+		return dev_err_probe(dev, ret, "Unable to init wakeup\n");
+
+	ret = devm_mfd_add_devices(dev, -1, max77659_devs, ARRAY_SIZE(max77659_devs),
+				   NULL, 0, NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add sub-devices\n");
+
+	enable_irq_wake(me->irq);
+
+	return 0;
+}
+
+static int max77659_i2c_probe(struct i2c_client *client)
+{
+	struct max77659_dev *me;
+
+	me = devm_kzalloc(&client->dev, sizeof(*me), GFP_KERNEL);
+	if (!me)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, me);
+	me->dev = &client->dev;
+	me->irq = client->irq;
+
+	me->regmap = devm_regmap_init_i2c(client, &max77659_regmap_config);
+
+	if (IS_ERR(me->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(me->regmap),
+				     "Failed to allocate register map\n");
+
+	return max77659_pmic_setup(me);
+}
+
+static const struct of_device_id max77659_of_id[] = {
+	{ .compatible = "adi,max77659" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max77659_of_id);
+
+static const struct i2c_device_id max77659_i2c_id[] = {
+	{ MAX77659_NAME, 0 },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, max77659_i2c_id);
+
+static struct i2c_driver max77659_i2c_driver = {
+	.driver = {
+		.name = MAX77659_NAME,
+		.of_match_table = of_match_ptr(max77659_of_id),
+	},
+	.probe_new = max77659_i2c_probe,
+	.id_table = max77659_i2c_id,
+};
+
+module_i2c_driver(max77659_i2c_driver);
+
+MODULE_DESCRIPTION("max77659 MFD Driver");
+MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
diff --git a/include/linux/mfd/max77659.h b/include/linux/mfd/max77659.h
new file mode 100644
index 000000000000..ef781e6e54c2
--- /dev/null
+++ b/include/linux/mfd/max77659.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __MAX77659_MFD_H__
+#define __MAX77659_MFD_H__
+
+#include <linux/bits.h>
+
+#define MAX77659_NAME "max77659"
+
+#define MAX77659_REGULATOR_NAME MAX77659_NAME "-regulator"
+#define MAX77659_CHARGER_NAME MAX77659_NAME "-charger"
+
+#define MAX77659_REG_INT_GLBL0 0x00
+#define MAX77659_REG_INT_CHG 0x01
+#define MAX77659_REG_INT_GLBL1 0x04
+#define MAX77659_REG_INT_M_CHG 0x07
+#define MAX77659_REG_INTM_GLBL1 0x08
+#define MAX77659_REG_INTM_GLBL0 0x09
+
+#define MAX77659_INT_GLBL0_MASK 0xFF
+#define MAX77659_INT_GLBL1_MASK 0x33
+
+#define MAX77659_BIT_INT_GLBL0_GPIO0_F BIT(0)
+#define MAX77659_BIT_INT_GLBL0_GPIO0_R BIT(1)
+#define MAX77659_BIT_INT_GLBL0_EN_F BIT(2)
+#define MAX77659_BIT_INT_GLBL0_EN_R BIT(3)
+#define MAX77659_BIT_INT_GLBL0_TJAL1_R BIT(4)
+#define MAX77659_BIT_INT_GLBL0_TJAL2_R BIT(5)
+#define MAX77659_BIT_INT_GLBL0_DOD0_R BIT(7)
+
+#define MAX77659_BIT_INT_GLBL1_GPI1_F BIT(0)
+#define MAX77659_BIT_INT_GLBL1_GPI1_R BIT(1)
+#define MAX77659_BIT_INT_GLBL1_SBB_TO BIT(4)
+#define MAX77659_BIT_INT_GLBL1_LDO0_F BIT(5)
+
+#define MAX77659_BIT_INT_THM BIT(0)
+#define MAX77659_BIT_INT_CHG BIT(1)
+#define MAX77659_BIT_INT_CHGIN BIT(2)
+#define MAX77659_BIT_INT_TJ_REG BIT(3)
+#define MAX77659_BIT_INT_SYS_CTRL BIT(4)
+
+enum {
+	MAX77659_DEV_PMIC,
+	MAX77659_DEV_CHARGER,
+	MAX77659_DEV_REGULATOR,
+	MAX77659_NUM_DEVS,
+};
+
+struct max77659_dev {
+	struct device *dev;
+
+	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;
+};
+
+#endif /* __MAX77659_MFD_H__ */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding
  2022-12-20 13:22 [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers Zeynep Arslanbenzer
  2022-12-20 13:22 ` [PATCH 1/6] drivers: mfd: add MAX77659 PMIC support Zeynep Arslanbenzer
@ 2022-12-20 13:22 ` Zeynep Arslanbenzer
  2022-12-20 14:43   ` Krzysztof Kozlowski
  2022-12-20 16:40   ` Rob Herring
  2022-12-20 13:22 ` [PATCH 3/6] drivers: power: supply: add MAX77659 charger support Zeynep Arslanbenzer
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Zeynep Arslanbenzer @ 2022-12-20 13:22 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, sre, lgirdwood, broonie
  Cc: Zeynep.Arslanbenzer, Nurettin.Bolucu, linux-kernel, devicetree,
	linux-pm

This patch adds binding document for MAX77659 MFD driver.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 .../devicetree/bindings/mfd/adi,max77659.yaml | 70 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77659.yaml

diff --git a/Documentation/devicetree/bindings/mfd/adi,max77659.yaml b/Documentation/devicetree/bindings/mfd/adi,max77659.yaml
new file mode 100644
index 000000000000..6bec11607615
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/adi,max77659.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/adi,max77659.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MAX77659 SIMO PMIC from ADI.
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+description: |
+  MAX77659 is an PMIC providing battery charging and power
+  supply solutions for low-power applications.
+
+  For device-tree bindings of other sub-modules (regulator, power supply
+  refer to the binding documents under the respective
+  sub-system directories.
+
+properties:
+  compatible:
+    const: adi,max77659
+
+  reg:
+    description:
+      I2C device address.
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  charger:
+    $ref: ../power/supply/adi,max77659-charger.yaml
+
+  regulator:
+    $ref: ../regulator/adi,max77659-regulator.yaml
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pmic@40 {
+            compatible = "adi,max77659";
+            reg = <0x40>;
+            interrupt-parent = <&gpio>;
+            #interrupt-cells = <2>;
+            interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
+            regulator {
+                compatible = "adi,max77659-regulator";
+                regulator-boot-on;
+                regulator-always-on;
+            };
+            charger {
+                compatible = "adi,max77659-charger";
+                adi,fast-charge-timer   = <5>;
+                adi,fast-charge-microamp = <15000>;
+                adi,topoff-timer  =  <30>;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 45a8a471c7c0..e7a9cadf0ff2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12645,6 +12645,7 @@ M:	Nurettin Bolucu <Nurettin.Bolucu@analog.com>
 M:	Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
 F:	drivers/mfd/max77659.c
 F:	include/linux/mfd/max77659.h
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/6] drivers: power: supply: add MAX77659 charger support
  2022-12-20 13:22 [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers Zeynep Arslanbenzer
  2022-12-20 13:22 ` [PATCH 1/6] drivers: mfd: add MAX77659 PMIC support Zeynep Arslanbenzer
  2022-12-20 13:22 ` [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding Zeynep Arslanbenzer
@ 2022-12-20 13:22 ` Zeynep Arslanbenzer
  2022-12-20 14:51   ` Krzysztof Kozlowski
  2023-01-02 20:44   ` Sebastian Reichel
  2022-12-20 13:22 ` [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding Zeynep Arslanbenzer
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 18+ messages in thread
From: Zeynep Arslanbenzer @ 2022-12-20 13:22 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, sre, lgirdwood, broonie
  Cc: Zeynep.Arslanbenzer, Nurettin.Bolucu, linux-kernel, devicetree,
	linux-pm

This patch adds battery charger driver for MAX77659.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index e7a9cadf0ff2..b3e307163063 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12647,6 +12647,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
 F:	drivers/mfd/max77659.c
+F:	drivers/power/supply/max77659-charger.c
 F:	include/linux/mfd/max77659.h
 
 MAXIM MAX77714 PMIC MFD DRIVER
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 0bbfe6a7ce4d..d38ef40ae9ee 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_MAX77659
+	tristate "Analog Devices MAX77659 battery charger driver"
+	depends on MFD_MAX77659
+	help
+	  Say Y to enable support for the battery charger control of MAX77659
+	  PMICs.
+
 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..e8646896bcd2 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_MAX77659)	+= max77659-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/max77659-charger.c b/drivers/power/supply/max77659-charger.c
new file mode 100644
index 000000000000..dfdbd484f7cd
--- /dev/null
+++ b/drivers/power/supply/max77659-charger.c
@@ -0,0 +1,547 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/bitfield.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max77659.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+#define MAX77659_IRQ_WORK_DELAY 0
+#define MAX77659_CHARGER_CURRENT_MAX 300000
+#define MAX77659_CHARGER_CURRENT_MIN 7500
+#define MAX77659_CHARGER_CURRENT_STEP 7500
+#define MAX77659_CHARGER_TOPOFF_TIMER_STEP 5
+
+#define MAX77659_REG_STAT_CHG_A 0x02
+#define MAX77659_REG_STAT_CHG_B 0x03
+#define MAX77659_REG_CNFG_CHG_A 0x20
+#define MAX77659_REG_CNFG_CHG_B 0x21
+#define MAX77659_REG_CNFG_CHG_C 0x22
+#define MAX77659_REG_CNFG_CHG_D 0x23
+#define MAX77659_REG_CNFG_CHG_E 0x24
+#define MAX77659_REG_CNFG_CHG_F 0x25
+#define MAX77659_REG_CNFG_CHG_G 0x26
+#define MAX77659_REG_CNFG_CHG_H 0x27
+#define MAX77659_REG_CNFG_CHG_I 0x28
+
+#define MAX77659_BIT_STAT_A_VSYSY_MIN_STAT	BIT(4)
+#define MAX77659_BIT_STAT_A_TJ_REG_STAT		BIT(3)
+#define MAX77659_BIT_STAT_A_THM_DTLS		GENMASK(2, 0)
+#define MAX77659_BIT_STAT_B_CHG_DTLS		GENMASK(7, 4)
+#define MAX77659_BIT_STAT_B_CHGIN_DTSL		GENMASK(3, 2)
+#define MAX77659_BIT_STAT_B_CHG			BIT(1)
+#define MAX77659_BIT_CNFG_B_CHG_EN		BIT(0)
+#define MAX77659_BIT_CNFG_C_TOPOFFTIMER		GENMASK(2, 0)
+#define MAX77659_BIT_CNFG_E_CC			GENMASK(7, 2)
+#define MAX77659_BIT_CNFG_E_TFASTCHG		GENMASK(1, 0)
+#define MAX77659_BIT_CNFG_G_CHG_CV		GENMASK(7, 2)
+
+enum {
+	MAX77659_CHG_DTLS_OFF,
+	MAX77659_CHG_DTLS_PREQUAL,
+	MAX77659_CHG_DTLS_FASTCHARGE_CC,
+	MAX77659_CHG_DTLS_JEITA_FASTCHARGE_CC,
+	MAX77659_CHG_DTLS_FASTCHARGE_CV,
+	MAX77659_CHG_DTLS_JEITA_FASTCHARGE_CV,
+	MAX77659_CHG_DTLS_TOPOFF,
+	MAX77659_CHG_DTLS_JEITA_TOPOFF,
+	MAX77659_CHG_DTLS_DONE,
+	MAX77659_CHG_DTLS_JEITA_DONE,
+	MAX77659_CHG_DTLS_OFF_TIMER_FAULT,
+	MAX77659_CHG_DTLS_OFF_CHARGE_TIMER_FAULT,
+	MAX77659_CHG_DTLS_OFF_BATTERY_TEMP_FAULT,
+	MAX77659_CHG_DTLS_RESERVED_13,
+};
+
+enum {
+	MAX77659_CHG_IRQ_THM_I,
+	MAX77659_CHG_IRQ_CHG_I,
+	MAX77659_CHG_IRQ_CHGIN_I,
+	MAX77659_CHG_IRQ_TJ_REG_I,
+	MAX77659_CHG_IRQ_SYS_CTRL_I,
+
+	MAX77659_CHG_IRQ_MAX,
+};
+
+struct max77659_charger_dev {
+	struct device *dev;
+	struct max77659_dev *max77659;
+	struct regmap *regmap;
+
+	struct power_supply *psy_chg;
+	struct power_supply_desc psy_chg_d;
+
+	int irq;
+	int irq_arr[MAX77659_CHG_IRQ_MAX];
+	int irq_mask;
+
+	struct delayed_work irq_work;
+	/* mutex for charger operations*/
+	struct mutex lock;
+
+	int present;
+	int health;
+	int status;
+	int charge_type;
+
+	unsigned int fast_charge_current_ua;
+	unsigned int fast_charge_timer_hr;
+	unsigned int topoff_timer_min;
+};
+
+static char *chg_supplied_to[] = {
+	MAX77659_CHARGER_NAME,
+};
+
+static int max77659_set_charge_current(struct max77659_charger_dev *charger,
+				       int fast_charge_current_ua)
+{
+	unsigned int reg_data;
+
+	fast_charge_current_ua = clamp_val(fast_charge_current_ua, MAX77659_CHARGER_CURRENT_MIN,
+					   MAX77659_CHARGER_CURRENT_MAX);
+	reg_data = fast_charge_current_ua / MAX77659_CHARGER_CURRENT_STEP - 1;
+	reg_data = FIELD_PREP(MAX77659_BIT_CNFG_E_CC, reg_data);
+
+	return regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_E,
+				  MAX77659_BIT_CNFG_E_CC, reg_data);
+}
+
+static int max77659_get_charge_current(struct max77659_charger_dev *charger,
+				       int *get_current)
+{
+	struct regmap *regmap = charger->regmap;
+	unsigned int reg_data, current_val;
+	int ret;
+
+	ret = regmap_read(regmap, MAX77659_REG_CNFG_CHG_E, &reg_data);
+	if (ret)
+		return ret;
+
+	reg_data = FIELD_GET(MAX77659_BIT_CNFG_E_CC, reg_data);
+	current_val = (reg_data + 1) * MAX77659_CHARGER_CURRENT_STEP;
+
+	*get_current = clamp_val(current_val, MAX77659_CHARGER_CURRENT_MIN,
+				 MAX77659_CHARGER_CURRENT_MAX);
+
+	return 0;
+}
+
+static int max77659_set_topoff_timer(struct max77659_charger_dev *charger, int termination_time_min)
+{
+	unsigned int reg_data;
+
+	termination_time_min = clamp_val(termination_time_min, 0, MAX77659_BIT_CNFG_C_TOPOFFTIMER
+					 * MAX77659_CHARGER_TOPOFF_TIMER_STEP);
+	reg_data = FIELD_PREP(MAX77659_BIT_CNFG_C_TOPOFFTIMER,
+			      termination_time_min / MAX77659_CHARGER_TOPOFF_TIMER_STEP);
+
+	return regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_C,
+				  MAX77659_BIT_CNFG_C_TOPOFFTIMER, reg_data);
+}
+
+static int max77659_charger_initialize(struct max77659_charger_dev *charger)
+{
+	int ret;
+	unsigned int val;
+
+	val = (MAX77659_BIT_INT_SYS_CTRL | MAX77659_BIT_INT_TJ_REG | MAX77659_BIT_INT_CHGIN |
+	       MAX77659_BIT_INT_CHG | MAX77659_BIT_INT_THM);
+
+	ret = regmap_write(charger->regmap, MAX77659_REG_INT_M_CHG, val);
+	if (ret)
+		return dev_err_probe(charger->dev, ret, "Error in writing register INT_M_CHG\n");
+
+	ret = max77659_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, MAX77659_REG_CNFG_CHG_E,
+				 MAX77659_BIT_CNFG_E_TFASTCHG, val);
+	if (ret)
+		return dev_err_probe(charger->dev, ret, "Error in writing register CNFG_CHG_E\n");
+
+	return max77659_set_topoff_timer(charger, charger->topoff_timer_min);
+}
+
+static void max77659_charger_parse_dt(struct max77659_charger_dev *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 adi,fast-charge-timer DT property\n");
+
+	ret = device_property_read_u32(charger->dev, "adi,fast-charge-microamp",
+				       &charger->fast_charge_current_ua);
+	if (ret)
+		dev_dbg(charger->dev, "Could not read adi,fast-charge-microamp DT property\n");
+
+	ret = device_property_read_u32(charger->dev, "adi,topoff-timer",
+				       &charger->topoff_timer_min);
+	if (ret)
+		dev_dbg(charger->dev, "Could not read adi,topoff-timer DT property\n");
+}
+
+struct max77659_charger_status_map {
+	int health;
+	int status;
+	int charge_type;
+};
+
+#define STATUS_MAP(_MAX77659_CHG_DTLS, _health, _status, _charge_type) \
+	[MAX77659_CHG_DTLS_##_MAX77659_CHG_DTLS] = {\
+		.health = POWER_SUPPLY_HEALTH_##_health,\
+		.status = POWER_SUPPLY_STATUS_##_status,\
+		.charge_type = POWER_SUPPLY_CHARGE_TYPE_##_charge_type,\
+	}
+
+static struct max77659_charger_status_map max77659_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 max77659_charger_update(struct max77659_charger_dev *charger)
+{
+	unsigned int stat_chg_b, chg_dtls;
+	int ret;
+
+	ret = regmap_read(charger->regmap, MAX77659_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 & MAX77659_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(MAX77659_BIT_STAT_B_CHG_DTLS, stat_chg_b);
+
+	charger->health = max77659_charger_status_map[chg_dtls].health;
+	charger->status = max77659_charger_status_map[chg_dtls].status;
+	charger->charge_type = max77659_charger_status_map[chg_dtls].charge_type;
+
+	return 0;
+}
+
+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 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 max77659_charger_get_property(struct power_supply *psy,
+					 enum power_supply_property psp,
+					 union power_supply_propval *val)
+{
+	struct max77659_charger_dev *charger = power_supply_get_drvdata(psy);
+	int ret;
+
+	mutex_lock(&charger->lock);
+
+	ret = max77659_charger_update(charger);
+	if (ret)
+		goto out;
+
+	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 = max77659_get_charge_current(charger, &val->intval);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+out:
+	mutex_unlock(&charger->lock);
+
+	return ret;
+}
+
+static int max77659_charger_set_property(struct power_supply *psy, enum power_supply_property psp,
+					 const union power_supply_propval *val)
+{
+	struct max77659_charger_dev *charger = power_supply_get_drvdata(psy);
+	int ret;
+
+	mutex_lock(&charger->lock);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_B,
+					 MAX77659_BIT_CNFG_B_CHG_EN, !!val->intval);
+		if (ret)
+			goto out;
+
+		ret = max77659_set_charge_current(charger, charger->fast_charge_current_ua);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		/* val->intval - uA */
+		ret = max77659_set_charge_current(charger, val->intval);
+		if (ret)
+			goto out;
+
+		charger->fast_charge_current_ua = val->intval;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+out:
+	mutex_unlock(&charger->lock);
+
+	return ret;
+}
+
+static void max77659_charger_irq_handler(struct max77659_charger_dev *charger, int irq)
+{
+	unsigned int val, stat_chg_b;
+	int chg_present;
+	int ret;
+
+	switch (irq) {
+	case MAX77659_CHG_IRQ_THM_I:
+		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_A, &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77659_BIT_STAT_A_THM_DTLS, val);
+		dev_dbg(charger->dev, "CHG_INT_THM: thm_dtls = %02Xh\n", val);
+		break;
+
+	case MAX77659_CHG_IRQ_CHG_I:
+		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_B, &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_B\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77659_BIT_STAT_B_CHG_DTLS, val);
+		dev_dbg(charger->dev, "CHG_INT_CHG: MAX77659_CHG_DTLS = %02Xh\n", val);
+		break;
+
+	case MAX77659_CHG_IRQ_CHGIN_I:
+		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_B, &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_B\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77659_BIT_STAT_B_CHG_DTLS, val);
+		dev_dbg(charger->dev, "CHG_INT_CHG: MAX77659_CHG_DTLS = %02Xh\n", val);
+
+		ret = regmap_read(charger->regmap, MAX77659_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 & MAX77659_BIT_STAT_B_CHG;
+
+		regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_B,
+				   MAX77659_BIT_CNFG_B_CHG_EN, !!chg_present);
+		if (!chg_present)
+			max77659_set_charge_current(charger, charger->fast_charge_current_ua);
+
+		dev_dbg(charger->dev, "CHG_INT_CHGIN: Charger input %s\n",
+			chg_present ? "inserted" : "removed");
+		break;
+
+	case MAX77659_CHG_IRQ_TJ_REG_I:
+		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_A, &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77659_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 MAX77659_CHG_IRQ_SYS_CTRL_I:
+		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_A, &val);
+		if (ret) {
+			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
+			return;
+		}
+
+		val = FIELD_GET(MAX77659_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;
+
+	default:
+		break;
+	}
+
+	power_supply_changed(charger->psy_chg);
+}
+
+static irqreturn_t max77659_charger_isr(int irq, void *data)
+{
+	struct max77659_charger_dev *charger = data;
+
+	charger->irq = irq;
+	max77659_charger_update(charger);
+	max77659_charger_irq_handler(charger, charger->irq - charger->irq_arr[0]);
+
+	return IRQ_HANDLED;
+}
+
+static const char * const max77659_irq_descs[] = {
+	"charger-thm",
+	"charger-chg",
+	"charger-chgin",
+	"charger-tj-reg",
+	"charger-sys-ctrl",
+};
+
+static int max77659_charger_probe(struct platform_device *pdev)
+{
+	struct max77659_dev *max77659 = dev_get_drvdata(pdev->dev.parent);
+	struct max77659_charger_dev *charger;
+	struct device *dev = &pdev->dev;
+	int i, ret;
+	struct power_supply_config charger_cfg = {};
+
+	charger = devm_kzalloc(dev, sizeof(*charger), GFP_KERNEL);
+	if (!charger)
+		return -ENOMEM;
+
+	mutex_init(&charger->lock);
+
+	charger->dev = dev;
+	charger->regmap = dev_get_regmap(charger->dev->parent, NULL);
+	charger->fast_charge_current_ua = 15000;
+	charger->topoff_timer_min = 30;
+
+	max77659_charger_parse_dt(charger);
+
+	charger->psy_chg_d.name = MAX77659_CHARGER_NAME;
+	charger->psy_chg_d.type = POWER_SUPPLY_TYPE_UNKNOWN;
+	charger->psy_chg_d.get_property	= max77659_charger_get_property;
+	charger->psy_chg_d.set_property	= max77659_charger_set_property;
+	charger->psy_chg_d.properties = max77659_charger_props;
+	charger->psy_chg_d.property_is_writeable = max77659_property_is_writeable;
+	charger->psy_chg_d.num_properties = ARRAY_SIZE(max77659_charger_props);
+	charger_cfg.drv_data = charger;
+	charger_cfg.supplied_to = chg_supplied_to;
+	charger_cfg.of_node = dev->of_node;
+	charger_cfg.num_supplicants = ARRAY_SIZE(chg_supplied_to);
+
+	ret = max77659_charger_initialize(charger);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize charger\n");
+
+	for (i = 0; i < MAX77659_CHG_IRQ_MAX; i++) {
+		charger->irq_arr[i] = regmap_irq_get_virq(max77659->irqc_chg, i);
+
+		if (charger->irq_arr[i] < 0)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid IRQ for MAX77659_CHG_IRQ %d\n", i);
+
+		ret = devm_request_threaded_irq(dev, charger->irq_arr[i],
+						NULL, max77659_charger_isr, IRQF_TRIGGER_FALLING,
+						max77659_irq_descs[i], charger);
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to request irq: %d\n",
+					     charger->irq_arr[i]);
+	}
+
+	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");
+
+	return 0;
+}
+
+static const struct of_device_id max77659_charger_of_id[] = {
+	{ .compatible = "adi,max77659-charger" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max77659_charger_of_id);
+
+static const struct platform_device_id max77659_charger_id[] = {
+	{ MAX77659_CHARGER_NAME, 0, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, max77659_charger_id);
+
+static struct platform_driver max77659_charger_driver = {
+	.driver = {
+		.name = MAX77659_CHARGER_NAME,
+		.of_match_table = of_match_ptr(max77659_charger_of_id),
+	},
+	.probe = max77659_charger_probe,
+	.id_table = max77659_charger_id,
+};
+
+module_platform_driver(max77659_charger_driver);
+
+MODULE_DESCRIPTION("MAX77659 Charger Driver");
+MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
  2022-12-20 13:22 [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers Zeynep Arslanbenzer
                   ` (2 preceding siblings ...)
  2022-12-20 13:22 ` [PATCH 3/6] drivers: power: supply: add MAX77659 charger support Zeynep Arslanbenzer
@ 2022-12-20 13:22 ` Zeynep Arslanbenzer
  2022-12-20 14:54   ` Krzysztof Kozlowski
  2022-12-20 13:22 ` [PATCH 5/6] drivers: regulator: add MAX77659 regulator support Zeynep Arslanbenzer
  2022-12-20 13:22 ` [PATCH 6/6] dt-bindings: regulator: add MAX77659 regulator binding Zeynep Arslanbenzer
  5 siblings, 1 reply; 18+ messages in thread
From: Zeynep Arslanbenzer @ 2022-12-20 13:22 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, sre, lgirdwood, broonie
  Cc: Zeynep.Arslanbenzer, Nurettin.Bolucu, linux-kernel, devicetree,
	linux-pm

This patch adds device tree binding documentation for MAX77659 charger.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 .../power/supply/adi,max77659-charger.yaml    | 42 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
new file mode 100644
index 000000000000..24f8b5a2bd84
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
@@ -0,0 +1,42 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/adi,max77659-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Battery charger for MAX77659 PMIC from ADI.
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+description: |
+  This module is part of the MAX77659 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/adi,max77659.yaml.
+
+  The charger is represented as a sub-node of the PMIC node on the device tree.
+
+properties:
+  compatible:
+    const: adi,max77659-charger
+
+  reg:
+    maxItems: 1
+
+  adi,fast-charge-timer:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Fast-charge safety timer value (in hours).
+
+  adi,fast-charge-microamp:
+    description: Fast-charge constant current value.
+
+  adi,topoff-timer:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Top-Off timer value (in minutes).
+
+required:
+  - compatible
+
+additionalProperties: false
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index b3e307163063..5cb8fa452f2d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12646,6 +12646,7 @@ M:	Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
+F:	Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
 F:	drivers/mfd/max77659.c
 F:	drivers/power/supply/max77659-charger.c
 F:	include/linux/mfd/max77659.h
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/6] drivers: regulator: add MAX77659 regulator support
  2022-12-20 13:22 [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers Zeynep Arslanbenzer
                   ` (3 preceding siblings ...)
  2022-12-20 13:22 ` [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding Zeynep Arslanbenzer
@ 2022-12-20 13:22 ` Zeynep Arslanbenzer
  2022-12-20 13:22 ` [PATCH 6/6] dt-bindings: regulator: add MAX77659 regulator binding Zeynep Arslanbenzer
  5 siblings, 0 replies; 18+ messages in thread
From: Zeynep Arslanbenzer @ 2022-12-20 13:22 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, sre, lgirdwood, broonie
  Cc: Zeynep.Arslanbenzer, Nurettin.Bolucu, linux-kernel, devicetree,
	linux-pm

This patch adds regulator driver for MAX77659.

Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
---
 MAINTAINERS                            |  1 +
 drivers/regulator/Kconfig              |  8 +++
 drivers/regulator/Makefile             |  1 +
 drivers/regulator/max77659-regulator.c | 98 ++++++++++++++++++++++++++
 4 files changed, 108 insertions(+)
 create mode 100644 drivers/regulator/max77659-regulator.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 5cb8fa452f2d..13c062a8cda2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12649,6 +12649,7 @@ F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
 F:	Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
 F:	drivers/mfd/max77659.c
 F:	drivers/power/supply/max77659-charger.c
+F:	drivers/regulator/max77659-regulator.c
 F:	include/linux/mfd/max77659.h
 
 MAXIM MAX77714 PMIC MFD DRIVER
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 820c9a0788e5..4a9852c7050f 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_MAX77659
+	tristate "Analog Devices MAX77659 Regulator"
+	depends on MFD_MAX77659
+	help
+	  Regulator driver for MAX77659 PMIC from Analog Devices.
+	  This driver supports an LDO regulator.
+	  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..a7e91b56fb3f 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_MAX77659) += max77659-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/max77659-regulator.c b/drivers/regulator/max77659-regulator.c
new file mode 100644
index 000000000000..03a565013851
--- /dev/null
+++ b/drivers/regulator/max77659-regulator.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/mfd/max77659.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#define MAX77659_LDO_VOLT_REG_MAX 0x7F
+#define MAX77659_LDO_VOLT_N_RANGE 0x80
+#define MAX77659_LDO_VOLT_STEP 25000
+#define MAX77659_LDO_VOLT_BASE 500000
+
+#define MAX77659_REG_CNFG_LDO0_A 0x38
+#define MAX77659_REG_CNFG_LDO0_B 0x39
+
+#define MAX77659_BITS_CONFIG_LDO0_A_TV_LDO GENMASK(6, 0)
+#define MAX77659_BITS_CONFIG_LDO0_B_EN_LDO GENMASK(2, 0)
+
+/*
+ * 0.500 to 3.675V (25mV step)
+ */
+static const struct linear_range MAX77659_LDO_volts[] = {
+	REGULATOR_LINEAR_RANGE(MAX77659_LDO_VOLT_BASE, 0x00, MAX77659_LDO_VOLT_REG_MAX,
+			       MAX77659_LDO_VOLT_STEP),
+};
+
+static const struct regulator_ops max77659_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,
+};
+
+static struct regulator_desc max77659_LDO_desc = {
+	.name		 = "LDO",
+	.id		 = 0,
+	.regulators_node = of_match_ptr("regulator"),
+	.ops		 = &max77659_LDO_ops,
+	.type		 = REGULATOR_VOLTAGE,
+	.owner		 = THIS_MODULE,
+	.n_linear_ranges = MAX77659_LDO_VOLT_N_RANGE,
+	.linear_ranges	 = MAX77659_LDO_volts,
+	.vsel_reg	 = MAX77659_REG_CNFG_LDO0_A,
+	.vsel_mask	 = MAX77659_BITS_CONFIG_LDO0_A_TV_LDO,
+	.enable_reg	 = MAX77659_REG_CNFG_LDO0_B,
+	.enable_mask	 = MAX77659_BITS_CONFIG_LDO0_B_EN_LDO,
+	.enable_val	 = 0x06,
+	.disable_val	 = 0x04,
+};
+
+static int max77659_regulator_probe(struct platform_device *pdev)
+{
+	struct regulator_dev *rdev;
+	struct regulator_config config = {};
+
+	config.dev = &pdev->dev;
+
+	rdev = devm_regulator_register(&pdev->dev, &max77659_LDO_desc, &config);
+
+	if (IS_ERR(rdev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rdev),
+				     "Failed to register regulator %s\n", max77659_LDO_desc.name);
+
+	return 0;
+}
+
+static const struct of_device_id max77659_regulator_of_id[] = {
+	{ .compatible = "adi,max77659-regulator" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, max77659_regulator_of_id);
+
+static const struct platform_device_id max77659_regulator_id[] = {
+	{ MAX77659_REGULATOR_NAME, 0, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, max77659_regulator_id);
+
+static struct platform_driver max77659_regulator_driver = {
+	.driver = {
+		.name = MAX77659_REGULATOR_NAME,
+		.of_match_table = of_match_ptr(max77659_regulator_of_id),
+	},
+	.probe = max77659_regulator_probe,
+	.id_table = max77659_regulator_id,
+};
+
+module_platform_driver(max77659_regulator_driver);
+
+MODULE_DESCRIPTION("max77659 Regulator Driver");
+MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/6] dt-bindings: regulator: add MAX77659 regulator binding
  2022-12-20 13:22 [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers Zeynep Arslanbenzer
                   ` (4 preceding siblings ...)
  2022-12-20 13:22 ` [PATCH 5/6] drivers: regulator: add MAX77659 regulator support Zeynep Arslanbenzer
@ 2022-12-20 13:22 ` Zeynep Arslanbenzer
  2022-12-20 14:55   ` Krzysztof Kozlowski
  5 siblings, 1 reply; 18+ messages in thread
From: Zeynep Arslanbenzer @ 2022-12-20 13:22 UTC (permalink / raw)
  To: lee, robh+dt, krzysztof.kozlowski+dt, sre, lgirdwood, broonie
  Cc: Zeynep.Arslanbenzer, Nurettin.Bolucu, linux-kernel, devicetree,
	linux-pm

This patch adds device tree binding documentation for MAX77659 regulator.

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

diff --git a/Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml
new file mode 100644
index 000000000000..c1e2d88be25b
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/adi,max77659-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulator driver for MAX77659 PMIC from ADI.
+
+maintainers:
+  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
+  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
+
+description: |
+  This module is part of the MAX77659 MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/adi,max77659.yaml.
+
+  The regulator is represented as a sub-node of the PMIC node on the device tree.
+
+properties:
+  compatible:
+    const: adi,max77650-regulator
+
+  regulator-boot-on: true
+  regulator-always-on: true
+
+required:
+  - compatible
+
+additionalProperties: false
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 13c062a8cda2..a10fff677b0b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12647,6 +12647,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
 F:	Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
+F:	Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml
 F:	drivers/mfd/max77659.c
 F:	drivers/power/supply/max77659-charger.c
 F:	drivers/regulator/max77659-regulator.c
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding
  2022-12-20 13:22 ` [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding Zeynep Arslanbenzer
@ 2022-12-20 14:43   ` Krzysztof Kozlowski
  2022-12-20 16:40   ` Rob Herring
  1 sibling, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-20 14:43 UTC (permalink / raw)
  To: Zeynep Arslanbenzer, lee, robh+dt, krzysztof.kozlowski+dt, sre,
	lgirdwood, broonie
  Cc: Nurettin.Bolucu, linux-kernel, devicetree, linux-pm

On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
> This patch adds binding document for MAX77659 MFD driver.

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

2. Subject: drop second, redundant "binding".

> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77659.yaml | 70 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77659.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/adi,max77659.yaml b/Documentation/devicetree/bindings/mfd/adi,max77659.yaml
> new file mode 100644
> index 000000000000..6bec11607615
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/adi,max77659.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/adi,max77659.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MAX77659 SIMO PMIC from ADI.

Drop full stop.

> +
> +maintainers:
> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> +
> +description: |
> +  MAX77659 is an PMIC providing battery charging and power
> +  supply solutions for low-power applications.
> +
> +  For device-tree bindings of other sub-modules (regulator, power supply
> +  refer to the binding documents under the respective
> +  sub-system directories.

Drop this part, not really relevant.

> +
> +properties:
> +  compatible:
> +    const: adi,max77659
> +
> +  reg:
> +    description:
> +      I2C device address.

Drop description.

> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  charger:
> +    $ref: ../power/supply/adi,max77659-charger.yaml

Full path, so /schemas/

There is no such file so you did not test the patch.

> +
> +  regulator:
> +    $ref: ../regulator/adi,max77659-regulator.yaml

Ditto


There is no such file so you did not test the patch.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        pmic@40 {
> +            compatible = "adi,max77659";
> +            reg = <0x40>;
> +            interrupt-parent = <&gpio>;
> +            #interrupt-cells = <2>;
> +            interrupts = <16 IRQ_TYPE_EDGE_FALLING>;
> +            regulator {
> +                compatible = "adi,max77659-regulator";
> +                regulator-boot-on;
> +                regulator-always-on;
> +            };
> +            charger {
> +                compatible = "adi,max77659-charger";
> +                adi,fast-charge-timer   = <5>;

That's not DT coding style...

> +                adi,fast-charge-microamp = <15000>;
> +                adi,topoff-timer  =  <30>;

Same problem.


>  

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/6] drivers: mfd: add MAX77659 PMIC support
  2022-12-20 13:22 ` [PATCH 1/6] drivers: mfd: add MAX77659 PMIC support Zeynep Arslanbenzer
@ 2022-12-20 14:47   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-20 14:47 UTC (permalink / raw)
  To: Zeynep Arslanbenzer, lee, robh+dt, krzysztof.kozlowski+dt, sre,
	lgirdwood, broonie
  Cc: Nurettin.Bolucu, linux-kernel, devicetree, linux-pm

On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
> This patch adds MFD driver for MAX77659 to enable its sub
> devices.
> 
> The MAX77659 is a multi-function devices. It includes
> charger and regulator
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  MAINTAINERS                  |   8 ++
>  drivers/mfd/Kconfig          |  14 +++
>  drivers/mfd/Makefile         |   1 +
>  drivers/mfd/max77659.c       | 188 +++++++++++++++++++++++++++++++++++
>  include/linux/mfd/max77659.h |  60 +++++++++++
>  5 files changed, 271 insertions(+)
>  create mode 100644 drivers/mfd/max77659.c
>  create mode 100644 include/linux/mfd/max77659.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a608f19da3a9..45a8a471c7c0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12640,6 +12640,14 @@ F:	drivers/power/supply/max77650-charger.c
>  F:	drivers/regulator/max77650-regulator.c
>  F:	include/linux/mfd/max77650.h


> +	ret = regmap_read(me->regmap, MAX77659_REG_INT_CHG, &val);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Unable to read Charger Interrupt Status register\n");
> +	ret = device_init_wakeup(dev, true);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Unable to init wakeup\n");
> +
> +	ret = devm_mfd_add_devices(dev, -1, max77659_devs, ARRAY_SIZE(max77659_devs),
> +				   NULL, 0, NULL);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add sub-devices\n");
> +
> +	enable_irq_wake(me->irq);

You do not allow a wakeup-source in DT, do you?

> +
> +	return 0;
> +}
> +
> +static int max77659_i2c_probe(struct i2c_client *client)
> +{
> +	struct max77659_dev *me;

Do you see other MFD driver calling this "me"? Please submit code which
is consistent with Linux style, not with your own.

> +
> +	me = devm_kzalloc(&client->dev, sizeof(*me), GFP_KERNEL);
> +	if (!me)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, me);
> +	me->dev = &client->dev;
> +	me->irq = client->irq;
> +
> +	me->regmap = devm_regmap_init_i2c(client, &max77659_regmap_config);
> +
> +	if (IS_ERR(me->regmap))
> +		return dev_err_probe(&client->dev, PTR_ERR(me->regmap),
> +				     "Failed to allocate register map\n");
> +
> +	return max77659_pmic_setup(me);
> +}
> +
> +static const struct of_device_id max77659_of_id[] = {
> +	{ .compatible = "adi,max77659" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max77659_of_id);
> +
> +static const struct i2c_device_id max77659_i2c_id[] = {
> +	{ MAX77659_NAME, 0 },

Nope. Use proper string.

> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max77659_i2c_id);
> +
> +static struct i2c_driver max77659_i2c_driver = {
> +	.driver = {
> +		.name = MAX77659_NAME,
> +		.of_match_table = of_match_ptr(max77659_of_id),

You will have warnings here, so the patch was not really compile tested.
Drop of_match_ptr() and check your code before sending (W=1, smatch,
sparse, coccinelle)

> +	},
> +	.probe_new = max77659_i2c_probe,
> +	.id_table = max77659_i2c_id,
> +};
> +
> +module_i2c_driver(max77659_i2c_driver);
> +
> +MODULE_DESCRIPTION("max77659 MFD Driver");
> +MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION("1.0");
> diff --git a/include/linux/mfd/max77659.h b/include/linux/mfd/max77659.h
> new file mode 100644
> index 000000000000..ef781e6e54c2
> --- /dev/null
> +++ b/include/linux/mfd/max77659.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef __MAX77659_MFD_H__
> +#define __MAX77659_MFD_H__

Header guard should be more descriptive (with pieces of path)

> +
> +#include <linux/bits.h>
> +
> +#define MAX77659_NAME "max77659"

Not really for include.

> +
> +#define MAX77659_REGULATOR_NAME MAX77659_NAME "-regulator"
> +#define MAX77659_CHARGER_NAME MAX77659_NAME "-charger"

Neither these.

> +
> +#define MAX77659_REG_INT_GLBL0 0x00
> +#define MAX77659_REG_INT_CHG 0x01
> +#define MAX77659_REG_INT_GLBL1 0x04
> +#define MAX77659_REG_INT_M_CHG 0x07
> +#define MAX77659_REG_INTM_GLBL1 0x08
> +#define MAX77659_REG_INTM_GLBL0 0x09

That's absolutely unreadable code.

> +
> +#define MAX77659_INT_GLBL0_MASK 0xFF
> +#define MAX77659_INT_GLBL1_MASK 0x33
> +
> +#define MAX77659_BIT_INT_GLBL0_GPIO0_F BIT(0)
> +#define MAX77659_BIT_INT_GLBL0_GPIO0_R BIT(1)
> +#define MAX77659_BIT_INT_GLBL0_EN_F BIT(2)
> +#define MAX77659_BIT_INT_GLBL0_EN_R BIT(3)
> +#define MAX77659_BIT_INT_GLBL0_TJAL1_R BIT(4)
> +#define MAX77659_BIT_INT_GLBL0_TJAL2_R BIT(5)
> +#define MAX77659_BIT_INT_GLBL0_DOD0_R BIT(7)
> +
> +#define MAX77659_BIT_INT_GLBL1_GPI1_F BIT(0)
> +#define MAX77659_BIT_INT_GLBL1_GPI1_R BIT(1)
> +#define MAX77659_BIT_INT_GLBL1_SBB_TO BIT(4)
> +#define MAX77659_BIT_INT_GLBL1_LDO0_F BIT(5)
> +
> +#define MAX77659_BIT_INT_THM BIT(0)
> +#define MAX77659_BIT_INT_CHG BIT(1)
> +#define MAX77659_BIT_INT_CHGIN BIT(2)
> +#define MAX77659_BIT_INT_TJ_REG BIT(3)
> +#define MAX77659_BIT_INT_SYS_CTRL BIT(4)
> +
> +enum {
> +	MAX77659_DEV_PMIC,
> +	MAX77659_DEV_CHARGER,
> +	MAX77659_DEV_REGULATOR,
> +	MAX77659_NUM_DEVS,
> +};
> +
> +struct max77659_dev {
> +	struct device *dev;
> +
> +	int irq;
> +	struct regmap_irq_chip_data *irqc_glbl0;
> +	struct regmap_irq_chip_data *irqc_glbl1;
> +	struct regmap_irq_chip_data	*irqc_chg;

Keep your code consistent.

> +
> +	struct regmap *regmap;
> +};
> +
> +#endif /* __MAX77659_MFD_H__ */

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/6] drivers: power: supply: add MAX77659 charger support
  2022-12-20 13:22 ` [PATCH 3/6] drivers: power: supply: add MAX77659 charger support Zeynep Arslanbenzer
@ 2022-12-20 14:51   ` Krzysztof Kozlowski
  2023-01-02 20:44   ` Sebastian Reichel
  1 sibling, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-20 14:51 UTC (permalink / raw)
  To: Zeynep Arslanbenzer, lee, robh+dt, krzysztof.kozlowski+dt, sre,
	lgirdwood, broonie
  Cc: Nurettin.Bolucu, linux-kernel, devicetree, linux-pm

On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
> This patch adds battery charger driver for MAX77659.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  MAINTAINERS                             |   1 +
>  drivers/power/supply/Kconfig            |   7 +
>  drivers/power/supply/Makefile           |   1 +
>  drivers/power/supply/max77659-charger.c | 547 ++++++++++++++++++++++++
>  4 files changed, 556 insertions(+)
>  create mode 100644 drivers/power/supply/max77659-charger.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e7a9cadf0ff2..b3e307163063 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12647,6 +12647,7 @@ L:	linux-kernel@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
>  F:	drivers/mfd/max77659.c
> +F:	drivers/power/supply/max77659-charger.c
>  F:	include/linux/mfd/max77659.h
>  
>  MAXIM MAX77714 PMIC MFD DRIVER
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 0bbfe6a7ce4d..d38ef40ae9ee 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_MAX77659
> +	tristate "Analog Devices MAX77659 battery charger driver"
> +	depends on MFD_MAX77659
> +	help
> +	  Say Y to enable support for the battery charger control of MAX77659
> +	  PMICs.
> +
>  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..e8646896bcd2 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_MAX77659)	+= max77659-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/max77659-charger.c b/drivers/power/supply/max77659-charger.c
> new file mode 100644
> index 000000000000..dfdbd484f7cd
> --- /dev/null
> +++ b/drivers/power/supply/max77659-charger.c
> @@ -0,0 +1,547 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/bitfield.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max77659.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77659_IRQ_WORK_DELAY 0
> +#define MAX77659_CHARGER_CURRENT_MAX 300000
> +#define MAX77659_CHARGER_CURRENT_MIN 7500
> +#define MAX77659_CHARGER_CURRENT_STEP 7500
> +#define MAX77659_CHARGER_TOPOFF_TIMER_STEP 5
> +
> +#define MAX77659_REG_STAT_CHG_A 0x02

Same problems as your other patch - unreadable code. Two different
coding styles...

> +#define MAX77659_REG_STAT_CHG_B 0x03
> +#define MAX77659_REG_CNFG_CHG_A 0x20
> +#define MAX77659_REG_CNFG_CHG_B 0x21
> +#define MAX77659_REG_CNFG_CHG_C 0x22
> +#define MAX77659_REG_CNFG_CHG_D 0x23
> +#define MAX77659_REG_CNFG_CHG_E 0x24
> +#define MAX77659_REG_CNFG_CHG_F 0x25
> +#define MAX77659_REG_CNFG_CHG_G 0x26
> +#define MAX77659_REG_CNFG_CHG_H 0x27
> +#define MAX77659_REG_CNFG_CHG_I 0x28
> +
> +#define MAX77659_BIT_STAT_A_VSYSY_MIN_STAT	BIT(4)
> +#define MAX77659_BIT_STAT_A_TJ_REG_STAT		BIT(3)
> +#define MAX77659_BIT_STAT_A_THM_DTLS		GENMASK(2, 0)
> +#define MAX77659_BIT_STAT_B_CHG_DTLS		GENMASK(7, 4)
> +#define MAX77659_BIT_STAT_B_CHGIN_DTSL		GENMASK(3, 2)
> +#define MAX77659_BIT_STAT_B_CHG			BIT(1)
> +#define MAX77659_BIT_CNFG_B_CHG_EN		BIT(0)
> +#define MAX77659_BIT_CNFG_C_TOPOFFTIMER		GENMASK(2, 0)
> +#define MAX77659_BIT_CNFG_E_CC			GENMASK(7, 2)
> +#define MAX77659_BIT_CNFG_E_TFASTCHG		GENMASK(1, 0)
> +#define MAX77659_BIT_CNFG_G_CHG_CV		GENMASK(7, 2)
> +

(...)

> +
> +	charger->psy_chg_d.name = MAX77659_CHARGER_NAME;
> +	charger->psy_chg_d.type = POWER_SUPPLY_TYPE_UNKNOWN;
> +	charger->psy_chg_d.get_property	= max77659_charger_get_property;
> +	charger->psy_chg_d.set_property	= max77659_charger_set_property;
> +	charger->psy_chg_d.properties = max77659_charger_props;
> +	charger->psy_chg_d.property_is_writeable = max77659_property_is_writeable;
> +	charger->psy_chg_d.num_properties = ARRAY_SIZE(max77659_charger_props);
> +	charger_cfg.drv_data = charger;
> +	charger_cfg.supplied_to = chg_supplied_to;
> +	charger_cfg.of_node = dev->of_node;
> +	charger_cfg.num_supplicants = ARRAY_SIZE(chg_supplied_to);
> +
> +	ret = max77659_charger_initialize(charger);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to initialize charger\n");
> +
> +	for (i = 0; i < MAX77659_CHG_IRQ_MAX; i++) {
> +		charger->irq_arr[i] = regmap_irq_get_virq(max77659->irqc_chg, i);
> +
> +		if (charger->irq_arr[i] < 0)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid IRQ for MAX77659_CHG_IRQ %d\n", i);
> +
> +		ret = devm_request_threaded_irq(dev, charger->irq_arr[i],
> +						NULL, max77659_charger_isr, IRQF_TRIGGER_FALLING,

This does not look like wrapped to Linux coding style (which is
explicitly set at 80).

> +						max77659_irq_descs[i], charger);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to request irq: %d\n",
> +					     charger->irq_arr[i]);
> +	}
> +
> +	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");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id max77659_charger_of_id[] = {
> +	{ .compatible = "adi,max77659-charger" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max77659_charger_of_id);
> +
> +static const struct platform_device_id max77659_charger_id[] = {
> +	{ MAX77659_CHARGER_NAME, 0, },

Same comment.

> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, max77659_charger_id);
> +
> +static struct platform_driver max77659_charger_driver = {
> +	.driver = {
> +		.name = MAX77659_CHARGER_NAME,
> +		.of_match_table = of_match_ptr(max77659_charger_of_id),

Same problem.

> +	},
> +	.probe = max77659_charger_probe,
> +	.id_table = max77659_charger_id,
> +};
> +
> +module_platform_driver(max77659_charger_driver);
> +
> +MODULE_DESCRIPTION("MAX77659 Charger Driver");
> +MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION("1.0");

Drop version. From all possible places.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
  2022-12-20 13:22 ` [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding Zeynep Arslanbenzer
@ 2022-12-20 14:54   ` Krzysztof Kozlowski
  2023-04-17 21:43     ` Arslanbenzer, Zeynep
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-20 14:54 UTC (permalink / raw)
  To: Zeynep Arslanbenzer, lee, robh+dt, krzysztof.kozlowski+dt, sre,
	lgirdwood, broonie
  Cc: Nurettin.Bolucu, linux-kernel, devicetree, linux-pm

On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
> This patch adds device tree binding documentation for MAX77659 charger.

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

> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../power/supply/adi,max77659-charger.yaml    | 42 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
> new file mode 100644
> index 000000000000..24f8b5a2bd84
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/adi,max77659-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Battery charger for MAX77659 PMIC from ADI.

Drop full stop.

> +
> +maintainers:
> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> +
> +description: |
> +  This module is part of the MAX77659 MFD device. For more details
> +  see Documentation/devicetree/bindings/mfd/adi,max77659.yaml.
> +
> +  The charger is represented as a sub-node of the PMIC node on the device tree.
> +
> +properties:
> +  compatible:
> +    const: adi,max77659-charger
> +
> +  reg:
> +    maxItems: 1
> +
> +  adi,fast-charge-timer:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Fast-charge safety timer value (in hours).

No, use suffixes for common units.

> +
> +  adi,fast-charge-microamp:
> +    description: Fast-charge constant current value.

> +
> +  adi,topoff-timer:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Top-Off timer value (in minutes).

No, use suffixes for common units.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/6] dt-bindings: regulator: add MAX77659 regulator binding
  2022-12-20 13:22 ` [PATCH 6/6] dt-bindings: regulator: add MAX77659 regulator binding Zeynep Arslanbenzer
@ 2022-12-20 14:55   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-12-20 14:55 UTC (permalink / raw)
  To: Zeynep Arslanbenzer, lee, robh+dt, krzysztof.kozlowski+dt, sre,
	lgirdwood, broonie
  Cc: Nurettin.Bolucu, linux-kernel, devicetree, linux-pm

On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
> This patch adds device tree binding documentation for MAX77659 regulator.

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

> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../regulator/adi,max77659-regulator.yaml     | 31 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml b/Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml
> new file mode 100644
> index 000000000000..c1e2d88be25b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/adi,max77659-regulator.yaml
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/adi,max77659-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Regulator driver for MAX77659 PMIC from ADI.

It's not a driver. Drop.
Also full stop

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

Missing reference to regulator schema.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding
  2022-12-20 13:22 ` [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding Zeynep Arslanbenzer
  2022-12-20 14:43   ` Krzysztof Kozlowski
@ 2022-12-20 16:40   ` Rob Herring
  1 sibling, 0 replies; 18+ messages in thread
From: Rob Herring @ 2022-12-20 16:40 UTC (permalink / raw)
  To: Zeynep Arslanbenzer
  Cc: lgirdwood, linux-pm, broonie, lee, Nurettin.Bolucu, robh+dt,
	linux-kernel, krzysztof.kozlowski+dt, sre, devicetree


On Tue, 20 Dec 2022 16:22:46 +0300, Zeynep Arslanbenzer wrote:
> This patch adds binding document for MAX77659 MFD driver.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---
>  .../devicetree/bindings/mfd/adi,max77659.yaml | 70 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/adi,max77659.yaml
> 

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/mfd/adi,max77659.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/power/supply/adi,max77659-charger.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77659.example.dtb: pmic@40: charger: False schema does not allow {'compatible': ['adi,max77659-charger'], 'adi,fast-charge-timer': [[5]], 'adi,fast-charge-microamp': [[15000]], 'adi,topoff-timer': [[30]]}
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77659.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77659.example.dtb: pmic@40: regulator: False schema does not allow {'compatible': ['adi,max77659-regulator'], 'regulator-boot-on': True, 'regulator-always-on': True}
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77659.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77659.example.dtb: pmic@40: '#interrupt-cells' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/adi,max77659.yaml
Documentation/devicetree/bindings/mfd/adi,max77659.example.dtb:0:0: /example-0/i2c/pmic@40/regulator: failed to match any schema with compatible: ['adi,max77659-regulator']
Documentation/devicetree/bindings/mfd/adi,max77659.example.dtb:0:0: /example-0/i2c/pmic@40/charger: failed to match any schema with compatible: ['adi,max77659-charger']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221220132250.19383-3-Zeynep.Arslanbenzer@analog.com

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

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

pip3 install dtschema --upgrade

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/6] drivers: power: supply: add MAX77659 charger support
  2022-12-20 13:22 ` [PATCH 3/6] drivers: power: supply: add MAX77659 charger support Zeynep Arslanbenzer
  2022-12-20 14:51   ` Krzysztof Kozlowski
@ 2023-01-02 20:44   ` Sebastian Reichel
  1 sibling, 0 replies; 18+ messages in thread
From: Sebastian Reichel @ 2023-01-02 20:44 UTC (permalink / raw)
  To: Zeynep Arslanbenzer
  Cc: lee, robh+dt, krzysztof.kozlowski+dt, lgirdwood, broonie,
	Nurettin.Bolucu, linux-kernel, devicetree, linux-pm

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

Hi,

On Tue, Dec 20, 2022 at 04:22:47PM +0300, Zeynep Arslanbenzer wrote:
> This patch adds battery charger driver for MAX77659.
> 
> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> ---

Some additional feedback.

>  MAINTAINERS                             |   1 +
>  drivers/power/supply/Kconfig            |   7 +
>  drivers/power/supply/Makefile           |   1 +
>  drivers/power/supply/max77659-charger.c | 547 ++++++++++++++++++++++++
>  4 files changed, 556 insertions(+)
>  create mode 100644 drivers/power/supply/max77659-charger.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e7a9cadf0ff2..b3e307163063 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12647,6 +12647,7 @@ L:	linux-kernel@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/mfd/adi,max77659.yaml
>  F:	drivers/mfd/max77659.c
> +F:	drivers/power/supply/max77659-charger.c
>  F:	include/linux/mfd/max77659.h
>  
>  MAXIM MAX77714 PMIC MFD DRIVER
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 0bbfe6a7ce4d..d38ef40ae9ee 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_MAX77659
> +	tristate "Analog Devices MAX77659 battery charger driver"
> +	depends on MFD_MAX77659
> +	help
> +	  Say Y to enable support for the battery charger control of MAX77659
> +	  PMICs.
> +
>  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..e8646896bcd2 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_MAX77659)	+= max77659-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/max77659-charger.c b/drivers/power/supply/max77659-charger.c
> new file mode 100644
> index 000000000000..dfdbd484f7cd
> --- /dev/null
> +++ b/drivers/power/supply/max77659-charger.c
> @@ -0,0 +1,547 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/bitfield.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/max77659.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77659_IRQ_WORK_DELAY 0
> +#define MAX77659_CHARGER_CURRENT_MAX 300000
> +#define MAX77659_CHARGER_CURRENT_MIN 7500
> +#define MAX77659_CHARGER_CURRENT_STEP 7500
> +#define MAX77659_CHARGER_TOPOFF_TIMER_STEP 5
> +
> +#define MAX77659_REG_STAT_CHG_A 0x02
> +#define MAX77659_REG_STAT_CHG_B 0x03
> +#define MAX77659_REG_CNFG_CHG_A 0x20
> +#define MAX77659_REG_CNFG_CHG_B 0x21
> +#define MAX77659_REG_CNFG_CHG_C 0x22
> +#define MAX77659_REG_CNFG_CHG_D 0x23
> +#define MAX77659_REG_CNFG_CHG_E 0x24
> +#define MAX77659_REG_CNFG_CHG_F 0x25
> +#define MAX77659_REG_CNFG_CHG_G 0x26
> +#define MAX77659_REG_CNFG_CHG_H 0x27
> +#define MAX77659_REG_CNFG_CHG_I 0x28
> +
> +#define MAX77659_BIT_STAT_A_VSYSY_MIN_STAT	BIT(4)
> +#define MAX77659_BIT_STAT_A_TJ_REG_STAT		BIT(3)
> +#define MAX77659_BIT_STAT_A_THM_DTLS		GENMASK(2, 0)
> +#define MAX77659_BIT_STAT_B_CHG_DTLS		GENMASK(7, 4)
> +#define MAX77659_BIT_STAT_B_CHGIN_DTSL		GENMASK(3, 2)
> +#define MAX77659_BIT_STAT_B_CHG			BIT(1)
> +#define MAX77659_BIT_CNFG_B_CHG_EN		BIT(0)
> +#define MAX77659_BIT_CNFG_C_TOPOFFTIMER		GENMASK(2, 0)
> +#define MAX77659_BIT_CNFG_E_CC			GENMASK(7, 2)
> +#define MAX77659_BIT_CNFG_E_TFASTCHG		GENMASK(1, 0)
> +#define MAX77659_BIT_CNFG_G_CHG_CV		GENMASK(7, 2)
> +
> +enum {
> +	MAX77659_CHG_DTLS_OFF,
> +	MAX77659_CHG_DTLS_PREQUAL,
> +	MAX77659_CHG_DTLS_FASTCHARGE_CC,
> +	MAX77659_CHG_DTLS_JEITA_FASTCHARGE_CC,
> +	MAX77659_CHG_DTLS_FASTCHARGE_CV,
> +	MAX77659_CHG_DTLS_JEITA_FASTCHARGE_CV,
> +	MAX77659_CHG_DTLS_TOPOFF,
> +	MAX77659_CHG_DTLS_JEITA_TOPOFF,
> +	MAX77659_CHG_DTLS_DONE,
> +	MAX77659_CHG_DTLS_JEITA_DONE,
> +	MAX77659_CHG_DTLS_OFF_TIMER_FAULT,
> +	MAX77659_CHG_DTLS_OFF_CHARGE_TIMER_FAULT,
> +	MAX77659_CHG_DTLS_OFF_BATTERY_TEMP_FAULT,
> +	MAX77659_CHG_DTLS_RESERVED_13,
> +};
> +
> +enum {
> +	MAX77659_CHG_IRQ_THM_I,
> +	MAX77659_CHG_IRQ_CHG_I,
> +	MAX77659_CHG_IRQ_CHGIN_I,
> +	MAX77659_CHG_IRQ_TJ_REG_I,
> +	MAX77659_CHG_IRQ_SYS_CTRL_I,
> +
> +	MAX77659_CHG_IRQ_MAX,
> +};
> +
> +struct max77659_charger_dev {
> +	struct device *dev;
> +	struct max77659_dev *max77659;
> +	struct regmap *regmap;
> +
> +	struct power_supply *psy_chg;
> +	struct power_supply_desc psy_chg_d;
> +
> +	int irq;
> +	int irq_arr[MAX77659_CHG_IRQ_MAX];
> +	int irq_mask;
> +
> +	struct delayed_work irq_work;
> +	/* mutex for charger operations*/
> +	struct mutex lock;
> +
> +	int present;
> +	int health;
> +	int status;
> +	int charge_type;
> +
> +	unsigned int fast_charge_current_ua;
> +	unsigned int fast_charge_timer_hr;
> +	unsigned int topoff_timer_min;
> +};
> +
> +static char *chg_supplied_to[] = {
> +	MAX77659_CHARGER_NAME,
> +};
> +
> +static int max77659_set_charge_current(struct max77659_charger_dev *charger,
> +				       int fast_charge_current_ua)
> +{
> +	unsigned int reg_data;
> +
> +	fast_charge_current_ua = clamp_val(fast_charge_current_ua, MAX77659_CHARGER_CURRENT_MIN,
> +					   MAX77659_CHARGER_CURRENT_MAX);
> +	reg_data = fast_charge_current_ua / MAX77659_CHARGER_CURRENT_STEP - 1;
> +	reg_data = FIELD_PREP(MAX77659_BIT_CNFG_E_CC, reg_data);
> +
> +	return regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_E,
> +				  MAX77659_BIT_CNFG_E_CC, reg_data);
> +}
> +
> +static int max77659_get_charge_current(struct max77659_charger_dev *charger,
> +				       int *get_current)
> +{
> +	struct regmap *regmap = charger->regmap;
> +	unsigned int reg_data, current_val;
> +	int ret;
> +
> +	ret = regmap_read(regmap, MAX77659_REG_CNFG_CHG_E, &reg_data);
> +	if (ret)
> +		return ret;
> +
> +	reg_data = FIELD_GET(MAX77659_BIT_CNFG_E_CC, reg_data);
> +	current_val = (reg_data + 1) * MAX77659_CHARGER_CURRENT_STEP;
> +
> +	*get_current = clamp_val(current_val, MAX77659_CHARGER_CURRENT_MIN,
> +				 MAX77659_CHARGER_CURRENT_MAX);
> +
> +	return 0;
> +}
> +
> +static int max77659_set_topoff_timer(struct max77659_charger_dev *charger, int termination_time_min)
> +{
> +	unsigned int reg_data;
> +
> +	termination_time_min = clamp_val(termination_time_min, 0, MAX77659_BIT_CNFG_C_TOPOFFTIMER
> +					 * MAX77659_CHARGER_TOPOFF_TIMER_STEP);
> +	reg_data = FIELD_PREP(MAX77659_BIT_CNFG_C_TOPOFFTIMER,
> +			      termination_time_min / MAX77659_CHARGER_TOPOFF_TIMER_STEP);
> +
> +	return regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_C,
> +				  MAX77659_BIT_CNFG_C_TOPOFFTIMER, reg_data);
> +}
> +
> +static int max77659_charger_initialize(struct max77659_charger_dev *charger)
> +{
> +	int ret;
> +	unsigned int val;
> +
> +	val = (MAX77659_BIT_INT_SYS_CTRL | MAX77659_BIT_INT_TJ_REG | MAX77659_BIT_INT_CHGIN |
> +	       MAX77659_BIT_INT_CHG | MAX77659_BIT_INT_THM);
> +
> +	ret = regmap_write(charger->regmap, MAX77659_REG_INT_M_CHG, val);
> +	if (ret)
> +		return dev_err_probe(charger->dev, ret, "Error in writing register INT_M_CHG\n");
> +
> +	ret = max77659_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, MAX77659_REG_CNFG_CHG_E,
> +				 MAX77659_BIT_CNFG_E_TFASTCHG, val);
> +	if (ret)
> +		return dev_err_probe(charger->dev, ret, "Error in writing register CNFG_CHG_E\n");
> +
> +	return max77659_set_topoff_timer(charger, charger->topoff_timer_min);
> +}
> +
> +static void max77659_charger_parse_dt(struct max77659_charger_dev *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 adi,fast-charge-timer DT property\n");
> +
> +	ret = device_property_read_u32(charger->dev, "adi,fast-charge-microamp",
> +				       &charger->fast_charge_current_ua);
> +	if (ret)
> +		dev_dbg(charger->dev, "Could not read adi,fast-charge-microamp DT property\n");

As far as I can tell this is power_supply_battery_info.constant_charge_current_max_ua
from power_supply_get_battery_info().

> +	ret = device_property_read_u32(charger->dev, "adi,topoff-timer",
> +				       &charger->topoff_timer_min);
> +	if (ret)
> +		dev_dbg(charger->dev, "Could not read adi,topoff-timer DT property\n");
> +}
> +
> +struct max77659_charger_status_map {
> +	int health;
> +	int status;
> +	int charge_type;
> +};
> +
> +#define STATUS_MAP(_MAX77659_CHG_DTLS, _health, _status, _charge_type) \
> +	[MAX77659_CHG_DTLS_##_MAX77659_CHG_DTLS] = {\
> +		.health = POWER_SUPPLY_HEALTH_##_health,\
> +		.status = POWER_SUPPLY_STATUS_##_status,\
> +		.charge_type = POWER_SUPPLY_CHARGE_TYPE_##_charge_type,\
> +	}
> +
> +static struct max77659_charger_status_map max77659_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 max77659_charger_update(struct max77659_charger_dev *charger)
> +{
> +	unsigned int stat_chg_b, chg_dtls;
> +	int ret;
> +
> +	ret = regmap_read(charger->regmap, MAX77659_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 & MAX77659_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(MAX77659_BIT_STAT_B_CHG_DTLS, stat_chg_b);
> +
> +	charger->health = max77659_charger_status_map[chg_dtls].health;
> +	charger->status = max77659_charger_status_map[chg_dtls].status;
> +	charger->charge_type = max77659_charger_status_map[chg_dtls].charge_type;
> +
> +	return 0;
> +}
> +
> +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 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 max77659_charger_get_property(struct power_supply *psy,
> +					 enum power_supply_property psp,
> +					 union power_supply_propval *val)
> +{
> +	struct max77659_charger_dev *charger = power_supply_get_drvdata(psy);
> +	int ret;
> +
> +	mutex_lock(&charger->lock);
> +
> +	ret = max77659_charger_update(charger);
> +	if (ret)
> +		goto out;
> +
> +	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 = max77659_get_charge_current(charger, &val->intval);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +out:
> +	mutex_unlock(&charger->lock);
> +
> +	return ret;
> +}
> +
> +static int max77659_charger_set_property(struct power_supply *psy, enum power_supply_property psp,
> +					 const union power_supply_propval *val)
> +{
> +	struct max77659_charger_dev *charger = power_supply_get_drvdata(psy);
> +	int ret;
> +
> +	mutex_lock(&charger->lock);
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		ret = regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_B,
> +					 MAX77659_BIT_CNFG_B_CHG_EN, !!val->intval);
> +		if (ret)
> +			goto out;
> +
> +		ret = max77659_set_charge_current(charger, charger->fast_charge_current_ua);
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		/* val->intval - uA */
> +		ret = max77659_set_charge_current(charger, val->intval);
> +		if (ret)
> +			goto out;
> +
> +		charger->fast_charge_current_ua = val->intval;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +out:
> +	mutex_unlock(&charger->lock);
> +
> +	return ret;
> +}
> +
> +static void max77659_charger_irq_handler(struct max77659_charger_dev *charger, int irq)
> +{
> +	unsigned int val, stat_chg_b;
> +	int chg_present;
> +	int ret;
> +
> +	switch (irq) {
> +	case MAX77659_CHG_IRQ_THM_I:
> +		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_A, &val);
> +		if (ret) {
> +			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
> +			return;
> +		}
> +
> +		val = FIELD_GET(MAX77659_BIT_STAT_A_THM_DTLS, val);
> +		dev_dbg(charger->dev, "CHG_INT_THM: thm_dtls = %02Xh\n", val);
> +		break;
> +
> +	case MAX77659_CHG_IRQ_CHG_I:
> +		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_B, &val);
> +		if (ret) {
> +			dev_err(charger->dev, "Failed to read STAT_CHG_B\n");
> +			return;
> +		}
> +
> +		val = FIELD_GET(MAX77659_BIT_STAT_B_CHG_DTLS, val);
> +		dev_dbg(charger->dev, "CHG_INT_CHG: MAX77659_CHG_DTLS = %02Xh\n", val);
> +		break;
> +
> +	case MAX77659_CHG_IRQ_CHGIN_I:
> +		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_B, &val);
> +		if (ret) {
> +			dev_err(charger->dev, "Failed to read STAT_CHG_B\n");
> +			return;
> +		}
> +
> +		val = FIELD_GET(MAX77659_BIT_STAT_B_CHG_DTLS, val);
> +		dev_dbg(charger->dev, "CHG_INT_CHG: MAX77659_CHG_DTLS = %02Xh\n", val);
> +
> +		ret = regmap_read(charger->regmap, MAX77659_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 & MAX77659_BIT_STAT_B_CHG;
> +
> +		regmap_update_bits(charger->regmap, MAX77659_REG_CNFG_CHG_B,
> +				   MAX77659_BIT_CNFG_B_CHG_EN, !!chg_present);
> +		if (!chg_present)
> +			max77659_set_charge_current(charger, charger->fast_charge_current_ua);
> +
> +		dev_dbg(charger->dev, "CHG_INT_CHGIN: Charger input %s\n",
> +			chg_present ? "inserted" : "removed");
> +		break;
> +
> +	case MAX77659_CHG_IRQ_TJ_REG_I:
> +		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_A, &val);
> +		if (ret) {
> +			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
> +			return;
> +		}
> +
> +		val = FIELD_GET(MAX77659_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 MAX77659_CHG_IRQ_SYS_CTRL_I:
> +		ret = regmap_read(charger->regmap, MAX77659_REG_STAT_CHG_A, &val);
> +		if (ret) {
> +			dev_err(charger->dev, "Failed to read STAT_CHG_A\n");
> +			return;
> +		}
> +
> +		val = FIELD_GET(MAX77659_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;
> +
> +	default:
> +		break;
> +	}
> +
> +	power_supply_changed(charger->psy_chg);
> +}
> +
> +static irqreturn_t max77659_charger_isr(int irq, void *data)
> +{
> +	struct max77659_charger_dev *charger = data;
> +
> +	charger->irq = irq;
> +	max77659_charger_update(charger);
> +	max77659_charger_irq_handler(charger, charger->irq - charger->irq_arr[0]);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static const char * const max77659_irq_descs[] = {
> +	"charger-thm",
> +	"charger-chg",
> +	"charger-chgin",
> +	"charger-tj-reg",
> +	"charger-sys-ctrl",
> +};
> +
> +static int max77659_charger_probe(struct platform_device *pdev)
> +{
> +	struct max77659_dev *max77659 = dev_get_drvdata(pdev->dev.parent);
> +	struct max77659_charger_dev *charger;
> +	struct device *dev = &pdev->dev;
> +	int i, ret;
> +	struct power_supply_config charger_cfg = {};
> +
> +	charger = devm_kzalloc(dev, sizeof(*charger), GFP_KERNEL);
> +	if (!charger)
> +		return -ENOMEM;
> +
> +	mutex_init(&charger->lock);
> +
> +	charger->dev = dev;
> +	charger->regmap = dev_get_regmap(charger->dev->parent, NULL);
> +	charger->fast_charge_current_ua = 15000;
> +	charger->topoff_timer_min = 30;
> +
> +	max77659_charger_parse_dt(charger);
> +
> +	charger->psy_chg_d.name = MAX77659_CHARGER_NAME;
> +	charger->psy_chg_d.type = POWER_SUPPLY_TYPE_UNKNOWN;

POWER_SUPPLY_TYPE_USB

> +	charger->psy_chg_d.get_property	= max77659_charger_get_property;
> +	charger->psy_chg_d.set_property	= max77659_charger_set_property;
> +	charger->psy_chg_d.properties = max77659_charger_props;
> +	charger->psy_chg_d.property_is_writeable = max77659_property_is_writeable;
> +	charger->psy_chg_d.num_properties = ARRAY_SIZE(max77659_charger_props);
> +	charger_cfg.drv_data = charger;
> +	charger_cfg.supplied_to = chg_supplied_to;
> +	charger_cfg.of_node = dev->of_node;
> +	charger_cfg.num_supplicants = ARRAY_SIZE(chg_supplied_to);

It does not make sense that the charger supplies itself.

> +	ret = max77659_charger_initialize(charger);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to initialize charger\n");
> +
> +	for (i = 0; i < MAX77659_CHG_IRQ_MAX; i++) {
> +		charger->irq_arr[i] = regmap_irq_get_virq(max77659->irqc_chg, i);
> +
> +		if (charger->irq_arr[i] < 0)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid IRQ for MAX77659_CHG_IRQ %d\n", i);
> +
> +		ret = devm_request_threaded_irq(dev, charger->irq_arr[i],
> +						NULL, max77659_charger_isr, IRQF_TRIGGER_FALLING,
> +						max77659_irq_descs[i], charger);
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to request irq: %d\n",
> +					     charger->irq_arr[i]);
> +	}
> +
> +	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");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id max77659_charger_of_id[] = {
> +	{ .compatible = "adi,max77659-charger" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, max77659_charger_of_id);
> +
> +static const struct platform_device_id max77659_charger_id[] = {
> +	{ MAX77659_CHARGER_NAME, 0, },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, max77659_charger_id);
> +
> +static struct platform_driver max77659_charger_driver = {
> +	.driver = {
> +		.name = MAX77659_CHARGER_NAME,
> +		.of_match_table = of_match_ptr(max77659_charger_of_id),
> +	},
> +	.probe = max77659_charger_probe,
> +	.id_table = max77659_charger_id,
> +};
> +
> +module_platform_driver(max77659_charger_driver);
> +
> +MODULE_DESCRIPTION("MAX77659 Charger Driver");
> +MODULE_AUTHOR("Nurettin.Bolucu@analog.com, Zeynep.Arslanbenzer@analog.com");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION("1.0");

-- Sebastian

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
  2022-12-20 14:54   ` Krzysztof Kozlowski
@ 2023-04-17 21:43     ` Arslanbenzer, Zeynep
  2023-04-18  7:09       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Arslanbenzer, Zeynep @ 2023-04-17 21:43 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, lee@kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, sre@kernel.org,
	lgirdwood@gmail.com, broonie@kernel.org

>On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
>> This patch adds device tree binding documentation for MAX77659 charger.
>
>Do not use "This commit/patch".
>https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;>Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoR62rXB0W$ 
>
>> 
>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>> ---
>>  .../power/supply/adi,max77659-charger.yaml    | 42 +++++++++++++++++++
>>  MAINTAINERS                                   |  1 +
>>  2 files changed, 43 insertions(+)
>>  create mode 100644 
>> Documentation/devicetree/bindings/power/supply/adi,max77659-charger.ya
>> ml
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>> yaml 
>> b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>> yaml
>> new file mode 100644
>> index 000000000000..24f8b5a2bd84
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77659-char
>> +++ ger.yaml
>> @@ -0,0 +1,42 @@
>> +# 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,max77659-charger.yaml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEm
>> +sXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywv
>> +oR5rKUR11$
>> +$schema: 
>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>> +aml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusM
>> +xP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoRy_yWWBS$
>> +
>> +title: Battery charger for MAX77659 PMIC from ADI.
>
>Drop full stop.
>
>> +
>> +maintainers:
>> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>> +
>> +description: |
>> +  This module is part of the MAX77659 MFD device. For more details
>> +  see Documentation/devicetree/bindings/mfd/adi,max77659.yaml.
>> +
>> +  The charger is represented as a sub-node of the PMIC node on the device tree.
>> +
>> +properties:
>> +  compatible:
>> +    const: adi,max77659-charger
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  adi,fast-charge-timer:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Fast-charge safety timer value (in hours).
>
>No, use suffixes for common units.

Hi Krzysztof,

Thank you for your review. The possible register values of the fast charge safety timer are described in the datasheet as follows. I was undecided about using the common unit, second, as it may affect the comprehensibility of the code. Of course, I can use second as the common unit if you think it's more appropriate.

0b00 = Timer disabled 
0b01 = 3 hours 
0b10 = 5 hours 
0b11 = 7 hours

>
>> +
>> +  adi,fast-charge-microamp:
>> +    description: Fast-charge constant current value.
>
>> +
>> +  adi,topoff-timer:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: Top-Off timer value (in minutes).
>
>No, use suffixes for common units.

Similar case:

0b000 = 0 minutes 
0b001 = 5 minutes 
0b010 = 10 minutes 
0b011 = 15 minutes 
0b100 = 20 minutes 
0b101 = 25 minutes 
0b110 = 30 minutes 
0b111 = 35 minutes

>
>
>Best regards,
>Krzysztof

Best regards,
Zeynep

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
  2023-04-17 21:43     ` Arslanbenzer, Zeynep
@ 2023-04-18  7:09       ` Krzysztof Kozlowski
  2023-04-18  9:41         ` Arslanbenzer, Zeynep
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Kozlowski @ 2023-04-18  7:09 UTC (permalink / raw)
  To: Arslanbenzer, Zeynep
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, lee@kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, sre@kernel.org,
	lgirdwood@gmail.com, broonie@kernel.org

On 17/04/2023 23:43, Arslanbenzer, Zeynep wrote:
>> On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
>>> This patch adds device tree binding documentation for MAX77659 charger.
>>
>> Do not use "This commit/patch".
>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst*L95__;>Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoR62rXB0W$ 
>>
>>>
>>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>> ---
>>>  .../power/supply/adi,max77659-charger.yaml    | 42 +++++++++++++++++++
>>>  MAINTAINERS                                   |  1 +
>>>  2 files changed, 43 insertions(+)
>>>  create mode 100644 
>>> Documentation/devicetree/bindings/power/supply/adi,max77659-charger.ya
>>> ml
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>> yaml 
>>> b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>> yaml
>>> new file mode 100644
>>> index 000000000000..24f8b5a2bd84
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77659-char
>>> +++ ger.yaml
>>> @@ -0,0 +1,42 @@
>>> +# 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,max77659-charger.yaml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEm
>>> +sXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywv
>>> +oR5rKUR11$
>>> +$schema: 
>>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>>> +aml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusM
>>> +xP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoRy_yWWBS$
>>> +
>>> +title: Battery charger for MAX77659 PMIC from ADI.
>>
>> Drop full stop.
>>
>>> +
>>> +maintainers:
>>> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>> +
>>> +description: |
>>> +  This module is part of the MAX77659 MFD device. For more details
>>> +  see Documentation/devicetree/bindings/mfd/adi,max77659.yaml.
>>> +
>>> +  The charger is represented as a sub-node of the PMIC node on the device tree.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: adi,max77659-charger
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  adi,fast-charge-timer:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: Fast-charge safety timer value (in hours).
>>
>> No, use suffixes for common units.
> 
> Hi Krzysztof,
> 
> Thank you for your review. The possible register values of the fast charge safety timer are described in the datasheet as follows. I was undecided about using the common unit, second, as it may affect the comprehensibility of the code. Of course, I can use second as the common unit if you think it's more appropriate.

This is quite common property, so I do not understand why this one
driver should have it written differently. I understand that parsing
0/1/2/3 is easier for the machine than 0/3/5/7 but it is not easier for
humans.

> 
> 0b00 = Timer disabled 
> 0b01 = 3 hours 
> 0b10 = 5 hours 
> 0b11 = 7 hours
> 
>>
>>> +
>>> +  adi,fast-charge-microamp:
>>> +    description: Fast-charge constant current value.
>>
>>> +
>>> +  adi,topoff-timer:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: Top-Off timer value (in minutes).
>>
>> No, use suffixes for common units.
> 
> Similar case:
> 
> 0b000 = 0 minutes 
> 0b001 = 5 minutes 
> 0b010 = 10 minutes 
> 0b011 = 15 minutes 
> 0b100 = 20 minutes 
> 0b101 = 25 minutes 
> 0b110 = 30 minutes 
> 0b111 = 35 minutes

Come on, this is easy to parse. Divide by 5 and where is the code
complexity?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
  2023-04-18  7:09       ` Krzysztof Kozlowski
@ 2023-04-18  9:41         ` Arslanbenzer, Zeynep
  2023-04-18 12:30           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 18+ messages in thread
From: Arslanbenzer, Zeynep @ 2023-04-18  9:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, lee@kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, sre@kernel.org,
	lgirdwood@gmail.com, broonie@kernel.org

>On 17/04/2023 23:43, Arslanbenzer, Zeynep wrote:
>>> On 20/12/2022 14:22, Zeynep Arslanbenzer wrote:
>>>> This patch adds device tree binding documentation for MAX77659 charger.
>>>
>>> Do not use "This commit/patch".
>>> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v5.17.1/
>>> source/Documentation/process/submitting-patches.rst*L95__;>Iw!!A3Ni8C
>>> S0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26L
>>> TvoaIIMvEDr2YxDECrGlMwR8-ywvoR62rXB0W$
>>>
>>>>
>>>> Signed-off-by: Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>>>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>>> ---
>>>>  .../power/supply/adi,max77659-charger.yaml    | 42 +++++++++++++++++++
>>>>  MAINTAINERS                                   |  1 +
>>>>  2 files changed, 43 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>>> ya
>>>> ml
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>>> yaml
>>>> b/Documentation/devicetree/bindings/power/supply/adi,max77659-charger.
>>>> yaml
>>>> new file mode 100644
>>>> index 000000000000..24f8b5a2bd84
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/supply/adi,max77659-ch
>>>> +++ ar
>>>> +++ ger.yaml
>>>> @@ -0,0 +1,42 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
>>>> +---
>>>> +$id: 
>>>> +https://urldefense.com/v3/__http://devicetree.org/schemas/power/sup
>>>> +pl 
>>>> +y/adi,max77659-charger.yaml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdve
>>>> +Em 
>>>> +sXlsfflRXDabf6RVE5ySRusMxP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-y
>>>> +wv
>>>> +oR5rKUR11$
>>>> +$schema: 
>>>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core
>>>> +.y 
>>>> +aml*__;Iw!!A3Ni8CS0y2Y!-AnHmIThbB5Q88_dFdveEmsXlsfflRXDabf6RVE5ySRu
>>>> +sM xP24NEfAr8RCsu26LTvoaIIMvEDr2YxDECrGlMwR8-ywvoRy_yWWBS$
>>>> +
>>>> +title: Battery charger for MAX77659 PMIC from ADI.
>>>
>>> Drop full stop.
>>>
>>>> +
>>>> +maintainers:
>>>> +  - Nurettin Bolucu <Nurettin.Bolucu@analog.com>>
>>>> +  - Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>>
>>>> +
>>>> +description: |
>>>> +  This module is part of the MAX77659 MFD device. For more details
>>>> +  see Documentation/devicetree/bindings/mfd/adi,max77659.yaml.
>>>> +
>>>> +  The charger is represented as a sub-node of the PMIC node on the device tree.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: adi,max77659-charger
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  adi,fast-charge-timer:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description: Fast-charge safety timer value (in hours).
>>>
>>> No, use suffixes for common units.
>> 
>> Hi Krzysztof,
>> 
>> Thank you for your review. The possible register values of the fast charge safety timer are described in the datasheet as follows. I was undecided about using the common unit, second, as it may affect the comprehensibility of the code. Of course, I can use second as the common unit if you think it's more appropriate.
>
>This is quite common property, so I do not understand why this one driver should have it written differently. I understand that parsing
>0/1/2/3 is easier for the machine than 0/3/5/7 but it is not easier for humans.

I referenced property-units.yaml for common units.

As I understood hours and minutes are not common units in Linux for time so I cannot use them as suffixes. Therefore, I thought I had to use "seconds" instead of "hours" or "minutes". I am totally fine if I can use " adi,fast-charge-timer-hours" and "adi,topoff-timer-minutes". 

>
>> 
>> 0b00 = Timer disabled
>> 0b01 = 3 hours
>> 0b10 = 5 hours
>> 0b11 = 7 hours
>> 
>>>
>>>> +
>>>> +  adi,fast-charge-microamp:
>>>> +    description: Fast-charge constant current value.
>>>
>>>> +
>>>> +  adi,topoff-timer:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description: Top-Off timer value (in minutes).
>>>
>>> No, use suffixes for common units.
>> 
>> Similar case:
>> 
>> 0b000 = 0 minutes 
>> 0b001 = 5 minutes 
>> 0b010 = 10 minutes 
>> 0b011 = 15 minutes 
>> 0b100 = 20 minutes 
>> 0b101 = 25 minutes 
>> 0b110 = 30 minutes 
>> 0b111 = 35 minutes
>
>Come on, this is easy to parse. Divide by 5 and where is the code
>complexity?
>
>Best regards,
>Krzysztof
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding
  2023-04-18  9:41         ` Arslanbenzer, Zeynep
@ 2023-04-18 12:30           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2023-04-18 12:30 UTC (permalink / raw)
  To: Arslanbenzer, Zeynep
  Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-pm@vger.kernel.org, lee@kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, sre@kernel.org,
	lgirdwood@gmail.com, broonie@kernel.org

On 18/04/2023 11:41, Arslanbenzer, Zeynep wrote:
>>>> No, use suffixes for common units.
>>>
>>> Hi Krzysztof,
>>>
>>> Thank you for your review. The possible register values of the fast charge safety timer are described in the datasheet as follows. I was undecided about using the common unit, second, as it may affect the comprehensibility of the code. Of course, I can use second as the common unit if you think it's more appropriate.
>>
>> This is quite common property, so I do not understand why this one driver should have it written differently. I understand that parsing
>> 0/1/2/3 is easier for the machine than 0/3/5/7 but it is not easier for humans.
> 
> I referenced property-units.yaml for common units.
> 
> As I understood hours and minutes are not common units in Linux for time so I cannot use them as suffixes. Therefore, I thought I had to use "seconds" instead of "hours" or "minutes". I am totally fine if I can use " adi,fast-charge-timer-hours" and "adi,topoff-timer-minutes". 

Wrap your replies to match mailing list style.

Yeah... indeed... because it should not be in DTS in the first place.
It's not a property of the hardware, so the same as with other drivers -
we don't store it in DT. Check other maxim drivers.

> 
>>
>>>
>>> 0b00 = Timer disabled
>>> 0b01 = 3 hours
>>> 0b10 = 5 hours
>>> 0b11 = 7 hours
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-04-18 12:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-20 13:22 [PATCH 0/6] drivers: mfd: Add MAX77659 MFD and related device drivers Zeynep Arslanbenzer
2022-12-20 13:22 ` [PATCH 1/6] drivers: mfd: add MAX77659 PMIC support Zeynep Arslanbenzer
2022-12-20 14:47   ` Krzysztof Kozlowski
2022-12-20 13:22 ` [PATCH 2/6] dt-bindings: mfd: add MAX77659 binding Zeynep Arslanbenzer
2022-12-20 14:43   ` Krzysztof Kozlowski
2022-12-20 16:40   ` Rob Herring
2022-12-20 13:22 ` [PATCH 3/6] drivers: power: supply: add MAX77659 charger support Zeynep Arslanbenzer
2022-12-20 14:51   ` Krzysztof Kozlowski
2023-01-02 20:44   ` Sebastian Reichel
2022-12-20 13:22 ` [PATCH 4/6] dt-bindings: power: supply: add MAX77659 charger binding Zeynep Arslanbenzer
2022-12-20 14:54   ` Krzysztof Kozlowski
2023-04-17 21:43     ` Arslanbenzer, Zeynep
2023-04-18  7:09       ` Krzysztof Kozlowski
2023-04-18  9:41         ` Arslanbenzer, Zeynep
2023-04-18 12:30           ` Krzysztof Kozlowski
2022-12-20 13:22 ` [PATCH 5/6] drivers: regulator: add MAX77659 regulator support Zeynep Arslanbenzer
2022-12-20 13:22 ` [PATCH 6/6] dt-bindings: regulator: add MAX77659 regulator binding Zeynep Arslanbenzer
2022-12-20 14:55   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).