[PATCH v3 0/2] power: supply: add support for MAX1720x standalone fuel

[PATCH v3 0/2] power: supply: add support for MAX1720x standalone fuel

[Dimitri Fedrau]

Changes to max1721x_battery.c:
  - reading manufacturer, model name and serial number is only possible
    when SBS functions of the IC are enabled.(nNVCfg0.enSBS) Factory
    default is off. Manufacturer is "Maxim Integrated" and the model name
    can be derived by register MAX172XX_DEV_NAME. Serial number is not
    available anymore.
  - According to the datasheet MAX172XX_BAT_PRESENT is at BIT(3) not
    BIT(4). Furthermore the naming is misleading, when BIT(3) is set the
    battery is not present.
  - Removed DeviceName, ManufacturerName and SerialNumber from struct
    max17211_device_info

Changes in V2:
  - Changed E-Mail in Patch (2/2) Signed-Off

Changes in V3:
  - Changed E-Mail in Patch (2/2) Author
  - Sorry for the confusion

Dimitri Fedrau (2):
  dt-bindings: power: supply: add support for MAX17201/MAX17205 fuel
    gauge
  power: supply: add support for MAX1720x standalone fuel gauge

 .../bindings/power/supply/maxim,max1720x.yaml |  51 +++
 drivers/power/supply/Kconfig                  |  12 +
 drivers/power/supply/Makefile                 |   1 +
 drivers/power/supply/max1720x_battery.c       | 324 ++++++++++++++++++
 4 files changed, 388 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml
 create mode 100644 drivers/power/supply/max1720x_battery.c

-- 
2.39.2

[PATCH v3 1/2] dt-bindings: power: supply: add support for MAX17201/MAX17205 fuel gauge

[Dimitri Fedrau]

Adding documentation for MAXIMs MAX17201/MAX17205 fuel gauge.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
 .../bindings/power/supply/maxim,max1720x.yaml | 51 +++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml

diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml
new file mode 100644
index 000000000000..4414bc6f214f
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/maxim,max1720x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX1720x fuel gauge
+
+maintainers:
+  - Dimitri Fedrau <dima.fedrau@gmail.com>
+
+properties:
+  compatible:
+    const: maxim,max1720x
+
+  reg:
+    items:
+      - description: ModelGauge m5 registers
+      - description: Nonvolatile registers
+
+  reg-names:
+    items:
+      - const: m5
+      - const: nvmem
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      max17201@36 {
+        compatible = "maxim,max1720x";
+        reg = <0x36>, <0xb>;
+        reg-names = "m5", "nvmem";
+        interrupt-parent = <&gpio0>;
+        interrupts = <31 IRQ_TYPE_LEVEL_LOW>;
+        status = "okay";
+      };
+    };
-- 
2.39.2

[PATCH v3 2/2] power: supply: add support for MAX1720x standalone fuel gauge

[Dimitri Fedrau]

The MAX17201 monitor a single cell pack. The MAX17205 monitor and balance
a 2S or 3S pack or monitor a multiple-series cell pack. Both device use
I2C interface.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
 drivers/power/supply/Kconfig            |  12 +
 drivers/power/supply/Makefile           |   1 +
 drivers/power/supply/max1720x_battery.c | 324 ++++++++++++++++++++++++
 3 files changed, 337 insertions(+)
 create mode 100644 drivers/power/supply/max1720x_battery.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 3e31375491d5..e620ae1a9c8b 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -402,6 +402,18 @@ config BATTERY_MAX17042
 
 	  Driver can be build as a module (max17042_battery).
 
+config BATTERY_MAX1720X
+tristate "Maxim MAX17201/MAX17205 Fuel Gauge"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  MAX1720x is a family of fuel-gauge systems for lithium-ion (Li+)
+	  batteries in handheld and portable equipment. MAX17201 are
+	  configured to operate with a single lithium cell, the MAX17205
+	  can operate with multiple cells.
+
+	  Say Y to include support for the MAX17201/MAX17205 Fuel Gauges.
+
 config BATTERY_MAX1721X
 	tristate "MAX17211/MAX17215 standalone gas-gauge"
 	depends on W1
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 58b567278034..6f02d0248826 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_CHARGER_DA9150)	+= da9150-charger.o
 obj-$(CONFIG_BATTERY_DA9150)	+= da9150-fg.o
 obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
 obj-$(CONFIG_BATTERY_MAX17042)	+= max17042_battery.o
+obj-$(CONFIG_BATTERY_MAX1720X)	+= max1720x_battery.o
 obj-$(CONFIG_BATTERY_MAX1721X)	+= max1721x_battery.o
 obj-$(CONFIG_BATTERY_RT5033)	+= rt5033_battery.o
 obj-$(CONFIG_CHARGER_RT5033)	+= rt5033_charger.o
diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
new file mode 100644
index 000000000000..68d317d01c9d
--- /dev/null
+++ b/drivers/power/supply/max1720x_battery.c
@@ -0,0 +1,324 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Fuel gauge driver for Maxim 17201/17205
+ *
+ * based on max1721x_battery.c
+ *
+ * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH
+ */
+
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+/* Nonvilatile registers */
+#define MAX1720_NRSENSE			0xCF	/* RSense in 10^-5 Ohm */
+
+/* ModelGauge m5 */
+#define MAX172XX_STATUS			0x00	/* Status */
+#define MAX172XX_STATUS_BAT_ABSENT	BIT(3)	/* Battery absent */
+#define MAX172XX_REPCAP			0x05	/* Average capacity */
+#define MAX172XX_REPSOC			0x06	/* Percentage of charge */
+#define MAX172XX_TEMP			0x08	/* Temperature */
+#define MAX172XX_CURRENT		0x0A	/* Actual current */
+#define MAX172XX_AVG_CURRENT		0x0B	/* Average current */
+#define MAX172XX_TTE			0x11	/* Time to empty */
+#define MAX172XX_AVG_TA			0x16	/* Average temperature */
+#define MAX172XX_DESIGN_CAP		0x18	/* Design capacity */
+#define MAX172XX_TTF			0x20	/* Time to full */
+#define MAX172XX_DEV_NAME		0x21	/* Device name */
+#define MAX172XX_DEV_NAME_TYPE_MASK	GENMASK(3, 0)
+#define MAX172XX_DEV_NAME_TYPE_MAX17201	BIT(0)
+#define MAX172XX_DEV_NAME_TYPE_MAX17205	(BIT(0) | BIT(2))
+#define MAX172XX_BATT			0xDA	/* Battery voltage */
+#define MAX172XX_ATAVCAP		0xDF
+
+static const char *max1720x_manufacturer = "Maxim Integrated";
+static const char *max17201_model = "MAX17201";
+static const char *max17205_model = "MAX17205";
+
+struct max1720x_device_info {
+	struct power_supply *bat;
+	struct power_supply_desc bat_desc;
+	struct i2c_client *ancillary;
+	struct regmap *regmap;
+	int rsense;
+};
+
+/*
+ * Model Gauge M5 Algorithm output register
+ * Volatile data (must not be cached)
+ */
+static const struct regmap_range max1720x_volatile_allow[] = {
+	regmap_reg_range(MAX172XX_STATUS, MAX172XX_ATAVCAP),	/* volatile data */
+};
+
+static const struct regmap_range max1720x_volatile_deny[] = {
+	/* volatile data unused registers */
+	regmap_reg_range(0x24, 0x26),
+	regmap_reg_range(0x30, 0x31),
+	regmap_reg_range(0x33, 0x34),
+	regmap_reg_range(0x37, 0x37),
+	regmap_reg_range(0x3B, 0x3C),
+	regmap_reg_range(0x40, 0x41),
+	regmap_reg_range(0x43, 0x44),
+	regmap_reg_range(0x47, 0x49),
+	regmap_reg_range(0x4B, 0x4C),
+	regmap_reg_range(0x4E, 0xAF),
+	regmap_reg_range(0xB1, 0xB3),
+	regmap_reg_range(0xB5, 0xB7),
+	regmap_reg_range(0xBF, 0xD0),
+	regmap_reg_range(0xDB, 0xDB),
+	regmap_reg_range(0xE0, 0xFF),
+};
+
+static const struct regmap_access_table max1720x_volatile_regs = {
+	.yes_ranges	= max1720x_volatile_allow,
+	.n_yes_ranges	= ARRAY_SIZE(max1720x_volatile_allow),
+	.no_ranges	= max1720x_volatile_deny,
+	.n_no_ranges	= ARRAY_SIZE(max1720x_volatile_deny),
+};
+
+static const struct regmap_config max1720x_regmap_cfg = {
+	.reg_bits = 8,
+	.val_bits = 16,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+	.rd_table = &max1720x_volatile_regs,
+	.volatile_table = &max1720x_volatile_regs,
+};
+
+static enum power_supply_property max1720x_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_CAPACITY,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
+	POWER_SUPPLY_PROP_CHARGE_AVG,
+	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
+	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CURRENT_AVG,
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+/* Convert regs value to power_supply units */
+
+static int max172xx_time_to_ps(unsigned int reg)
+{
+	return reg * 5625 / 1000;	/* in sec. */
+}
+
+static int max172xx_percent_to_ps(unsigned int reg)
+{
+	return reg / 256;	/* in percent from 0 to 100 */
+}
+
+static int max172xx_voltage_to_ps(unsigned int reg)
+{
+	return reg * 1250;	/* in uV */
+}
+
+static int max172xx_capacity_to_ps(unsigned int reg)
+{
+	return reg * 500;	/* in uAh */
+}
+
+/*
+ * Current and temperature is signed values, so unsigned regs
+ * value must be converted to signed type
+ */
+
+static int max172xx_temperature_to_ps(unsigned int reg)
+{
+	int val = (int16_t)(reg);
+
+	return val * 10 / 256; /* in tenths of deg. C */
+}
+
+/*
+ * Calculating current registers resolution:
+ *
+ * RSense stored in 10^-5 Ohm, so mesaurment voltage must be
+ * in 10^-11 Volts for get current in uA.
+ * 16 bit current reg fullscale +/-51.2mV is 102400 uV.
+ * So: 102400 / 65535 * 10^5 = 156252
+ */
+static int max172xx_current_to_voltage(unsigned int reg)
+{
+	int val = (int16_t)(reg);
+
+	return val * 156252;
+}
+
+static int max1720x_battery_get_property(struct power_supply *psy,
+					 enum power_supply_property psp,
+					 union power_supply_propval *val)
+{
+	struct max1720x_device_info *info = power_supply_get_drvdata(psy);
+	unsigned int reg_val = 0;
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		/*
+		 * POWER_SUPPLY_PROP_PRESENT will always readable via
+		 * sysfs interface. Value return 0 if battery not
+		 * present or unaccesable via I2c.
+		 */
+		ret = regmap_read(info->regmap, MAX172XX_STATUS, &reg_val);
+		if (ret < 0)
+			return ret;
+
+		val->intval = !(FIELD_GET(MAX172XX_STATUS_BAT_ABSENT, reg_val));
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		ret = regmap_read(info->regmap, MAX172XX_REPSOC, &reg_val);
+		val->intval = max172xx_percent_to_ps(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = regmap_read(info->regmap, MAX172XX_BATT, &reg_val);
+		val->intval = max172xx_voltage_to_ps(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
+		ret = regmap_read(info->regmap, MAX172XX_DESIGN_CAP, &reg_val);
+		val->intval = max172xx_capacity_to_ps(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_REPCAP, &reg_val);
+		val->intval = max172xx_capacity_to_ps(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_TTE, &reg_val);
+		val->intval = max172xx_time_to_ps(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_TTF, &reg_val);
+		val->intval = max172xx_time_to_ps(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		ret = regmap_read(info->regmap, MAX172XX_TEMP, &reg_val);
+		val->intval = max172xx_temperature_to_ps(reg_val);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = regmap_read(info->regmap, MAX172XX_CURRENT, &reg_val);
+		val->intval = max172xx_current_to_voltage(reg_val) / info->rsense;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_AVG:
+		ret = regmap_read(info->regmap, MAX172XX_AVG_CURRENT, &reg_val);
+		val->intval = max172xx_current_to_voltage(reg_val) / info->rsense;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		ret = regmap_read(info->regmap, MAX172XX_DEV_NAME, &reg_val);
+		reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val);
+		if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201)
+			val->strval = max17201_model;
+		else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205)
+			val->strval = max17205_model;
+		else
+			return -ENODEV;
+		break;
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		val->strval = max1720x_manufacturer;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static int max1720x_probe_sense_resistor(struct max1720x_device_info *info)
+{
+	struct device *dev = &info->ancillary->dev;
+	int ret;
+
+	ret = i2c_smbus_read_word_data(info->ancillary, MAX1720_NRSENSE);
+	if (ret < 0)
+		return ret;
+
+	info->rsense = ret;
+	if (!info->rsense) {
+		dev_warn(dev, "RSense not calibrated, set 10 mOhms!\n");
+		info->rsense = 1000; /* in regs in 10^-5 */
+	}
+
+	return 0;
+}
+
+static int max1720x_probe(struct i2c_client *client)
+{
+	struct power_supply_config psy_cfg = {};
+	struct device *dev = &client->dev;
+	struct max1720x_device_info *info;
+	int ret;
+
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, info);
+	info->bat_desc.name = "max1720x";
+	info->bat_desc.no_thermal = true;
+	info->bat_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	info->bat_desc.properties = max1720x_battery_props;
+	info->bat_desc.num_properties = ARRAY_SIZE(max1720x_battery_props);
+	info->bat_desc.get_property = max1720x_battery_get_property;
+	psy_cfg.drv_data = info;
+
+	info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg);
+	if (IS_ERR(info->regmap))
+		return dev_err_probe(dev, PTR_ERR(info->regmap),
+				     "regmap initialization failed\n");
+
+	info->ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
+	if (IS_ERR(info->ancillary))
+		return dev_err_probe(dev, PTR_ERR(info->ancillary),
+				     "Failed to initialize ancillary i2c device\n");
+
+	i2c_set_clientdata(info->ancillary, info);
+	ret = max1720x_probe_sense_resistor(info);
+	if (ret) {
+		i2c_unregister_device(info->ancillary);
+		return dev_err_probe(dev, PTR_ERR(info->bat),
+				     "Failed to read sense resistor value\n");
+	}
+
+	info->bat = devm_power_supply_register(dev, &info->bat_desc, &psy_cfg);
+	if (IS_ERR(info->bat)) {
+		i2c_unregister_device(info->ancillary);
+		return dev_err_probe(dev, PTR_ERR(info->bat),
+				     "Failed to register power supply\n");
+	}
+
+	return 0;
+}
+
+static void max1720x_remove(struct i2c_client *client)
+{
+	struct max1720x_device_info *info = i2c_get_clientdata(client);
+
+	i2c_unregister_device(info->ancillary);
+}
+
+static const struct of_device_id max1720x_of_match[] = {
+	{ .compatible = "maxim,max1720x" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max1720x_of_match);
+
+static struct i2c_driver max1720x_i2c_driver = {
+	.driver = {
+		   .name = "max1720x",
+		   .of_match_table = max1720x_of_match,
+		   },
+	.probe = max1720x_probe,
+	.remove = max1720x_remove,
+};
+module_i2c_driver(max1720x_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dimitri Fedrau <dima.fedrau@gmail.com>");
+MODULE_DESCRIPTION("Maxim MAX17201/MAX17205 Fuel Gauge IC driver");
-- 
2.39.2

Re: [PATCH v3 2/2] power: supply: add support for MAX1720x standalone fuel gauge

[Thomas Weißschuh]

On 2024-06-15 22:33:50+0000, Dimitri Fedrau wrote:
> The MAX17201 monitor a single cell pack. The MAX17205 monitor and balance

"monitors" / "monitors and balances".

> a 2S or 3S pack or monitor a multiple-series cell pack. Both device use

"devices"

> I2C interface.

"a" I2C.

> 
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> ---
>  drivers/power/supply/Kconfig            |  12 +
>  drivers/power/supply/Makefile           |   1 +
>  drivers/power/supply/max1720x_battery.c | 324 ++++++++++++++++++++++++
>  3 files changed, 337 insertions(+)
>  create mode 100644 drivers/power/supply/max1720x_battery.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 3e31375491d5..e620ae1a9c8b 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -402,6 +402,18 @@ config BATTERY_MAX17042
>  
>  	  Driver can be build as a module (max17042_battery).
>  
> +config BATTERY_MAX1720X
> +tristate "Maxim MAX17201/MAX17205 Fuel Gauge"

Indentation?

> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  MAX1720x is a family of fuel-gauge systems for lithium-ion (Li+)
> +	  batteries in handheld and portable equipment. MAX17201 are
> +	  configured to operate with a single lithium cell, the MAX17205
> +	  can operate with multiple cells.
> +
> +	  Say Y to include support for the MAX17201/MAX17205 Fuel Gauges.
> +
>  config BATTERY_MAX1721X
>  	tristate "MAX17211/MAX17215 standalone gas-gauge"
>  	depends on W1
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 58b567278034..6f02d0248826 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_CHARGER_DA9150)	+= da9150-charger.o
>  obj-$(CONFIG_BATTERY_DA9150)	+= da9150-fg.o
>  obj-$(CONFIG_BATTERY_MAX17040)	+= max17040_battery.o
>  obj-$(CONFIG_BATTERY_MAX17042)	+= max17042_battery.o
> +obj-$(CONFIG_BATTERY_MAX1720X)	+= max1720x_battery.o
>  obj-$(CONFIG_BATTERY_MAX1721X)	+= max1721x_battery.o
>  obj-$(CONFIG_BATTERY_RT5033)	+= rt5033_battery.o
>  obj-$(CONFIG_CHARGER_RT5033)	+= rt5033_charger.o
> diff --git a/drivers/power/supply/max1720x_battery.c b/drivers/power/supply/max1720x_battery.c
> new file mode 100644
> index 000000000000..68d317d01c9d
> --- /dev/null
> +++ b/drivers/power/supply/max1720x_battery.c
> @@ -0,0 +1,324 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Fuel gauge driver for Maxim 17201/17205
> + *
> + * based on max1721x_battery.c
> + *
> + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +/* Nonvilatile registers */

Nonvolatile

> +#define MAX1720_NRSENSE			0xCF	/* RSense in 10^-5 Ohm */
> +
> +/* ModelGauge m5 */
> +#define MAX172XX_STATUS			0x00	/* Status */
> +#define MAX172XX_STATUS_BAT_ABSENT	BIT(3)	/* Battery absent */
> +#define MAX172XX_REPCAP			0x05	/* Average capacity */
> +#define MAX172XX_REPSOC			0x06	/* Percentage of charge */
> +#define MAX172XX_TEMP			0x08	/* Temperature */
> +#define MAX172XX_CURRENT		0x0A	/* Actual current */
> +#define MAX172XX_AVG_CURRENT		0x0B	/* Average current */
> +#define MAX172XX_TTE			0x11	/* Time to empty */
> +#define MAX172XX_AVG_TA			0x16	/* Average temperature */
> +#define MAX172XX_DESIGN_CAP		0x18	/* Design capacity */
> +#define MAX172XX_TTF			0x20	/* Time to full */
> +#define MAX172XX_DEV_NAME		0x21	/* Device name */
> +#define MAX172XX_DEV_NAME_TYPE_MASK	GENMASK(3, 0)
> +#define MAX172XX_DEV_NAME_TYPE_MAX17201	BIT(0)
> +#define MAX172XX_DEV_NAME_TYPE_MAX17205	(BIT(0) | BIT(2))
> +#define MAX172XX_BATT			0xDA	/* Battery voltage */
> +#define MAX172XX_ATAVCAP		0xDF
> +
> +static const char *max1720x_manufacturer = "Maxim Integrated";

static const char *const max1720x_manufacturer = "Maxim Integrated";

> +static const char *max17201_model = "MAX17201";
> +static const char *max17205_model = "MAX17205";
> +
> +struct max1720x_device_info {
> +	struct power_supply *bat;
> +	struct power_supply_desc bat_desc;

No need for bat or bat_desc in the info struct.

> +	struct i2c_client *ancillary;
> +	struct regmap *regmap;
> +	int rsense;
> +};
> +
> +/*
> + * Model Gauge M5 Algorithm output register
> + * Volatile data (must not be cached)
> + */
> +static const struct regmap_range max1720x_volatile_allow[] = {
> +	regmap_reg_range(MAX172XX_STATUS, MAX172XX_ATAVCAP),	/* volatile data */
> +};

Are MAX172XX_DEV_NAME and MAX172XX_DESIGN_CAP really volatile?
> +
> +static const struct regmap_range max1720x_volatile_deny[] = {
> +	/* volatile data unused registers */
> +	regmap_reg_range(0x24, 0x26),
> +	regmap_reg_range(0x30, 0x31),
> +	regmap_reg_range(0x33, 0x34),
> +	regmap_reg_range(0x37, 0x37),
> +	regmap_reg_range(0x3B, 0x3C),
> +	regmap_reg_range(0x40, 0x41),
> +	regmap_reg_range(0x43, 0x44),
> +	regmap_reg_range(0x47, 0x49),
> +	regmap_reg_range(0x4B, 0x4C),
> +	regmap_reg_range(0x4E, 0xAF),
> +	regmap_reg_range(0xB1, 0xB3),
> +	regmap_reg_range(0xB5, 0xB7),
> +	regmap_reg_range(0xBF, 0xD0),
> +	regmap_reg_range(0xDB, 0xDB),
> +	regmap_reg_range(0xE0, 0xFF),
> +};
> +
> +static const struct regmap_access_table max1720x_volatile_regs = {
> +	.yes_ranges	= max1720x_volatile_allow,
> +	.n_yes_ranges	= ARRAY_SIZE(max1720x_volatile_allow),
> +	.no_ranges	= max1720x_volatile_deny,
> +	.n_no_ranges	= ARRAY_SIZE(max1720x_volatile_deny),
> +};
> +
> +static const struct regmap_config max1720x_regmap_cfg = {
> +	.reg_bits = 8,
> +	.val_bits = 16,
> +	.val_format_endian = REGMAP_ENDIAN_LITTLE,
> +	.rd_table = &max1720x_volatile_regs,
> +	.volatile_table = &max1720x_volatile_regs,
> +};
> +
> +static enum power_supply_property max1720x_battery_props[] = {

const.

> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
> +	POWER_SUPPLY_PROP_CHARGE_AVG,
> +	POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG,
> +	POWER_SUPPLY_PROP_TIME_TO_FULL_AVG,
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_AVG,
> +	POWER_SUPPLY_PROP_MODEL_NAME,
> +	POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +/* Convert regs value to power_supply units */
> +
> +static int max172xx_time_to_ps(unsigned int reg)
> +{
> +	return reg * 5625 / 1000;	/* in sec. */
> +}
> +
> +static int max172xx_percent_to_ps(unsigned int reg)
> +{
> +	return reg / 256;	/* in percent from 0 to 100 */
> +}
> +
> +static int max172xx_voltage_to_ps(unsigned int reg)
> +{
> +	return reg * 1250;	/* in uV */
> +}
> +
> +static int max172xx_capacity_to_ps(unsigned int reg)
> +{
> +	return reg * 500;	/* in uAh */
> +}
> +
> +/*
> + * Current and temperature is signed values, so unsigned regs
> + * value must be converted to signed type
> + */
> +
> +static int max172xx_temperature_to_ps(unsigned int reg)
> +{
> +	int val = (int16_t)(reg);
> +
> +	return val * 10 / 256; /* in tenths of deg. C */
> +}
> +
> +/*
> + * Calculating current registers resolution:
> + *
> + * RSense stored in 10^-5 Ohm, so mesaurment voltage must be
> + * in 10^-11 Volts for get current in uA.
> + * 16 bit current reg fullscale +/-51.2mV is 102400 uV.
> + * So: 102400 / 65535 * 10^5 = 156252
> + */
> +static int max172xx_current_to_voltage(unsigned int reg)
> +{
> +	int val = (int16_t)(reg);

No need for braces around (reg).

> +
> +	return val * 156252;
> +}
> +
> +static int max1720x_battery_get_property(struct power_supply *psy,
> +					 enum power_supply_property psp,
> +					 union power_supply_propval *val)
> +{
> +	struct max1720x_device_info *info = power_supply_get_drvdata(psy);
> +	unsigned int reg_val = 0;

No need to initialize.

> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		/*
> +		 * POWER_SUPPLY_PROP_PRESENT will always readable via
> +		 * sysfs interface. Value return 0 if battery not
> +		 * present or unaccesable via I2c.
> +		 */
> +		ret = regmap_read(info->regmap, MAX172XX_STATUS, &reg_val);
> +		if (ret < 0)
> +			return ret;
> +
> +		val->intval = !(FIELD_GET(MAX172XX_STATUS_BAT_ABSENT, reg_val));

No need for the braces around FIELD_GET().

> +		break;
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		ret = regmap_read(info->regmap, MAX172XX_REPSOC, &reg_val);

In case POWER_SUPPLY_PROP_PRESENT there is an early return if ret < 0.
Why are the other ones different?

> +		val->intval = max172xx_percent_to_ps(reg_val);
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = regmap_read(info->regmap, MAX172XX_BATT, &reg_val);
> +		val->intval = max172xx_voltage_to_ps(reg_val);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
> +		ret = regmap_read(info->regmap, MAX172XX_DESIGN_CAP, &reg_val);
> +		val->intval = max172xx_capacity_to_ps(reg_val);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_REPCAP, &reg_val);
> +		val->intval = max172xx_capacity_to_ps(reg_val);
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_EMPTY_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_TTE, &reg_val);
> +		val->intval = max172xx_time_to_ps(reg_val);
> +		break;
> +	case POWER_SUPPLY_PROP_TIME_TO_FULL_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_TTF, &reg_val);
> +		val->intval = max172xx_time_to_ps(reg_val);
> +		break;
> +	case POWER_SUPPLY_PROP_TEMP:
> +		ret = regmap_read(info->regmap, MAX172XX_TEMP, &reg_val);
> +		val->intval = max172xx_temperature_to_ps(reg_val);
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = regmap_read(info->regmap, MAX172XX_CURRENT, &reg_val);
> +		val->intval = max172xx_current_to_voltage(reg_val) / info->rsense;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_AVG:
> +		ret = regmap_read(info->regmap, MAX172XX_AVG_CURRENT, &reg_val);
> +		val->intval = max172xx_current_to_voltage(reg_val) / info->rsense;
> +		break;
> +	case POWER_SUPPLY_PROP_MODEL_NAME:
> +		ret = regmap_read(info->regmap, MAX172XX_DEV_NAME, &reg_val);
> +		reg_val = FIELD_GET(MAX172XX_DEV_NAME_TYPE_MASK, reg_val);
> +		if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17201)
> +			val->strval = max17201_model;
> +		else if (reg_val == MAX172XX_DEV_NAME_TYPE_MAX17205)
> +			val->strval = max17205_model;
> +		else
> +			return -ENODEV;
> +		break;
> +	case POWER_SUPPLY_PROP_MANUFACTURER:
> +		val->strval = max1720x_manufacturer;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int max1720x_probe_sense_resistor(struct max1720x_device_info *info)
> +{
> +	struct device *dev = &info->ancillary->dev;
> +	int ret;
> +
> +	ret = i2c_smbus_read_word_data(info->ancillary, MAX1720_NRSENSE);
> +	if (ret < 0)
> +		return ret;

Is this really backed by an smbus?
AFAIK if it isn't then it's better to manually implement multi-byte
readings instead of relying on the smbus emulation.

> +
> +	info->rsense = ret;
> +	if (!info->rsense) {
> +		dev_warn(dev, "RSense not calibrated, set 10 mOhms!\n");
> +		info->rsense = 1000; /* in regs in 10^-5 */
> +	}
> +
> +	return 0;
> +}
> +
> +static int max1720x_probe(struct i2c_client *client)
> +{
> +	struct power_supply_config psy_cfg = {};
> +	struct device *dev = &client->dev;
> +	struct max1720x_device_info *info;
> +	int ret;
> +
> +	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, info);
> +	info->bat_desc.name = "max1720x";
> +	info->bat_desc.no_thermal = true;
> +	info->bat_desc.type = POWER_SUPPLY_TYPE_BATTERY;
> +	info->bat_desc.properties = max1720x_battery_props;
> +	info->bat_desc.num_properties = ARRAY_SIZE(max1720x_battery_props);
> +	info->bat_desc.get_property = max1720x_battery_get_property;
> +	psy_cfg.drv_data = info;
> +
> +	info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg);
> +	if (IS_ERR(info->regmap))
> +		return dev_err_probe(dev, PTR_ERR(info->regmap),
> +				     "regmap initialization failed\n");
> +
> +	info->ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
> +	if (IS_ERR(info->ancillary))
> +		return dev_err_probe(dev, PTR_ERR(info->ancillary),
> +				     "Failed to initialize ancillary i2c device\n");

