[PATCH v3 0/6] power: supply: max17040 support compatible devices

[PATCH v3 0/6] power: supply: max17040 support compatible devices

[Iskren Chernev]

The max17040 fuel gauge is part of a family of 8 chips that have very similar
mode of operations and registers.

This patch set adds:
- compatible strings for all supported devices and handles the minor
  differences between them;
- handling for devices reporting double capacity via maxim,double-soc;
- handling for setting rcomp, a compensation value for more accurate reading,
  affected by battery chemistry and operating temps;
- suppot for SOC alerts (capacity changes by +/- 1%), to prevent polling every
  second;
- improved max17040 driver with regmap and devm_

The datasheets of the supported devices are linked [0] [1] [2] [3].

[0] https://datasheets.maximintegrated.com/en/ds/MAX17040-MAX17041.pdf
[1] https://datasheets.maximintegrated.com/en/ds/MAX17043-MAX17044.pdf
[2] https://datasheets.maximintegrated.com/en/ds/MAX17048-MAX17049.pdf
[3] https://datasheets.maximintegrated.com/en/ds/MAX17058-MAX17059.pdf

v2: https://lkml.org/lkml/2020/6/18/260
v1: https://lkml.org/lkml/2020/6/8/682

Changes in v2:
- remove maxim,skip-reset property in favor of device id
- split driver change into 4 pieces

Iskren Chernev (6):
  power: supply: max17040: Use regmap i2c
  dt-bindings: power: supply: Extend max17040 compatibility
  power: supply: max17040: Support compatible devices
  dt-bindings: power: supply: max17040: Add maxim,rcomp
  power: supply: max17040: Support setting rcomp
  power: supply: max17040: Support soc alert

 .../power/supply/max17040_battery.txt         |  21 +-
 drivers/power/supply/Kconfig                  |  11 +-
 drivers/power/supply/max17040_battery.c       | 473 ++++++++++++------
 3 files changed, 357 insertions(+), 148 deletions(-)


base-commit: cfafde3c949cae39483639c03c5da5fd91bb234e
--
2.27.0

[PATCH v3 1/6] power: supply: max17040: Use regmap i2c

[Iskren Chernev]

Rewrite i2c operations from i2c client read/write to regmap i2c. As
a result, most private functions now accept the private driver data
instead of an i2c client pointer.

Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
---
 drivers/power/supply/Kconfig            |   1 +
 drivers/power/supply/max17040_battery.c | 250 +++++++++++-------------
 2 files changed, 110 insertions(+), 141 deletions(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 44d3c8512fb8d..12ca79952768f 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -367,6 +367,7 @@ config AXP288_FUEL_GAUGE
 config BATTERY_MAX17040
 	tristate "Maxim MAX17040 Fuel Gauge"
 	depends on I2C
+	select REGMAP_I2C
 	help
 	  MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
 	  in handheld and portable equipment. The MAX17040 is configured
diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index 48aa44665e2f1..678241fcc2548 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -16,32 +16,30 @@
 #include <linux/interrupt.h>
 #include <linux/power_supply.h>
 #include <linux/max17040_battery.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #define MAX17040_VCELL	0x02
 #define MAX17040_SOC	0x04
 #define MAX17040_MODE	0x06
 #define MAX17040_VER	0x08
-#define MAX17040_RCOMP	0x0C
+#define MAX17040_CONFIG	0x0C
 #define MAX17040_CMD	0xFE
 
 
 #define MAX17040_DELAY		1000
 #define MAX17040_BATTERY_FULL	95
 
-#define MAX17040_ATHD_MASK		0xFFC0
+#define MAX17040_ATHD_MASK		0x3f
 #define MAX17040_ATHD_DEFAULT_POWER_UP	4
 
 struct max17040_chip {
 	struct i2c_client		*client;
+	struct regmap			*regmap;
 	struct delayed_work		work;
 	struct power_supply		*battery;
 	struct max17040_platform_data	*pdata;
 
-	/* State Of Connect */
-	int online;
-	/* battery voltage */
-	int vcell;
 	/* battery capacity */
 	int soc;
 	/* State Of Charge */
@@ -50,135 +48,68 @@ struct max17040_chip {
 	u32 low_soc_alert;
 };
 
-static int max17040_get_property(struct power_supply *psy,
-			    enum power_supply_property psp,
-			    union power_supply_propval *val)
-{
-	struct max17040_chip *chip = power_supply_get_drvdata(psy);
-
-	switch (psp) {
-	case POWER_SUPPLY_PROP_STATUS:
-		val->intval = chip->status;
-		break;
-	case POWER_SUPPLY_PROP_ONLINE:
-		val->intval = chip->online;
-		break;
-	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
-		val->intval = chip->vcell;
-		break;
-	case POWER_SUPPLY_PROP_CAPACITY:
-		val->intval = chip->soc;
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
-
-static int max17040_write_reg(struct i2c_client *client, int reg, u16 value)
-{
-	int ret;
-
-	ret = i2c_smbus_write_word_swapped(client, reg, value);
-
-	if (ret < 0)
-		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
-
-	return ret;
-}
-
-static int max17040_read_reg(struct i2c_client *client, int reg)
-{
-	int ret;
-
-	ret = i2c_smbus_read_word_swapped(client, reg);
-
-	if (ret < 0)
-		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
-
-	return ret;
-}
-
-static void max17040_reset(struct i2c_client *client)
+static int max17040_reset(struct max17040_chip *chip)
 {
-	max17040_write_reg(client, MAX17040_CMD, 0x0054);
+	return regmap_write(chip->regmap, MAX17040_CMD, 0x0054);
 }
 
-static int max17040_set_low_soc_alert(struct i2c_client *client, u32 level)
+static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
 {
-	int ret;
-	u16 data;
-
 	level = 32 - level;
-	data = max17040_read_reg(client, MAX17040_RCOMP);
-	/* clear the alrt bit and set LSb 5 bits */
-	data &= MAX17040_ATHD_MASK;
-	data |= level;
-	ret = max17040_write_reg(client, MAX17040_RCOMP, data);
-
-	return ret;
+	return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
+			MAX17040_ATHD_MASK, level);
 }
 
-static void max17040_get_vcell(struct i2c_client *client)
+static int max17040_get_vcell(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-	u16 vcell;
+	u32 vcell;
 
-	vcell = max17040_read_reg(client, MAX17040_VCELL);
+	regmap_read(chip->regmap, MAX17040_VCELL, &vcell);
 
-	chip->vcell = (vcell >> 4) * 1250;
+	return (vcell >> 4) * 1250;
 }
 
-static void max17040_get_soc(struct i2c_client *client)
+static int max17040_get_soc(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-	u16 soc;
+	u32 soc;
 
-	soc = max17040_read_reg(client, MAX17040_SOC);
+	regmap_read(chip->regmap, MAX17040_SOC, &soc);
 
-	chip->soc = (soc >> 8);
+	return soc >> 8;
 }
 
-static void max17040_get_version(struct i2c_client *client)
+static int max17040_get_version(struct max17040_chip *chip)
 {
-	u16 version;
+	int ret;
+	u32 version;
 
-	version = max17040_read_reg(client, MAX17040_VER);
+	ret = regmap_read(chip->regmap, MAX17040_VER, &version);
 
-	dev_info(&client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", version);
+	return ret ? ret : version;
 }
 
-static void max17040_get_online(struct i2c_client *client)
+static int max17040_get_online(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-
-	if (chip->pdata && chip->pdata->battery_online)
-		chip->online = chip->pdata->battery_online();
-	else
-		chip->online = 1;
+	return chip->pdata && chip->pdata->battery_online ?
+		chip->pdata->battery_online() : 1;
 }
 
-static void max17040_get_status(struct i2c_client *client)
+static int max17040_get_status(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-
 	if (!chip->pdata || !chip->pdata->charger_online
-			|| !chip->pdata->charger_enable) {
-		chip->status = POWER_SUPPLY_STATUS_UNKNOWN;
-		return;
-	}
+			|| !chip->pdata->charger_enable)
+		return POWER_SUPPLY_STATUS_UNKNOWN;
+
+	if (max17040_get_soc(chip) > MAX17040_BATTERY_FULL)
+		return POWER_SUPPLY_STATUS_FULL;
 
-	if (chip->pdata->charger_online()) {
+	if (chip->pdata->charger_online())
 		if (chip->pdata->charger_enable())
-			chip->status = POWER_SUPPLY_STATUS_CHARGING;
+			return POWER_SUPPLY_STATUS_CHARGING;
 		else
-			chip->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
-	} else {
-		chip->status = POWER_SUPPLY_STATUS_DISCHARGING;
-	}
-
-	if (chip->soc > MAX17040_BATTERY_FULL)
-		chip->status = POWER_SUPPLY_STATUS_FULL;
+			return POWER_SUPPLY_STATUS_NOT_CHARGING;
+	else
+		return POWER_SUPPLY_STATUS_DISCHARGING;
 }
 
 static int max17040_get_of_data(struct max17040_chip *chip)
@@ -190,18 +121,31 @@ static int max17040_get_of_data(struct max17040_chip *chip)
 				 "maxim,alert-low-soc-level",
 				 &chip->low_soc_alert);
 
-	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33)
+	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33) {
+		dev_err(dev, "maxim,alert-low-soc-level out of bounds\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
 
-static void max17040_check_changes(struct i2c_client *client)
+static void max17040_check_changes(struct max17040_chip *chip)
+{
+	chip->soc = max17040_get_soc(chip);
+	chip->status = max17040_get_status(chip);
+}
+
+static void max17040_queue_work(struct max17040_chip *chip)
+{
+	queue_delayed_work(system_power_efficient_wq, &chip->work,
+			   MAX17040_DELAY);
+}
+
+static void max17040_stop_work(void *data)
 {
-	max17040_get_vcell(client);
-	max17040_get_soc(client);
-	max17040_get_online(client);
-	max17040_get_status(client);
+	struct max17040_chip *chip = data;
+
+	cancel_delayed_work_sync(&chip->work);
 }
 
 static void max17040_work(struct work_struct *work)
@@ -214,30 +158,29 @@ static void max17040_work(struct work_struct *work)
 	/* store SOC and status to check changes */
 	last_soc = chip->soc;
 	last_status = chip->status;
-	max17040_check_changes(chip->client);
+	max17040_check_changes(chip);
 
 	/* check changes and send uevent */
 	if (last_soc != chip->soc || last_status != chip->status)
 		power_supply_changed(chip->battery);
 
-	queue_delayed_work(system_power_efficient_wq, &chip->work,
-			   MAX17040_DELAY);
+	max17040_queue_work(chip);
 }
 
 static irqreturn_t max17040_thread_handler(int id, void *dev)
 {
 	struct max17040_chip *chip = dev;
-	struct i2c_client *client = chip->client;
 
-	dev_warn(&client->dev, "IRQ: Alert battery low level");
+	dev_warn(&chip->client->dev, "IRQ: Alert battery low level");
+
 	/* read registers */
-	max17040_check_changes(chip->client);
+	max17040_check_changes(chip);
 
 	/* send uevent */
 	power_supply_changed(chip->battery);
 
 	/* reset alert bit */
-	max17040_set_low_soc_alert(client, chip->low_soc_alert);
+	max17040_set_low_soc_alert(chip, chip->low_soc_alert);
 
 	return IRQ_HANDLED;
 }
@@ -256,6 +199,38 @@ static int max17040_enable_alert_irq(struct max17040_chip *chip)
 	return ret;
 }
 
