[PATCH 0/3] platform/x86: dell-ddv: Rework battery temperature handling

[PATCH 0/3] platform/x86: dell-ddv: Rework battery temperature handling

[Armin Wolf]

This patch series reworks the handling of the battery temperature
inside the dell-wmi-ddv driver.

The first patch fixes an issue inside the calculation formula for
the temperature value that resulted in strange temperature values
like 29.1 degrees celcius.

The second patch then simplifies the battery hook handling by using
devm_battery_hook_register().

The third patch finally makes use of the new power supply extension
mechanism to expose the battery temperature to userspace. The
power supply extension mechanism also takes care that the temperature
shows up inside the hwmon interface of the associated battery.

All patches where tested on a Dell Inspiron 3505 and appear to work.

Armin Wolf (3):
  platform/x86: dell-ddv: Fix temperature calculation
  platform/x86: dell-ddv: Use devm_battery_hook_register
  platform/x86: dell-ddv: Use the power supply extension mechanism

 drivers/platform/x86/dell/dell-wmi-ddv.c | 84 +++++++++++++-----------
 1 file changed, 46 insertions(+), 38 deletions(-)

--
2.39.5

[PATCH 1/3] platform/x86: dell-ddv: Fix temperature calculation

[Armin Wolf]

On the Dell Inspiron 3505 the battery temperature is always
0.1 degrees larger than the temperature show inside the OEM
application.

Emulate this behaviour to avoid showing strange looking values
like 29.1 degrees.

Fixes: 0331b1b0ba653 ("platform/x86: dell-ddv: Fix temperature scaling")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-ddv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index e75cd6e1efe6..ab5f7d3ab824 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -665,8 +665,10 @@ static ssize_t temp_show(struct device *dev, struct device_attribute *attr, char
 	if (ret < 0)
 		return ret;

-	/* Use 2731 instead of 2731.5 to avoid unnecessary rounding */
-	return sysfs_emit(buf, "%d\n", value - 2731);
+	/* Use 2732 instead of 2731.5 to avoid unnecessary rounding and to emulate
+	 * the behaviour of the OEM application which seems to round down the result.
+	 */
+	return sysfs_emit(buf, "%d\n", value - 2732);
 }

 static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, char *buf)
--
2.39.5

[PATCH 2/3] platform/x86: dell-ddv: Use devm_battery_hook_register

[Armin Wolf]

Use devm_battery_hook_register() instead of manually calling
devm_add_action_or_reset() to simplify the code.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-ddv.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index ab5f7d3ab824..811cddab57fc 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -732,13 +732,6 @@ static int dell_wmi_ddv_remove_battery(struct power_supply *battery, struct acpi
 	return 0;
 }

-static void dell_wmi_ddv_battery_remove(void *data)
-{
-	struct acpi_battery_hook *hook = data;
-
-	battery_hook_unregister(hook);
-}
-
 static int dell_wmi_ddv_battery_add(struct dell_wmi_ddv_data *data)
 {
 	data->hook.name = "Dell DDV Battery Extension";
@@ -755,9 +748,7 @@ static int dell_wmi_ddv_battery_add(struct dell_wmi_ddv_data *data)
 	data->eppid_attr.attr.mode = 0444;
 	data->eppid_attr.show = eppid_show;

-	battery_hook_register(&data->hook);
-
-	return devm_add_action_or_reset(&data->wdev->dev, dell_wmi_ddv_battery_remove, &data->hook);
+	return devm_battery_hook_register(&data->wdev->dev, &data->hook);
 }

 static int dell_wmi_ddv_buffer_read(struct seq_file *seq, enum dell_ddv_method method)
--
2.39.5

[PATCH 3/3] platform/x86: dell-ddv: Use the power supply extension mechanism

[Armin Wolf]

Use the power supply extension mechanism for registering the battery
temperature properties so that they can show up in the hwmon device
associated with the ACPI battery.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-ddv.c | 75 ++++++++++++++----------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index 811cddab57fc..f27739da380f 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -104,7 +104,6 @@ struct dell_wmi_ddv_sensors {

 struct dell_wmi_ddv_data {
 	struct acpi_battery_hook hook;
-	struct device_attribute temp_attr;
 	struct device_attribute eppid_attr;
 	struct dell_wmi_ddv_sensors fans;
 	struct dell_wmi_ddv_sensors temps;
@@ -651,26 +650,6 @@ static int dell_wmi_ddv_battery_index(struct acpi_device *acpi_dev, u32 *index)
 	return kstrtou32(uid_str, 10, index);
 }

-static ssize_t temp_show(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct dell_wmi_ddv_data *data = container_of(attr, struct dell_wmi_ddv_data, temp_attr);
-	u32 index, value;
-	int ret;
-
-	ret = dell_wmi_ddv_battery_index(to_acpi_device(dev->parent), &index);
-	if (ret < 0)
-		return ret;
-
-	ret = dell_wmi_ddv_query_integer(data->wdev, DELL_DDV_BATTERY_TEMPERATURE, index, &value);
-	if (ret < 0)
-		return ret;
-
-	/* Use 2732 instead of 2731.5 to avoid unnecessary rounding and to emulate
-	 * the behaviour of the OEM application which seems to round down the result.
-	 */
-	return sysfs_emit(buf, "%d\n", value - 2732);
-}
-
 static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct dell_wmi_ddv_data *data = container_of(attr, struct dell_wmi_ddv_data, eppid_attr);
@@ -697,6 +676,46 @@ static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, cha
 	return ret;
 }

