[v3] power: supply: add support for MAX1720x standalone fuel

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

Posted by Dimitri Fedrau 1 year, 6 months ago

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

Posted by Dan Carpenter 1 year, 6 months ago

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 2/2] power: supply: add support for MAX1720x standalone fuel gauge

Posted by Thomas Weißschuh 1 year, 6 months ago

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
>

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