+static int max17040_get_property(struct power_supply *psy,
+			    enum power_supply_property psp,
+			    union power_supply_propval *val)
+{
+	struct max17040_chip *chip = power_supply_get_drvdata(psy);
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = max17040_get_status(chip);
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = max17040_get_online(chip);
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		val->intval = max17040_get_vcell(chip);
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = max17040_get_soc(chip);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static const struct regmap_config max17040_regmap = {
+	.reg_bits	= 8,
+	.reg_stride	= 2,
+	.val_bits	= 16,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+};
+
 static enum power_supply_property max17040_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_ONLINE,
@@ -287,31 +262,33 @@ static int max17040_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	chip->client = client;
+	chip->regmap = devm_regmap_init_i2c(client, &max17040_regmap);
 	chip->pdata = client->dev.platform_data;
 	ret = max17040_get_of_data(chip);
-	if (ret) {
-		dev_err(&client->dev,
-			"failed: low SOC alert OF data out of bounds\n");
+	if (ret)
 		return ret;
-	}
 
 	i2c_set_clientdata(client, chip);
 	psy_cfg.drv_data = chip;
 
-	chip->battery = power_supply_register(&client->dev,
+	chip->battery = devm_power_supply_register(&client->dev,
 				&max17040_battery_desc, &psy_cfg);
 	if (IS_ERR(chip->battery)) {
 		dev_err(&client->dev, "failed: power supply register\n");
 		return PTR_ERR(chip->battery);
 	}
 
-	max17040_reset(client);
-	max17040_get_version(client);
+	ret = max17040_get_version(chip);
+	if (ret < 0)
+		return ret;
+	dev_dbg(&chip->client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", ret);
+
+	max17040_reset(chip);
 
 	/* check interrupt */
 	if (client->irq && of_device_is_compatible(client->dev.of_node,
 						   "maxim,max77836-battery")) {
-		ret = max17040_set_low_soc_alert(client, chip->low_soc_alert);
+		ret = max17040_set_low_soc_alert(chip, chip->low_soc_alert);
 		if (ret) {
 			dev_err(&client->dev,
 				"Failed to set low SOC alert: err %d\n", ret);
@@ -327,18 +304,11 @@ static int max17040_probe(struct i2c_client *client,
 	}
 
 	INIT_DEFERRABLE_WORK(&chip->work, max17040_work);
-	queue_delayed_work(system_power_efficient_wq, &chip->work,
-			   MAX17040_DELAY);
-
-	return 0;
-}
-
-static int max17040_remove(struct i2c_client *client)
-{
-	struct max17040_chip *chip = i2c_get_clientdata(client);
+	ret = devm_add_action(&client->dev, max17040_stop_work, chip);
+	if (ret)
+		return ret;
+	max17040_queue_work(chip);
 
-	power_supply_unregister(chip->battery);
-	cancel_delayed_work(&chip->work);
 	return 0;
 }
 
@@ -362,12 +332,11 @@ static int max17040_resume(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct max17040_chip *chip = i2c_get_clientdata(client);
 
-	queue_delayed_work(system_power_efficient_wq, &chip->work,
-			   MAX17040_DELAY);
-
 	if (client->irq && device_may_wakeup(dev))
 		disable_irq_wake(client->irq);
 
+	max17040_queue_work(chip);
+
 	return 0;
 }
 
@@ -401,7 +370,6 @@ static struct i2c_driver max17040_i2c_driver = {
 		.pm	= MAX17040_PM_OPS,
 	},
 	.probe		= max17040_probe,
-	.remove		= max17040_remove,
 	.id_table	= max17040_id,
 };
 module_i2c_driver(max17040_i2c_driver);
-- 
2.27.0

[PATCH v3 2/6] dt-bindings: power: supply: Extend max17040 compatibility

[Iskren Chernev]

Maxim max17040 is a fuel gauge from a larger family utilising the Model
Gauge technology. Document all different compatible strings that the
max17040 driver recognizes.

Some devices in the wild report double the capacity. The
maxim,double-soc (from State-Of-Charge) property fixes that.

Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
---
 .../bindings/power/supply/max17040_battery.txt    | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
index 4e0186b8380fa..554bce82a08e6 100644
--- a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -2,7 +2,9 @@ max17040_battery
 ~~~~~~~~~~~~~~~~
 
 Required properties :
- - compatible : "maxim,max17040" or "maxim,max77836-battery"
+ - compatible : "maxim,max17040", "maxim,max17041", "maxim,max17043",
+ 		"maxim,max17044", "maxim,max17048", "maxim,max17049",
+		"maxim,max17058", "maxim,max17059" or "maxim,max77836-battery"
  - reg: i2c slave address
 
 Optional properties :
@@ -11,6 +13,10 @@ Optional properties :
 				generated. Can be configured from 1 up to 32
 				(%). If skipped the power up default value of
 				4 (%) will be used.
+- maxim,double-soc : 		Certain devices return double the capacity.
+				Specify this boolean property to divide the
+				reported value in 2 and thus normalize it.
+				SOC == State of Charge == Capacity.
 - interrupts : 			Interrupt line see Documentation/devicetree/
 				bindings/interrupt-controller/interrupts.txt
 - wakeup-source :		This device has wakeup capabilities. Use this
@@ -31,3 +37,10 @@ Example:
 		interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
 		wakeup-source;
 	};
+
+	battery-fuel-gauge@36 {
+		compatible = "maxim,max17048";
+		reg = <0x36>;
+		maxim,alert-low-soc-level = <10>;
+		maxim,double-soc;
+	};
-- 
2.27.0

[PATCH v3 3/6] power: supply: max17040: Support compatible devices

[Iskren Chernev]

The max17040 fuel gauge is part of a family of 8 chips that have very
similar mode of operations and registers.

This change adds:
- compatible strings for all supported devices and handling for the
  minor differences between them;
- handling for devices reporting double capacity via maxim,double-soc;

The datasheets of the supported devices are linked [0] [1] [2] [3].

[0] https://datasheets.maximintegrated.com/en/ds/MAX17040-MAX17041.pdf
[1] https://datasheets.maximintegrated.com/en/ds/MAX17043-MAX17044.pdf
[2] https://datasheets.maximintegrated.com/en/ds/MAX17048-MAX17049.pdf
[3] https://datasheets.maximintegrated.com/en/ds/MAX17058-MAX17059.pdf

Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
---
 drivers/power/supply/Kconfig            |  10 +-
 drivers/power/supply/max17040_battery.c | 143 +++++++++++++++++++++---
 2 files changed, 135 insertions(+), 18 deletions(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 12ca79952768f..0efdb282fea28 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -369,9 +369,13 @@ config BATTERY_MAX17040
 	depends on I2C
 	select REGMAP_I2C
 	help
-	  MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
-	  in handheld and portable equipment. The MAX17040 is configured
-	  to operate with a single lithium cell
+	  Maxim models with ModelGauge are fuel-gauge systems for lithium-ion
+	  (Li+) batteries in handheld and portable equipment, including
+	  max17040, max17041, max17043, max17044, max17048, max17049, max17058,
+	  max17059. It is also included in some batteries like max77836.
+
+	  Driver supports reporting SOC (State of Charge, i.e capacity),
+	  voltage and configurable low-SOC wakeup interrupt.
 
 config BATTERY_MAX17042
 	tristate "Maxim MAX17042/17047/17050/8997/8966 Fuel Gauge"
diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index 678241fcc2548..a6ecd84194e51 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -15,6 +15,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/power_supply.h>
+#include <linux/of_device.h>
 #include <linux/max17040_battery.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -33,12 +34,92 @@
 #define MAX17040_ATHD_MASK		0x3f
 #define MAX17040_ATHD_DEFAULT_POWER_UP	4
 
+enum chip_id {
+	ID_MAX17040,
+	ID_MAX17041,
+	ID_MAX17043,
+	ID_MAX17044,
+	ID_MAX17048,
+	ID_MAX17049,
+	ID_MAX17058,
+	ID_MAX17059,
+};
+
+/* values that differ by chip_id */
+struct chip_data {
+	u16 reset_val;
+	u16 vcell_shift;
+	u16 vcell_mul;
+	u16 vcell_div;
+	u8  has_low_soc_alert;
+};
+
+static struct chip_data max17040_family[] = {
+	[ID_MAX17040] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 1250,
+		.vcell_div = 1,
+		.has_low_soc_alert = 0,
+	},
+	[ID_MAX17041] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 2500,
+		.vcell_div = 1,
+		.has_low_soc_alert = 0,
+	},
+	[ID_MAX17043] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 1250,
+		.vcell_div = 1,
+		.has_low_soc_alert = 1,
+	},
+	[ID_MAX17044] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 2500,
+		.vcell_div = 1,
+		.has_low_soc_alert = 1,
+	},
+	[ID_MAX17048] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 8,
+		.has_low_soc_alert = 1,
+	},
+	[ID_MAX17049] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 4,
+		.has_low_soc_alert = 1,
+	},
+	[ID_MAX17058] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 8,
+		.has_low_soc_alert = 1,
+	},
+	[ID_MAX17059] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 4,
+		.has_low_soc_alert = 1,
+	},
+};
+
 struct max17040_chip {
 	struct i2c_client		*client;
 	struct regmap			*regmap;
 	struct delayed_work		work;
 	struct power_supply		*battery;
 	struct max17040_platform_data	*pdata;
+	struct chip_data		data;
 
 	/* battery capacity */
 	int soc;
@@ -46,11 +127,13 @@ struct max17040_chip {
 	int status;
 	/* Low alert threshold from 32% to 1% of the State of Charge */
 	u32 low_soc_alert;
+	/* some devices return twice the capacity */
+	bool quirk_double_soc;
 };
 
 static int max17040_reset(struct max17040_chip *chip)
 {
-	return regmap_write(chip->regmap, MAX17040_CMD, 0x0054);
+	return regmap_write(chip->regmap, MAX17040_CMD, chip->data.reset_val);
 }
 
 static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
@@ -60,13 +143,21 @@ static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
 			MAX17040_ATHD_MASK, level);
 }
 