+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)
+{
+	struct dell_wmi_ddv_data *data = drvdata;
+	u32 index, value;
+	int ret;
+
+	ret = dell_wmi_ddv_battery_index(to_acpi_device(psy->dev.parent), &index);
+	if (ret < 0)
+		return ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_TEMP:
+		ret = dell_wmi_ddv_query_integer(data->wdev, DELL_DDV_BATTERY_TEMPERATURE, index,
+						 &value);
+		if (ret < 0)
+			return ret;
+
+		/* Use 2732 instead of 2731.5 to avoid unnecessary rounding and to emulate
+		 * the behaviour of the OEM application which seems to round down the result.
+		 */
+		val->intval = value - 2732;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const enum power_supply_property dell_wmi_ddv_properties[] = {
+	POWER_SUPPLY_PROP_TEMP,
+};
+
+static const struct power_supply_ext dell_wmi_ddv_extension = {
+	.name = DRIVER_NAME,
+	.properties = dell_wmi_ddv_properties,
+	.num_properties = ARRAY_SIZE(dell_wmi_ddv_properties),
+	.get_property = dell_wmi_ddv_get_property,
+};
+
 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);
@@ -708,13 +727,14 @@ static int dell_wmi_ddv_add_battery(struct power_supply *battery, struct acpi_ba
 	if (ret < 0)
 		return 0;

-	ret = device_create_file(&battery->dev, &data->temp_attr);
+	ret = device_create_file(&battery->dev, &data->eppid_attr);
 	if (ret < 0)
 		return ret;

-	ret = device_create_file(&battery->dev, &data->eppid_attr);
+	ret = power_supply_register_extension(battery, &dell_wmi_ddv_extension, &data->wdev->dev,
+					      data);
 	if (ret < 0) {
-		device_remove_file(&battery->dev, &data->temp_attr);
+		device_remove_file(&battery->dev, &data->eppid_attr);

 		return ret;
 	}
@@ -726,8 +746,8 @@ static int dell_wmi_ddv_remove_battery(struct power_supply *battery, struct acpi
 {
 	struct dell_wmi_ddv_data *data = container_of(hook, struct dell_wmi_ddv_data, hook);

-	device_remove_file(&battery->dev, &data->temp_attr);
 	device_remove_file(&battery->dev, &data->eppid_attr);
+	power_supply_unregister_extension(battery, &dell_wmi_ddv_extension);

 	return 0;
 }
@@ -738,11 +758,6 @@ static int dell_wmi_ddv_battery_add(struct dell_wmi_ddv_data *data)
 	data->hook.add_battery = dell_wmi_ddv_add_battery;
 	data->hook.remove_battery = dell_wmi_ddv_remove_battery;

-	sysfs_attr_init(&data->temp_attr.attr);
-	data->temp_attr.attr.name = "temp";
-	data->temp_attr.attr.mode = 0444;
-	data->temp_attr.show = temp_show;
-
 	sysfs_attr_init(&data->eppid_attr.attr);
 	data->eppid_attr.attr.name = "eppid";
 	data->eppid_attr.attr.mode = 0444;
--
2.39.5

Re: [PATCH 0/3] platform/x86: dell-ddv: Rework battery temperature handling

[Sebastian Reichel]

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

Hi,

On Wed, Mar 05, 2025 at 06:30:06AM +0100, Armin Wolf wrote:
> This patch series reworks the handling of the battery temperature
> inside the dell-wmi-ddv driver.
> 
> The first patch fixes an issue inside the calculation formula for
> the temperature value that resulted in strange temperature values
> like 29.1 degrees celcius.
> 
> The second patch then simplifies the battery hook handling by using
> devm_battery_hook_register().
> 
> The third patch finally makes use of the new power supply extension
> mechanism to expose the battery temperature to userspace. The
> power supply extension mechanism also takes care that the temperature
> shows up inside the hwmon interface of the associated battery.
> 
> All patches where tested on a Dell Inspiron 3505 and appear to work.

LGTM.

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

-- Sebastian

> Armin Wolf (3):
>   platform/x86: dell-ddv: Fix temperature calculation
>   platform/x86: dell-ddv: Use devm_battery_hook_register
>   platform/x86: dell-ddv: Use the power supply extension mechanism
> 
>  drivers/platform/x86/dell/dell-wmi-ddv.c | 84 +++++++++++++-----------
>  1 file changed, 46 insertions(+), 38 deletions(-)
> 
> --
> 2.39.5
> 
> 

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

Re: [PATCH 0/3] platform/x86: dell-ddv: Rework battery temperature handling

[Ilpo Järvinen]

On Wed, 05 Mar 2025 06:30:06 +0100, Armin Wolf wrote:

> This patch series reworks the handling of the battery temperature
> inside the dell-wmi-ddv driver.
> 
> The first patch fixes an issue inside the calculation formula for
> the temperature value that resulted in strange temperature values
> like 29.1 degrees celcius.
> 
> [...]


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/3] platform/x86: dell-ddv: Fix temperature calculation
      commit: 7a248294a3145bc65eb0d8980a0a8edbb1b92db4
[2/3] platform/x86: dell-ddv: Use devm_battery_hook_register
      commit: 8dc3f0161e35d6ceb12de4a70cbed593e5b0583f
[3/3] platform/x86: dell-ddv: Use the power supply extension mechanism
      commit: 99923a0df7852311fa3d01eaddb430c958780143

--
 i.