* [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* 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
* [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* 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 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
* [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, ®_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* 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 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, ®_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
* [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* 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 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
* [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 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