[PATCH 0/8] power: supply: bq24190 updates + new ug3105 driver

[PATCH 0/8] power: supply: bq24190 updates + new ug3105 driver

[Hans de Goede]

Hi Sebastian,

Here is a series of patches which:

1. Modifies power_supply_get_battery_info() to also work with non
   of/dt device-properties

2. Modifies bq24190_charger to use and apply more settings returned
   by power_supply_get_battery_info()

3. Adds a new driver for the ug3105 battery monitoring chip, note
   this chip is not really a full/standalone fuel-gauge so
   the functionality of the is limited

Regards,

Hans


Hans de Goede (8):
  power: supply: core: Use fwnode_property_*() in
    power_supply_get_battery_info()
  power: supply: core: Add support for generic fwnodes to
    power_supply_get_battery_info()
  power: supply: bq24190_charger: Turn off 5V boost regulator on
    shutdown
  power: supply: bq24190_charger: Always call
    power_supply_get_battery_info()
  power: supply: bq24190_charger: Store ichg-max and vreg-max in
    bq24190_dev_info
  power: supply: bq24190_charger: Program charger with fwnode supplied
    ccc_ireg and cvc_vreg
  power: supply: bq24190_charger: Disallow ccc_ireg and cvc_vreg to be
    higher then the fwnode values
  power: supply: ug3105_battery: Add driver for uPI uG3105 battery
    monitor

 drivers/power/supply/Kconfig             |  14 +
 drivers/power/supply/Makefile            |   1 +
 drivers/power/supply/bq24190_charger.c   | 114 ++++--
 drivers/power/supply/power_supply_core.c |  93 +++--
 drivers/power/supply/ug3105_battery.c    | 486 +++++++++++++++++++++++
 5 files changed, 642 insertions(+), 66 deletions(-)
 create mode 100644 drivers/power/supply/ug3105_battery.c

-- 
2.33.1

[PATCH 1/8] power: supply: core: Use fwnode_property_*() in power_supply_get_battery_info()

[Hans de Goede]

Switch power_supply_get_battery_info() over to use the generic
fwnode_property_*() property read functions. This is a preparation patch
for adding support for reading properties from other fwnode types such
as swnode properties added by platform code on x86 devices.

Note the parsing of the 2d matrix "ocv-capacity-table-%d" and
"resistance-temp-table" properties is not converted since this depends on
the raw of_get_property() accessor function of which there is no
fwnode_property_*() equivalent AFAICT. This means that these properties
will not be supported in swnodes for now.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_core.c | 69 ++++++++++++++----------
 1 file changed, 42 insertions(+), 27 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index df4471e50d33..fd08c018c18e 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -572,9 +572,11 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	struct power_supply_resistance_temp_table *resist_table;
 	struct power_supply_battery_info *info;
 	struct device_node *battery_np;
+	struct fwnode_handle *fwnode;
 	const char *value;
 	int err, len, index;
 	const __be32 *list;