The ancillary is only used during probing.
So it doesn't need to be kept around.
You can pass it directly to max1720x_probe_sense_resistor(), drop it
from info clean it up after usage in _probe() and get rid of max1720x_remove().

Also drop *both* calls to i2c_set_clientdata().

> +
> +	i2c_set_clientdata(info->ancillary, info);
> +	ret = max1720x_probe_sense_resistor(info);
> +	if (ret) {
> +		i2c_unregister_device(info->ancillary);
> +		return dev_err_probe(dev, PTR_ERR(info->bat),
> +				     "Failed to read sense resistor value\n");
> +	}
> +
> +	info->bat = devm_power_supply_register(dev, &info->bat_desc, &psy_cfg);
> +	if (IS_ERR(info->bat)) {
> +		i2c_unregister_device(info->ancillary);
> +		return dev_err_probe(dev, PTR_ERR(info->bat),
> +				     "Failed to register power supply\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static void max1720x_remove(struct i2c_client *client)
> +{
> +	struct max1720x_device_info *info = i2c_get_clientdata(client);
> +
> +	i2c_unregister_device(info->ancillary);
> +}
> +
> +static const struct of_device_id max1720x_of_match[] = {
> +	{ .compatible = "maxim,max1720x" },
> +	{},

No comma after sentinel value.

> +};
> +MODULE_DEVICE_TABLE(of, max1720x_of_match);
> +
> +static struct i2c_driver max1720x_i2c_driver = {
> +	.driver = {
> +		   .name = "max1720x",
> +		   .of_match_table = max1720x_of_match,
> +		   },

Alignment.

> +	.probe = max1720x_probe,
> +	.remove = max1720x_remove,
> +};
> +module_i2c_driver(max1720x_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Dimitri Fedrau <dima.fedrau@gmail.com>");
> +MODULE_DESCRIPTION("Maxim MAX17201/MAX17205 Fuel Gauge IC driver");
> -- 
> 2.39.2
> 

Re: [PATCH v3 1/2] dt-bindings: power: supply: add support for MAX17201/MAX17205 fuel gauge

[Krzysztof Kozlowski]

On 15/06/2024 22:33, Dimitri Fedrau wrote:
> Adding documentation for MAXIMs MAX17201/MAX17205 fuel gauge.
> 

Three patchsets within 30 minutes. No changelog et all.

Slow down (one posting per 24h) to give people chances to review. Then
provide changelog under --- and describe what happened.

> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
> ---
>  .../bindings/power/supply/maxim,max1720x.yaml | 51 +++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml
> new file mode 100644
> index 000000000000..4414bc6f214f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max1720x.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/maxim,max1720x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim MAX1720x fuel gauge
> +
> +maintainers:
> +  - Dimitri Fedrau <dima.fedrau@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: maxim,max1720x

Nope, this must be specific.

Filename follows compatible then.

> +
> +  reg:
> +    items:
> +      - description: ModelGauge m5 registers
> +      - description: Nonvolatile registers
> +
> +  reg-names:
> +    items:
> +      - const: m5
> +      - const: nvmem
> +
> +  interrupts:
> +    maxItems: 1

This is incomplete. Missing battery and probably more... Look how other
bindings are written.

> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      max17201@36 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +        compatible = "maxim,max1720x";
> +        reg = <0x36>, <0xb>;
> +        reg-names = "m5", "nvmem";
> +        interrupt-parent = <&gpio0>;
> +        interrupts = <31 IRQ_TYPE_LEVEL_LOW>;
> +        status = "okay";

Drop

Best regards,
Krzysztof

Re: [PATCH v3 1/2] dt-bindings: power: supply: add support for MAX17201/MAX17205 fuel gauge

[Dimitri Fedrau]

Am Sun, Jun 16, 2024 at 09:27:21AM +0200 schrieb Krzysztof Kozlowski:
> On 15/06/2024 22:33, Dimitri Fedrau wrote:
> > Adding documentation for MAXIMs MAX17201/MAX17205 fuel gauge.
> > 
> 
> Three patchsets within 30 minutes. No changelog et all.
>
Sorry, had to fix my mail address in the commit message. Changelog was
in the cover letter. Anyway, could have fixed that in a later version.

> Slow down (one posting per 24h) to give people chances to review. Then
> provide changelog under --- and describe what happened.
> 
[...]
> > +maintainers:
> > +  - Dimitri Fedrau <dima.fedrau@gmail.com>
> > +
> > +properties:
> > +      - description: ModelGauge m5 registers
> > +      - description: Nonvolatile registers
> > +
> > +  reg-names:
> > +    items:
> > +      - const: m5
> > +      - const: nvmem
> > +
> > +  interrupts:
> > +    maxItems: 1
> 
> This is incomplete. Missing battery and probably more... Look how other
> bindings are written.
> 
Some fuel gauges used monitored-battery and/or power-supplies others none
of them(mitsumi,mm8013.yaml). I'm not sure when to use them.

Best regards,
Dimitri Fedrau

Re: [PATCH v3 2/2] power: supply: add support for MAX1720x standalone fuel gauge

[Dan Carpenter]

Hi Dimitri,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dimitri-Fedrau/dt-bindings-power-supply-add-support-for-MAX17201-MAX17205-fuel-gauge/20240616-043602
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
patch link:    https://lore.kernel.org/r/20240615203352.164234-3-dima.fedrau%40gmail.com
patch subject: [PATCH v3 2/2] power: supply: add support for MAX1720x standalone fuel gauge
config: nios2-randconfig-r081-20240616 (https://download.01.org/0day-ci/archive/20240617/202406170040.gxB0dYxg-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 13.2.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202406170040.gxB0dYxg-lkp@intel.com/

smatch warnings:
drivers/power/supply/max1720x_battery.c:285 max1720x_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +285 drivers/power/supply/max1720x_battery.c

134a669e205435 Dimitri Fedrau 2024-06-15  251  static int max1720x_probe(struct i2c_client *client)
134a669e205435 Dimitri Fedrau 2024-06-15  252  {
134a669e205435 Dimitri Fedrau 2024-06-15  253  	struct power_supply_config psy_cfg = {};
134a669e205435 Dimitri Fedrau 2024-06-15  254  	struct device *dev = &client->dev;
134a669e205435 Dimitri Fedrau 2024-06-15  255  	struct max1720x_device_info *info;
134a669e205435 Dimitri Fedrau 2024-06-15  256  	int ret;
134a669e205435 Dimitri Fedrau 2024-06-15  257  
134a669e205435 Dimitri Fedrau 2024-06-15  258  	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
134a669e205435 Dimitri Fedrau 2024-06-15  259  	if (!info)
134a669e205435 Dimitri Fedrau 2024-06-15  260  		return -ENOMEM;
134a669e205435 Dimitri Fedrau 2024-06-15  261  
134a669e205435 Dimitri Fedrau 2024-06-15  262  	i2c_set_clientdata(client, info);
134a669e205435 Dimitri Fedrau 2024-06-15  263  	info->bat_desc.name = "max1720x";
134a669e205435 Dimitri Fedrau 2024-06-15  264  	info->bat_desc.no_thermal = true;
134a669e205435 Dimitri Fedrau 2024-06-15  265  	info->bat_desc.type = POWER_SUPPLY_TYPE_BATTERY;
134a669e205435 Dimitri Fedrau 2024-06-15  266  	info->bat_desc.properties = max1720x_battery_props;
134a669e205435 Dimitri Fedrau 2024-06-15  267  	info->bat_desc.num_properties = ARRAY_SIZE(max1720x_battery_props);
134a669e205435 Dimitri Fedrau 2024-06-15  268  	info->bat_desc.get_property = max1720x_battery_get_property;
134a669e205435 Dimitri Fedrau 2024-06-15  269  	psy_cfg.drv_data = info;
134a669e205435 Dimitri Fedrau 2024-06-15  270  
134a669e205435 Dimitri Fedrau 2024-06-15  271  	info->regmap = devm_regmap_init_i2c(client, &max1720x_regmap_cfg);
134a669e205435 Dimitri Fedrau 2024-06-15  272  	if (IS_ERR(info->regmap))
134a669e205435 Dimitri Fedrau 2024-06-15  273  		return dev_err_probe(dev, PTR_ERR(info->regmap),
134a669e205435 Dimitri Fedrau 2024-06-15  274  				     "regmap initialization failed\n");
134a669e205435 Dimitri Fedrau 2024-06-15  275  
134a669e205435 Dimitri Fedrau 2024-06-15  276  	info->ancillary = i2c_new_ancillary_device(client, "nvmem", 0xb);
134a669e205435 Dimitri Fedrau 2024-06-15  277  	if (IS_ERR(info->ancillary))
134a669e205435 Dimitri Fedrau 2024-06-15  278  		return dev_err_probe(dev, PTR_ERR(info->ancillary),
134a669e205435 Dimitri Fedrau 2024-06-15  279  				     "Failed to initialize ancillary i2c device\n");
134a669e205435 Dimitri Fedrau 2024-06-15  280  
134a669e205435 Dimitri Fedrau 2024-06-15  281  	i2c_set_clientdata(info->ancillary, info);
134a669e205435 Dimitri Fedrau 2024-06-15  282  	ret = max1720x_probe_sense_resistor(info);
134a669e205435 Dimitri Fedrau 2024-06-15  283  	if (ret) {
134a669e205435 Dimitri Fedrau 2024-06-15  284  		i2c_unregister_device(info->ancillary);
134a669e205435 Dimitri Fedrau 2024-06-15 @285  		return dev_err_probe(dev, PTR_ERR(info->bat),

s/PTR_ERR(info->bat)/ret/

134a669e205435 Dimitri Fedrau 2024-06-15  286  				     "Failed to read sense resistor value\n");
134a669e205435 Dimitri Fedrau 2024-06-15  287  	}
134a669e205435 Dimitri Fedrau 2024-06-15  288  
134a669e205435 Dimitri Fedrau 2024-06-15  289  	info->bat = devm_power_supply_register(dev, &info->bat_desc, &psy_cfg);
134a669e205435 Dimitri Fedrau 2024-06-15  290  	if (IS_ERR(info->bat)) {
134a669e205435 Dimitri Fedrau 2024-06-15  291  		i2c_unregister_device(info->ancillary);
134a669e205435 Dimitri Fedrau 2024-06-15  292  		return dev_err_probe(dev, PTR_ERR(info->bat),
134a669e205435 Dimitri Fedrau 2024-06-15  293  				     "Failed to register power supply\n");
134a669e205435 Dimitri Fedrau 2024-06-15  294  	}
134a669e205435 Dimitri Fedrau 2024-06-15  295  
134a669e205435 Dimitri Fedrau 2024-06-15  296  	return 0;
134a669e205435 Dimitri Fedrau 2024-06-15  297  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 1/2] dt-bindings: power: supply: add support for MAX17201/MAX17205 fuel gauge

[Krzysztof Kozlowski]

On 17/06/2024 14:59, Dimitri Fedrau wrote:
> Am Sun, Jun 16, 2024 at 09:27:21AM +0200 schrieb Krzysztof Kozlowski:
>> On 15/06/2024 22:33, Dimitri Fedrau wrote:
>>> Adding documentation for MAXIMs MAX17201/MAX17205 fuel gauge.
>>>
>>
>> Three patchsets within 30 minutes. No changelog et all.
>>
> Sorry, had to fix my mail address in the commit message. Changelog was
> in the cover letter. Anyway, could have fixed that in a later version.

There was no cover letter attached to this patchset. If you do not send
cover letter to interested parties, then it does not count.

> 
>> Slow down (one posting per 24h) to give people chances to review. Then
>> provide changelog under --- and describe what happened.
>>
> [...]
>>> +maintainers:
>>> +  - Dimitri Fedrau <dima.fedrau@gmail.com>
>>> +
>>> +properties:
>>> +      - description: ModelGauge m5 registers
>>> +      - description: Nonvolatile registers
>>> +
>>> +  reg-names:
>>> +    items:
>>> +      - const: m5
>>> +      - const: nvmem
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>
>> This is incomplete. Missing battery and probably more... Look how other
>> bindings are written.
>>
> Some fuel gauges used monitored-battery and/or power-supplies others none
> of them(mitsumi,mm8013.yaml). I'm not sure when to use them.

Look at your hardware, datasheet if it is available. Then look at
monitored battery properties. If you see anything in common, then that's
a sign.

I did not get your driver changes, so I cannot help here. Kind of your call.

Best regards,
Krzysztof