[PATCH v2 1/4] power: supply: core: Add additional health status values

[PATCH v2 1/4] power: supply: core: Add additional health status values

[Armin Wolf]

Some batteries can signal when an internal fuse was blown. In such a
case POWER_SUPPLY_HEALTH_DEAD is too vague for userspace applications
to perform meaningful diagnostics.

Additionally some batteries can also signal when some of their
internal cells are imbalanced. In such a case returning
POWER_SUPPLY_HEALTH_UNSPEC_FAILURE is again too vague for userspace
applications to perform meaningful diagnostics.

Add new health status values for both cases.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
Changes since v1:
 - rename "Fuse blown" to "Blown fuse"
 - rename "Cell imbalanced" to "Cell imbalance"
---
 Documentation/ABI/testing/sysfs-class-power | 2 +-
 drivers/power/supply/power_supply_sysfs.c   | 2 ++
 include/linux/power_supply.h                | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
index 2a5c1a09a28f..be8be54b183d 100644
--- a/Documentation/ABI/testing/sysfs-class-power
+++ b/Documentation/ABI/testing/sysfs-class-power
@@ -456,7 +456,7 @@ Description:
 			      "Over voltage", "Under voltage", "Unspecified failure", "Cold",
 			      "Watchdog timer expire", "Safety timer expire",
 			      "Over current", "Calibration required", "Warm",
