[PATCH 1/2] dt-bindints: power: supply: Document max17040 extensions

[PATCH 1/2] dt-bindints: power: supply: Document max17040 extensions

[Iskren Chernev]

Maxim max17040 is a fuel gauge from a larger family utilising the Model
Gauge thechnology. 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.

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>
---
 .../power/supply/max17040_battery.txt         | 22 ++++++++++++++++++-
 1 file changed, 21 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..be11cab4530b9 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,15 @@ 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.
+- 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
@@ -27,7 +38,16 @@ Example:
 		compatible = "maxim,max77836-battery";
 		reg = <0x36>;
 		maxim,alert-low-soc-level = <10>;
+		maxim,rcomp = /bits/ 8 <0x97 0x00>;
 		interrupt-parent = <&gpio7>;
 		interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
 		wakeup-source;
 	};
+
+	battery-fuel-gauge@36 {
+		compatible = "maxim,max17048";
+		reg = <0x36>;
+		maxim,rcomp = /bits/ 8 <0x97>;
+		maxim,alert-low-soc-level = <10>;
+		maxim,double-soc;
+	};

base-commit: 1713116fa907cc7290020f0d8632ec646d2936f8
-- 
2.27.0

[PATCH 2/2] 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 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;
- redo i2c read/writes via regmap

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            |  11 +-
 drivers/power/supply/max17040_battery.c | 458 +++++++++++++++++-------
 2 files changed, 330 insertions(+), 139 deletions(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 44d3c8512fb8d..c5580b858ce4b 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -368,9 +368,14 @@ config BATTERY_MAX17040
 	tristate "Maxim MAX17040 Fuel Gauge"
 	depends on 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, include max17040,
+	  max17041, max17043, max17044, max17048, max17049, max17058, max17059.
+	  It is also included in some mfd devices like max77836.
+
+	  Driver supports reporting SOC (State of Charge, i.e capacity),
+	  voltage and configurable low-SOC wakeup interrupt. For certain models
+	  a SOC change interrupt is also supported.
 
 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 48aa44665e2f1..4fd6cf7b60c71 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -15,193 +15,336 @@
 #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>
 
 #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_STATUS	0x1A
 #define MAX17040_CMD	0xFE
 
 
 #define MAX17040_DELAY		1000
 #define MAX17040_BATTERY_FULL	95
+#define MAX17040_RCOMP_DEFAULT  0x9700
 
-#define MAX17040_ATHD_MASK		0xFFC0
+#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 {
+	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  rcomp_bytes;
+	u8  has_low_soc_alert;
+	u8  has_soc_alert;
+};
+
+struct chip_data max17040_family[] = {
+	[ID_MAX17040] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 1250,
+		.vcell_div = 1,
+		.rcomp_bytes = 2,
+		.has_low_soc_alert = 0,
+		.has_soc_alert = 0,
+	},
+	[ID_MAX17041] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 2500,
+		.vcell_div = 1,
+		.rcomp_bytes = 2,
+		.has_low_soc_alert = 0,
+		.has_soc_alert = 0,
+	},
+	[ID_MAX17043] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 1250,
+		.vcell_div = 1,
+		.rcomp_bytes = 1,
+		.has_low_soc_alert = 1,
+		.has_soc_alert = 0,
+	},
+	[ID_MAX17044] = {
+		.reset_val = 0x0054,
+		.vcell_shift = 4,
+		.vcell_mul = 2500,
+		.vcell_div = 1,
+		.rcomp_bytes = 1,
+		.has_low_soc_alert = 1,
+		.has_soc_alert = 0,
+	},
+	[ID_MAX17048] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 8,
+		.rcomp_bytes = 1,
+		.has_low_soc_alert = 1,
+		.has_soc_alert = 1,
+	},
+	[ID_MAX17049] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 4,
+		.rcomp_bytes = 1,
+		.has_low_soc_alert = 1,
+		.has_soc_alert = 1,
+	},
+	[ID_MAX17058] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 8,
+		.rcomp_bytes = 1,
+		.has_low_soc_alert = 1,
+		.has_soc_alert = 0,
+	},
+	[ID_MAX17059] = {
+		.reset_val = 0x5400,
+		.vcell_shift = 0,
+		.vcell_mul = 625,
+		.vcell_div = 4,
+		.rcomp_bytes = 1,
+		.has_low_soc_alert = 1,
+		.has_soc_alert = 0,
+	},
+};
 
 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;
 
-	/* State Of Connect */
-	int online;
-	/* battery voltage */
-	int vcell;
 	/* battery capacity */
 	int soc;
 	/* State Of Charge */
 	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;
+	/* higher 8 bits for 17043+, 16 bits for 17040,41 */
+	u16 rcomp;
 };
 