+static int max17040_raw_vcell_to_uvolts(struct max17040_chip *chip, u16 vcell)
+{
+	struct chip_data *d = &chip->data;
+
+	return (vcell >> d->vcell_shift) * d->vcell_mul / d->vcell_div;
+}
+
+
 static int max17040_get_vcell(struct max17040_chip *chip)
 {
 	u32 vcell;
 
 	regmap_read(chip->regmap, MAX17040_VCELL, &vcell);
 
-	return (vcell >> 4) * 1250;
+	return max17040_raw_vcell_to_uvolts(chip, vcell);
 }
 
 static int max17040_get_soc(struct max17040_chip *chip)
@@ -75,7 +166,7 @@ static int max17040_get_soc(struct max17040_chip *chip)
 
 	regmap_read(chip->regmap, MAX17040_SOC, &soc);
 
-	return soc >> 8;
+	return soc >> (chip->quirk_double_soc ? 9 : 8);
 }
 
 static int max17040_get_version(struct max17040_chip *chip)
@@ -125,6 +216,8 @@ static int max17040_get_of_data(struct max17040_chip *chip)
 		dev_err(dev, "maxim,alert-low-soc-level out of bounds\n");
 		return -EINVAL;
 	}
+	chip->quirk_double_soc = device_property_read_bool(dev,
+							   "maxim,double-soc");
 
 	return 0;
 }