-			      "Cool", "Hot", "No battery"
+			      "Cool", "Hot", "No battery", "Blown fuse", "Cell imbalance"
 
 What:		/sys/class/power_supply/<supply_name>/precharge_current
 Date:		June 2017
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index edb058c19c9c..2703ed1dd943 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -110,6 +110,8 @@ static const char * const POWER_SUPPLY_HEALTH_TEXT[] = {
 	[POWER_SUPPLY_HEALTH_COOL]		    = "Cool",
 	[POWER_SUPPLY_HEALTH_HOT]		    = "Hot",
 	[POWER_SUPPLY_HEALTH_NO_BATTERY]	    = "No battery",
+	[POWER_SUPPLY_HEALTH_BLOWN_FUSE]	    = "Blown fuse",
+	[POWER_SUPPLY_HEALTH_CELL_IMBALANCE]	    = "Cell imbalance",
 };
 
 static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = {
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 888824592953..69df3a452918 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -71,6 +71,8 @@ enum {
 	POWER_SUPPLY_HEALTH_COOL,
 	POWER_SUPPLY_HEALTH_HOT,
 	POWER_SUPPLY_HEALTH_NO_BATTERY,
+	POWER_SUPPLY_HEALTH_BLOWN_FUSE,
+	POWER_SUPPLY_HEALTH_CELL_IMBALANCE,
 };
 
 enum {
-- 
2.39.5

[PATCH v2 2/4] platform/x86: dell-ddv: Implement the battery matching algorithm

[Armin Wolf]

Since commit db0a507cb24d ("ACPICA: Update integer-to-hex-string
conversions") the battery serial number is no longer distorted,
allowing us to finally implement the battery matching algorithm.

The battery matchign algorithm is necessary when translating between
ACPI batteries and the associated indices used by the WMI interface
based on the battery serial number. Since this serial number can only
be retrieved when the battery is present we cannot perform the initial
translation inside dell_wmi_ddv_add_battery() because the ACPI battery
might be absent at this point in time.

Introduce dell_wmi_ddv_battery_translate() which implements the
battery matching algorithm and replaces dell_wmi_ddv_battery_index().
Also implement a translation cache for caching previous translations
between ACPI batteries and indices. This is necessary because
performing a translation can be very expensive.

Tested on a Dell Inspiron 3505.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 Documentation/wmi/devices/dell-wmi-ddv.rst |   8 --
 drivers/platform/x86/dell/dell-wmi-ddv.c   | 101 ++++++++++++++++++---
 2 files changed, 88 insertions(+), 21 deletions(-)

diff --git a/Documentation/wmi/devices/dell-wmi-ddv.rst b/Documentation/wmi/devices/dell-wmi-ddv.rst
index e0c20af30948..f10a623acca1 100644
--- a/Documentation/wmi/devices/dell-wmi-ddv.rst
+++ b/Documentation/wmi/devices/dell-wmi-ddv.rst
@@ -260,14 +260,6 @@ Some machines like the Dell Inspiron 3505 only support a single battery and thus
 ignore the battery index. Because of this the driver depends on the ACPI battery
 hook mechanism to discover batteries.
 
-.. note::
-   The ACPI battery matching algorithm currently used inside the driver is
-   outdated and does not match the algorithm described above. The reasons for
-   this are differences in the handling of the ToHexString() ACPI opcode between
-   Linux and Windows, which distorts the serial number of ACPI batteries on many
-   machines. Until this issue is resolved, the driver cannot use the above
-   algorithm.
-
 Reverse-Engineering the DDV WMI interface
 =========================================
 
diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index f27739da380f..711639001be4 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -39,6 +39,9 @@
 #define DELL_DDV_SUPPORTED_VERSION_MAX	3
 #define DELL_DDV_GUID	"8A42EA14-4F2A-FD45-6422-0087F7A7E608"
 
+/* Battery indices 1, 2 and 3 */
+#define DELL_DDV_NUM_BATTERIES		3
+
 #define DELL_EPPID_LENGTH	20
 #define DELL_EPPID_EXT_LENGTH	23
 
@@ -105,6 +108,8 @@ struct dell_wmi_ddv_sensors {
 struct dell_wmi_ddv_data {
 	struct acpi_battery_hook hook;
 	struct device_attribute eppid_attr;
+	struct mutex translation_cache_lock;	/* Protects the translation cache */
+	struct power_supply *translation_cache[DELL_DDV_NUM_BATTERIES];
 	struct dell_wmi_ddv_sensors fans;
 	struct dell_wmi_ddv_sensors temps;
 	struct wmi_device *wdev;
@@ -639,15 +644,78 @@ static int dell_wmi_ddv_hwmon_add(struct dell_wmi_ddv_data *data)
 	return ret;
 }
 
-static int dell_wmi_ddv_battery_index(struct acpi_device *acpi_dev, u32 *index)
+static int dell_wmi_ddv_battery_translate(struct dell_wmi_ddv_data *data,
+					  struct power_supply *battery, u32 *index)
 {
-	const char *uid_str;
+	u32 serial_dec, serial_hex, serial;
+	union power_supply_propval val;
+	int ret;
+
+	guard(mutex)(&data->translation_cache_lock);
+
+	for (int i = 0; i < ARRAY_SIZE(data->translation_cache); i++) {
+		if (data->translation_cache[i] == battery) {
+			dev_dbg(&data->wdev->dev, "Translation cache hit for battery index %u\n",
+				i + 1);
+			*index = i + 1;
+			return 0;
+		}
+	}
+
+	dev_dbg(&data->wdev->dev, "Translation cache miss\n");
+
+	/* Perform a translation between a ACPI battery and a battery index */
+
+	ret = power_supply_get_property(battery, POWER_SUPPLY_PROP_SERIAL_NUMBER, &val);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Some devices display the serial number of the ACPI battery (string!) as a decimal
+	 * number while other devices display it as a hexadecimal number. Because of this we
+	 * have to check both cases.
+	 */
+	ret = kstrtou32(val.strval, 16, &serial_hex);
+	if (ret < 0)
+		return ret;	/* Should never fail */
+
+	ret = kstrtou32(val.strval, 10, &serial_dec);
+	if (ret < 0)
+		serial_dec = 0; /* Can fail, thus we only mark serial_dec as invalid */
+
+	for (int i = 0; i < ARRAY_SIZE(data->translation_cache); i++) {
+		ret = dell_wmi_ddv_query_integer(data->wdev, DELL_DDV_BATTERY_SERIAL_NUMBER, i + 1,
+						 &serial);
+		if (ret < 0)
+			return ret;
 
-	uid_str = acpi_device_uid(acpi_dev);
-	if (!uid_str)
-		return -ENODEV;
+		/* A serial number of 0 signals that this index is not associated with a battery */
+		if (!serial)
+			continue;
 
-	return kstrtou32(uid_str, 10, index);
+		if (serial == serial_dec || serial == serial_hex) {
+			dev_dbg(&data->wdev->dev, "Translation cache update for battery index %u\n",
+				i + 1);
+			data->translation_cache[i] = battery;
+			*index = i + 1;
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+static void dell_wmi_battery_invalidate(struct dell_wmi_ddv_data *data,
+					struct power_supply *battery)
+{
+	guard(mutex)(&data->translation_cache_lock);
+
+	for (int i = 0; i < ARRAY_SIZE(data->translation_cache); i++) {
+		if (data->translation_cache[i] == battery) {
+			data->translation_cache[i] = NULL;
+			return;
+		}
+	}
 }
 
 static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, char *buf)
@@ -657,7 +725,7 @@ static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, cha
 	u32 index;
 	int ret;
 
-	ret = dell_wmi_ddv_battery_index(to_acpi_device(dev->parent), &index);
+	ret = dell_wmi_ddv_battery_translate(data, to_power_supply(dev), &index);
 	if (ret < 0)
 		return ret;
 
@@ -684,7 +752,7 @@ static int dell_wmi_ddv_get_property(struct power_supply *psy, const struct powe
 	u32 index, value;
 	int ret;
 
-	ret = dell_wmi_ddv_battery_index(to_acpi_device(psy->dev.parent), &index);
+	ret = dell_wmi_ddv_battery_translate(data, psy, &index);
 	if (ret < 0)
 		return ret;
 
@@ -719,13 +787,12 @@ static const struct power_supply_ext dell_wmi_ddv_extension = {
 static int dell_wmi_ddv_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
 {
 	struct dell_wmi_ddv_data *data = container_of(hook, struct dell_wmi_ddv_data, hook);
-	u32 index;
 	int ret;
 
-	/* Return 0 instead of error to avoid being unloaded */
-	ret = dell_wmi_ddv_battery_index(to_acpi_device(battery->dev.parent), &index);
-	if (ret < 0)
-		return 0;
+	/*
+	 * We cannot do the battery matching here since the battery might be absent, preventing
+	 * us from reading the serial number.
+	 */
 
 	ret = device_create_file(&battery->dev, &data->eppid_attr);
 	if (ret < 0)
@@ -749,11 +816,19 @@ static int dell_wmi_ddv_remove_battery(struct power_supply *battery, struct acpi
 	device_remove_file(&battery->dev, &data->eppid_attr);
 	power_supply_unregister_extension(battery, &dell_wmi_ddv_extension);
 
+	dell_wmi_battery_invalidate(data, battery);
+
 	return 0;
 }
 
 static int dell_wmi_ddv_battery_add(struct dell_wmi_ddv_data *data)
 {
+	int ret;
+
+	ret = devm_mutex_init(&data->wdev->dev, &data->translation_cache_lock);
+	if (ret < 0)
+		return ret;
+
 	data->hook.name = "Dell DDV Battery Extension";
 	data->hook.add_battery = dell_wmi_ddv_add_battery;
 	data->hook.remove_battery = dell_wmi_ddv_remove_battery;
-- 
2.39.5

[PATCH v2 3/4] platform/x86: dell-ddv: Expose the battery manufacture date to userspace

[Armin Wolf]

The manufacture date of a given battery is exposed over the Dell DDV
WMI interface using the "BatteryManufactureDate" WMI method. The
resulting data contains the manufacture date of the battery encoded
inside a 16-bit value as described in the Smart Battery Data
Specification.

Expose this value to userspace using the power supply extension
interface.

Tested on a Dell Inspiron 3505.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 Documentation/wmi/devices/dell-wmi-ddv.rst |  3 --
 drivers/platform/x86/dell/dell-wmi-ddv.c   | 58 ++++++++++++++++++++++
 2 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/Documentation/wmi/devices/dell-wmi-ddv.rst b/Documentation/wmi/devices/dell-wmi-ddv.rst
index f10a623acca1..41c553d5c77d 100644
--- a/Documentation/wmi/devices/dell-wmi-ddv.rst
+++ b/Documentation/wmi/devices/dell-wmi-ddv.rst
@@ -118,9 +118,6 @@ The date is encoded in the following manner:
 - bits 5 to 8 contain the manufacture month.
 - bits 9 to 15 contain the manufacture year biased by 1980.
 
-.. note::
-   The data format needs to be verified on more machines.
-
 WMI method BatterySerialNumber()
 --------------------------------
 
diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index 711639001be4..8bd3ff76bb91 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -8,6 +8,7 @@
 #define pr_format(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/acpi.h>
+#include <linux/bitfield.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/device/driver.h>
@@ -42,6 +43,10 @@
 /* Battery indices 1, 2 and 3 */
 #define DELL_DDV_NUM_BATTERIES		3
 
+#define SBS_MANUFACTURE_YEAR_MASK	GENMASK(15, 9)
+#define SBS_MANUFACTURE_MONTH_MASK	GENMASK(8, 5)
+#define SBS_MANUFACTURE_DAY_MASK	GENMASK(4, 0)
+
 #define DELL_EPPID_LENGTH	20
 #define DELL_EPPID_EXT_LENGTH	23
 
@@ -744,6 +749,52 @@ static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, cha
 	return ret;
 }
 
+static int dell_wmi_ddv_get_manufacture_date(struct dell_wmi_ddv_data *data, u32 index,
+					     enum power_supply_property psp,
+					     union power_supply_propval *val)
+{
+	u16 year, month, day;
+	u32 value;
+	int ret;
+
+	ret = dell_wmi_ddv_query_integer(data->wdev, DELL_DDV_BATTERY_MANUFACTURE_DATE,
+					 index, &value);
+
+	if (ret < 0)
+		return ret;
+
+	if (value > U16_MAX)
+		return -ENXIO;
+
+	/*
+	 * Some devices report a invalid manufacture date value
+	 * like 0.0.1980. Because of this we have to check the
+	 * whole value before exposing parts of it to user space.
+	 */
+	year = FIELD_GET(SBS_MANUFACTURE_YEAR_MASK, value) + 1980;
+	month = FIELD_GET(SBS_MANUFACTURE_MONTH_MASK, value);
+	if (month < 1 || month > 12)
+		return -ENODATA;
+
+	day = FIELD_GET(SBS_MANUFACTURE_DAY_MASK, value);
+	if (day < 1 || day > 31)
+		return -ENODATA;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_MANUFACTURE_YEAR:
+		val->intval = year;
+		return 0;
+	case POWER_SUPPLY_PROP_MANUFACTURE_MONTH:
+		val->intval = month;
+		return 0;
+	case POWER_SUPPLY_PROP_MANUFACTURE_DAY:
+		val->intval = day;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int dell_wmi_ddv_get_property(struct power_supply *psy, const struct power_supply_ext *ext,
 				     void *drvdata, enum power_supply_property psp,
 				     union power_supply_propval *val)
@@ -768,6 +819,10 @@ static int dell_wmi_ddv_get_property(struct power_supply *psy, const struct powe
 		 */
 		val->intval = value - 2732;
 		return 0;
+	case POWER_SUPPLY_PROP_MANUFACTURE_YEAR:
+	case POWER_SUPPLY_PROP_MANUFACTURE_MONTH:
+	case POWER_SUPPLY_PROP_MANUFACTURE_DAY:
+		return dell_wmi_ddv_get_manufacture_date(data, index, psp, val);
 	default:
 		return -EINVAL;
 	}
@@ -775,6 +830,9 @@ static int dell_wmi_ddv_get_property(struct power_supply *psy, const struct powe
 
 static const enum power_supply_property dell_wmi_ddv_properties[] = {
 	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
+	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
+	POWER_SUPPLY_PROP_MANUFACTURE_DAY,
 };
 
 static const struct power_supply_ext dell_wmi_ddv_extension = {
-- 
2.39.5

[PATCH v2 4/4] platform/x86: dell-ddv: Expose the battery health to userspace

[Armin Wolf]

The health of a given battery is exposed over the Dell DDV WMI
interface using the "BatteryManufactureAceess" WMI method. The
resulting data contains, among other data, the health status of
the battery.

Expose this value to userspace using the power supply extension
interface.

Tested on a Dell Inspiron 3505.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 Documentation/wmi/devices/dell-wmi-ddv.rst | 35 ++++++++-
 drivers/platform/x86/dell/dell-wmi-ddv.c   | 89 ++++++++++++++++++++++
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/Documentation/wmi/devices/dell-wmi-ddv.rst b/Documentation/wmi/devices/dell-wmi-ddv.rst
index 41c553d5c77d..109d4c5c922e 100644
--- a/Documentation/wmi/devices/dell-wmi-ddv.rst
+++ b/Documentation/wmi/devices/dell-wmi-ddv.rst
@@ -150,7 +150,40 @@ Returns the voltage flow of the battery in mV as an u16.
 WMI method BatteryManufactureAccess()
 -------------------------------------
 
-Returns a manufacture-defined value as an u16.
+Returns the health status of the battery as a u16.
+The health status encoded in the following manner:
+
+ - the third nibble contains the general failure mode
+ - the fourth nibble contains the specific failure code
+
+Valid failure modes are:
+
+ - permanent failure (``0x9``)
+ - overheat failure (``0xa``)
+ - overcurrent failure (``0xb``)
+
+All other failure modes are to be considered normal.
+
+The following failure codes are valid for a permanent failure:
+
+ - fuse blown (``0x0``)
+ - cell imbalance (``0x1``)
+ - overvoltage (``0x2``)
+ - fet failure (``0x3``)
+
+The last two bits of the failure code are to be ignored when the battery
+signals a permanent failure.
+
+The following failure codes a valid for a overheat failure:
+
+ - overheat at start of charging (``0x5``)
+ - overheat during charging (``0x7``)
+ - overheat during discharging (``0x8``)
+
+The following failure codes are valid for a overcurrent failure:
+
+ - overcurrent during charging (``0x6``)
+ - overcurrent during discharging (``0xb``)
 
 WMI method BatteryRelativeStateOfCharge()
 -----------------------------------------
diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index 8bd3ff76bb91..cb4e0c1b908e 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -47,6 +47,26 @@
 #define SBS_MANUFACTURE_MONTH_MASK	GENMASK(8, 5)
 #define SBS_MANUFACTURE_DAY_MASK	GENMASK(4, 0)
 
+#define MA_FAILURE_MODE_MASK			GENMASK(11, 8)
+#define MA_FAILURE_MODE_PERMANENT		0x9
+#define MA_FAILURE_MODE_OVERHEAT		0xA
+#define MA_FAILURE_MODE_OVERCURRENT		0xB
+
+#define MA_PERMANENT_FAILURE_CODE_MASK		GENMASK(13, 12)
+#define MA_PERMANENT_FAILURE_FUSE_BLOWN		0x0
+#define MA_PERMANENT_FAILURE_CELL_IMBALANCE	0x1
+#define MA_PERMANENT_FAILURE_OVERVOLTAGE	0x2
+#define MA_PERMANENT_FAILURE_FET_FAILURE	0x3
+
+#define MA_OVERHEAT_FAILURE_CODE_MASK		GENMASK(15, 12)
+#define MA_OVERHEAT_FAILURE_START		0x5
+#define MA_OVERHEAT_FAILURE_CHARGING		0x7
+#define MA_OVERHEAT_FAILURE_DISCHARGING		0x8
+
+#define MA_OVERCURRENT_FAILURE_CODE_MASK	GENMASK(15, 12)
+#define MA_OVERCURRENT_FAILURE_CHARGING		0x6
+#define MA_OVERCURRENT_FAILURE_DISCHARGING	0xB
+
 #define DELL_EPPID_LENGTH	20
 #define DELL_EPPID_EXT_LENGTH	23
 
@@ -749,6 +769,72 @@ static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, cha
 	return ret;
 }
 
+static int dell_wmi_ddv_get_health(struct dell_wmi_ddv_data *data, u32 index,
+				   union power_supply_propval *val)
+{
+	u32 value, code;
+	int ret;
+
+	ret = dell_wmi_ddv_query_integer(data->wdev, DELL_DDV_BATTERY_MANUFACTURER_ACCESS, index,
+					 &value);
+	if (ret < 0)
+		return ret;
+
+	switch (FIELD_GET(MA_FAILURE_MODE_MASK, value)) {
+	case MA_FAILURE_MODE_PERMANENT:
+		code = FIELD_GET(MA_PERMANENT_FAILURE_CODE_MASK, value);
+		switch (code) {
+		case MA_PERMANENT_FAILURE_FUSE_BLOWN:
+			val->intval = POWER_SUPPLY_HEALTH_BLOWN_FUSE;
+			return 0;
+		case MA_PERMANENT_FAILURE_CELL_IMBALANCE:
+			val->intval = POWER_SUPPLY_HEALTH_CELL_IMBALANCE;
+			return 0;
+		case MA_PERMANENT_FAILURE_OVERVOLTAGE:
+			val->intval = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+			return 0;
+		case MA_PERMANENT_FAILURE_FET_FAILURE:
+			val->intval = POWER_SUPPLY_HEALTH_DEAD;
+			return 0;
+		default:
+			dev_notice_once(&data->wdev->dev, "Unknown permanent failure code %u\n",
+					code);
+			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+			return 0;
+		}
+	case MA_FAILURE_MODE_OVERHEAT:
+		code = FIELD_GET(MA_OVERHEAT_FAILURE_CODE_MASK, value);
+		switch (code) {
+		case MA_OVERHEAT_FAILURE_START:
+		case MA_OVERHEAT_FAILURE_CHARGING:
+		case MA_OVERHEAT_FAILURE_DISCHARGING:
+			val->intval = POWER_SUPPLY_HEALTH_OVERHEAT;
+			return 0;
+		default:
+			dev_notice_once(&data->wdev->dev, "Unknown overheat failure code %u\n",
+					code);
+			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+			return 0;
+		}
+	case MA_FAILURE_MODE_OVERCURRENT:
+		code = FIELD_GET(MA_OVERCURRENT_FAILURE_CODE_MASK, value);
+		switch (code) {
+		case MA_OVERCURRENT_FAILURE_CHARGING:
+		case MA_OVERCURRENT_FAILURE_DISCHARGING:
+			val->intval = POWER_SUPPLY_HEALTH_OVERCURRENT;
+			return 0;
+		default:
+			dev_notice_once(&data->wdev->dev, "Unknown overcurrent failure code %u\n",
+					code);
+			val->intval = POWER_SUPPLY_HEALTH_UNSPEC_FAILURE;
+			return 0;
+		}
+	default:
+		val->intval = POWER_SUPPLY_HEALTH_GOOD;
+		return 0;
+	}
+}
+
 static int dell_wmi_ddv_get_manufacture_date(struct dell_wmi_ddv_data *data, u32 index,
 					     enum power_supply_property psp,
 					     union power_supply_propval *val)
@@ -808,6 +894,8 @@ static int dell_wmi_ddv_get_property(struct power_supply *psy, const struct powe
 		return ret;
 
 	switch (psp) {
+	case POWER_SUPPLY_PROP_HEALTH:
+		return dell_wmi_ddv_get_health(data, index, val);
 	case POWER_SUPPLY_PROP_TEMP:
 		ret = dell_wmi_ddv_query_integer(data->wdev, DELL_DDV_BATTERY_TEMPERATURE, index,
 						 &value);
@@ -829,6 +917,7 @@ static int dell_wmi_ddv_get_property(struct power_supply *psy, const struct powe
 }
 
 static const enum power_supply_property dell_wmi_ddv_properties[] = {
+	POWER_SUPPLY_PROP_HEALTH,
 	POWER_SUPPLY_PROP_TEMP,
 	POWER_SUPPLY_PROP_MANUFACTURE_YEAR,
 	POWER_SUPPLY_PROP_MANUFACTURE_MONTH,
-- 
2.39.5

Re: [PATCH v2 1/4] power: supply: core: Add additional health status values

[Sebastian Reichel]

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

Hi,

On Tue, Apr 29, 2025 at 02:36:03AM +0200, Armin Wolf wrote:
> Some batteries can signal when an internal fuse was blown. In such a
> case POWER_SUPPLY_HEALTH_DEAD is too vague for userspace applications
> to perform meaningful diagnostics.
> 
> Additionally some batteries can also signal when some of their
> internal cells are imbalanced. In such a case returning
> POWER_SUPPLY_HEALTH_UNSPEC_FAILURE is again too vague for userspace
> applications to perform meaningful diagnostics.
> 
> Add new health status values for both cases.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

> ---
> Changes since v1:
>  - rename "Fuse blown" to "Blown fuse"
>  - rename "Cell imbalanced" to "Cell imbalance"
> ---
>  Documentation/ABI/testing/sysfs-class-power | 2 +-
>  drivers/power/supply/power_supply_sysfs.c   | 2 ++
>  include/linux/power_supply.h                | 2 ++
>  3 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> index 2a5c1a09a28f..be8be54b183d 100644
> --- a/Documentation/ABI/testing/sysfs-class-power
> +++ b/Documentation/ABI/testing/sysfs-class-power
> @@ -456,7 +456,7 @@ Description:
>  			      "Over voltage", "Under voltage", "Unspecified failure", "Cold",
>  			      "Watchdog timer expire", "Safety timer expire",
>  			      "Over current", "Calibration required", "Warm",
> -			      "Cool", "Hot", "No battery"
> +			      "Cool", "Hot", "No battery", "Blown fuse", "Cell imbalance"
>  
>  What:		/sys/class/power_supply/<supply_name>/precharge_current
>  Date:		June 2017
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index edb058c19c9c..2703ed1dd943 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -110,6 +110,8 @@ static const char * const POWER_SUPPLY_HEALTH_TEXT[] = {
>  	[POWER_SUPPLY_HEALTH_COOL]		    = "Cool",
>  	[POWER_SUPPLY_HEALTH_HOT]		    = "Hot",
>  	[POWER_SUPPLY_HEALTH_NO_BATTERY]	    = "No battery",
> +	[POWER_SUPPLY_HEALTH_BLOWN_FUSE]	    = "Blown fuse",
> +	[POWER_SUPPLY_HEALTH_CELL_IMBALANCE]	    = "Cell imbalance",
>  };
>  
>  static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = {
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 888824592953..69df3a452918 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -71,6 +71,8 @@ enum {
>  	POWER_SUPPLY_HEALTH_COOL,
>  	POWER_SUPPLY_HEALTH_HOT,
>  	POWER_SUPPLY_HEALTH_NO_BATTERY,
> +	POWER_SUPPLY_HEALTH_BLOWN_FUSE,
> +	POWER_SUPPLY_HEALTH_CELL_IMBALANCE,
>  };
>  
>  enum {
> -- 
> 2.39.5
> 

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

Re: [PATCH v2 1/4] power: supply: core: Add additional health status values

[Ilpo Järvinen]

On Wed, 30 Apr 2025, Sebastian Reichel wrote:
> On Tue, Apr 29, 2025 at 02:36:03AM +0200, Armin Wolf wrote:
> > Some batteries can signal when an internal fuse was blown. In such a
> > case POWER_SUPPLY_HEALTH_DEAD is too vague for userspace applications
> > to perform meaningful diagnostics.
> > 
> > Additionally some batteries can also signal when some of their
> > internal cells are imbalanced. In such a case returning
> > POWER_SUPPLY_HEALTH_UNSPEC_FAILURE is again too vague for userspace
> > applications to perform meaningful diagnostics.
> > 
> > Add new health status values for both cases.
> > 
> > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> 
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Hi Sebastian,

Is it okay with you I take this through pdx86 tree?

--
 i.

> 
> -- Sebastian
> 
> > ---
> > Changes since v1:
> >  - rename "Fuse blown" to "Blown fuse"
> >  - rename "Cell imbalanced" to "Cell imbalance"
> > ---
> >  Documentation/ABI/testing/sysfs-class-power | 2 +-
> >  drivers/power/supply/power_supply_sysfs.c   | 2 ++
> >  include/linux/power_supply.h                | 2 ++
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> > index 2a5c1a09a28f..be8be54b183d 100644
> > --- a/Documentation/ABI/testing/sysfs-class-power
> > +++ b/Documentation/ABI/testing/sysfs-class-power
> > @@ -456,7 +456,7 @@ Description:
> >  			      "Over voltage", "Under voltage", "Unspecified failure", "Cold",
> >  			      "Watchdog timer expire", "Safety timer expire",
> >  			      "Over current", "Calibration required", "Warm",
> > -			      "Cool", "Hot", "No battery"
> > +			      "Cool", "Hot", "No battery", "Blown fuse", "Cell imbalance"
> >  
> >  What:		/sys/class/power_supply/<supply_name>/precharge_current
> >  Date:		June 2017
> > diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> > index edb058c19c9c..2703ed1dd943 100644
> > --- a/drivers/power/supply/power_supply_sysfs.c
> > +++ b/drivers/power/supply/power_supply_sysfs.c
> > @@ -110,6 +110,8 @@ static const char * const POWER_SUPPLY_HEALTH_TEXT[] = {
> >  	[POWER_SUPPLY_HEALTH_COOL]		    = "Cool",
> >  	[POWER_SUPPLY_HEALTH_HOT]		    = "Hot",
> >  	[POWER_SUPPLY_HEALTH_NO_BATTERY]	    = "No battery",
> > +	[POWER_SUPPLY_HEALTH_BLOWN_FUSE]	    = "Blown fuse",
> > +	[POWER_SUPPLY_HEALTH_CELL_IMBALANCE]	    = "Cell imbalance",
> >  };
> >  
> >  static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = {
> > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > index 888824592953..69df3a452918 100644
> > --- a/include/linux/power_supply.h
> > +++ b/include/linux/power_supply.h
> > @@ -71,6 +71,8 @@ enum {
> >  	POWER_SUPPLY_HEALTH_COOL,
> >  	POWER_SUPPLY_HEALTH_HOT,
> >  	POWER_SUPPLY_HEALTH_NO_BATTERY,
> > +	POWER_SUPPLY_HEALTH_BLOWN_FUSE,
> > +	POWER_SUPPLY_HEALTH_CELL_IMBALANCE,
> >  };
> >  
> >  enum {
> > -- 
> > 2.39.5
> > 
> 

Re: [PATCH v2 1/4] power: supply: core: Add additional health status values

[Ilpo Järvinen]

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

On Mon, 5 May 2025, Ilpo Järvinen wrote:
> On Wed, 30 Apr 2025, Sebastian Reichel wrote:
> > On Tue, Apr 29, 2025 at 02:36:03AM +0200, Armin Wolf wrote:
> > > Some batteries can signal when an internal fuse was blown. In such a
> > > case POWER_SUPPLY_HEALTH_DEAD is too vague for userspace applications
> > > to perform meaningful diagnostics.
> > > 
> > > Additionally some batteries can also signal when some of their
> > > internal cells are imbalanced. In such a case returning
> > > POWER_SUPPLY_HEALTH_UNSPEC_FAILURE is again too vague for userspace
> > > applications to perform meaningful diagnostics.
> > > 
> > > Add new health status values for both cases.
> > > 
> > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > 
> > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> Hi Sebastian,
> 
> Is it okay with you I take this through pdx86 tree?

Ping?

-- 
 i.

> 
> --
>  i.
> 
> > 
> > -- Sebastian
> > 
> > > ---
> > > Changes since v1:
> > >  - rename "Fuse blown" to "Blown fuse"
> > >  - rename "Cell imbalanced" to "Cell imbalance"
> > > ---
> > >  Documentation/ABI/testing/sysfs-class-power | 2 +-
> > >  drivers/power/supply/power_supply_sysfs.c   | 2 ++
> > >  include/linux/power_supply.h                | 2 ++
> > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> > > index 2a5c1a09a28f..be8be54b183d 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-power
> > > +++ b/Documentation/ABI/testing/sysfs-class-power
> > > @@ -456,7 +456,7 @@ Description:
> > >  			      "Over voltage", "Under voltage", "Unspecified failure", "Cold",
> > >  			      "Watchdog timer expire", "Safety timer expire",
> > >  			      "Over current", "Calibration required", "Warm",
> > > -			      "Cool", "Hot", "No battery"
> > > +			      "Cool", "Hot", "No battery", "Blown fuse", "Cell imbalance"
> > >  
> > >  What:		/sys/class/power_supply/<supply_name>/precharge_current
> > >  Date:		June 2017
> > > diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> > > index edb058c19c9c..2703ed1dd943 100644
> > > --- a/drivers/power/supply/power_supply_sysfs.c
> > > +++ b/drivers/power/supply/power_supply_sysfs.c
> > > @@ -110,6 +110,8 @@ static const char * const POWER_SUPPLY_HEALTH_TEXT[] = {
> > >  	[POWER_SUPPLY_HEALTH_COOL]		    = "Cool",
> > >  	[POWER_SUPPLY_HEALTH_HOT]		    = "Hot",
> > >  	[POWER_SUPPLY_HEALTH_NO_BATTERY]	    = "No battery",
> > > +	[POWER_SUPPLY_HEALTH_BLOWN_FUSE]	    = "Blown fuse",
> > > +	[POWER_SUPPLY_HEALTH_CELL_IMBALANCE]	    = "Cell imbalance",
> > >  };
> > >  
> > >  static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = {
> > > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > > index 888824592953..69df3a452918 100644
> > > --- a/include/linux/power_supply.h
> > > +++ b/include/linux/power_supply.h
> > > @@ -71,6 +71,8 @@ enum {
> > >  	POWER_SUPPLY_HEALTH_COOL,
> > >  	POWER_SUPPLY_HEALTH_HOT,
> > >  	POWER_SUPPLY_HEALTH_NO_BATTERY,
> > > +	POWER_SUPPLY_HEALTH_BLOWN_FUSE,
> > > +	POWER_SUPPLY_HEALTH_CELL_IMBALANCE,
> > >  };
> > >  
> > >  enum {
> > > -- 
> > > 2.39.5
> > > 
> > 
> 

Re: [PATCH v2 1/4] power: supply: core: Add additional health status values

[Sebastian Reichel]

Hi,

On Fri, May 09, 2025 at 01:09:43PM +0300, Ilpo Järvinen wrote:
> On Mon, 5 May 2025, Ilpo Järvinen wrote:
> > On Wed, 30 Apr 2025, Sebastian Reichel wrote:
> > > On Tue, Apr 29, 2025 at 02:36:03AM +0200, Armin Wolf wrote:
> > > > Some batteries can signal when an internal fuse was blown. In such a
> > > > case POWER_SUPPLY_HEALTH_DEAD is too vague for userspace applications
> > > > to perform meaningful diagnostics.
> > > > 
> > > > Additionally some batteries can also signal when some of their
> > > > internal cells are imbalanced. In such a case returning
> > > > POWER_SUPPLY_HEALTH_UNSPEC_FAILURE is again too vague for userspace
> > > > applications to perform meaningful diagnostics.
> > > > 
> > > > Add new health status values for both cases.
> > > > 
> > > > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > > 
> > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > 
> > Hi Sebastian,
> > 
> > Is it okay with you I take this through pdx86 tree?
> 
> Ping?

I just checked and that's fine. I don't expect any merge issues.

Greetings,

-- Sebastian

> 
> -- 
>  i.
> 
> > 
> > --
> >  i.
> > 
> > > 
> > > -- Sebastian
> > > 
> > > > ---
> > > > Changes since v1:
> > > >  - rename "Fuse blown" to "Blown fuse"
> > > >  - rename "Cell imbalanced" to "Cell imbalance"
> > > > ---
> > > >  Documentation/ABI/testing/sysfs-class-power | 2 +-
> > > >  drivers/power/supply/power_supply_sysfs.c   | 2 ++
> > > >  include/linux/power_supply.h                | 2 ++
> > > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/ABI/testing/sysfs-class-power b/Documentation/ABI/testing/sysfs-class-power
> > > > index 2a5c1a09a28f..be8be54b183d 100644
> > > > --- a/Documentation/ABI/testing/sysfs-class-power
> > > > +++ b/Documentation/ABI/testing/sysfs-class-power
> > > > @@ -456,7 +456,7 @@ Description:
> > > >  			      "Over voltage", "Under voltage", "Unspecified failure", "Cold",
> > > >  			      "Watchdog timer expire", "Safety timer expire",
> > > >  			      "Over current", "Calibration required", "Warm",
> > > > -			      "Cool", "Hot", "No battery"
> > > > +			      "Cool", "Hot", "No battery", "Blown fuse", "Cell imbalance"
> > > >  
> > > >  What:		/sys/class/power_supply/<supply_name>/precharge_current
> > > >  Date:		June 2017
> > > > diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> > > > index edb058c19c9c..2703ed1dd943 100644
> > > > --- a/drivers/power/supply/power_supply_sysfs.c
> > > > +++ b/drivers/power/supply/power_supply_sysfs.c
> > > > @@ -110,6 +110,8 @@ static const char * const POWER_SUPPLY_HEALTH_TEXT[] = {
> > > >  	[POWER_SUPPLY_HEALTH_COOL]		    = "Cool",
> > > >  	[POWER_SUPPLY_HEALTH_HOT]		    = "Hot",
> > > >  	[POWER_SUPPLY_HEALTH_NO_BATTERY]	    = "No battery",
> > > > +	[POWER_SUPPLY_HEALTH_BLOWN_FUSE]	    = "Blown fuse",
> > > > +	[POWER_SUPPLY_HEALTH_CELL_IMBALANCE]	    = "Cell imbalance",
> > > >  };
> > > >  
> > > >  static const char * const POWER_SUPPLY_TECHNOLOGY_TEXT[] = {
> > > > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > > > index 888824592953..69df3a452918 100644
> > > > --- a/include/linux/power_supply.h
> > > > +++ b/include/linux/power_supply.h
> > > > @@ -71,6 +71,8 @@ enum {
> > > >  	POWER_SUPPLY_HEALTH_COOL,
> > > >  	POWER_SUPPLY_HEALTH_HOT,
> > > >  	POWER_SUPPLY_HEALTH_NO_BATTERY,
> > > > +	POWER_SUPPLY_HEALTH_BLOWN_FUSE,
> > > > +	POWER_SUPPLY_HEALTH_CELL_IMBALANCE,
> > > >  };
> > > >  
> > > >  enum {
> > > > -- 
> > > > 2.39.5
> > > > 
> > > 
> > 

Re: [PATCH v2 1/4] power: supply: core: Add additional health status values

[Ilpo Järvinen]

On Tue, 29 Apr 2025 02:36:03 +0200, Armin Wolf wrote:

> Some batteries can signal when an internal fuse was blown. In such a
> case POWER_SUPPLY_HEALTH_DEAD is too vague for userspace applications
> to perform meaningful diagnostics.
> 
> Additionally some batteries can also signal when some of their
> internal cells are imbalanced. In such a case returning
> POWER_SUPPLY_HEALTH_UNSPEC_FAILURE is again too vague for userspace
> applications to perform meaningful diagnostics.
> 
> [...]


Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

The list of commits applied:
[1/4] power: supply: core: Add additional health status values
      commit: e6b07a34038716e010d9fd1ac74c1d84a501f369
[2/4] platform/x86: dell-ddv: Implement the battery matching algorithm
      commit: 52e59cf1332dc4da5aecaa64c20f4a9f902e3186
[3/4] platform/x86: dell-ddv: Expose the battery manufacture date to userspace
      commit: 366a50722c7071120a494aaf91c9193922e3d8f6
[4/4] platform/x86: dell-ddv: Expose the battery health to userspace
      commit: d5251eef71bab8bd0b9ea3fe0005ad3d2553c3bb

--
 i.

Re: [PATCH v2 1/4] power: supply: core: Add additional health status values

[Armin Wolf]

Am 12.05.25 um 00:32 schrieb Ilpo Järvinen:

> On Tue, 29 Apr 2025 02:36:03 +0200, Armin Wolf wrote:
>
>> Some batteries can signal when an internal fuse was blown. In such a
>> case POWER_SUPPLY_HEALTH_DEAD is too vague for userspace applications
>> to perform meaningful diagnostics.
>>
>> Additionally some batteries can also signal when some of their
>> internal cells are imbalanced. In such a case returning
>> POWER_SUPPLY_HEALTH_UNSPEC_FAILURE is again too vague for userspace
>> applications to perform meaningful diagnostics.
>>
>> [...]
>
> Thank you for your contribution, it has been applied to my local
> review-ilpo-next branch. Note it will show up in the public
> platform-drivers-x86/review-ilpo-next branch only once I've pushed my
> local branch there, which might take a while.

Thank you :)

> The list of commits applied:
> [1/4] power: supply: core: Add additional health status values
>        commit: e6b07a34038716e010d9fd1ac74c1d84a501f369
> [2/4] platform/x86: dell-ddv: Implement the battery matching algorithm
>        commit: 52e59cf1332dc4da5aecaa64c20f4a9f902e3186
> [3/4] platform/x86: dell-ddv: Expose the battery manufacture date to userspace
>        commit: 366a50722c7071120a494aaf91c9193922e3d8f6
> [4/4] platform/x86: dell-ddv: Expose the battery health to userspace
>        commit: d5251eef71bab8bd0b9ea3fe0005ad3d2553c3bb
>
> --
>   i.
>