+	u32 min_max[2];
 
 	info = devm_kmalloc(&psy->dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
@@ -618,7 +620,9 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	if (!battery_np)
 		return -ENODEV;
 
-	err = of_property_read_string(battery_np, "compatible", &value);
+	fwnode = of_fwnode_handle(battery_np);
+
+	err = fwnode_property_read_string(fwnode, "compatible", &value);
 	if (err)
 		goto out_put_node;
 
@@ -632,7 +636,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	 * Documentation/power/power_supply_class.rst.
 	 */
 
-	if (!of_property_read_string(battery_np, "device-chemistry", &value)) {
+	if (!fwnode_property_read_string(fwnode, "device-chemistry", &value)) {
 		if (!strcmp("nickel-cadmium", value))
 			info->technology = POWER_SUPPLY_TECHNOLOGY_NiCd;
 		else if (!strcmp("nickel-metal-hydride", value))
@@ -650,45 +654,56 @@ int power_supply_get_battery_info(struct power_supply *psy,
 			dev_warn(&psy->dev, "%s unknown battery type\n", value);
 	}
 
-	of_property_read_u32(battery_np, "energy-full-design-microwatt-hours",
+	fwnode_property_read_u32(fwnode, "energy-full-design-microwatt-hours",
 			     &info->energy_full_design_uwh);
-	of_property_read_u32(battery_np, "charge-full-design-microamp-hours",
+	fwnode_property_read_u32(fwnode, "charge-full-design-microamp-hours",
 			     &info->charge_full_design_uah);
-	of_property_read_u32(battery_np, "voltage-min-design-microvolt",
+	fwnode_property_read_u32(fwnode, "voltage-min-design-microvolt",
 			     &info->voltage_min_design_uv);
-	of_property_read_u32(battery_np, "voltage-max-design-microvolt",
+	fwnode_property_read_u32(fwnode, "voltage-max-design-microvolt",
 			     &info->voltage_max_design_uv);
-	of_property_read_u32(battery_np, "trickle-charge-current-microamp",
+	fwnode_property_read_u32(fwnode, "trickle-charge-current-microamp",
 			     &info->tricklecharge_current_ua);
-	of_property_read_u32(battery_np, "precharge-current-microamp",
+	fwnode_property_read_u32(fwnode, "precharge-current-microamp",
 			     &info->precharge_current_ua);
-	of_property_read_u32(battery_np, "precharge-upper-limit-microvolt",
+	fwnode_property_read_u32(fwnode, "precharge-upper-limit-microvolt",
 			     &info->precharge_voltage_max_uv);
-	of_property_read_u32(battery_np, "charge-term-current-microamp",
+	fwnode_property_read_u32(fwnode, "charge-term-current-microamp",
 			     &info->charge_term_current_ua);
-	of_property_read_u32(battery_np, "re-charge-voltage-microvolt",
+	fwnode_property_read_u32(fwnode, "re-charge-voltage-microvolt",
 			     &info->charge_restart_voltage_uv);
-	of_property_read_u32(battery_np, "over-voltage-threshold-microvolt",
+	fwnode_property_read_u32(fwnode, "over-voltage-threshold-microvolt",
 			     &info->overvoltage_limit_uv);
-	of_property_read_u32(battery_np, "constant-charge-current-max-microamp",
+	fwnode_property_read_u32(fwnode, "constant-charge-current-max-microamp",
 			     &info->constant_charge_current_max_ua);
-	of_property_read_u32(battery_np, "constant-charge-voltage-max-microvolt",
+	fwnode_property_read_u32(fwnode, "constant-charge-voltage-max-microvolt",
 			     &info->constant_charge_voltage_max_uv);
-	of_property_read_u32(battery_np, "factory-internal-resistance-micro-ohms",
+	fwnode_property_read_u32(fwnode, "factory-internal-resistance-micro-ohms",
 			     &info->factory_internal_resistance_uohm);
 
-	of_property_read_u32_index(battery_np, "ambient-celsius",
-				   0, &info->temp_ambient_alert_min);
-	of_property_read_u32_index(battery_np, "ambient-celsius",
-				   1, &info->temp_ambient_alert_max);
-	of_property_read_u32_index(battery_np, "alert-celsius",
-				   0, &info->temp_alert_min);
-	of_property_read_u32_index(battery_np, "alert-celsius",
-				   1, &info->temp_alert_max);
-	of_property_read_u32_index(battery_np, "operating-range-celsius",
-				   0, &info->temp_min);
-	of_property_read_u32_index(battery_np, "operating-range-celsius",
-				   1, &info->temp_max);
+	if (!fwnode_property_read_u32_array(fwnode, "ambient-celsius",
+					    min_max, ARRAY_SIZE(min_max))) {
+		info->temp_ambient_alert_min = min_max[0];
+		info->temp_ambient_alert_max = min_max[1];
+	}
+	if (!fwnode_property_read_u32_array(fwnode, "alert-celsius",
+					    min_max, ARRAY_SIZE(min_max))) {
+		info->temp_alert_min = min_max[0];
+		info->temp_alert_max = min_max[1];
+	}
+	if (!fwnode_property_read_u32_array(fwnode, "operating-range-celsius",
+					    min_max, ARRAY_SIZE(min_max))) {
+		info->temp_min = min_max[0];
+		info->temp_max = min_max[1];
+	}
+
+	/*
+	 * The below code uses raw of-data parsing to parse
+	 * /schemas/types.yaml#/definitions/uint32-matrix
+	 * data, so for now this is only support with of.
+	 */
+	if (!battery_np)
+		goto out_ret_pointer;
 
 	len = of_property_count_u32_elems(battery_np, "ocv-capacity-celsius");
 	if (len < 0 && len != -EINVAL) {
-- 
2.33.1

[PATCH 2/8] power: supply: core: Add support for generic fwnodes to power_supply_get_battery_info()

[Hans de Goede]

Add support to power_supply_get_battery_info() to read the properties from
other fwnode types such as swnodes added by platform code on x86 devices.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/power_supply_core.c | 26 +++++++++++++++---------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index fd08c018c18e..1a2440e212a7 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -571,7 +571,8 @@ int power_supply_get_battery_info(struct power_supply *psy,
 {
 	struct power_supply_resistance_temp_table *resist_table;
 	struct power_supply_battery_info *info;
-	struct device_node *battery_np;
+	struct device_node *battery_np = NULL;
+	struct fwnode_reference_args args;
 	struct fwnode_handle *fwnode;
 	const char *value;
 	int err, len, index;
@@ -610,17 +611,21 @@ int power_supply_get_battery_info(struct power_supply *psy,
 		info->ocv_table_size[index]  = -EINVAL;
 	}
 
-	if (!psy->of_node) {
-		dev_warn(&psy->dev, "%s currently only supports devicetree\n",
-			 __func__);
-		return -ENXIO;
-	}
+	if (psy->of_node) {
+		battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);
+		if (!battery_np)
+			return -ENODEV;
 
-	battery_np = of_parse_phandle(psy->of_node, "monitored-battery", 0);
-	if (!battery_np)
-		return -ENODEV;
+		fwnode = fwnode_handle_get(of_fwnode_handle(battery_np));
+	} else {
+		err = fwnode_property_get_reference_args(
+					dev_fwnode(psy->dev.parent),
+					"monitored-battery", NULL, 0, 0, &args);
+		if (err)
+			return err;
 
-	fwnode = of_fwnode_handle(battery_np);
+		fwnode = args.fwnode;
+	}
 
 	err = fwnode_property_read_string(fwnode, "compatible", &value);
 	if (err)
@@ -778,6 +783,7 @@ int power_supply_get_battery_info(struct power_supply *psy,
 	*info_out = info;
 
 out_put_node:
+	fwnode_handle_put(fwnode);
 	of_node_put(battery_np);
 	return err;
 }
-- 
2.33.1

[PATCH 3/8] power: supply: bq24190_charger: Turn off 5V boost regulator on shutdown

[Hans de Goede]

Turn off the 5V boost regulator on shutdown, there are 3 reasons for
doing this:

1. It drains he battery if left on
2. If left on the device will not charge when plugged into a charger
3. If left on and the powered peripheral attached to a Type-C port is
   removed before the next boot, then the Type-C port-controller will
   see VBus being present while nothing is attached confusing the
   TCPM state-machine.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/bq24190_charger.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index a1c957a26f07..7414830a70e4 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -497,10 +497,8 @@ static ssize_t bq24190_sysfs_store(struct device *dev,
 }
 #endif
 
-#ifdef CONFIG_REGULATOR
-static int bq24190_set_charge_mode(struct regulator_dev *dev, u8 val)
+static int bq24190_set_charge_mode(struct bq24190_dev_info *bdi, u8 val)
 {
-	struct bq24190_dev_info *bdi = rdev_get_drvdata(dev);
 	int ret;
 
 	ret = pm_runtime_get_sync(bdi->dev);
@@ -520,14 +518,17 @@ static int bq24190_set_charge_mode(struct regulator_dev *dev, u8 val)
 	return ret;
 }
 
+#ifdef CONFIG_REGULATOR
 static int bq24190_vbus_enable(struct regulator_dev *dev)
 {
-	return bq24190_set_charge_mode(dev, BQ24190_REG_POC_CHG_CONFIG_OTG);
+	return bq24190_set_charge_mode(rdev_get_drvdata(dev),
+				       BQ24190_REG_POC_CHG_CONFIG_OTG);
 }
 
 static int bq24190_vbus_disable(struct regulator_dev *dev)
 {
-	return bq24190_set_charge_mode(dev, BQ24190_REG_POC_CHG_CONFIG_CHARGE);
+	return bq24190_set_charge_mode(rdev_get_drvdata(dev),
+				       BQ24190_REG_POC_CHG_CONFIG_CHARGE);
 }
 
 static int bq24190_vbus_is_enabled(struct regulator_dev *dev)
@@ -1870,6 +1871,14 @@ static int bq24190_remove(struct i2c_client *client)
 	return 0;
 }
 
+static void bq24190_shutdown(struct i2c_client *client)
+{
+	struct bq24190_dev_info *bdi = i2c_get_clientdata(client);
+
+	/* Turn off 5V boost regulator on shutdown */
+	bq24190_set_charge_mode(bdi, BQ24190_REG_POC_CHG_CONFIG_CHARGE);
+}
+
 static __maybe_unused int bq24190_runtime_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -1980,6 +1989,7 @@ MODULE_DEVICE_TABLE(of, bq24190_of_match);
 static struct i2c_driver bq24190_driver = {
 	.probe		= bq24190_probe,
 	.remove		= bq24190_remove,
+	.shutdown	= bq24190_shutdown,
 	.id_table	= bq24190_i2c_ids,
 	.driver = {
 		.name		= "bq24190-charger",
-- 
2.33.1

[PATCH 4/8] power: supply: bq24190_charger: Always call power_supply_get_battery_info()

[Hans de Goede]

power_supply_get_battery_info() now also supports getting battery_info
on boards not using dt/of. Remove the of_node check. If neither of nor
other battery-info is present the function will fail making this change
a no-op in that case.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/bq24190_charger.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 7414830a70e4..83873119ba24 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1693,8 +1693,7 @@ static int bq24190_get_config(struct bq24190_dev_info *bdi)
 			dev_warn(bdi->dev, "invalid value for %s: %u\n", s, v);
 	}
 
-	if (bdi->dev->of_node &&
-	    !power_supply_get_battery_info(bdi->charger, &info)) {
+	if (!power_supply_get_battery_info(bdi->charger, &info)) {
 		v = info->precharge_current_ua / 1000;
 		if (v >= BQ24190_REG_PCTCC_IPRECHG_MIN
 		 && v <= BQ24190_REG_PCTCC_IPRECHG_MAX)
-- 
2.33.1

[PATCH 5/8] power: supply: bq24190_charger: Store ichg-max and vreg-max in bq24190_dev_info

[Hans de Goede]

Store ichg-max and vreg-max in bq24190_dev_info once from
bq24190_get_config() and drop the bq24190_charger_get_current_max() and
bq24190_charger_get_voltage_max() helpers.

This is a preparation patch for honoring the
constant_charge_current_max_ua and constant_charge_voltage_max_uv
values from power_supply_get_battery_info().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/bq24190_charger.c | 34 ++++++++++----------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 83873119ba24..cb36ccbb731a 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -165,6 +165,8 @@ struct bq24190_dev_info {
 	u16				sys_min;
 	u16				iprechg;
 	u16				iterm;
+	u32				ichg_max;
+	u32				vreg_max;
 	struct mutex			f_reg_lock;
 	u8				f_reg;
 	u8				ss_reg;
@@ -977,15 +979,6 @@ static int bq24190_charger_get_current(struct bq24190_dev_info *bdi,
 	return 0;
 }
 
-static int bq24190_charger_get_current_max(struct bq24190_dev_info *bdi,
-		union power_supply_propval *val)
-{
-	int idx = ARRAY_SIZE(bq24190_ccc_ichg_values) - 1;
-
-	val->intval = bq24190_ccc_ichg_values[idx];
-	return 0;
-}
-
 static int bq24190_charger_set_current(struct bq24190_dev_info *bdi,
 		const union power_supply_propval *val)
 {
@@ -1024,15 +1017,6 @@ static int bq24190_charger_get_voltage(struct bq24190_dev_info *bdi,
 	return 0;
 }
 
-static int bq24190_charger_get_voltage_max(struct bq24190_dev_info *bdi,
-		union power_supply_propval *val)
-{
-	int idx = ARRAY_SIZE(bq24190_cvc_vreg_values) - 1;
-
-	val->intval = bq24190_cvc_vreg_values[idx];
-	return 0;
-}
-
 static int bq24190_charger_set_voltage(struct bq24190_dev_info *bdi,
 		const union power_supply_propval *val)
 {
@@ -1109,13 +1093,15 @@ static int bq24190_charger_get_property(struct power_supply *psy,
 		ret = bq24190_charger_get_current(bdi, val);
 		break;
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
-		ret = bq24190_charger_get_current_max(bdi, val);
+		val->intval = bdi->ichg_max;
+		ret = 0;
 		break;
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
 		ret = bq24190_charger_get_voltage(bdi, val);
 		break;
 	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
-		ret = bq24190_charger_get_voltage_max(bdi, val);
+		val->intval = bdi->vreg_max;
+		ret = 0;
 		break;
 	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
 		ret = bq24190_charger_get_iinlimit(bdi, val);
@@ -1682,7 +1668,13 @@ static int bq24190_get_config(struct bq24190_dev_info *bdi)
 {
 	const char * const s = "ti,system-minimum-microvolt";
 	struct power_supply_battery_info *info;
-	int v;
+	int v, idx;
+
+	idx = ARRAY_SIZE(bq24190_ccc_ichg_values) - 1;
+	bdi->ichg_max = bq24190_ccc_ichg_values[idx];
+
+	idx = ARRAY_SIZE(bq24190_cvc_vreg_values) - 1;
+	bdi->vreg_max = bq24190_cvc_vreg_values[idx];
 
 	if (device_property_read_u32(bdi->dev, s, &v) == 0) {
 		v /= 1000;
-- 
2.33.1

[PATCH 6/8] power: supply: bq24190_charger: Program charger with fwnode supplied ccc_ireg and cvc_vreg

[Hans de Goede]

So far the bq24190_charger driver has been relying on either the chips
default constant_charge_current_max_ua and constant_charge_voltage_max_uv
values, or on the BIOS or bootloader to program these for us.

This does not happen on all boards, causing e.g. the wrong (too low)
values to be used on Lenovo Yoga Tablet 2 830F/L and 1050F/L tablets.

If power_supply_get_battery_info() provides us with values for these
settings, then program the charger accordingly.

And if the user later overrides these values then save the user-values
so that these will be restored after a suspend/resume.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/bq24190_charger.c | 51 +++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index cb36ccbb731a..974e4b6b8d4d 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -165,7 +165,9 @@ struct bq24190_dev_info {
 	u16				sys_min;
 	u16				iprechg;
 	u16				iterm;
+	u32				ichg;
 	u32				ichg_max;
+	u32				vreg;
 	u32				vreg_max;
 	struct mutex			f_reg_lock;
 	u8				f_reg;
@@ -662,6 +664,28 @@ static int bq24190_set_config(struct bq24190_dev_info *bdi)
 			return ret;
 	}
 
+	if (bdi->ichg) {
+		ret = bq24190_set_field_val(bdi, BQ24190_REG_CCC,
+					    BQ24190_REG_CCC_ICHG_MASK,
+					    BQ24190_REG_CCC_ICHG_SHIFT,
+					    bq24190_ccc_ichg_values,
+					    ARRAY_SIZE(bq24190_ccc_ichg_values),
+					    bdi->ichg);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (bdi->vreg) {
+		ret = bq24190_set_field_val(bdi, BQ24190_REG_CVC,
+					    BQ24190_REG_CVC_VREG_MASK,
+					    BQ24190_REG_CVC_VREG_SHIFT,
+					    bq24190_cvc_vreg_values,
+					    ARRAY_SIZE(bq24190_cvc_vreg_values),
+					    bdi->vreg);
+		if (ret < 0)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -995,10 +1019,16 @@ static int bq24190_charger_set_current(struct bq24190_dev_info *bdi,
 	if (v)
 		curr *= 5;
 
-	return bq24190_set_field_val(bdi, BQ24190_REG_CCC,
+	ret = bq24190_set_field_val(bdi, BQ24190_REG_CCC,
 			BQ24190_REG_CCC_ICHG_MASK, BQ24190_REG_CCC_ICHG_SHIFT,
 			bq24190_ccc_ichg_values,
 			ARRAY_SIZE(bq24190_ccc_ichg_values), curr);
+	if (ret < 0)
+		return ret;
+
+	bdi->ichg = curr;
+
+	return 0;
 }
 
 static int bq24190_charger_get_voltage(struct bq24190_dev_info *bdi,
@@ -1020,10 +1050,18 @@ static int bq24190_charger_get_voltage(struct bq24190_dev_info *bdi,
 static int bq24190_charger_set_voltage(struct bq24190_dev_info *bdi,
 		const union power_supply_propval *val)
 {
-	return bq24190_set_field_val(bdi, BQ24190_REG_CVC,
+	int ret;
+
+	ret = bq24190_set_field_val(bdi, BQ24190_REG_CVC,
 			BQ24190_REG_CVC_VREG_MASK, BQ24190_REG_CVC_VREG_SHIFT,
 			bq24190_cvc_vreg_values,
 			ARRAY_SIZE(bq24190_cvc_vreg_values), val->intval);
+	if (ret < 0)
+		return ret;
+
+	bdi->vreg = val->intval;
+
+	return 0;
 }
 
 static int bq24190_charger_get_iinlimit(struct bq24190_dev_info *bdi,
@@ -1701,6 +1739,15 @@ static int bq24190_get_config(struct bq24190_dev_info *bdi)
 		else
 			dev_warn(bdi->dev, "invalid value for battery:charge-term-current-microamp: %d\n",
 				 v);
+
+		/* These are optional, so no warning when not set */
+		v = info->constant_charge_current_max_ua;
+		if (v >= bq24190_ccc_ichg_values[0] && v <= bdi->ichg_max)
+			bdi->ichg = v;
+
+		v = info->constant_charge_voltage_max_uv;
+		if (v >= bq24190_cvc_vreg_values[0] && v <= bdi->vreg_max)
+			bdi->vreg = v;
 	}
 
 	return 0;
-- 
2.33.1

[PATCH 7/8] power: supply: bq24190_charger: Disallow ccc_ireg and cvc_vreg to be higher then the fwnode values

[Hans de Goede]

If the fwnode data as parsed by power_supply_get_battery_info() provides
max values for ccc_ireg and cvc_vreg then do not allow the user to later
set these to higher values then those specified by the firmware,
otherwise the battery might get damaged.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/bq24190_charger.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 974e4b6b8d4d..04aa25f2d033 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1019,6 +1019,9 @@ static int bq24190_charger_set_current(struct bq24190_dev_info *bdi,
 	if (v)
 		curr *= 5;
 
+	if (curr > bdi->ichg_max)
+		return -EINVAL;
+
 	ret = bq24190_set_field_val(bdi, BQ24190_REG_CCC,
 			BQ24190_REG_CCC_ICHG_MASK, BQ24190_REG_CCC_ICHG_SHIFT,
 			bq24190_ccc_ichg_values,
@@ -1052,6 +1055,9 @@ static int bq24190_charger_set_voltage(struct bq24190_dev_info *bdi,
 {
 	int ret;
 
+	if (val->intval > bdi->vreg_max)
+		return -EINVAL;
+
 	ret = bq24190_set_field_val(bdi, BQ24190_REG_CVC,
 			BQ24190_REG_CVC_VREG_MASK, BQ24190_REG_CVC_VREG_SHIFT,
 			bq24190_cvc_vreg_values,
@@ -1743,11 +1749,11 @@ static int bq24190_get_config(struct bq24190_dev_info *bdi)
 		/* These are optional, so no warning when not set */
 		v = info->constant_charge_current_max_ua;
 		if (v >= bq24190_ccc_ichg_values[0] && v <= bdi->ichg_max)
-			bdi->ichg = v;
+			bdi->ichg = bdi->ichg_max = v;
 
 		v = info->constant_charge_voltage_max_uv;
 		if (v >= bq24190_cvc_vreg_values[0] && v <= bdi->vreg_max)
-			bdi->vreg = v;
+			bdi->vreg = bdi->vreg_max = v;
 	}
 
 	return 0;
-- 
2.33.1

[PATCH 8/8] power: supply: ug3105_battery: Add driver for uPI uG3105 battery monitor

[Hans de Goede]

Add a new battery driver for the uPI uG3105 battery monitor.

Note the uG3105 is not a full-featured autonomous fuel-gauge. Instead it
is expected to be use in combination with some always on microcontroller
reading its coulomb-counter before it can wrap (must be read every 400
seconds!).

Since Linux does not monitor coulomb-counter changes while the device is
off or suspended, the coulomb counter is not used atm.

So far this driver is only used on x86/ACPI (non devicetree) devs
(also note there is no of_match table). Therefor there is no devicetree
bindings documentation for this driver's "upi,rsns-microohm" property
since this is not used in actual devicetree files and the dt bindings
maintainers have requested properties with no actual dt users to
_not_ be added to the dt bindings.

The property's name has been chosen so that it should not need to be
changed if/when devicetree enumeration support gets added later, as it
mirrors "maxim,rsns-microohm" from the "maxim,max17042" bindings.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/Kconfig          |  15 +
 drivers/power/supply/Makefile         |   1 +
 drivers/power/supply/ug3105_battery.c | 486 ++++++++++++++++++++++++++
 3 files changed, 502 insertions(+)
 create mode 100644 drivers/power/supply/ug3105_battery.c

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index b366e2fd8e97..6b15eb184072 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -866,4 +866,19 @@ config CHARGER_SURFACE
 	  Microsoft Surface devices, i.e. Surface Pro 7, Surface Laptop 3,
 	  Surface Book 3, and Surface Laptop Go.
 
+config BATTERY_UG3105
+	tristate "uPI uG3105 battery monitor driver"
+	depends on I2C
+	help
+	  Battery monitor driver for the uPI uG3105 battery monitor.
+
+	  Note the uG3105 is not a full-featured autonomous fuel-gauge. Instead
+	  it is expected to be use in combination with some always on
+	  microcontroller reading its coulomb-counter before it can wrap
+	  (it must be read every 400 seconds!).
+
+	  Since Linux does not monitor coulomb-counter changes while the
+	  device is off or suspended, the functionality of this driver is
+	  limited to reporting capacity only.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index 2c1b264b2046..edf983676799 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -105,3 +105,4 @@ obj-$(CONFIG_RN5T618_POWER)	+= rn5t618_power.o
 obj-$(CONFIG_BATTERY_ACER_A500)	+= acer_a500_battery.o
 obj-$(CONFIG_BATTERY_SURFACE)	+= surface_battery.o
 obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
+obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
diff --git a/drivers/power/supply/ug3105_battery.c b/drivers/power/supply/ug3105_battery.c
new file mode 100644
index 000000000000..d92a7ebeb26e
--- /dev/null
+++ b/drivers/power/supply/ug3105_battery.c
@@ -0,0 +1,486 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Battery monitor driver for the uPI uG3105 battery monitor
+ *
+ * Note the uG3105 is not a full-featured autonomous fuel-gauge. Instead it is
+ * expected to be use in combination with some always on microcontroller reading
+ * its coulomb-counter before it can wrap (must be read every 400 seconds!).
+ *
+ * Since Linux does not monitor coulomb-counter changes while the device
+ * is off or suspended, the coulomb counter is not used atm.
+ *
+ * Possible improvements:
+ * 1. Activate commented out total_coulomb_count code
+ * 2. Reset total_coulomb_count val to 0 when the battery is as good as empty
+ *    and remember that we did this (and clear the flag for this on susp/resume)
+ * 3. When the battery is full check if the flag that we set total_coulomb_count
+ *    to when the battery was empty is set. If so we now know the capacity,
+ *    not the design, but actual capacity, of the battery
+ * 4. Add some mechanism (needs userspace help, or maybe use efivar?) to remember
+ *    the actual capacity of the battery over reboots
+ * 5. When we know the actual capacity at probe time, add energy_now and
+ *    energy_full attributes. Guess boot + resume energy_now value based on ocv
+ *    and then use total_coulomb_count to report energy_now over time, resetting
+ *    things to adjust for drift when empty/full. This should give more accurate
+ *    readings, esp. in the 30-70% range and allow userspace to estimate time
+ *    remaining till empty/full
+ * 6. Maybe unregister + reregister the psy device when we learn the actual
+ *    capacity during run-time ?
+ *
+ * The above will also require some sort of mwh_per_unit calculation. Testing
+ * has shown that an estimated 7404mWh increase of the battery's energy results
+ * in a total_coulomb_count increase of 3277 units with a 5 milli-ohm sense R.
+ *
+ * Copyright (C) 2021 Hans de Goede <hdegoede@redhat.com>
+ */
+
+#include <linux/devm-helpers.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/power_supply.h>
+#include <linux/workqueue.h>
+
+#define UG3105_MOV_AVG_WINDOW					8
+#define UG3105_INIT_POLL_TIME					(5 * HZ)
+#define UG3105_POLL_TIME					(30 * HZ)
+#define UG3105_SETTLE_TIME					(1 * HZ)
+
+#define UG3105_INIT_POLL_COUNT					30
+
+#define UG3105_REG_MODE						0x00
+#define UG3105_REG_CTRL1					0x01
+#define UG3105_REG_COULOMB_CNT					0x02
+#define UG3105_REG_BAT_VOLT					0x08
+#define UG3105_REG_BAT_CURR					0x0c
+
+#define UG3105_MODE_STANDBY					0x00
+#define UG3105_MODE_RUN						0x10
+
+#define UG3105_CTRL1_RESET_COULOMB_CNT				0x03
+
+#define UG3105_CURR_HYST_UA					65000
+
+#define UG3105_LOW_BAT_UV					3700000
+#define UG3105_FULL_BAT_HYST_UV					38000
+
+struct ug3105_chip {
+	struct i2c_client *client;
+	struct power_supply *psy;
+	struct power_supply_battery_info *info;
+	struct delayed_work work;
+	struct mutex lock;
+	int ocv[UG3105_MOV_AVG_WINDOW];		/* micro-volt */
+	int intern_res[UG3105_MOV_AVG_WINDOW];	/* milli-ohm */
+	int poll_count;
+	int ocv_avg_index;
+	int ocv_avg;				/* micro-volt */
+	int intern_res_poll_count;
+	int intern_res_avg_index;
+	int intern_res_avg;			/* milli-ohm */
+	int volt;				/* micro-volt */
+	int curr;				/* micro-ampere */
+	int total_coulomb_count;
+	int uv_per_unit;
+	int ua_per_unit;
+	int status;
+	int capacity;
+	bool supplied;
+};
+
+static int ug3105_read_word(struct i2c_client *client, u8 reg)
+{
+	int val;
+
+	val = i2c_smbus_read_word_data(client, reg);
+	if (val < 0)
+		dev_err(&client->dev, "Error reading reg 0x%02x\n", reg);
+
+	return val;
+}
+
+static int ug3105_get_status(struct ug3105_chip *chip)
+{
+	int full = chip->info->constant_charge_voltage_max_uv - UG3105_FULL_BAT_HYST_UV;
+
+	if (chip->curr > UG3105_CURR_HYST_UA)
+		return POWER_SUPPLY_STATUS_CHARGING;
+
+	if (chip->curr < -UG3105_CURR_HYST_UA)
+		return POWER_SUPPLY_STATUS_DISCHARGING;
+
+	if (chip->supplied && chip->ocv_avg > full)
+		return POWER_SUPPLY_STATUS_FULL;
+
+	return POWER_SUPPLY_STATUS_NOT_CHARGING;
+}
+
+static int ug3105_get_capacity(struct ug3105_chip *chip)
+{
+	/*
+	 * OCV voltages in uV for 0-110% in 5% increments, the 100-110% is
+	 * for LiPo HV (High-Voltage) bateries which can go up to 4.35V
+	 * instead of the usual 4.2V.
+	 */
+	static const int ocv_capacity_tbl[23] = {
+		3350000,
+		3610000,
+		3690000,
+		3710000,
+		3730000,
+		3750000,
+		3770000,
+		3786667,
+		3803333,
+		3820000,
+		3836667,
+		3853333,
+		3870000,
+		3907500,
+		3945000,
+		3982500,
+		4020000,
+		4075000,
+		4110000,
+		4150000,
+		4200000,
+		4250000,
+		4300000,
+	};
+	int i, ocv_diff, ocv_step;
+
+	if (chip->ocv_avg < ocv_capacity_tbl[0])
+		return 0;
+
+	if (chip->status == POWER_SUPPLY_STATUS_FULL)
+		return 100;
+
+	for (i = 1; i < ARRAY_SIZE(ocv_capacity_tbl); i++) {
+		if (chip->ocv_avg > ocv_capacity_tbl[i])
+			continue;
+
+		ocv_diff = ocv_capacity_tbl[i] - chip->ocv_avg;
+		ocv_step = ocv_capacity_tbl[i] - ocv_capacity_tbl[i - 1];
+		/* scale 0-110% down to 0-100% for LiPo HV */
+		if (chip->info->constant_charge_voltage_max_uv >= 4300000)
+			return (i * 500 - ocv_diff * 500 / ocv_step) / 110;
+		else
+			return i * 5 - ocv_diff * 5 / ocv_step;
+	}
+
+	return 100;
+}
+
+static void ug3105_work(struct work_struct *work)
+{
+	struct ug3105_chip *chip = container_of(work, struct ug3105_chip,
+						work.work);
+	int i, val, curr_diff, volt_diff, res, win_size;
+	bool prev_supplied = chip->supplied;
+	int prev_status = chip->status;
+	int prev_volt = chip->volt;
+	int prev_curr = chip->curr;
+	struct power_supply *psy;
+
+	mutex_lock(&chip->lock);
+
+	psy = chip->psy;
+	if (!psy)
+		goto out;
+
+	val = ug3105_read_word(chip->client, UG3105_REG_BAT_VOLT);
+	if (val < 0)
+		goto out;
+	chip->volt = val * chip->uv_per_unit;
+
+	val = ug3105_read_word(chip->client, UG3105_REG_BAT_CURR);
+	if (val < 0)
+		goto out;
+	chip->curr = (s16)val * chip->ua_per_unit;
+
+	chip->ocv[chip->ocv_avg_index] =
+		chip->volt - chip->curr * chip->intern_res_avg / 1000;
+	chip->ocv_avg_index = (chip->ocv_avg_index + 1) % UG3105_MOV_AVG_WINDOW;
+	chip->poll_count++;
+
+	/*
+	 * See possible improvements comment above.
+	 *
+	 * Read + reset coulomb counter every 10 polls (every 300 seconds)
+	 * if ((chip->poll_count % 10) == 0) {
+	 *	val = ug3105_read_word(chip->client, UG3105_REG_COULOMB_CNT);
+	 *	if (val < 0)
+	 *		goto out;
+	 *
+	 *	i2c_smbus_write_byte_data(chip->client, UG3105_REG_CTRL1,
+	 *				  UG3105_CTRL1_RESET_COULOMB_CNT);
+	 *
+	 *	chip->total_coulomb_count += (s16)val;
+	 *	dev_dbg(&chip->client->dev, "coulomb count %d total %d\n",
+	 *		(s16)val, chip->total_coulomb_count);
+	 * }
+	 */
+
+	chip->ocv_avg = 0;
+	win_size = min(chip->poll_count, UG3105_MOV_AVG_WINDOW);
+	for (i = 0; i < win_size; i++)
+		chip->ocv_avg += chip->ocv[i];
+	chip->ocv_avg /= win_size;
+
+	chip->supplied = power_supply_am_i_supplied(psy);
+	chip->status = ug3105_get_status(chip);
+	chip->capacity = ug3105_get_capacity(chip);
+
+	/*
+	 * Skip internal resistance calc on charger [un]plug and
+	 * when the battery is almost empty (voltage low).
+	 */
+	if (chip->supplied != prev_supplied ||
+	    chip->volt < UG3105_LOW_BAT_UV ||
+	    chip->poll_count < 2)
+		goto out;
+
+	/*
+	 * Assuming that the OCV voltage does not change significantly
+	 * between 2 polls, then we can calculate the internal resistance
+	 * on a significant current change by attributing all voltage
+	 * change between the 2 readings to the internal resistance.
+	 */
+	curr_diff = abs(chip->curr - prev_curr);
+	if (curr_diff < UG3105_CURR_HYST_UA)
+		goto out;
+
+	volt_diff = abs(chip->volt - prev_volt);
+	res = volt_diff * 1000 / curr_diff;
+
+	if ((res < (chip->intern_res_avg * 2 / 3)) ||
+	    (res > (chip->intern_res_avg * 4 / 3))) {
+		dev_dbg(&chip->client->dev, "Ignoring outlier internal resistance %d mOhm\n", res);
+		goto out;
+	}
+
+	dev_dbg(&chip->client->dev, "Internal resistance %d mOhm\n", res);
+
+	chip->intern_res[chip->intern_res_avg_index] = res;
+	chip->intern_res_avg_index = (chip->intern_res_avg_index + 1) % UG3105_MOV_AVG_WINDOW;
+	chip->intern_res_poll_count++;
+
+	chip->intern_res_avg = 0;
+	win_size = min(chip->intern_res_poll_count, UG3105_MOV_AVG_WINDOW);
+	for (i = 0; i < win_size; i++)
+		chip->intern_res_avg += chip->intern_res[i];
+	chip->intern_res_avg /= win_size;
+
+out:
+	mutex_unlock(&chip->lock);
+
+	queue_delayed_work(system_wq, &chip->work,
+			   (chip->poll_count <= UG3105_INIT_POLL_COUNT) ?
+					UG3105_INIT_POLL_TIME : UG3105_POLL_TIME);
+
+	if (chip->status != prev_status && psy)
+		power_supply_changed(psy);
+}
+
+static enum power_supply_property ug3105_battery_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_TECHNOLOGY,
+	POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_VOLTAGE_NOW,
+	POWER_SUPPLY_PROP_VOLTAGE_OCV,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int ug3105_get_property(struct power_supply *psy,
+			       enum power_supply_property psp,
+			       union power_supply_propval *val)
+{
+	struct ug3105_chip *chip = power_supply_get_drvdata(psy);
+	int ret = 0;
+
+	mutex_lock(&chip->lock);
+
+	if (!chip->psy) {
+		ret = -EAGAIN;
+		goto out;
+	}
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = chip->status;
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_TECHNOLOGY:
+		val->intval = chip->info->technology;
+		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+		ret = ug3105_read_word(chip->client, UG3105_REG_BAT_VOLT);
+		if (ret < 0)
+			break;
+		val->intval = ret * chip->uv_per_unit;
+		ret = 0;
+		break;
+	case POWER_SUPPLY_PROP_VOLTAGE_OCV:
+		val->intval = chip->ocv_avg;
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+		ret = ug3105_read_word(chip->client, UG3105_REG_BAT_CURR);
+		if (ret < 0)
+			break;
+		val->intval = (s16)ret * chip->ua_per_unit;
+		ret = 0;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = chip->capacity;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+out:
+	mutex_unlock(&chip->lock);
+	return ret;
+}
+
+static void ug3105_external_power_changed(struct power_supply *psy)
+{
+	struct ug3105_chip *chip = power_supply_get_drvdata(psy);
+
+	dev_dbg(&chip->client->dev, "external power changed\n");
+	mod_delayed_work(system_wq, &chip->work, UG3105_SETTLE_TIME);
+}
+
+static const struct power_supply_desc ug3105_psy_desc = {
+	.name		= "ug3105_battery",
+	.type		= POWER_SUPPLY_TYPE_BATTERY,
+	.get_property	= ug3105_get_property,
+	.external_power_changed	= ug3105_external_power_changed,
+	.properties	= ug3105_battery_props,
+	.num_properties	= ARRAY_SIZE(ug3105_battery_props),
+};
+
+static void ug3105_init(struct ug3105_chip *chip)
+{
+	chip->poll_count = 0;
+	chip->ocv_avg_index = 0;
+	chip->total_coulomb_count = 0;
+	i2c_smbus_write_byte_data(chip->client, UG3105_REG_MODE,
+				  UG3105_MODE_RUN);
+	i2c_smbus_write_byte_data(chip->client, UG3105_REG_CTRL1,
+				  UG3105_CTRL1_RESET_COULOMB_CNT);
+	queue_delayed_work(system_wq, &chip->work, 0);
+	flush_delayed_work(&chip->work);
+}
+
+static int ug3105_probe(struct i2c_client *client)
+{
+	struct power_supply_config psy_cfg = {};
+	struct device *dev = &client->dev;
+	u32 curr_sense_res_uohm = 10000;
+	struct power_supply *psy;
+	struct ug3105_chip *chip;
+	int ret;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->client = client;
+	mutex_init(&chip->lock);
+	ret = devm_delayed_work_autocancel(dev, &chip->work, ug3105_work);
+	if (ret)
+		return ret;
+
+	psy_cfg.drv_data = chip;
+	psy = devm_power_supply_register(dev, &ug3105_psy_desc, &psy_cfg);
+	if (IS_ERR(psy))
+		return PTR_ERR(psy);
+
+	ret = power_supply_get_battery_info(psy, &chip->info);
+	if (ret)
+		return ret;
+
+	if (chip->info->factory_internal_resistance_uohm == -EINVAL ||
+	    chip->info->constant_charge_voltage_max_uv == -EINVAL) {
+		dev_err(dev, "error required properties are missing\n");
+		return -ENODEV;
+	}
+
+	device_property_read_u32(dev, "upi,rsns-microohm", &curr_sense_res_uohm);
+
+	/*
+	 * DAC maximum is 4.5V divided by 65536 steps + an unknown factor of 10
+	 * coming from somewhere for some reason (verified with a volt-meter).
+	 */
+	chip->uv_per_unit = 45000000/65536;
+	/* Datasheet says 8.1 uV per unit for the current ADC */
+	chip->ua_per_unit = 8100000 / curr_sense_res_uohm;
+
+	/* Use provided internal resistance as start point (in milli-ohm) */
+	chip->intern_res_avg = chip->info->factory_internal_resistance_uohm / 1000;
+	/* Also add it to the internal resistance moving average window */
+	chip->intern_res[0] = chip->intern_res_avg;
+	chip->intern_res_avg_index = 1;
+	chip->intern_res_poll_count = 1;
+
+	mutex_lock(&chip->lock);
+	chip->psy = psy;
+	mutex_unlock(&chip->lock);
+
+	ug3105_init(chip);
+
+	i2c_set_clientdata(client, chip);
+	return 0;
+}
+
+static int __maybe_unused ug3105_suspend(struct device *dev)
+{
+	struct ug3105_chip *chip = dev_get_drvdata(dev);
+
+	cancel_delayed_work_sync(&chip->work);
+	i2c_smbus_write_byte_data(chip->client, UG3105_REG_MODE,
+				  UG3105_MODE_STANDBY);
+
+	return 0;
+}
+
+static int __maybe_unused ug3105_resume(struct device *dev)
+{
+	struct ug3105_chip *chip = dev_get_drvdata(dev);
+
+	ug3105_init(chip);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(ug3105_pm_ops, ug3105_suspend,
+			ug3105_resume);
+
+static const struct i2c_device_id ug3105_id[] = {
+	{ "ug3105" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ug3105_id);
+
+static struct i2c_driver ug3105_i2c_driver = {
+	.driver	= {
+		.name = "ug3105",
+		.pm = &ug3105_pm_ops,
+	},
+	.probe_new = ug3105_probe,
+	.id_table = ug3105_id,
+};
+module_i2c_driver(ug3105_i2c_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com");
+MODULE_DESCRIPTION("uPI uG3105 battery monitor driver");
+MODULE_LICENSE("GPL");
-- 
2.33.1

Re: [PATCH 8/8] power: supply: ug3105_battery: Add driver for uPI uG3105 battery monitor

[Sebastian Reichel]

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

Hi,

On Mon, Jan 31, 2022 at 04:57:30PM +0100, Hans de Goede wrote:
> Add a new battery driver for the uPI uG3105 battery monitor.
> 
> Note the uG3105 is not a full-featured autonomous fuel-gauge. Instead it
> is expected to be use in combination with some always on microcontroller
> reading its coulomb-counter before it can wrap (must be read every 400
> seconds!).
> 
> Since Linux does not monitor coulomb-counter changes while the device is
> off or suspended, the coulomb counter is not used atm.
> 
> So far this driver is only used on x86/ACPI (non devicetree) devs
> (also note there is no of_match table). Therefor there is no devicetree
> bindings documentation for this driver's "upi,rsns-microohm" property
> since this is not used in actual devicetree files and the dt bindings
> maintainers have requested properties with no actual dt users to
> _not_ be added to the dt bindings.
> 
> The property's name has been chosen so that it should not need to be
> changed if/when devicetree enumeration support gets added later, as it
> mirrors "maxim,rsns-microohm" from the "maxim,max17042" bindings.

Except the vendor prefix being incorrect; please use this one:

$ grep -i upi Documentation/devicetree/bindings/vendor-prefixes.yaml
  "^upisemi,.*":
    description: uPI Semiconductor Corp.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/power/supply/Kconfig          |  15 +
>  drivers/power/supply/Makefile         |   1 +
>  drivers/power/supply/ug3105_battery.c | 486 ++++++++++++++++++++++++++
>  3 files changed, 502 insertions(+)
>  create mode 100644 drivers/power/supply/ug3105_battery.c
> 
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index b366e2fd8e97..6b15eb184072 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -866,4 +866,19 @@ config CHARGER_SURFACE
>  	  Microsoft Surface devices, i.e. Surface Pro 7, Surface Laptop 3,
>  	  Surface Book 3, and Surface Laptop Go.
>  
> +config BATTERY_UG3105
> +	tristate "uPI uG3105 battery monitor driver"
> +	depends on I2C
> +	help
> +	  Battery monitor driver for the uPI uG3105 battery monitor.
> +
> +	  Note the uG3105 is not a full-featured autonomous fuel-gauge. Instead
> +	  it is expected to be use in combination with some always on
> +	  microcontroller reading its coulomb-counter before it can wrap
> +	  (it must be read every 400 seconds!).
> +
> +	  Since Linux does not monitor coulomb-counter changes while the
> +	  device is off or suspended, the functionality of this driver is
> +	  limited to reporting capacity only.
> +
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 2c1b264b2046..edf983676799 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -105,3 +105,4 @@ obj-$(CONFIG_RN5T618_POWER)	+= rn5t618_power.o
>  obj-$(CONFIG_BATTERY_ACER_A500)	+= acer_a500_battery.o
>  obj-$(CONFIG_BATTERY_SURFACE)	+= surface_battery.o
>  obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
> +obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
> diff --git a/drivers/power/supply/ug3105_battery.c b/drivers/power/supply/ug3105_battery.c
> new file mode 100644
> index 000000000000..d92a7ebeb26e
> --- /dev/null
> +++ b/drivers/power/supply/ug3105_battery.c
> @@ -0,0 +1,486 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Battery monitor driver for the uPI uG3105 battery monitor
> + *
> + * Note the uG3105 is not a full-featured autonomous fuel-gauge. Instead it is
> + * expected to be use in combination with some always on microcontroller reading
> + * its coulomb-counter before it can wrap (must be read every 400 seconds!).
> + *
> + * Since Linux does not monitor coulomb-counter changes while the device
> + * is off or suspended, the coulomb counter is not used atm.
> + *
> + * Possible improvements:
> + * 1. Activate commented out total_coulomb_count code
> + * 2. Reset total_coulomb_count val to 0 when the battery is as good as empty
> + *    and remember that we did this (and clear the flag for this on susp/resume)
> + * 3. When the battery is full check if the flag that we set total_coulomb_count
> + *    to when the battery was empty is set. If so we now know the capacity,
> + *    not the design, but actual capacity, of the battery
> + * 4. Add some mechanism (needs userspace help, or maybe use efivar?) to remember
> + *    the actual capacity of the battery over reboots
> + * 5. When we know the actual capacity at probe time, add energy_now and
> + *    energy_full attributes. Guess boot + resume energy_now value based on ocv
> + *    and then use total_coulomb_count to report energy_now over time, resetting
> + *    things to adjust for drift when empty/full. This should give more accurate
> + *    readings, esp. in the 30-70% range and allow userspace to estimate time
> + *    remaining till empty/full
> + * 6. Maybe unregister + reregister the psy device when we learn the actual
> + *    capacity during run-time ?

Same problem also exists on a few mobile phones, e.g.
drivers/power/supply/cpcap-battery.c, but no good solution has been
found yet.

> + * The above will also require some sort of mwh_per_unit calculation. Testing
> + * has shown that an estimated 7404mWh increase of the battery's energy results
> + * in a total_coulomb_count increase of 3277 units with a 5 milli-ohm sense R.
> + *
> + * Copyright (C) 2021 Hans de Goede <hdegoede@redhat.com>
> + */
> +
> +#include <linux/devm-helpers.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/power_supply.h>
> +#include <linux/workqueue.h>
> +
> +#define UG3105_MOV_AVG_WINDOW					8
> +#define UG3105_INIT_POLL_TIME					(5 * HZ)
> +#define UG3105_POLL_TIME					(30 * HZ)
> +#define UG3105_SETTLE_TIME					(1 * HZ)
> +
> +#define UG3105_INIT_POLL_COUNT					30
> +
> +#define UG3105_REG_MODE						0x00
> +#define UG3105_REG_CTRL1					0x01
> +#define UG3105_REG_COULOMB_CNT					0x02
> +#define UG3105_REG_BAT_VOLT					0x08
> +#define UG3105_REG_BAT_CURR					0x0c
> +
> +#define UG3105_MODE_STANDBY					0x00
> +#define UG3105_MODE_RUN						0x10
> +
> +#define UG3105_CTRL1_RESET_COULOMB_CNT				0x03
> +
> +#define UG3105_CURR_HYST_UA					65000
> +
> +#define UG3105_LOW_BAT_UV					3700000
> +#define UG3105_FULL_BAT_HYST_UV					38000
> +
> +struct ug3105_chip {
> +	struct i2c_client *client;
> +	struct power_supply *psy;
> +	struct power_supply_battery_info *info;
> +	struct delayed_work work;
> +	struct mutex lock;
> +	int ocv[UG3105_MOV_AVG_WINDOW];		/* micro-volt */
> +	int intern_res[UG3105_MOV_AVG_WINDOW];	/* milli-ohm */
> +	int poll_count;
> +	int ocv_avg_index;
> +	int ocv_avg;				/* micro-volt */
> +	int intern_res_poll_count;
> +	int intern_res_avg_index;
> +	int intern_res_avg;			/* milli-ohm */
> +	int volt;				/* micro-volt */
> +	int curr;				/* micro-ampere */
> +	int total_coulomb_count;
> +	int uv_per_unit;
> +	int ua_per_unit;
> +	int status;
> +	int capacity;
> +	bool supplied;
> +};
> +
> +static int ug3105_read_word(struct i2c_client *client, u8 reg)
> +{
> +	int val;
> +
> +	val = i2c_smbus_read_word_data(client, reg);
> +	if (val < 0)
> +		dev_err(&client->dev, "Error reading reg 0x%02x\n", reg);
> +
> +	return val;
> +}

can we use regmap instead? :)

> +static int ug3105_get_status(struct ug3105_chip *chip)
> +{
> +	int full = chip->info->constant_charge_voltage_max_uv - UG3105_FULL_BAT_HYST_UV;
> +
> +	if (chip->curr > UG3105_CURR_HYST_UA)
> +		return POWER_SUPPLY_STATUS_CHARGING;
> +
> +	if (chip->curr < -UG3105_CURR_HYST_UA)
> +		return POWER_SUPPLY_STATUS_DISCHARGING;
> +
> +	if (chip->supplied && chip->ocv_avg > full)
> +		return POWER_SUPPLY_STATUS_FULL;
> +
> +	return POWER_SUPPLY_STATUS_NOT_CHARGING;
> +}
> +
> +static int ug3105_get_capacity(struct ug3105_chip *chip)
> +{
> +	/*
> +	 * OCV voltages in uV for 0-110% in 5% increments, the 100-110% is
> +	 * for LiPo HV (High-Voltage) bateries which can go up to 4.35V
> +	 * instead of the usual 4.2V.
> +	 */
> +	static const int ocv_capacity_tbl[23] = {
> +		3350000,
> +		3610000,
> +		3690000,
> +		3710000,
> +		3730000,
> +		3750000,
> +		3770000,
> +		3786667,
> +		3803333,
> +		3820000,
> +		3836667,
> +		3853333,
> +		3870000,
> +		3907500,
> +		3945000,
> +		3982500,
> +		4020000,
> +		4075000,
> +		4110000,
> +		4150000,
> +		4200000,
> +		4250000,
> +		4300000,
> +	};
> +	int i, ocv_diff, ocv_step;
> +
> +	if (chip->ocv_avg < ocv_capacity_tbl[0])
> +		return 0;
> +
> +	if (chip->status == POWER_SUPPLY_STATUS_FULL)
> +		return 100;
> +
> +	for (i = 1; i < ARRAY_SIZE(ocv_capacity_tbl); i++) {
> +		if (chip->ocv_avg > ocv_capacity_tbl[i])
> +			continue;
> +
> +		ocv_diff = ocv_capacity_tbl[i] - chip->ocv_avg;
> +		ocv_step = ocv_capacity_tbl[i] - ocv_capacity_tbl[i - 1];
> +		/* scale 0-110% down to 0-100% for LiPo HV */
> +		if (chip->info->constant_charge_voltage_max_uv >= 4300000)
> +			return (i * 500 - ocv_diff * 500 / ocv_step) / 110;
> +		else
> +			return i * 5 - ocv_diff * 5 / ocv_step;
> +	}
> +
> +	return 100;
> +}
> +
> +static void ug3105_work(struct work_struct *work)
> +{
> +	struct ug3105_chip *chip = container_of(work, struct ug3105_chip,
> +						work.work);
> +	int i, val, curr_diff, volt_diff, res, win_size;
> +	bool prev_supplied = chip->supplied;
> +	int prev_status = chip->status;
> +	int prev_volt = chip->volt;
> +	int prev_curr = chip->curr;
> +	struct power_supply *psy;
> +
> +	mutex_lock(&chip->lock);
> +
> +	psy = chip->psy;
> +	if (!psy)
> +		goto out;
> +
> +	val = ug3105_read_word(chip->client, UG3105_REG_BAT_VOLT);
> +	if (val < 0)
> +		goto out;
> +	chip->volt = val * chip->uv_per_unit;
> +
> +	val = ug3105_read_word(chip->client, UG3105_REG_BAT_CURR);
> +	if (val < 0)
> +		goto out;
> +	chip->curr = (s16)val * chip->ua_per_unit;
> +
> +	chip->ocv[chip->ocv_avg_index] =
> +		chip->volt - chip->curr * chip->intern_res_avg / 1000;
> +	chip->ocv_avg_index = (chip->ocv_avg_index + 1) % UG3105_MOV_AVG_WINDOW;
> +	chip->poll_count++;
> +
> +	/*
> +	 * See possible improvements comment above.
> +	 *
> +	 * Read + reset coulomb counter every 10 polls (every 300 seconds)
> +	 * if ((chip->poll_count % 10) == 0) {
> +	 *	val = ug3105_read_word(chip->client, UG3105_REG_COULOMB_CNT);
> +	 *	if (val < 0)
> +	 *		goto out;
> +	 *
> +	 *	i2c_smbus_write_byte_data(chip->client, UG3105_REG_CTRL1,
> +	 *				  UG3105_CTRL1_RESET_COULOMB_CNT);
> +	 *
> +	 *	chip->total_coulomb_count += (s16)val;
> +	 *	dev_dbg(&chip->client->dev, "coulomb count %d total %d\n",
> +	 *		(s16)val, chip->total_coulomb_count);
> +	 * }
> +	 */
> +
> +	chip->ocv_avg = 0;
> +	win_size = min(chip->poll_count, UG3105_MOV_AVG_WINDOW);
> +	for (i = 0; i < win_size; i++)
> +		chip->ocv_avg += chip->ocv[i];
> +	chip->ocv_avg /= win_size;
> +
> +	chip->supplied = power_supply_am_i_supplied(psy);
> +	chip->status = ug3105_get_status(chip);
> +	chip->capacity = ug3105_get_capacity(chip);
> +
> +	/*
> +	 * Skip internal resistance calc on charger [un]plug and
> +	 * when the battery is almost empty (voltage low).
> +	 */
> +	if (chip->supplied != prev_supplied ||
> +	    chip->volt < UG3105_LOW_BAT_UV ||
> +	    chip->poll_count < 2)
> +		goto out;
> +
> +	/*
> +	 * Assuming that the OCV voltage does not change significantly
> +	 * between 2 polls, then we can calculate the internal resistance
> +	 * on a significant current change by attributing all voltage
> +	 * change between the 2 readings to the internal resistance.
> +	 */
> +	curr_diff = abs(chip->curr - prev_curr);
> +	if (curr_diff < UG3105_CURR_HYST_UA)
> +		goto out;
> +
> +	volt_diff = abs(chip->volt - prev_volt);
> +	res = volt_diff * 1000 / curr_diff;
> +
> +	if ((res < (chip->intern_res_avg * 2 / 3)) ||
> +	    (res > (chip->intern_res_avg * 4 / 3))) {
> +		dev_dbg(&chip->client->dev, "Ignoring outlier internal resistance %d mOhm\n", res);
> +		goto out;
> +	}
> +
> +	dev_dbg(&chip->client->dev, "Internal resistance %d mOhm\n", res);
> +
> +	chip->intern_res[chip->intern_res_avg_index] = res;
> +	chip->intern_res_avg_index = (chip->intern_res_avg_index + 1) % UG3105_MOV_AVG_WINDOW;
> +	chip->intern_res_poll_count++;
> +
> +	chip->intern_res_avg = 0;
> +	win_size = min(chip->intern_res_poll_count, UG3105_MOV_AVG_WINDOW);
> +	for (i = 0; i < win_size; i++)
> +		chip->intern_res_avg += chip->intern_res[i];
> +	chip->intern_res_avg /= win_size;
> +
> +out:
> +	mutex_unlock(&chip->lock);
> +
> +	queue_delayed_work(system_wq, &chip->work,
> +			   (chip->poll_count <= UG3105_INIT_POLL_COUNT) ?
> +					UG3105_INIT_POLL_TIME : UG3105_POLL_TIME);
> +
> +	if (chip->status != prev_status && psy)
> +		power_supply_changed(psy);
> +}
> +
> +static enum power_supply_property ug3105_battery_props[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_PRESENT,
> +	POWER_SUPPLY_PROP_TECHNOLOGY,
> +	POWER_SUPPLY_PROP_SCOPE,
> +	POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +	POWER_SUPPLY_PROP_VOLTAGE_OCV,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CAPACITY,
> +};
> +
> +static int ug3105_get_property(struct power_supply *psy,
> +			       enum power_supply_property psp,
> +			       union power_supply_propval *val)
> +{
> +	struct ug3105_chip *chip = power_supply_get_drvdata(psy);
> +	int ret = 0;
> +
> +	mutex_lock(&chip->lock);
> +
> +	if (!chip->psy) {
> +		ret = -EAGAIN;
> +		goto out;
> +	}
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		val->intval = chip->status;
> +		break;
> +	case POWER_SUPPLY_PROP_PRESENT:
> +		val->intval = 1;
> +		break;
> +	case POWER_SUPPLY_PROP_TECHNOLOGY:
> +		val->intval = chip->info->technology;
> +		break;
> +	case POWER_SUPPLY_PROP_SCOPE:
> +		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> +		ret = ug3105_read_word(chip->client, UG3105_REG_BAT_VOLT);
> +		if (ret < 0)
> +			break;
> +		val->intval = ret * chip->uv_per_unit;
> +		ret = 0;
> +		break;
> +	case POWER_SUPPLY_PROP_VOLTAGE_OCV:
> +		val->intval = chip->ocv_avg;
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +		ret = ug3105_read_word(chip->client, UG3105_REG_BAT_CURR);
> +		if (ret < 0)
> +			break;
> +		val->intval = (s16)ret * chip->ua_per_unit;
> +		ret = 0;
> +		break;
> +	case POWER_SUPPLY_PROP_CAPACITY:
> +		val->intval = chip->capacity;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +out:
> +	mutex_unlock(&chip->lock);
> +	return ret;
> +}
> +
> +static void ug3105_external_power_changed(struct power_supply *psy)
> +{
> +	struct ug3105_chip *chip = power_supply_get_drvdata(psy);
> +
> +	dev_dbg(&chip->client->dev, "external power changed\n");
> +	mod_delayed_work(system_wq, &chip->work, UG3105_SETTLE_TIME);
> +}
> +
> +static const struct power_supply_desc ug3105_psy_desc = {
> +	.name		= "ug3105_battery",
> +	.type		= POWER_SUPPLY_TYPE_BATTERY,
> +	.get_property	= ug3105_get_property,
> +	.external_power_changed	= ug3105_external_power_changed,
> +	.properties	= ug3105_battery_props,
> +	.num_properties	= ARRAY_SIZE(ug3105_battery_props),
> +};
> +
> +static void ug3105_init(struct ug3105_chip *chip)
> +{
> +	chip->poll_count = 0;
> +	chip->ocv_avg_index = 0;
> +	chip->total_coulomb_count = 0;
> +	i2c_smbus_write_byte_data(chip->client, UG3105_REG_MODE,
> +				  UG3105_MODE_RUN);
> +	i2c_smbus_write_byte_data(chip->client, UG3105_REG_CTRL1,
> +				  UG3105_CTRL1_RESET_COULOMB_CNT);
> +	queue_delayed_work(system_wq, &chip->work, 0);
> +	flush_delayed_work(&chip->work);
> +}
> +
> +static int ug3105_probe(struct i2c_client *client)
> +{
> +	struct power_supply_config psy_cfg = {};
> +	struct device *dev = &client->dev;
> +	u32 curr_sense_res_uohm = 10000;
> +	struct power_supply *psy;
> +	struct ug3105_chip *chip;
> +	int ret;
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	chip->client = client;
> +	mutex_init(&chip->lock);
> +	ret = devm_delayed_work_autocancel(dev, &chip->work, ug3105_work);
> +	if (ret)
> +		return ret;
> +
> +	psy_cfg.drv_data = chip;
> +	psy = devm_power_supply_register(dev, &ug3105_psy_desc, &psy_cfg);
> +	if (IS_ERR(psy))
> +		return PTR_ERR(psy);
> +
> +	ret = power_supply_get_battery_info(psy, &chip->info);
> +	if (ret)
> +		return ret;

this allocates data, so you need to call power_supply_put_battery_info()
when chip->info is no loner required.

> +
> +	if (chip->info->factory_internal_resistance_uohm == -EINVAL ||
> +	    chip->info->constant_charge_voltage_max_uv == -EINVAL) {
> +		dev_err(dev, "error required properties are missing\n");
> +		return -ENODEV;
> +	}
> +
> +	device_property_read_u32(dev, "upi,rsns-microohm", &curr_sense_res_uohm);
> +
> +	/*
> +	 * DAC maximum is 4.5V divided by 65536 steps + an unknown factor of 10
> +	 * coming from somewhere for some reason (verified with a volt-meter).
> +	 */
> +	chip->uv_per_unit = 45000000/65536;
> +	/* Datasheet says 8.1 uV per unit for the current ADC */
> +	chip->ua_per_unit = 8100000 / curr_sense_res_uohm;
> +
> +	/* Use provided internal resistance as start point (in milli-ohm) */
> +	chip->intern_res_avg = chip->info->factory_internal_resistance_uohm / 1000;
> +	/* Also add it to the internal resistance moving average window */
> +	chip->intern_res[0] = chip->intern_res_avg;
> +	chip->intern_res_avg_index = 1;
> +	chip->intern_res_poll_count = 1;
> +
> +	mutex_lock(&chip->lock);
> +	chip->psy = psy;
> +	mutex_unlock(&chip->lock);
> +
> +	ug3105_init(chip);
> +
> +	i2c_set_clientdata(client, chip);
> +	return 0;
> +}
> +
> +static int __maybe_unused ug3105_suspend(struct device *dev)
> +{
> +	struct ug3105_chip *chip = dev_get_drvdata(dev);
> +
> +	cancel_delayed_work_sync(&chip->work);
> +	i2c_smbus_write_byte_data(chip->client, UG3105_REG_MODE,
> +				  UG3105_MODE_STANDBY);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused ug3105_resume(struct device *dev)
> +{
> +	struct ug3105_chip *chip = dev_get_drvdata(dev);
> +
> +	ug3105_init(chip);
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(ug3105_pm_ops, ug3105_suspend,
> +			ug3105_resume);
> +
> +static const struct i2c_device_id ug3105_id[] = {
> +	{ "ug3105" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ug3105_id);
> +
> +static struct i2c_driver ug3105_i2c_driver = {
> +	.driver	= {
> +		.name = "ug3105",
> +		.pm = &ug3105_pm_ops,
> +	},
> +	.probe_new = ug3105_probe,
> +	.id_table = ug3105_id,
> +};
> +module_i2c_driver(ug3105_i2c_driver);
> +
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com");
> +MODULE_DESCRIPTION("uPI uG3105 battery monitor driver");
> +MODULE_LICENSE("GPL");

Otherwise LGTM.

-- Sebastian

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

Re: [PATCH 8/8] power: supply: ug3105_battery: Add driver for uPI uG3105 battery monitor

[Hans de Goede]

Hi Sebastian,

Thank you for the review and thank you for merging the axp288-fuel-gauge
and the bug bq25890 series.

On 2/2/22 00:33, Sebastian Reichel wrote:
> Hi,
> 
> On Mon, Jan 31, 2022 at 04:57:30PM +0100, Hans de Goede wrote:
>> Add a new battery driver for the uPI uG3105 battery monitor.
>>
>> Note the uG3105 is not a full-featured autonomous fuel-gauge. Instead it
>> is expected to be use in combination with some always on microcontroller
>> reading its coulomb-counter before it can wrap (must be read every 400
>> seconds!).
>>
>> Since Linux does not monitor coulomb-counter changes while the device is
>> off or suspended, the coulomb counter is not used atm.
>>
>> So far this driver is only used on x86/ACPI (non devicetree) devs
>> (also note there is no of_match table). Therefor there is no devicetree
>> bindings documentation for this driver's "upi,rsns-microohm" property
>> since this is not used in actual devicetree files and the dt bindings
>> maintainers have requested properties with no actual dt users to
>> _not_ be added to the dt bindings.
>>
>> The property's name has been chosen so that it should not need to be
>> changed if/when devicetree enumeration support gets added later, as it
>> mirrors "maxim,rsns-microohm" from the "maxim,max17042" bindings.
> 
> Except the vendor prefix being incorrect; please use this one:
> 
> $ grep -i upi Documentation/devicetree/bindings/vendor-prefixes.yaml
>   "^upisemi,.*":
>     description: uPI Semiconductor Corp.

Ah I didn't realize there already was a vendor prefix for uPI,
will fix for v2.

<snip>

> +static int ug3105_read_word(struct i2c_client *client, u8 reg)
>> +{
>> +	int val;
>> +
>> +	val = i2c_smbus_read_word_data(client, reg);
>> +	if (val < 0)
>> +		dev_err(&client->dev, "Error reading reg 0x%02x\n", reg);
>> +
>> +	return val;
>> +}
> 
> can we use regmap instead? :)

That would be tricky, since some registers are 8 bit, where as
others are 16 bit. Where as the i2c-regmap code assumes a fixed
register-size.

Also I don't really see much added value for this driver to use
the regmap abstraction, so I believe it would be better to
just keep this as is and I'm going to keep this as is for v2.

<snip>

>> +static int ug3105_probe(struct i2c_client *client)
>> +{
>> +	struct power_supply_config psy_cfg = {};
>> +	struct device *dev = &client->dev;
>> +	u32 curr_sense_res_uohm = 10000;
>> +	struct power_supply *psy;
>> +	struct ug3105_chip *chip;
>> +	int ret;
>> +
>> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
>> +	if (!chip)
>> +		return -ENOMEM;
>> +
>> +	chip->client = client;
>> +	mutex_init(&chip->lock);
>> +	ret = devm_delayed_work_autocancel(dev, &chip->work, ug3105_work);
>> +	if (ret)
>> +		return ret;
>> +
>> +	psy_cfg.drv_data = chip;
>> +	psy = devm_power_supply_register(dev, &ug3105_psy_desc, &psy_cfg);
>> +	if (IS_ERR(psy))
>> +		return PTR_ERR(psy);
>> +
>> +	ret = power_supply_get_battery_info(psy, &chip->info);
>> +	if (ret)
>> +		return ret;
> 
> this allocates data, so you need to call power_supply_put_battery_info()
> when chip->info is no loner required.

ug3105_work() and ug3105_get_property() both use the info, so it
is needed until the driver gets unbound; and since the memory allocated
by power_supply_get_battery_info() is allocated using devm_kmalloc
it will get freed on driver-unbind automatically.

So AFAICT there is no need to call power_supply_put_battery_info()
(which uses devm_kfree() internally to avoid the memory being free-ed
a second time on driver unbind).

Regards,

Hans