@@ -252,6 +345,7 @@ static int max17040_probe(struct i2c_client *client,
 	struct i2c_adapter *adapter = client->adapter;
 	struct power_supply_config psy_cfg = {};
 	struct max17040_chip *chip;
+	enum chip_id chip_id;
 	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
@@ -264,9 +358,14 @@ static int max17040_probe(struct i2c_client *client,
 	chip->client = client;
 	chip->regmap = devm_regmap_init_i2c(client, &max17040_regmap);
 	chip->pdata = client->dev.platform_data;
-	ret = max17040_get_of_data(chip);
-	if (ret)
-		return ret;
+	chip_id = (enum chip_id) id->driver_data;
+	if (client->dev.of_node) {
+		ret = max17040_get_of_data(chip);
+		if (ret)
+			return ret;
+		chip_id = (enum chip_id) of_device_get_match_data(&client->dev);
+	}
+	chip->data = max17040_family[chip_id];
 
 	i2c_set_clientdata(client, chip);
 	psy_cfg.drv_data = chip;
@@ -283,11 +382,11 @@ static int max17040_probe(struct i2c_client *client,
 		return ret;
 	dev_dbg(&chip->client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", ret);
 
-	max17040_reset(chip);
+	if (chip_id == ID_MAX17040 || chip_id == ID_MAX17041)
+		max17040_reset(chip);
 
 	/* check interrupt */
-	if (client->irq && of_device_is_compatible(client->dev.of_node,
-						   "maxim,max77836-battery")) {
+	if (client->irq && chip->data.has_low_soc_alert) {
 		ret = max17040_set_low_soc_alert(chip, chip->low_soc_alert);
 		if (ret) {
 			dev_err(&client->dev,
@@ -350,16 +449,30 @@ static SIMPLE_DEV_PM_OPS(max17040_pm_ops, max17040_suspend, max17040_resume);
 #endif /* CONFIG_PM_SLEEP */
 
 static const struct i2c_device_id max17040_id[] = {
-	{ "max17040" },
-	{ "max77836-battery" },
-	{ }
+	{ "max17040", ID_MAX17040 },
+	{ "max17041", ID_MAX17041 },
+	{ "max17043", ID_MAX17043 },
+	{ "max77836-battery", ID_MAX17043 },
+	{ "max17044", ID_MAX17044 },
+	{ "max17048", ID_MAX17048 },
+	{ "max17049", ID_MAX17049 },
+	{ "max17058", ID_MAX17058 },
+	{ "max17059", ID_MAX17059 },
+	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(i2c, max17040_id);
 
 static const struct of_device_id max17040_of_match[] = {
-	{ .compatible = "maxim,max17040" },
-	{ .compatible = "maxim,max77836-battery" },
-	{ },
+	{ .compatible = "maxim,max17040", .data = (void *) ID_MAX17040 },
+	{ .compatible = "maxim,max17041", .data = (void *) ID_MAX17041 },
+	{ .compatible = "maxim,max17043", .data = (void *) ID_MAX17043 },
+	{ .compatible = "maxim,max77836-battery", .data = (void *) ID_MAX17043 },
+	{ .compatible = "maxim,max17044", .data = (void *) ID_MAX17044 },
+	{ .compatible = "maxim,max17048", .data = (void *) ID_MAX17048 },
+	{ .compatible = "maxim,max17049", .data = (void *) ID_MAX17049 },
+	{ .compatible = "maxim,max17058", .data = (void *) ID_MAX17058 },
+	{ .compatible = "maxim,max17059", .data = (void *) ID_MAX17059 },
+	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, max17040_of_match);
 
-- 
2.27.0

[PATCH v3 4/6] dt-bindings: power: supply: max17040: Add maxim,rcomp

[Iskren Chernev]

To compensate for the battery chemistry and operating conditions the
chips support a compensation value. Specify one or two byte compensation
via the maxim,rcomp byte array.

Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
---
 .../devicetree/bindings/power/supply/max17040_battery.txt   | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
index 554bce82a08e6..2cf046d12d922 100644
--- a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
+++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
@@ -17,6 +17,11 @@ Optional properties :
 				Specify this boolean property to divide the
 				reported value in 2 and thus normalize it.
 				SOC == State of Charge == Capacity.
+- maxim,rcomp :			A value to compensate readings for various
+				battery chemistries and operating temperatures.
+				max17040,41 have 2 byte rcomp, default to
+				0x97 0x00. All other devices have one byte
+				rcomp, default to 0x97.
 - interrupts : 			Interrupt line see Documentation/devicetree/
 				bindings/interrupt-controller/interrupts.txt
 - wakeup-source :		This device has wakeup capabilities. Use this
@@ -41,6 +46,7 @@ Example:
 	battery-fuel-gauge@36 {
 		compatible = "maxim,max17048";
 		reg = <0x36>;
+		maxim,rcomp = /bits/ 8 <0x56>;
 		maxim,alert-low-soc-level = <10>;
 		maxim,double-soc;
 	};
-- 
2.27.0

[PATCH v3 5/6] power: supply: max17040: Support setting rcomp

[Iskren Chernev]

The Maxim ModelGauge family supports fine-tuning by setting
a compensation value (named rcomp in the docs). The value is affected by
battery chemistry and ambient temperature.

Add support for reading maxim,rcomp from DT and configuring the device
with the supplied value. Temperature adjustment is not implemented, because
there is no provision for receiving the ambient temperature.

Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
---
 drivers/power/supply/max17040_battery.c | 40 +++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index a6ecd84194e51..54393f411211e 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -30,9 +30,11 @@
 
 #define MAX17040_DELAY		1000
 #define MAX17040_BATTERY_FULL	95
+#define MAX17040_RCOMP_DEFAULT  0x9700
 
 #define MAX17040_ATHD_MASK		0x3f
 #define MAX17040_ATHD_DEFAULT_POWER_UP	4
+#define MAX17040_CFG_RCOMP_MASK		0xff00
 
 enum chip_id {
 	ID_MAX17040,
@@ -52,6 +54,7 @@ struct chip_data {
 	u16 vcell_mul;
 	u16 vcell_div;
 	u8  has_low_soc_alert;
+	u8  rcomp_bytes;
 };
 
 static struct chip_data max17040_family[] = {
@@ -61,6 +64,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 1250,
 		.vcell_div = 1,
 		.has_low_soc_alert = 0,
+		.rcomp_bytes = 2,
 	},
 	[ID_MAX17041] = {
 		.reset_val = 0x0054,
@@ -68,6 +72,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 2500,
 		.vcell_div = 1,
 		.has_low_soc_alert = 0,
+		.rcomp_bytes = 2,
 	},
 	[ID_MAX17043] = {
 		.reset_val = 0x0054,
@@ -75,6 +80,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 1250,
 		.vcell_div = 1,
 		.has_low_soc_alert = 1,
+		.rcomp_bytes = 1,
 	},
 	[ID_MAX17044] = {
 		.reset_val = 0x0054,
@@ -82,6 +88,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 2500,
 		.vcell_div = 1,
 		.has_low_soc_alert = 1,
+		.rcomp_bytes = 1,
 	},
 	[ID_MAX17048] = {
 		.reset_val = 0x5400,
@@ -89,6 +96,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 625,
 		.vcell_div = 8,
 		.has_low_soc_alert = 1,
+		.rcomp_bytes = 1,
 	},
 	[ID_MAX17049] = {
 		.reset_val = 0x5400,
@@ -96,6 +104,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 625,
 		.vcell_div = 4,
 		.has_low_soc_alert = 1,
+		.rcomp_bytes = 1,
 	},
 	[ID_MAX17058] = {
 		.reset_val = 0x5400,
@@ -103,6 +112,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 625,
 		.vcell_div = 8,
 		.has_low_soc_alert = 1,
+		.rcomp_bytes = 1,
 	},
 	[ID_MAX17059] = {
 		.reset_val = 0x5400,
@@ -110,6 +120,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_mul = 625,
 		.vcell_div = 4,
 		.has_low_soc_alert = 1,
+		.rcomp_bytes = 1,
 	},
 };
 
@@ -129,6 +140,8 @@ struct max17040_chip {
 	u32 low_soc_alert;
 	/* some devices return twice the capacity */
 	bool quirk_double_soc;
+	/* higher 8 bits for 17043+, 16 bits for 17040,41 */
+	u16 rcomp;
 };
 
 static int max17040_reset(struct max17040_chip *chip)
@@ -143,6 +156,14 @@ static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
 			MAX17040_ATHD_MASK, level);
 }
 
+static int max17040_set_rcomp(struct max17040_chip *chip, u16 rcomp)
+{
+	u16 mask = chip->data.rcomp_bytes == 2 ?
+		0xffff : MAX17040_CFG_RCOMP_MASK;
+
+	return regmap_update_bits(chip->regmap, MAX17040_CONFIG, mask, rcomp);
+}
+
 static int max17040_raw_vcell_to_uvolts(struct max17040_chip *chip, u16 vcell)
 {
 	struct chip_data *d = &chip->data;
@@ -206,6 +227,10 @@ static int max17040_get_status(struct max17040_chip *chip)
 static int max17040_get_of_data(struct max17040_chip *chip)
 {
 	struct device *dev = &chip->client->dev;
+	struct chip_data *data = &max17040_family[
+		(enum chip_id) of_device_get_match_data(dev)];
+	int rcomp_len;
+	u8 rcomp[2];
 
 	chip->low_soc_alert = MAX17040_ATHD_DEFAULT_POWER_UP;
 	device_property_read_u32(dev,
@@ -219,6 +244,19 @@ static int max17040_get_of_data(struct max17040_chip *chip)
 	chip->quirk_double_soc = device_property_read_bool(dev,
 							   "maxim,double-soc");
 
+	rcomp_len = device_property_count_u8(dev, "maxim,rcomp");
+	chip->rcomp = MAX17040_RCOMP_DEFAULT;
+	if (rcomp_len == data->rcomp_bytes) {
+		device_property_read_u8_array(dev, "maxim,rcomp",
+					      rcomp, rcomp_len);
+		chip->rcomp = rcomp_len == 2 ?
+			rcomp[0] << 8 | rcomp[1] :
+			rcomp[0] << 8;
+	} else if (rcomp_len > 0) {
+		dev_err(dev, "maxim,rcomp has incorrect length\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -385,6 +423,8 @@ static int max17040_probe(struct i2c_client *client,
 	if (chip_id == ID_MAX17040 || chip_id == ID_MAX17041)
 		max17040_reset(chip);
 
+	max17040_set_rcomp(chip, chip->rcomp);
+
 	/* check interrupt */
 	if (client->irq && chip->data.has_low_soc_alert) {
 		ret = max17040_set_low_soc_alert(chip, chip->low_soc_alert);
-- 
2.27.0

[PATCH v3 6/6] power: supply: max17040: Support soc alert

[Iskren Chernev]

max17048 and max17049 support SOC alerts (interrupts when battery
capacity changes by +/- 1%). At the moment the driver polls for changes
every second. Using the alerts removes the need for polling.

Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
---
 drivers/power/supply/max17040_battery.c | 82 ++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 9 deletions(-)

diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index 54393f411211e..4425411775f26 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -25,6 +25,7 @@
 #define MAX17040_MODE	0x06
 #define MAX17040_VER	0x08
 #define MAX17040_CONFIG	0x0C
+#define MAX17040_STATUS	0x1A
 #define MAX17040_CMD	0xFE
 
 
@@ -33,7 +34,10 @@
 #define MAX17040_RCOMP_DEFAULT  0x9700
 
 #define MAX17040_ATHD_MASK		0x3f
+#define MAX17040_ALSC_MASK		0x40
 #define MAX17040_ATHD_DEFAULT_POWER_UP	4
+#define MAX17040_STATUS_HD_MASK		0x1000
+#define MAX17040_STATUS_SC_MASK		0x2000
 #define MAX17040_CFG_RCOMP_MASK		0xff00
 
 enum chip_id {
@@ -55,6 +59,7 @@ struct chip_data {
 	u16 vcell_div;
 	u8  has_low_soc_alert;
 	u8  rcomp_bytes;
+	u8  has_soc_alert;
 };
 
 static struct chip_data max17040_family[] = {
@@ -65,6 +70,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 1,
 		.has_low_soc_alert = 0,
 		.rcomp_bytes = 2,
+		.has_soc_alert = 0,
 	},
 	[ID_MAX17041] = {
 		.reset_val = 0x0054,
@@ -73,6 +79,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 1,
 		.has_low_soc_alert = 0,
 		.rcomp_bytes = 2,
+		.has_soc_alert = 0,
 	},
 	[ID_MAX17043] = {
 		.reset_val = 0x0054,
@@ -81,6 +88,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 1,
 		.has_low_soc_alert = 1,
 		.rcomp_bytes = 1,
+		.has_soc_alert = 0,
 	},
 	[ID_MAX17044] = {
 		.reset_val = 0x0054,
@@ -89,6 +97,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 1,
 		.has_low_soc_alert = 1,
 		.rcomp_bytes = 1,
+		.has_soc_alert = 0,
 	},
 	[ID_MAX17048] = {
 		.reset_val = 0x5400,
@@ -97,6 +106,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 8,
 		.has_low_soc_alert = 1,
 		.rcomp_bytes = 1,
+		.has_soc_alert = 1,
 	},
 	[ID_MAX17049] = {
 		.reset_val = 0x5400,
@@ -105,6 +115,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 4,
 		.has_low_soc_alert = 1,
 		.rcomp_bytes = 1,
+		.has_soc_alert = 1,
 	},
 	[ID_MAX17058] = {
 		.reset_val = 0x5400,
@@ -113,6 +124,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 8,
 		.has_low_soc_alert = 1,
 		.rcomp_bytes = 1,
+		.has_soc_alert = 0,
 	},
 	[ID_MAX17059] = {
 		.reset_val = 0x5400,
@@ -121,6 +133,7 @@ static struct chip_data max17040_family[] = {
 		.vcell_div = 4,
 		.has_low_soc_alert = 1,
 		.rcomp_bytes = 1,
+		.has_soc_alert = 0,
 	},
 };
 
@@ -156,6 +169,12 @@ static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
 			MAX17040_ATHD_MASK, level);
 }
 
+static int max17040_set_soc_alert(struct max17040_chip *chip, bool enable)
+{
+	return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
+			MAX17040_ALSC_MASK, enable ? MAX17040_ALSC_MASK : 0);
+}
+
 static int max17040_set_rcomp(struct max17040_chip *chip, u16 rcomp)
 {
 	u16 mask = chip->data.rcomp_bytes == 2 ?
@@ -298,11 +317,33 @@ static void max17040_work(struct work_struct *work)
 	max17040_queue_work(chip);
 }
 
+/* Returns true if alert cause was SOC change, not low SOC */
+static bool max17040_handle_soc_alert(struct max17040_chip *chip)
+{
+	bool ret = true;
+	u32 data;
+
+	regmap_read(chip->regmap, MAX17040_STATUS, &data);
+
+	if (data & MAX17040_STATUS_HD_MASK) {
+		// this alert was caused by low soc
+		ret = false;
+	}
+	if (data & MAX17040_STATUS_SC_MASK) {
+		// soc change bit -- deassert to mark as handled
+		regmap_write(chip->regmap, MAX17040_STATUS,
+				data & ~MAX17040_STATUS_SC_MASK);
+	}
+
+	return ret;
+}
+
 static irqreturn_t max17040_thread_handler(int id, void *dev)
 {
 	struct max17040_chip *chip = dev;
 
-	dev_warn(&chip->client->dev, "IRQ: Alert battery low level");
+	if (!(chip->data.has_soc_alert && max17040_handle_soc_alert(chip)))
+		dev_warn(&chip->client->dev, "IRQ: Alert battery low level\n");
 
 	/* read registers */
 	max17040_check_changes(chip);
@@ -384,6 +425,7 @@ static int max17040_probe(struct i2c_client *client,
 	struct power_supply_config psy_cfg = {};
 	struct max17040_chip *chip;
 	enum chip_id chip_id;
+	bool enable_irq = false;
 	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
@@ -434,6 +476,27 @@ static int max17040_probe(struct i2c_client *client,
 			return ret;
 		}
 
+		enable_irq = true;
+	}
+
+	if (client->irq && chip->data.has_soc_alert) {
+		ret = max17040_set_soc_alert(chip, 1);
+		if (ret) {
+			dev_err(&client->dev,
+				"Failed to set SOC alert: err %d\n", ret);
+			return ret;
+		}
+		enable_irq = true;
+	} else {
+		/* soc alerts negate the need for polling */
+		INIT_DEFERRABLE_WORK(&chip->work, max17040_work);
+		ret = devm_add_action(&client->dev, max17040_stop_work, chip);
+		if (ret)
+			return ret;
+		max17040_queue_work(chip);
+	}
+
+	if (enable_irq) {
 		ret = max17040_enable_alert_irq(chip);
 		if (ret) {
 			client->irq = 0;
@@ -442,12 +505,6 @@ static int max17040_probe(struct i2c_client *client,
 		}
 	}
 
-	INIT_DEFERRABLE_WORK(&chip->work, max17040_work);
-	ret = devm_add_action(&client->dev, max17040_stop_work, chip);
-	if (ret)
-		return ret;
-	max17040_queue_work(chip);
-
 	return 0;
 }
 
@@ -458,7 +515,11 @@ static int max17040_suspend(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct max17040_chip *chip = i2c_get_clientdata(client);
 
-	cancel_delayed_work(&chip->work);
+	if (client->irq && chip->data.has_soc_alert)
+		// disable soc alert to prevent wakeup
+		max17040_set_soc_alert(chip, 0);
+	else
+		cancel_delayed_work(&chip->work);
 
 	if (client->irq && device_may_wakeup(dev))
 		enable_irq_wake(client->irq);
@@ -474,7 +535,10 @@ static int max17040_resume(struct device *dev)
 	if (client->irq && device_may_wakeup(dev))
 		disable_irq_wake(client->irq);
 
-	max17040_queue_work(chip);
+	if (client->irq && chip->data.has_soc_alert)
+		max17040_set_soc_alert(chip, 1);
+	else
+		max17040_queue_work(chip);
 
 	return 0;
 }
-- 
2.27.0

Re: [PATCH v3 2/6] dt-bindings: power: supply: Extend max17040 compatibility

[Rob Herring]

On Wed, Jun 24, 2020 at 06:56:29PM +0300, Iskren Chernev wrote:
> Maxim max17040 is a fuel gauge from a larger family utilising the Model
> Gauge technology. Document all different compatible strings that the
> max17040 driver recognizes.
> 
> Some devices in the wild report double the capacity. The
> maxim,double-soc (from State-Of-Charge) property fixes that.
> 
> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
> ---
>  .../bindings/power/supply/max17040_battery.txt    | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> index 4e0186b8380fa..554bce82a08e6 100644
> --- a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> @@ -2,7 +2,9 @@ max17040_battery
>  ~~~~~~~~~~~~~~~~
>  
>  Required properties :
> - - compatible : "maxim,max17040" or "maxim,max77836-battery"
> + - compatible : "maxim,max17040", "maxim,max17041", "maxim,max17043",
> + 		"maxim,max17044", "maxim,max17048", "maxim,max17049",
> +		"maxim,max17058", "maxim,max17059" or "maxim,max77836-battery"
>   - reg: i2c slave address
>  
>  Optional properties :
> @@ -11,6 +13,10 @@ Optional properties :
>  				generated. Can be configured from 1 up to 32
>  				(%). If skipped the power up default value of
>  				4 (%) will be used.
> +- maxim,double-soc : 		Certain devices return double the capacity.
> +				Specify this boolean property to divide the
> +				reported value in 2 and thus normalize it.
> +				SOC == State of Charge == Capacity.

This can't be implied by the compatible string?

>  - interrupts : 			Interrupt line see Documentation/devicetree/
>  				bindings/interrupt-controller/interrupts.txt
>  - wakeup-source :		This device has wakeup capabilities. Use this
> @@ -31,3 +37,10 @@ Example:
>  		interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>  		wakeup-source;
>  	};
> +
> +	battery-fuel-gauge@36 {
> +		compatible = "maxim,max17048";
> +		reg = <0x36>;
> +		maxim,alert-low-soc-level = <10>;
> +		maxim,double-soc;
> +	};
> -- 
> 2.27.0
> 

Re: [PATCH v3 4/6] dt-bindings: power: supply: max17040: Add maxim,rcomp

[Rob Herring]

On Wed, 24 Jun 2020 18:56:31 +0300, Iskren Chernev wrote:
> To compensate for the battery chemistry and operating conditions the
> chips support a compensation value. Specify one or two byte compensation
> via the maxim,rcomp byte array.
> 
> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
> ---
>  .../devicetree/bindings/power/supply/max17040_battery.txt   | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 2/6] dt-bindings: power: supply: Extend max17040 compatibility

[Iskren Chernev]

On 7/13/20 10:03 PM, Rob Herring wrote:
> On Wed, Jun 24, 2020 at 06:56:29PM +0300, Iskren Chernev wrote:
>> Maxim max17040 is a fuel gauge from a larger family utilising the Model
>> Gauge technology. Document all different compatible strings that the
>> max17040 driver recognizes.
>>
>> Some devices in the wild report double the capacity. The
>> maxim,double-soc (from State-Of-Charge) property fixes that.
>>
>> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
>> ---
>>  .../bindings/power/supply/max17040_battery.txt    | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>> index 4e0186b8380fa..554bce82a08e6 100644
>> --- a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
>> @@ -2,7 +2,9 @@ max17040_battery
>>  ~~~~~~~~~~~~~~~~
>>
>>  Required properties :
>> - - compatible : "maxim,max17040" or "maxim,max77836-battery"
>> + - compatible : "maxim,max17040", "maxim,max17041", "maxim,max17043",
>> +         "maxim,max17044", "maxim,max17048", "maxim,max17049",
>> +        "maxim,max17058", "maxim,max17059" or "maxim,max77836-battery"
>>   - reg: i2c slave address
>>
>>  Optional properties :
>> @@ -11,6 +13,10 @@ Optional properties :
>>                  generated. Can be configured from 1 up to 32
>>                  (%). If skipped the power up default value of
>>                  4 (%) will be used.
>> +- maxim,double-soc :         Certain devices return double the capacity.
>> +                Specify this boolean property to divide the
>> +                reported value in 2 and thus normalize it.
>> +                SOC == State of Charge == Capacity.
>
> This can't be implied by the compatible string?
>

From what I can tell - no. Here are multiple examples of downstream code:

For max17043:
- single soc [1]
- double soc [2]
- both (toggle with macro) [3]

For max17048:
- single soc [4]
- double soc [5]
- both (toggle with dts) [6], docs [7]

For max17058:
- both (toggle with macro) [8], this device is single
- both (toggle with dts) [9], this device is double [10]

[1] https://github.com/LineageOS/lge-kernel-sniper/blob/9907b1312e9b4c5c4f73ac9bf2e772b12e1c9145/drivers/power/max17043_fuelgauge.c#L383
[2] https://github.com/LineageOS/android_kernel_lge_v500/blob/b4fe00e1f8f09c173a6c28a42ca69ff9529cc13b/drivers/power/max17043_fuelgauge.c#L307
[3] https://github.com/LineageOS/lge-kernel-p880/blob/c5795644a60338f88c7aa29208efadde835ea769/drivers/power/max17043_fuelgauge.c#L406
[4] https://github.com/LineageOS/lge-kernel-star/blob/d963160ebd8e64263ed740d5f1e1a0324085a826/drivers/power/max17048_battery.c#L168
[5] https://github.com/LineageOS/android_kernel_samsung_p4/blob/b190cf1bf4ca0e597a51c820a323f2aa3b2c8585/drivers/power/max17048_battery.c#L192
[6] https://github.com/LineageOS/android_kernel_htc_flounder/blob/03e0b4f36fc60c226adacdb48306df9ec65de33b/drivers/power/max17048_battery.c#L248
[7] https://github.com/LineageOS/android_kernel_htc_flounder/blob/03e0b4f36fc60c226adacdb48306df9ec65de33b/Documentation/devicetree/bindings/power_supply/max17048_battery.txt#L23
[8] https://github.com/LineageOS/android_kernel_asus_moorefield/blob/c3eae894ce8092c2a9a51f9a4924c8df714d6b3c/drivers/power/ASUS_BATTERY/max17058_battery.c#L551
[9] https://github.com/LineageOS/android_kernel_motorola_msm8916/blob/415000d938de1aa46206043e06f033edf33557ce/drivers/power/max17058_battery.c#L225
[10] https://github.com/LineageOS/android_kernel_motorola_msm8916/blob/415000d938de1aa46206043e06f033edf33557ce/arch/arm/boot/dts/qcom/msm8916-harpia-p0a.dts#L59

>>  - interrupts :             Interrupt line see Documentation/devicetree/
>>                  bindings/interrupt-controller/interrupts.txt
>>  - wakeup-source :        This device has wakeup capabilities. Use this
>> @@ -31,3 +37,10 @@ Example:
>>          interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>>          wakeup-source;
>>      };
>> +
>> +    battery-fuel-gauge@36 {
>> +        compatible = "maxim,max17048";
>> +        reg = <0x36>;
>> +        maxim,alert-low-soc-level = <10>;
>> +        maxim,double-soc;
>> +    };
>> --
>> 2.27.0
>>

Re: [PATCH v3 2/6] dt-bindings: power: supply: Extend max17040 compatibility

[Rob Herring]

On Tue, Jul 14, 2020 at 11:49:44AM +0300, Iskren Chernev wrote:
> 
> On 7/13/20 10:03 PM, Rob Herring wrote:
> > On Wed, Jun 24, 2020 at 06:56:29PM +0300, Iskren Chernev wrote:
> >> Maxim max17040 is a fuel gauge from a larger family utilising the Model
> >> Gauge technology. Document all different compatible strings that the
> >> max17040 driver recognizes.
> >>
> >> Some devices in the wild report double the capacity. The
> >> maxim,double-soc (from State-Of-Charge) property fixes that.
> >>
> >> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
> >> ---
> >>  .../bindings/power/supply/max17040_battery.txt    | 15 ++++++++++++++-
> >>  1 file changed, 14 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> >> index 4e0186b8380fa..554bce82a08e6 100644
> >> --- a/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> >> +++ b/Documentation/devicetree/bindings/power/supply/max17040_battery.txt
> >> @@ -2,7 +2,9 @@ max17040_battery
> >>  ~~~~~~~~~~~~~~~~
> >>
> >>  Required properties :
> >> - - compatible : "maxim,max17040" or "maxim,max77836-battery"
> >> + - compatible : "maxim,max17040", "maxim,max17041", "maxim,max17043",
> >> +         "maxim,max17044", "maxim,max17048", "maxim,max17049",
> >> +        "maxim,max17058", "maxim,max17059" or "maxim,max77836-battery"
> >>   - reg: i2c slave address
> >>
> >>  Optional properties :
> >> @@ -11,6 +13,10 @@ Optional properties :
> >>                  generated. Can be configured from 1 up to 32
> >>                  (%). If skipped the power up default value of
> >>                  4 (%) will be used.
> >> +- maxim,double-soc :         Certain devices return double the capacity.
> >> +                Specify this boolean property to divide the
> >> +                reported value in 2 and thus normalize it.
> >> +                SOC == State of Charge == Capacity.
> >
> > This can't be implied by the compatible string?
> >
> 
> >From what I can tell - no. Here are multiple examples of downstream code:

Okay,

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 1/6] power: supply: max17040: Use regmap i2c

[Sebastian Reichel]

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

Hi,

On Wed, Jun 24, 2020 at 06:56:28PM +0300, Iskren Chernev wrote:
> Rewrite i2c operations from i2c client read/write to regmap i2c. As
> a result, most private functions now accept the private driver data
> instead of an i2c client pointer.
> 
> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com>
> ---

This still does two things: converting the driver to devm_
and using regmap. Please split into two patches. Otherwise
the changes look good, but the patchset needs to be rebased.

-- Sebastian

>  drivers/power/supply/Kconfig            |   1 +
>  drivers/power/supply/max17040_battery.c | 250 +++++++++++-------------
>  2 files changed, 110 insertions(+), 141 deletions(-)
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 44d3c8512fb8d..12ca79952768f 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -367,6 +367,7 @@ config AXP288_FUEL_GAUGE
>  config BATTERY_MAX17040
>  	tristate "Maxim MAX17040 Fuel Gauge"
>  	depends on I2C
> +	select REGMAP_I2C
>  	help
>  	  MAX17040 is fuel-gauge systems for lithium-ion (Li+) batteries
>  	  in handheld and portable equipment. The MAX17040 is configured
> diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
> index 48aa44665e2f1..678241fcc2548 100644
> --- a/drivers/power/supply/max17040_battery.c
> +++ b/drivers/power/supply/max17040_battery.c
> @@ -16,32 +16,30 @@
>  #include <linux/interrupt.h>
>  #include <linux/power_supply.h>
>  #include <linux/max17040_battery.h>
> +#include <linux/regmap.h>
>  #include <linux/slab.h>
>  
>  #define MAX17040_VCELL	0x02
>  #define MAX17040_SOC	0x04
>  #define MAX17040_MODE	0x06
>  #define MAX17040_VER	0x08
> -#define MAX17040_RCOMP	0x0C
> +#define MAX17040_CONFIG	0x0C
>  #define MAX17040_CMD	0xFE
>  
>  
>  #define MAX17040_DELAY		1000
>  #define MAX17040_BATTERY_FULL	95
>  
> -#define MAX17040_ATHD_MASK		0xFFC0
> +#define MAX17040_ATHD_MASK		0x3f
>  #define MAX17040_ATHD_DEFAULT_POWER_UP	4
>  
>  struct max17040_chip {
>  	struct i2c_client		*client;
> +	struct regmap			*regmap;
>  	struct delayed_work		work;
>  	struct power_supply		*battery;
>  	struct max17040_platform_data	*pdata;
>  
> -	/* State Of Connect */
> -	int online;
> -	/* battery voltage */
> -	int vcell;
>  	/* battery capacity */
>  	int soc;
>  	/* State Of Charge */
> @@ -50,135 +48,68 @@ struct max17040_chip {
>  	u32 low_soc_alert;
>  };
>  
> -static int max17040_get_property(struct power_supply *psy,
> -			    enum power_supply_property psp,
> -			    union power_supply_propval *val)
> -{
> -	struct max17040_chip *chip = power_supply_get_drvdata(psy);
> -
> -	switch (psp) {
> -	case POWER_SUPPLY_PROP_STATUS:
> -		val->intval = chip->status;
> -		break;
> -	case POWER_SUPPLY_PROP_ONLINE:
> -		val->intval = chip->online;
> -		break;
> -	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> -		val->intval = chip->vcell;
> -		break;
> -	case POWER_SUPPLY_PROP_CAPACITY:
> -		val->intval = chip->soc;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -	return 0;
> -}
> -
> -static int max17040_write_reg(struct i2c_client *client, int reg, u16 value)
> -{
> -	int ret;
> -
> -	ret = i2c_smbus_write_word_swapped(client, reg, value);
> -
> -	if (ret < 0)
> -		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
> -
> -	return ret;
> -}
> -
> -static int max17040_read_reg(struct i2c_client *client, int reg)
> -{
> -	int ret;
> -
> -	ret = i2c_smbus_read_word_swapped(client, reg);
> -
> -	if (ret < 0)
> -		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
> -
> -	return ret;
> -}
> -
> -static void max17040_reset(struct i2c_client *client)
> +static int max17040_reset(struct max17040_chip *chip)
>  {
> -	max17040_write_reg(client, MAX17040_CMD, 0x0054);
> +	return regmap_write(chip->regmap, MAX17040_CMD, 0x0054);
>  }
>  
> -static int max17040_set_low_soc_alert(struct i2c_client *client, u32 level)
> +static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
>  {
> -	int ret;
> -	u16 data;
> -
>  	level = 32 - level;
> -	data = max17040_read_reg(client, MAX17040_RCOMP);
> -	/* clear the alrt bit and set LSb 5 bits */
> -	data &= MAX17040_ATHD_MASK;
> -	data |= level;
> -	ret = max17040_write_reg(client, MAX17040_RCOMP, data);
> -
> -	return ret;
> +	return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
> +			MAX17040_ATHD_MASK, level);
>  }
>  
> -static void max17040_get_vcell(struct i2c_client *client)
> +static int max17040_get_vcell(struct max17040_chip *chip)
>  {
> -	struct max17040_chip *chip = i2c_get_clientdata(client);
> -	u16 vcell;
> +	u32 vcell;
>  
> -	vcell = max17040_read_reg(client, MAX17040_VCELL);
> +	regmap_read(chip->regmap, MAX17040_VCELL, &vcell);
>  
> -	chip->vcell = (vcell >> 4) * 1250;
> +	return (vcell >> 4) * 1250;
>  }
>  
> -static void max17040_get_soc(struct i2c_client *client)
> +static int max17040_get_soc(struct max17040_chip *chip)
>  {
> -	struct max17040_chip *chip = i2c_get_clientdata(client);
> -	u16 soc;
> +	u32 soc;
>  
> -	soc = max17040_read_reg(client, MAX17040_SOC);
> +	regmap_read(chip->regmap, MAX17040_SOC, &soc);
>  
> -	chip->soc = (soc >> 8);
> +	return soc >> 8;
>  }
>  
> -static void max17040_get_version(struct i2c_client *client)
> +static int max17040_get_version(struct max17040_chip *chip)
>  {
> -	u16 version;
> +	int ret;
> +	u32 version;
>  
> -	version = max17040_read_reg(client, MAX17040_VER);
> +	ret = regmap_read(chip->regmap, MAX17040_VER, &version);
>  
> -	dev_info(&client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", version);
> +	return ret ? ret : version;
>  }
>  
> -static void max17040_get_online(struct i2c_client *client)
> +static int max17040_get_online(struct max17040_chip *chip)
>  {
> -	struct max17040_chip *chip = i2c_get_clientdata(client);
> -
> -	if (chip->pdata && chip->pdata->battery_online)
> -		chip->online = chip->pdata->battery_online();
> -	else
> -		chip->online = 1;
> +	return chip->pdata && chip->pdata->battery_online ?
> +		chip->pdata->battery_online() : 1;
>  }
>  
> -static void max17040_get_status(struct i2c_client *client)
> +static int max17040_get_status(struct max17040_chip *chip)
>  {
> -	struct max17040_chip *chip = i2c_get_clientdata(client);
> -
>  	if (!chip->pdata || !chip->pdata->charger_online
> -			|| !chip->pdata->charger_enable) {
> -		chip->status = POWER_SUPPLY_STATUS_UNKNOWN;
> -		return;
> -	}
> +			|| !chip->pdata->charger_enable)
> +		return POWER_SUPPLY_STATUS_UNKNOWN;
> +
> +	if (max17040_get_soc(chip) > MAX17040_BATTERY_FULL)
> +		return POWER_SUPPLY_STATUS_FULL;
>  
> -	if (chip->pdata->charger_online()) {
> +	if (chip->pdata->charger_online())
>  		if (chip->pdata->charger_enable())
> -			chip->status = POWER_SUPPLY_STATUS_CHARGING;
> +			return POWER_SUPPLY_STATUS_CHARGING;
>  		else
> -			chip->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> -	} else {
> -		chip->status = POWER_SUPPLY_STATUS_DISCHARGING;
> -	}
> -
> -	if (chip->soc > MAX17040_BATTERY_FULL)
> -		chip->status = POWER_SUPPLY_STATUS_FULL;
> +			return POWER_SUPPLY_STATUS_NOT_CHARGING;
> +	else
> +		return POWER_SUPPLY_STATUS_DISCHARGING;
>  }
>  
>  static int max17040_get_of_data(struct max17040_chip *chip)
> @@ -190,18 +121,31 @@ static int max17040_get_of_data(struct max17040_chip *chip)
>  				 "maxim,alert-low-soc-level",
>  				 &chip->low_soc_alert);
>  
> -	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33)
> +	if (chip->low_soc_alert <= 0 || chip->low_soc_alert >= 33) {
> +		dev_err(dev, "maxim,alert-low-soc-level out of bounds\n");
>  		return -EINVAL;
> +	}
>  
>  	return 0;
>  }
>  
> -static void max17040_check_changes(struct i2c_client *client)
> +static void max17040_check_changes(struct max17040_chip *chip)
> +{
> +	chip->soc = max17040_get_soc(chip);
> +	chip->status = max17040_get_status(chip);
> +}
> +
> +static void max17040_queue_work(struct max17040_chip *chip)
> +{
> +	queue_delayed_work(system_power_efficient_wq, &chip->work,
> +			   MAX17040_DELAY);
> +}
> +
> +static void max17040_stop_work(void *data)
>  {
> -	max17040_get_vcell(client);
> -	max17040_get_soc(client);
> -	max17040_get_online(client);
> -	max17040_get_status(client);
> +	struct max17040_chip *chip = data;
> +
> +	cancel_delayed_work_sync(&chip->work);
>  }
>  
>  static void max17040_work(struct work_struct *work)
> @@ -214,30 +158,29 @@ static void max17040_work(struct work_struct *work)
>  	/* store SOC and status to check changes */
>  	last_soc = chip->soc;
>  	last_status = chip->status;
> -	max17040_check_changes(chip->client);
> +	max17040_check_changes(chip);
>  
>  	/* check changes and send uevent */
>  	if (last_soc != chip->soc || last_status != chip->status)
>  		power_supply_changed(chip->battery);
>  
> -	queue_delayed_work(system_power_efficient_wq, &chip->work,
> -			   MAX17040_DELAY);
> +	max17040_queue_work(chip);
>  }
>  
>  static irqreturn_t max17040_thread_handler(int id, void *dev)
>  {
>  	struct max17040_chip *chip = dev;
> -	struct i2c_client *client = chip->client;
>  
> -	dev_warn(&client->dev, "IRQ: Alert battery low level");
> +	dev_warn(&chip->client->dev, "IRQ: Alert battery low level");
> +
>  	/* read registers */
> -	max17040_check_changes(chip->client);
> +	max17040_check_changes(chip);
>  
>  	/* send uevent */
>  	power_supply_changed(chip->battery);
>  
>  	/* reset alert bit */
> -	max17040_set_low_soc_alert(client, chip->low_soc_alert);
> +	max17040_set_low_soc_alert(chip, chip->low_soc_alert);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -256,6 +199,38 @@ static int max17040_enable_alert_irq(struct max17040_chip *chip)
>  	return ret;
>  }
>  
> +static int max17040_get_property(struct power_supply *psy,
> +			    enum power_supply_property psp,
> +			    union power_supply_propval *val)
> +{
> +	struct max17040_chip *chip = power_supply_get_drvdata(psy);
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		val->intval = max17040_get_status(chip);
> +		break;
> +	case POWER_SUPPLY_PROP_ONLINE:
> +		val->intval = max17040_get_online(chip);
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		val->intval = max17040_get_vcell(chip);
> +		break;
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		val->intval = max17040_get_soc(chip);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static const struct regmap_config max17040_regmap = {
> +	.reg_bits	= 8,
> +	.reg_stride	= 2,
> +	.val_bits	= 16,
> +	.val_format_endian = REGMAP_ENDIAN_BIG,
> +};
> +
>  static enum power_supply_property max17040_battery_props[] = {
>  	POWER_SUPPLY_PROP_STATUS,
>  	POWER_SUPPLY_PROP_ONLINE,
> @@ -287,31 +262,33 @@ static int max17040_probe(struct i2c_client *client,
>  		return -ENOMEM;
>  
>  	chip->client = client;
> +	chip->regmap = devm_regmap_init_i2c(client, &max17040_regmap);
>  	chip->pdata = client->dev.platform_data;
>  	ret = max17040_get_of_data(chip);
> -	if (ret) {
> -		dev_err(&client->dev,
> -			"failed: low SOC alert OF data out of bounds\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	i2c_set_clientdata(client, chip);
>  	psy_cfg.drv_data = chip;
>  
> -	chip->battery = power_supply_register(&client->dev,
> +	chip->battery = devm_power_supply_register(&client->dev,
>  				&max17040_battery_desc, &psy_cfg);
>  	if (IS_ERR(chip->battery)) {
>  		dev_err(&client->dev, "failed: power supply register\n");
>  		return PTR_ERR(chip->battery);
>  	}
>  
> -	max17040_reset(client);
> -	max17040_get_version(client);
> +	ret = max17040_get_version(chip);
> +	if (ret < 0)
> +		return ret;
> +	dev_dbg(&chip->client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", ret);
> +
> +	max17040_reset(chip);
>  
>  	/* check interrupt */
>  	if (client->irq && of_device_is_compatible(client->dev.of_node,
>  						   "maxim,max77836-battery")) {
> -		ret = max17040_set_low_soc_alert(client, chip->low_soc_alert);
> +		ret = max17040_set_low_soc_alert(chip, chip->low_soc_alert);
>  		if (ret) {
>  			dev_err(&client->dev,
>  				"Failed to set low SOC alert: err %d\n", ret);
> @@ -327,18 +304,11 @@ static int max17040_probe(struct i2c_client *client,
>  	}
>  
>  	INIT_DEFERRABLE_WORK(&chip->work, max17040_work);
> -	queue_delayed_work(system_power_efficient_wq, &chip->work,
> -			   MAX17040_DELAY);
> -
> -	return 0;
> -}
> -
> -static int max17040_remove(struct i2c_client *client)
> -{
> -	struct max17040_chip *chip = i2c_get_clientdata(client);
> +	ret = devm_add_action(&client->dev, max17040_stop_work, chip);
> +	if (ret)
> +		return ret;
> +	max17040_queue_work(chip);
>  
> -	power_supply_unregister(chip->battery);
> -	cancel_delayed_work(&chip->work);
>  	return 0;
>  }
>  
> @@ -362,12 +332,11 @@ static int max17040_resume(struct device *dev)
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct max17040_chip *chip = i2c_get_clientdata(client);
>  
> -	queue_delayed_work(system_power_efficient_wq, &chip->work,
> -			   MAX17040_DELAY);
> -
>  	if (client->irq && device_may_wakeup(dev))
>  		disable_irq_wake(client->irq);
>  
> +	max17040_queue_work(chip);
> +
>  	return 0;
>  }
>  
> @@ -401,7 +370,6 @@ static struct i2c_driver max17040_i2c_driver = {
>  		.pm	= MAX17040_PM_OPS,
>  	},
>  	.probe		= max17040_probe,
> -	.remove		= max17040_remove,
>  	.id_table	= max17040_id,
>  };
>  module_i2c_driver(max17040_i2c_driver);
> -- 
> 2.27.0
> 

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