-static int max17040_get_property(struct power_supply *psy,
-			    enum power_supply_property psp,
-			    union power_supply_propval *val)
+static int max17040_reset(struct max17040_chip *chip)
 {
-	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;
+	return regmap_write(chip->regmap, MAX17040_CMD, chip->data.reset_val);
 }
 
-static int max17040_write_reg(struct i2c_client *client, int reg, u16 value)
+static int max17040_set_low_soc_alert(struct max17040_chip *chip, u32 level)
 {
-	int ret;
-
-	ret = i2c_smbus_write_word_swapped(client, reg, value);
-
-	if (ret < 0)
-		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
+	level = 32 - level;
+	return regmap_update_bits(chip->regmap, MAX17040_CONFIG,
+			MAX17040_ATHD_MASK, level);
+}
 
-	return ret;
+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_read_reg(struct i2c_client *client, int reg)
+/* Returns true iff alert cause was SOC change, not low SOC */
+static bool max17040_handle_soc_alert(struct max17040_chip *chip)
 {
-	int ret;
+	bool ret = true;
+	u32 data;
 
-	ret = i2c_smbus_read_word_swapped(client, reg);
+	regmap_read(chip->regmap, MAX17040_STATUS, &data);
 
-	if (ret < 0)
-		dev_err(&client->dev, "%s: err %d\n", __func__, ret);
+	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 void max17040_reset(struct i2c_client *client)
+static int max17040_set_rcomp(struct max17040_chip *chip, u16 rcomp)
 {
-	max17040_write_reg(client, MAX17040_CMD, 0x0054);
+	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_set_low_soc_alert(struct i2c_client *client, u32 level)
+static int max17040_raw_vcell_to_uvolts(struct max17040_chip *chip, u16 vcell)
 {
-	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);
+	struct chip_data *d = &chip->data;
 
-	return ret;
+	return (vcell >> d->vcell_shift) * d->vcell_mul / d->vcell_div;
 }
 
-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 max17040_raw_vcell_to_uvolts(chip, vcell);
 }
 
-static void max17040_get_soc(struct i2c_client *client)
+static int max17040_raw_soc_to_percent(struct max17040_chip *chip, u16 soc)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
-	u16 soc;
+	int shift = 8;
 
-	soc = max17040_read_reg(client, MAX17040_SOC);
+	if (chip->quirk_double_soc)
+		shift += 1;
 
-	chip->soc = (soc >> 8);
+	return soc >> shift;
 }
 
-static void max17040_get_version(struct i2c_client *client)
+static int max17040_get_soc(struct max17040_chip *chip)
 {
-	u16 version;
+	u32 soc;
 
-	version = max17040_read_reg(client, MAX17040_VER);
+	regmap_read(chip->regmap, MAX17040_SOC, &soc);
 
-	dev_info(&client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", version);
+	return max17040_raw_soc_to_percent(chip, soc);
 }
 
-static void max17040_get_online(struct i2c_client *client)
+static void max17040_get_version(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
+	u32 version;
 
-	if (chip->pdata && chip->pdata->battery_online)
-		chip->online = chip->pdata->battery_online();
-	else
-		chip->online = 1;
+	regmap_read(chip->regmap, MAX17040_VER, &version);
+
+	dev_dbg(&chip->client->dev, "MAX17040 Fuel-Gauge Ver 0x%x\n", version);
 }
 
-static void max17040_get_status(struct i2c_client *client)
+static int max17040_get_online(struct max17040_chip *chip)
 {
-	struct max17040_chip *chip = i2c_get_clientdata(client);
+	return chip->pdata && chip->pdata->battery_online ?
+		chip->pdata->battery_online() : 1;
+}
 
+static int max17040_get_status(struct max17040_chip *chip)
+{
 	if (!chip->pdata || !chip->pdata->charger_online
 			|| !chip->pdata->charger_enable) {
-		chip->status = POWER_SUPPLY_STATUS_UNKNOWN;
-		return;
+		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_enable())
-			chip->status = POWER_SUPPLY_STATUS_CHARGING;
+			return POWER_SUPPLY_STATUS_CHARGING;
 		else
-			chip->status = POWER_SUPPLY_STATUS_NOT_CHARGING;
+			return POWER_SUPPLY_STATUS_NOT_CHARGING;
 	} else {
-		chip->status = POWER_SUPPLY_STATUS_DISCHARGING;
+		return POWER_SUPPLY_STATUS_DISCHARGING;
 	}
+}
 
-	if (chip->soc > MAX17040_BATTERY_FULL)
-		chip->status = POWER_SUPPLY_STATUS_FULL;
+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 int max17040_get_of_data(struct max17040_chip *chip)
 {
 	struct device *dev = &chip->client->dev;
+	int rcomp_len;
+	u8 rcomp[2];
 
 	chip->low_soc_alert = MAX17040_ATHD_DEFAULT_POWER_UP;
 	device_property_read_u32(dev,
 				 "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;
+	}
+
+	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 == chip->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;
 }
 
-static void max17040_check_changes(struct i2c_client *client)
+static void max17040_check_changes(struct max17040_chip *chip)
 {
-	max17040_get_vcell(client);
-	max17040_get_soc(client);
-	max17040_get_online(client);
-	max17040_get_status(client);
+	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_work(struct work_struct *work)
@@ -214,35 +357,42 @@ 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 void max17040_stop_work(void *data)
+{
+	struct max17040_chip *chip = data;
+
+	cancel_delayed_work_sync(&chip->work);
 }
 
 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");
+	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->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);
+	/* reset low soc alert bit */
+	max17040_set_low_soc_alert(chip, chip->low_soc_alert);
 
 	return IRQ_HANDLED;
 }
 
-static int max17040_enable_alert_irq(struct max17040_chip *chip)
+static int max17040_setup_irq(struct max17040_chip *chip)
 {
 	struct i2c_client *client = chip->client;
 	unsigned int flags;
@@ -256,6 +406,13 @@ static int max17040_enable_alert_irq(struct max17040_chip *chip)
 	return ret;
 }
 
+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,
@@ -277,6 +434,8 @@ 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;
+	bool enable_irq = false;
 	int ret;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
@@ -287,38 +446,61 @@ 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");
-		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;
 
-	chip->battery = power_supply_register(&client->dev,
-				&max17040_battery_desc, &psy_cfg);
+	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);
+	max17040_reset(chip);
+	max17040_get_version(chip);
+	max17040_set_rcomp(chip, chip->rcomp);
 
 	/* 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);
+	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,
 				"Failed to set low SOC alert: err %d\n", ret);
 			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);
+	}
 
-		ret = max17040_enable_alert_irq(chip);
+	if (enable_irq) {
+		ret = max17040_setup_irq(chip);
 		if (ret) {
 			client->irq = 0;
 			dev_warn(&client->dev,
@@ -326,19 +508,6 @@ 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);
-
-	power_supply_unregister(chip->battery);
-	cancel_delayed_work(&chip->work);
 	return 0;
 }
 
@@ -349,10 +518,13 @@ 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 && device_may_wakeup(dev))
 		enable_irq_wake(client->irq);
+	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);
 
 	return 0;
 }
@@ -362,11 +534,12 @@ 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);
+	if (client->irq && chip->data.has_soc_alert)
+		max17040_set_soc_alert(chip, 1);
+	else
+		max17040_queue_work(chip);
 
 	return 0;
 }
@@ -381,16 +554,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);
 
@@ -401,7 +588,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

Re: [PATCH 2/2] power: supply: max17040: Support compatible devices

[kernel test robot]

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

Hi Iskren,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on power-supply/for-next]
[also build test WARNING on next-20200608]
[cannot apply to v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Iskren-Chernev/dt-bindints-power-supply-Document-max17040-extensions/20200609-022950
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: i386-randconfig-s002-20200607 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-247-gcadbd124-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/power/supply/max17040_battery.c:65:18: sparse: sparse: symbol 'max17040_family' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39836 bytes --]

[RFC PATCH] power: supply: max17040: max17040_family[] can be static

[kernel test robot]

Signed-off-by: kernel test robot <lkp@intel.com>
---
 max17040_battery.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/power/supply/max17040_battery.c b/drivers/power/supply/max17040_battery.c
index 4fd6cf7b60c71..78f04aba4f26a 100644
--- a/drivers/power/supply/max17040_battery.c
+++ b/drivers/power/supply/max17040_battery.c
@@ -62,7 +62,7 @@ struct chip_data {
 	u8  has_soc_alert;
 };
 
-struct chip_data max17040_family[] = {
+static struct chip_data max17040_family[] = {
 	[ID_MAX17040] = {
 		.reset_val = 0x0054,
 		.vcell_shift = 4,

Re: [PATCH 2/2] power: supply: max17040: Support compatible devices

[kernel test robot]

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

Hi Iskren,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on power-supply/for-next]
[also build test WARNING on next-20200608]
[cannot apply to v5.7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Iskren-Chernev/dt-bindints-power-supply-Document-max17040-extensions/20200609-022950
base:   https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project e429cffd4f228f70c1d9df0e5d77c08590dd9766)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/power/supply/max17040_battery.c:456:13: warning: cast to smaller integer type 'enum chip_id' from 'const void *' [-Wvoid-pointer-to-enum-cast]
chip_id = (enum chip_id) of_device_get_match_data(&client->dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

vim +456 drivers/power/supply/max17040_battery.c

   430	
   431	static int max17040_probe(struct i2c_client *client,
   432				const struct i2c_device_id *id)
   433	{
   434		struct i2c_adapter *adapter = client->adapter;
   435		struct power_supply_config psy_cfg = {};
   436		struct max17040_chip *chip;
   437		enum chip_id chip_id;
   438		bool enable_irq = false;
   439		int ret;
   440	
   441		if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
   442			return -EIO;
   443	
   444		chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
   445		if (!chip)
   446			return -ENOMEM;
   447	
   448		chip->client = client;
   449		chip->regmap = devm_regmap_init_i2c(client, &max17040_regmap);
   450		chip->pdata = client->dev.platform_data;
   451		chip_id = (enum chip_id) id->driver_data;
   452		if (client->dev.of_node) {
   453			ret = max17040_get_of_data(chip);
   454			if (ret)
   455				return ret;
 > 456			chip_id = (enum chip_id) of_device_get_match_data(&client->dev);
   457		}
   458		chip->data = max17040_family[chip_id];
   459	
   460		i2c_set_clientdata(client, chip);
   461		psy_cfg.drv_data = chip;
   462	
   463		chip->battery = devm_power_supply_register(&client->dev,
   464						&max17040_battery_desc, &psy_cfg);
   465		if (IS_ERR(chip->battery)) {
   466			dev_err(&client->dev, "failed: power supply register\n");
   467			return PTR_ERR(chip->battery);
   468		}
   469	
   470		max17040_reset(chip);
   471		max17040_get_version(chip);
   472		max17040_set_rcomp(chip, chip->rcomp);
   473	
   474		/* check interrupt */
   475		if (client->irq && chip->data.has_low_soc_alert) {
   476			ret = max17040_set_low_soc_alert(chip, chip->low_soc_alert);
   477			if (ret) {
   478				dev_err(&client->dev,
   479					"Failed to set low SOC alert: err %d\n", ret);
   480				return ret;
   481			}
   482			enable_irq = true;
   483		}
   484	
   485		if (client->irq && chip->data.has_soc_alert) {
   486			ret = max17040_set_soc_alert(chip, 1);
   487			if (ret) {
   488				dev_err(&client->dev,
   489					"Failed to set SOC alert: err %d\n", ret);
   490				return ret;
   491			}
   492			enable_irq = true;
   493		} else {
   494			/* soc alerts negate the need for polling */
   495			INIT_DEFERRABLE_WORK(&chip->work, max17040_work);
   496			ret = devm_add_action(&client->dev, max17040_stop_work, chip);
   497			if (ret)
   498				return ret;
   499			max17040_queue_work(chip);
   500		}
   501	
   502		if (enable_irq) {
   503			ret = max17040_setup_irq(chip);
   504			if (ret) {
   505				client->irq = 0;
   506				dev_warn(&client->dev,
   507					 "Failed to get IRQ err %d\n", ret);
   508			}
   509		}
   510	
   511		return 0;
   512	}
   513	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73499 bytes --]

Re: [PATCH 1/2] dt-bindints: power: supply: Document max17040 extensions

[Rob Herring]

typo in the subject

On Mon, Jun 08, 2020 at 09:26:41PM +0300, Iskren Chernev wrote:
> Maxim max17040 is a fuel gauge from a larger family utilising the Model
> Gauge thechnology. Document all different compatible strings that the

typo

> max17040 driver recognizes.
> 
> Some devices in the wild report double the capacity. The
> maxim,double-soc (from State-Of-Charge) property fixes that.
> 
> 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>
> ---
>  .../power/supply/max17040_battery.txt         | 22 ++++++++++++++++++-
>  1 file changed, 21 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..be11cab4530b9 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,15 @@ 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.
> +- 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
> @@ -27,7 +38,16 @@ Example:
>  		compatible = "maxim,max77836-battery";
>  		reg = <0x36>;
>  		maxim,alert-low-soc-level = <10>;
> +		maxim,rcomp = /bits/ 8 <0x97 0x00>;
>  		interrupt-parent = <&gpio7>;
>  		interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>  		wakeup-source;
>  	};
> +
> +	battery-fuel-gauge@36 {
> +		compatible = "maxim,max17048";
> +		reg = <0x36>;
> +		maxim,rcomp = /bits/ 8 <0x97>;
> +		maxim,alert-low-soc-level = <10>;
> +		maxim,double-soc;
> +	};
> 
> base-commit: 1713116fa907cc7290020f0d8632ec646d2936f8
> -- 
> 2.27.0
>