[PATCH v2 0/1] add Acer battery control driver

[PATCH v2 0/1] add Acer battery control driver

[Jelle van der Waa]

This patch upstreams a part of the out of tree acer wmi battery
specifically the battery charge control and battery temperature. [1]
On my Acer Aspire A315-510P battery calibration did not work as expected
so for now this is left out.

Jelle van der Waa (1):
  platform/x86: add Acer battery control driver

 drivers/platform/x86/Kconfig            |  11 +
 drivers/platform/x86/Makefile           |   1 +
 drivers/platform/x86/acer-wmi-battery.c | 355 ++++++++++++++++++++++++
 3 files changed, 367 insertions(+)
 create mode 100644 drivers/platform/x86/acer-wmi-battery.c

-- 
2.52.0

[PATCH v2 1/1] platform/x86: add Acer battery control driver

[Jelle van der Waa]

Some Acer laptops can configure battery related features through Acer
Care Center on Windows. This driver uses the power supply extension to
set a battery charge limit and exposes the battery
temperature.

This driver is based on the existing acer-wmi-battery project on GitHub
and was tested on an Acer Aspire A315-510P.

Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>

---
v2:
- Alphabetically sort linux headers
- Include headers for types / _packed
- Use cleanup.h instead of goto + label
- Add missing prefix for set_battery_health_control
- General code formatting fixes
- Remove HWMON dependency in Kconfig
- Use wmidev_evaluate_method()
- Accept oversized ACPI buffers
- Use DRIVER_NAME for battery extension name
- Set no_singleton = true
- Implement DMI matching to support laptops with only battery
  temperature support.
---
 drivers/platform/x86/Kconfig            |  11 +
 drivers/platform/x86/Makefile           |   1 +
 drivers/platform/x86/acer-wmi-battery.c | 355 ++++++++++++++++++++++++
 3 files changed, 367 insertions(+)
 create mode 100644 drivers/platform/x86/acer-wmi-battery.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 4cb7d97a9fcc..88c11a698fb9 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -170,6 +170,17 @@ config ACER_WMI
 	  If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M
 	  here.
 
+config ACER_WMI_BATTERY
+	tristate "Acer WMI Battery"
+	depends on ACPI_WMI
+	depends on ACPI_BATTERY
+	help
+	  This is a driver for Acer laptops with battery health control. It
+	  adds charge limit control and battery temperature reporting.
+
+	  If you have an ACPI-WMI Battery compatible Acer laptop, say Y or M
+	  here.
+
 source "drivers/platform/x86/amd/Kconfig"
 
 config ADV_SWBUTTON
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index d25762f7114f..9cf28baff3ae 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_GIGABYTE_WMI)		+= gigabyte-wmi.o
 obj-$(CONFIG_ACERHDF)		+= acerhdf.o
 obj-$(CONFIG_ACER_WIRELESS)	+= acer-wireless.o
 obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
+obj-$(CONFIG_ACER_WMI_BATTERY)	+= acer-wmi-battery.o
 
 # AMD
 obj-y				+= amd/
diff --git a/drivers/platform/x86/acer-wmi-battery.c b/drivers/platform/x86/acer-wmi-battery.c
new file mode 100644
index 000000000000..abb45cfcc6fe
--- /dev/null
+++ b/drivers/platform/x86/acer-wmi-battery.c
@@ -0,0 +1,355 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * acer-wmi-battery.c: Acer battery health control driver
+ *
+ * This is a driver for the WMI battery health control interface found
+ * on some Acer laptops.  This interface allows to enable/disable a
+ * battery charge limit ("health mode") and exposes the battery temperature.
+ *
+ * Based on acer-wmi-battery https://github.com/frederik-h/acer-wmi-battery/
+ *
+ * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
+ */
+
+#include <linux/acpi.h>
+#include <linux/cleanup.h>
+#include <linux/compiler_attributes.h>
+#include <linux/dmi.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/power_supply.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/version.h>
+#include <linux/wmi.h>
+
+#include <acpi/battery.h>
+
+#define DRIVER_NAME	"acer-wmi-battery"
+
+#define ACER_BATTERY_GUID "79772EC5-04B1-4BFD-843C-61E7F77B6CC9"
+
+/*
+ * The Acer OEM software seems to always use this battery index,
+ * so we emulate this behaviour to not confuse the underlying firmware.
+ *
+ * However this also means that we only fully support devices with a
+ * single battery for now.
+ */
+#define ACER_BATTERY_INDEX	0x1
+
+struct get_battery_health_control_status_input {
+	u8 uBatteryNo;
+	u8 uFunctionQuery;
+	u8 uReserved[2];
+} __packed;
+
+struct get_battery_health_control_status_output {
+	u8 uFunctionList;
+	u8 uReturn[2];
+	u8 uFunctionStatus[5];
+} __packed;
+
+struct set_battery_health_control_input {
+	u8 uBatteryNo;
+	u8 uFunctionMask;
+	u8 uFunctionStatus;
+	u8 uReservedIn[5];
+} __packed;
+
+struct set_battery_health_control_output {
+	u8 uReturn;
+	u8 uReservedOut;
+} __packed;
+
+enum battery_mode {
+	HEALTH_MODE = 1,
+	CALIBRATION_MODE = 2,
+};
+
+struct acer_wmi_battery_data {
+	struct acpi_battery_hook hook;
+	struct wmi_device *wdev;
+	const struct power_supply_ext *battery_ext;
+	struct {
+		bool health_mode : 1;
+	} features;
+};
+
+static int acer_wmi_battery_get_information(struct acer_wmi_battery_data *data,
+					    u32 index, u32 battery, u32 *result)
+{
+	u32 args[2] = { index, battery };
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_buffer input = { sizeof(args), args };
+	int ret;
+
+	ret = wmidev_evaluate_method(data->wdev, 0, 19, &input, &output);
+	if (ACPI_FAILURE(ret))
+		return -EIO;
+
+	union acpi_object *obj __free(kfree) = output.pointer;
+	if (!obj)
+		return -EIO;
+
+	if (obj->type != ACPI_TYPE_BUFFER)
+		ret = -EIO;
+
+	if (obj->buffer.length < sizeof(u32)) {
+		dev_err(&data->wdev->dev, "WMI battery information call returned buffer of unexpected length %u\n",
+			obj->buffer.length);
+		ret = -EINVAL;
+	}
+
+	*result = get_unaligned_le32(obj->buffer.pointer);
+
+	return ret;
+}
+
+static int acer_wmi_battery_get_health_control_status(struct acer_wmi_battery_data *data,
+						      bool *health_mode)
+{
+	/*
+	 * Acer Care Center seems to always call the WMI method
+	 * with fixed parameters. This yields information about
+	 * the availability and state of both health and
+	 * calibration mode. The modes probably apply to
+	 * all batteries of the system.
+	 */
+	struct get_battery_health_control_status_input args = {
+		.uBatteryNo = ACER_BATTERY_INDEX,
+		.uFunctionQuery = 0x1,
+		.uReserved = { 0x0, 0x0 }
+	};
+	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
+	struct get_battery_health_control_status_output *status_output;
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	int ret;
+
+	ret = wmidev_evaluate_method(data->wdev, 0, 20, &input, &output);
+	if (ACPI_FAILURE(ret))
+		return -EIO;
+
+	union acpi_object *obj __free(kfree) = output.pointer;
+	if (!obj)
+		return -EIO;
+
+	if (obj->type != ACPI_TYPE_BUFFER)
+		ret = -EIO;
+
+	if (obj->buffer.length < 8) {
+		dev_err(&data->wdev->dev, "WMI battery status call returned a buffer of unexpected length %d\n",
+			obj->buffer.length);
+		ret = -EINVAL;
+	}
+
+	status_output = (struct get_battery_health_control_status_output *)obj->buffer.pointer;
+
+	if (health_mode) {
+		if (status_output->uFunctionList & HEALTH_MODE)
+			*health_mode = status_output->uFunctionStatus[0] > 0;
+		else
+			ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int acer_wmi_battery_set_battery_health_control(struct acer_wmi_battery_data *data,
+						       u8 function, bool function_status)
+{
+	struct set_battery_health_control_input args = {
+		.uBatteryNo = ACER_BATTERY_INDEX,
+		.uFunctionMask = function,
+		.uFunctionStatus = function_status ? 1 : 0,
+		.uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
+	};
+	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	int ret;
+
+	ret = wmidev_evaluate_method(data->wdev, 0, 21, &input, &output);
+	if (ACPI_FAILURE(ret))
+		return -EIO;
+
+	obj = output.pointer;
+
+	if (!obj)
+		return -EIO;
+
+	if (obj->type != ACPI_TYPE_BUFFER)
+		ret = -EIO;
+
+	if (obj->buffer.length < 4) {
+		dev_err(&data->wdev->dev, "WMI battery status set operation returned a buffer of unexpected length %d\n",
+			obj->buffer.length);
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static int acer_battery_ext_property_get(struct power_supply *psy,
+					 const struct power_supply_ext *ext,
+					 void *ext_data,
+					 enum power_supply_property psp,
+					 union power_supply_propval *val)
+{
+	struct acer_wmi_battery_data *data = ext_data;
+	bool health_mode;
+	u32 value;
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPES:
+		ret = acer_wmi_battery_get_health_control_status(data, &health_mode);
+		if (ret)
+			return ret;
+
+		val->intval = health_mode
+			      ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
+			      : POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+		break;
+	case POWER_SUPPLY_PROP_TEMP:
+		ret = acer_wmi_battery_get_information(data, 0x8, ACER_BATTERY_INDEX, &value);
+		if (ret)
+			return ret;
+
+		if (value > U16_MAX)
+			return -ERANGE;
+
+		val->intval = value - 2731;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int acer_battery_ext_property_set(struct power_supply *psy,
+					 const struct power_supply_ext *ext,
+					 void *ext_data,
+					 enum power_supply_property psp,
+					 const union power_supply_propval *val)
+{
+	struct acer_wmi_battery_data *data = ext_data;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPES:
+		return acer_wmi_battery_set_battery_health_control(data, HEALTH_MODE,
+				val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int acer_battery_ext_property_is_writeable(struct power_supply *psy,
+						  const struct power_supply_ext *ext,
+						  void *ext_data,
+						  enum power_supply_property psp)
+{
+	switch (psp) {
+	case POWER_SUPPLY_PROP_CHARGE_TYPES:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct dmi_system_id acer_wmi_battery_health_mode_table[] = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P")
+		}
+	},
+	{}
+};
+
+static const enum power_supply_property acer_battery_properties_v1[] = {
+	POWER_SUPPLY_PROP_TEMP,
+};
+
+static const enum power_supply_property acer_battery_properties_v2[] = {
+	POWER_SUPPLY_PROP_TEMP,
+	POWER_SUPPLY_PROP_CHARGE_TYPES,
+};
+
+#define DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(_name, _power_supply_props)		\
+	static const struct power_supply_ext _name = {					\
+		.name			= DRIVER_NAME,					\
+		.properties		= _power_supply_props,				\
+		.num_properties		= ARRAY_SIZE(_power_supply_props),		\
+		.charge_types           = BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |	\
+					  BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE),	\
+		.get_property		= acer_battery_ext_property_get,		\
+		.set_property		= acer_battery_ext_property_set,		\
+		.property_is_writeable	= acer_battery_ext_property_is_writeable,	\
+	}
+
+DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(acer_wmi_battery_extension_v1, acer_battery_properties_v1);
+DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(acer_wmi_battery_extension_v2, acer_battery_properties_v2);
+
+static int acer_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+	struct acer_wmi_battery_data *data = container_of(hook, struct acer_wmi_battery_data, hook);
+
+	return power_supply_register_extension(battery, data->battery_ext,
+					       &data->wdev->dev, data);
+}
+
+static int acer_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+	struct acer_wmi_battery_data *data = container_of(hook, struct acer_wmi_battery_data, hook);
+
+	power_supply_unregister_extension(battery, data->battery_ext);
+
+	return 0;
+}
+
+static int acer_wmi_battery_probe(struct wmi_device *wdev, const void *context)
+{
+	struct acer_wmi_battery_data *data;
+
+	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	dev_set_drvdata(&wdev->dev, data);
+	data->wdev = wdev;
+	data->features.health_mode = dmi_check_system(acer_wmi_battery_health_mode_table);
+	data->battery_ext = data->features.health_mode
+			    ? &acer_wmi_battery_extension_v2
+			    : &acer_wmi_battery_extension_v1;
+	data->hook.name = "Acer Battery Extension";
+	data->hook.add_battery = acer_battery_add;
+	data->hook.remove_battery = acer_battery_remove;
+
+	return devm_battery_hook_register(&data->wdev->dev, &data->hook);
+}
+
+static const struct wmi_device_id acer_wmi_battery_id_table[] = {
+	{ ACER_BATTERY_GUID, NULL },
+	{ }
+};
+MODULE_DEVICE_TABLE(wmi, acer_wmi_battery_id_table);
+
+static struct wmi_driver acer_wmi_battery_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+	},
+	.id_table = acer_wmi_battery_id_table,
+	.probe = acer_wmi_battery_probe,
+	.no_singleton = true,
+};
+module_wmi_driver(acer_wmi_battery_driver);
+
+MODULE_AUTHOR("Frederik Harwath <frederik@harwath.name>");
+MODULE_AUTHOR("Jelle van der Waa <jelle@vdwaa.nl>");
+MODULE_DESCRIPTION("Acer battery health control WMI driver");
+MODULE_LICENSE("GPL");
-- 
2.52.0

Re: [PATCH v2 1/1] platform/x86: add Acer battery control driver

[Ilpo Järvinen]

On Sun, 25 Jan 2026, Jelle van der Waa wrote:

> Some Acer laptops can configure battery related features through Acer
> Care Center on Windows. This driver uses the power supply extension to
> set a battery charge limit and exposes the battery
> temperature.
> 
> This driver is based on the existing acer-wmi-battery project on GitHub
> and was tested on an Acer Aspire A315-510P.
> 
> Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
> 
> ---
> v2:
> - Alphabetically sort linux headers
> - Include headers for types / _packed
> - Use cleanup.h instead of goto + label
> - Add missing prefix for set_battery_health_control
> - General code formatting fixes
> - Remove HWMON dependency in Kconfig
> - Use wmidev_evaluate_method()
> - Accept oversized ACPI buffers
> - Use DRIVER_NAME for battery extension name
> - Set no_singleton = true
> - Implement DMI matching to support laptops with only battery
>   temperature support.
> ---
>  drivers/platform/x86/Kconfig            |  11 +
>  drivers/platform/x86/Makefile           |   1 +
>  drivers/platform/x86/acer-wmi-battery.c | 355 ++++++++++++++++++++++++
>  3 files changed, 367 insertions(+)
>  create mode 100644 drivers/platform/x86/acer-wmi-battery.c
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 4cb7d97a9fcc..88c11a698fb9 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -170,6 +170,17 @@ config ACER_WMI
>  	  If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M
>  	  here.
>  
> +config ACER_WMI_BATTERY
> +	tristate "Acer WMI Battery"
> +	depends on ACPI_WMI
> +	depends on ACPI_BATTERY
> +	help
> +	  This is a driver for Acer laptops with battery health control. It
> +	  adds charge limit control and battery temperature reporting.
> +
> +	  If you have an ACPI-WMI Battery compatible Acer laptop, say Y or M
> +	  here.
> +
>  source "drivers/platform/x86/amd/Kconfig"
>  
>  config ADV_SWBUTTON
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index d25762f7114f..9cf28baff3ae 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_GIGABYTE_WMI)		+= gigabyte-wmi.o
>  obj-$(CONFIG_ACERHDF)		+= acerhdf.o
>  obj-$(CONFIG_ACER_WIRELESS)	+= acer-wireless.o
>  obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
> +obj-$(CONFIG_ACER_WMI_BATTERY)	+= acer-wmi-battery.o
>  
>  # AMD
>  obj-y				+= amd/
> diff --git a/drivers/platform/x86/acer-wmi-battery.c b/drivers/platform/x86/acer-wmi-battery.c
> new file mode 100644
> index 000000000000..abb45cfcc6fe
> --- /dev/null
> +++ b/drivers/platform/x86/acer-wmi-battery.c
> @@ -0,0 +1,355 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * acer-wmi-battery.c: Acer battery health control driver
> + *
> + * This is a driver for the WMI battery health control interface found
> + * on some Acer laptops.  This interface allows to enable/disable a
> + * battery charge limit ("health mode") and exposes the battery temperature.
> + *
> + * Based on acer-wmi-battery https://github.com/frederik-h/acer-wmi-battery/
> + *
> + * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/cleanup.h>
> +#include <linux/compiler_attributes.h>
> +#include <linux/dmi.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/module.h>
> +#include <linux/power_supply.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/version.h>
> +#include <linux/wmi.h>
> +
> +#include <acpi/battery.h>
> +
> +#define DRIVER_NAME	"acer-wmi-battery"
> +
> +#define ACER_BATTERY_GUID "79772EC5-04B1-4BFD-843C-61E7F77B6CC9"
> +
> +/*
> + * The Acer OEM software seems to always use this battery index,
> + * so we emulate this behaviour to not confuse the underlying firmware.
> + *
> + * However this also means that we only fully support devices with a
> + * single battery for now.
> + */
> +#define ACER_BATTERY_INDEX	0x1
> +
> +struct get_battery_health_control_status_input {
> +	u8 uBatteryNo;
> +	u8 uFunctionQuery;
> +	u8 uReserved[2];

All these should be lower-cased.

> +} __packed;
> +
> +struct get_battery_health_control_status_output {
> +	u8 uFunctionList;
> +	u8 uReturn[2];
> +	u8 uFunctionStatus[5];
> +} __packed;
> +
> +struct set_battery_health_control_input {
> +	u8 uBatteryNo;
> +	u8 uFunctionMask;
> +	u8 uFunctionStatus;
> +	u8 uReservedIn[5];
> +} __packed;
> +
> +struct set_battery_health_control_output {
> +	u8 uReturn;
> +	u8 uReservedOut;
> +} __packed;
> +
> +enum battery_mode {
> +	HEALTH_MODE = 1,
> +	CALIBRATION_MODE = 2,
> +};
> +
> +struct acer_wmi_battery_data {
> +	struct acpi_battery_hook hook;
> +	struct wmi_device *wdev;
> +	const struct power_supply_ext *battery_ext;
> +	struct {
> +		bool health_mode : 1;
> +	} features;
> +};
> +
> +static int acer_wmi_battery_get_information(struct acer_wmi_battery_data *data,
> +					    u32 index, u32 battery, u32 *result)
> +{
> +	u32 args[2] = { index, battery };
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer input = { sizeof(args), args };
> +	int ret;
> +
> +	ret = wmidev_evaluate_method(data->wdev, 0, 19, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	union acpi_object *obj __free(kfree) = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER)
> +		ret = -EIO;

return -EIO; ?

IMO, you could do:

	if (!obj || obj->type != ACPI_TYPE_BUFFER)
		return -EIO;

> +
> +	if (obj->buffer.length < sizeof(u32)) {
> +		dev_err(&data->wdev->dev, "WMI battery information call returned buffer of unexpected length %u\n",
> +			obj->buffer.length);

Add include.

> +		ret = -EINVAL;

return -EINVAL; ?

These seem to be a common pattern in your functions so please check them 
all.

> +	}
> +
> +	*result = get_unaligned_le32(obj->buffer.pointer);

Are these pointers directly compatible? I think sparse with endianness 
checking enabled will complain about passing obj->buffer.pointer directly 
here.

> +
> +	return ret;
> +}
> +
> +static int acer_wmi_battery_get_health_control_status(struct acer_wmi_battery_data *data,
> +						      bool *health_mode)
> +{
> +	/*
> +	 * Acer Care Center seems to always call the WMI method
> +	 * with fixed parameters. This yields information about
> +	 * the availability and state of both health and
> +	 * calibration mode. The modes probably apply to
> +	 * all batteries of the system.
> +	 */
> +	struct get_battery_health_control_status_input args = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionQuery = 0x1,
> +		.uReserved = { 0x0, 0x0 }

Add the comma to any non-terminating entry.

> +	};
> +	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
> +	struct get_battery_health_control_status_output *status_output;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	int ret;
> +
> +	ret = wmidev_evaluate_method(data->wdev, 0, 20, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	union acpi_object *obj __free(kfree) = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER)
> +		ret = -EIO;
> +
> +	if (obj->buffer.length < 8) {
> +		dev_err(&data->wdev->dev, "WMI battery status call returned a buffer of unexpected length %d\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +	}
> +
> +	status_output = (struct get_battery_health_control_status_output *)obj->buffer.pointer;
> +
> +	if (health_mode) {
> +		if (status_output->uFunctionList & HEALTH_MODE)
> +			*health_mode = status_output->uFunctionStatus[0] > 0;
> +		else
> +			ret = -EINVAL;

return right here.

I suggest you reverse the if logic and return the error first and remove 
else entirely.

> +	}
> +
> +	return ret;
> +}
> +
> +static int acer_wmi_battery_set_battery_health_control(struct acer_wmi_battery_data *data,
> +						       u8 function, bool function_status)
> +{
> +	struct set_battery_health_control_input args = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionMask = function,
> +		.uFunctionStatus = function_status ? 1 : 0,
> +		.uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
> +	};
> +	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmidev_evaluate_method(data->wdev, 0, 21, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	obj = output.pointer;
> +
> +	if (!obj)

Remove the empty line between these two, as this is kind of "error 
handling".

> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER)
> +		ret = -EIO;
> +
> +	if (obj->buffer.length < 4) {
> +		dev_err(&data->wdev->dev, "WMI battery status set operation returned a buffer of unexpected length %d\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int acer_battery_ext_property_get(struct power_supply *psy,
> +					 const struct power_supply_ext *ext,
> +					 void *ext_data,
> +					 enum power_supply_property psp,
> +					 union power_supply_propval *val)
> +{
> +	struct acer_wmi_battery_data *data = ext_data;
> +	bool health_mode;
> +	u32 value;
> +	int ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		ret = acer_wmi_battery_get_health_control_status(data, &health_mode);
> +		if (ret)
> +			return ret;
> +
> +		val->intval = health_mode
> +			      ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
> +			      : POWER_SUPPLY_CHARGE_TYPE_STANDARD;

		val->intval = health_mode ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
					  : POWER_SUPPLY_CHARGE_TYPE_STANDARD;

> +		break;
> +	case POWER_SUPPLY_PROP_TEMP:
> +		ret = acer_wmi_battery_get_information(data, 0x8, ACER_BATTERY_INDEX, &value);
> +		if (ret)
> +			return ret;
> +
> +		if (value > U16_MAX)

This is still error handling for the call so remove the empty line.

> +			return -ERANGE;
> +
> +		val->intval = value - 2731;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int acer_battery_ext_property_set(struct power_supply *psy,
> +					 const struct power_supply_ext *ext,
> +					 void *ext_data,
> +					 enum power_supply_property psp,
> +					 const union power_supply_propval *val)
> +{
> +	struct acer_wmi_battery_data *data = ext_data;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		return acer_wmi_battery_set_battery_health_control(data, HEALTH_MODE,

You seem to like very verbose function names, this even has "battery" 
twice :-). It would likely make the code slightly easier to read if you 
would try to make them shorter.

> +				val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int acer_battery_ext_property_is_writeable(struct power_supply *psy,
> +						  const struct power_supply_ext *ext,
> +						  void *ext_data,
> +						  enum power_supply_property psp)
> +{
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct dmi_system_id acer_wmi_battery_health_mode_table[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P")
> +		}
> +	},
> +	{}
> +};
> +
> +static const enum power_supply_property acer_battery_properties_v1[] = {
> +	POWER_SUPPLY_PROP_TEMP,
> +};
> +
> +static const enum power_supply_property acer_battery_properties_v2[] = {
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
> +};
> +
> +#define DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(_name, _power_supply_props)		\
> +	static const struct power_supply_ext _name = {					\
> +		.name			= DRIVER_NAME,					\
> +		.properties		= _power_supply_props,				\
> +		.num_properties		= ARRAY_SIZE(_power_supply_props),		\
> +		.charge_types           = BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |	\
> +					  BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE),	\
> +		.get_property		= acer_battery_ext_property_get,		\
> +		.set_property		= acer_battery_ext_property_set,		\
> +		.property_is_writeable	= acer_battery_ext_property_is_writeable,	\
> +	}
> +
> +DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(acer_wmi_battery_extension_v1, acer_battery_properties_v1);
> +DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(acer_wmi_battery_extension_v2, acer_battery_properties_v2);
> +
> +static int acer_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	struct acer_wmi_battery_data *data = container_of(hook, struct acer_wmi_battery_data, hook);
> +
> +	return power_supply_register_extension(battery, data->battery_ext,
> +					       &data->wdev->dev, data);
> +}
> +
> +static int acer_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	struct acer_wmi_battery_data *data = container_of(hook, struct acer_wmi_battery_data, hook);
> +
> +	power_supply_unregister_extension(battery, data->battery_ext);
> +
> +	return 0;
> +}
> +
> +static int acer_wmi_battery_probe(struct wmi_device *wdev, const void *context)
> +{
> +	struct acer_wmi_battery_data *data;
> +
> +	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&wdev->dev, data);
> +	data->wdev = wdev;
> +	data->features.health_mode = dmi_check_system(acer_wmi_battery_health_mode_table);
> +	data->battery_ext = data->features.health_mode
> +			    ? &acer_wmi_battery_extension_v2
> +			    : &acer_wmi_battery_extension_v1;
> +	data->hook.name = "Acer Battery Extension";
> +	data->hook.add_battery = acer_battery_add;
> +	data->hook.remove_battery = acer_battery_remove;
> +
> +	return devm_battery_hook_register(&data->wdev->dev, &data->hook);
> +}
> +
> +static const struct wmi_device_id acer_wmi_battery_id_table[] = {
> +	{ ACER_BATTERY_GUID, NULL },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(wmi, acer_wmi_battery_id_table);
> +
> +static struct wmi_driver acer_wmi_battery_driver = {
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +	.id_table = acer_wmi_battery_id_table,
> +	.probe = acer_wmi_battery_probe,
> +	.no_singleton = true,
> +};
> +module_wmi_driver(acer_wmi_battery_driver);
> +
> +MODULE_AUTHOR("Frederik Harwath <frederik@harwath.name>");
> +MODULE_AUTHOR("Jelle van der Waa <jelle@vdwaa.nl>");
> +MODULE_DESCRIPTION("Acer battery health control WMI driver");
> +MODULE_LICENSE("GPL");
> 

-- 
 i.

Re: [PATCH v2 1/1] platform/x86: add Acer battery control driver

[Armin Wolf]

Am 25.01.26 um 19:23 schrieb Jelle van der Waa:

> Some Acer laptops can configure battery related features through Acer
> Care Center on Windows. This driver uses the power supply extension to
> set a battery charge limit and exposes the battery
> temperature.
>
> This driver is based on the existing acer-wmi-battery project on GitHub
> and was tested on an Acer Aspire A315-510P.
>
> Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
>
> ---
> v2:
> - Alphabetically sort linux headers
> - Include headers for types / _packed
> - Use cleanup.h instead of goto + label
> - Add missing prefix for set_battery_health_control
> - General code formatting fixes
> - Remove HWMON dependency in Kconfig
> - Use wmidev_evaluate_method()
> - Accept oversized ACPI buffers
> - Use DRIVER_NAME for battery extension name
> - Set no_singleton = true
> - Implement DMI matching to support laptops with only battery
>    temperature support.
> ---
>   drivers/platform/x86/Kconfig            |  11 +
>   drivers/platform/x86/Makefile           |   1 +
>   drivers/platform/x86/acer-wmi-battery.c | 355 ++++++++++++++++++++++++
>   3 files changed, 367 insertions(+)
>   create mode 100644 drivers/platform/x86/acer-wmi-battery.c
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 4cb7d97a9fcc..88c11a698fb9 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -170,6 +170,17 @@ config ACER_WMI
>   	  If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y or M
>   	  here.
>   
> +config ACER_WMI_BATTERY
> +	tristate "Acer WMI Battery"
> +	depends on ACPI_WMI
> +	depends on ACPI_BATTERY

Please also depend on DMI so that dmi_check_system() will always be available
when building this driver.

> +	help
> +	  This is a driver for Acer laptops with battery health control. It
> +	  adds charge limit control and battery temperature reporting.
> +
> +	  If you have an ACPI-WMI Battery compatible Acer laptop, say Y or M
> +	  here.
> +
>   source "drivers/platform/x86/amd/Kconfig"
>   
>   config ADV_SWBUTTON
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index d25762f7114f..9cf28baff3ae 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_GIGABYTE_WMI)		+= gigabyte-wmi.o
>   obj-$(CONFIG_ACERHDF)		+= acerhdf.o
>   obj-$(CONFIG_ACER_WIRELESS)	+= acer-wireless.o
>   obj-$(CONFIG_ACER_WMI)		+= acer-wmi.o
> +obj-$(CONFIG_ACER_WMI_BATTERY)	+= acer-wmi-battery.o
>   
>   # AMD
>   obj-y				+= amd/
> diff --git a/drivers/platform/x86/acer-wmi-battery.c b/drivers/platform/x86/acer-wmi-battery.c
> new file mode 100644
> index 000000000000..abb45cfcc6fe
> --- /dev/null
> +++ b/drivers/platform/x86/acer-wmi-battery.c
> @@ -0,0 +1,355 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * acer-wmi-battery.c: Acer battery health control driver
> + *
> + * This is a driver for the WMI battery health control interface found
> + * on some Acer laptops.  This interface allows to enable/disable a
> + * battery charge limit ("health mode") and exposes the battery temperature.
> + *
> + * Based on acer-wmi-battery https://github.com/frederik-h/acer-wmi-battery/
> + *
> + * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/cleanup.h>
> +#include <linux/compiler_attributes.h>
> +#include <linux/dmi.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/module.h>
> +#include <linux/power_supply.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/version.h>
> +#include <linux/wmi.h>
> +
> +#include <acpi/battery.h>
> +
> +#define DRIVER_NAME	"acer-wmi-battery"
> +
> +#define ACER_BATTERY_GUID "79772EC5-04B1-4BFD-843C-61E7F77B6CC9"
> +
> +/*
> + * The Acer OEM software seems to always use this battery index,
> + * so we emulate this behaviour to not confuse the underlying firmware.
> + *
> + * However this also means that we only fully support devices with a
> + * single battery for now.
> + */
> +#define ACER_BATTERY_INDEX	0x1
> +
> +struct get_battery_health_control_status_input {
> +	u8 uBatteryNo;
> +	u8 uFunctionQuery;
> +	u8 uReserved[2];
> +} __packed;
> +
> +struct get_battery_health_control_status_output {
> +	u8 uFunctionList;
> +	u8 uReturn[2];
> +	u8 uFunctionStatus[5];
> +} __packed;
> +
> +struct set_battery_health_control_input {
> +	u8 uBatteryNo;
> +	u8 uFunctionMask;
> +	u8 uFunctionStatus;
> +	u8 uReservedIn[5];
> +} __packed;
> +
> +struct set_battery_health_control_output {
> +	u8 uReturn;
> +	u8 uReservedOut;
> +} __packed;
> +
> +enum battery_mode {
> +	HEALTH_MODE = 1,
> +	CALIBRATION_MODE = 2,
> +};
> +
> +struct acer_wmi_battery_data {
> +	struct acpi_battery_hook hook;
> +	struct wmi_device *wdev;
> +	const struct power_supply_ext *battery_ext;
> +	struct {
> +		bool health_mode : 1;
> +	} features;
> +};
> +
> +static int acer_wmi_battery_get_information(struct acer_wmi_battery_data *data,
> +					    u32 index, u32 battery, u32 *result)
> +{
> +	u32 args[2] = { index, battery };
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_buffer input = { sizeof(args), args };
> +	int ret;
> +
> +	ret = wmidev_evaluate_method(data->wdev, 0, 19, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	union acpi_object *obj __free(kfree) = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER)
> +		ret = -EIO;
> +
> +	if (obj->buffer.length < sizeof(u32)) {
> +		dev_err(&data->wdev->dev, "WMI battery information call returned buffer of unexpected length %u\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +	}
> +
> +	*result = get_unaligned_le32(obj->buffer.pointer);
> +
> +	return ret;
> +}
> +
> +static int acer_wmi_battery_get_health_control_status(struct acer_wmi_battery_data *data,
> +						      bool *health_mode)
> +{
> +	/*
> +	 * Acer Care Center seems to always call the WMI method
> +	 * with fixed parameters. This yields information about
> +	 * the availability and state of both health and
> +	 * calibration mode. The modes probably apply to
> +	 * all batteries of the system.
> +	 */
> +	struct get_battery_health_control_status_input args = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionQuery = 0x1,
> +		.uReserved = { 0x0, 0x0 }
> +	};
> +	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
> +	struct get_battery_health_control_status_output *status_output;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	int ret;
> +
> +	ret = wmidev_evaluate_method(data->wdev, 0, 20, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	union acpi_object *obj __free(kfree) = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER)
> +		ret = -EIO;
> +
> +	if (obj->buffer.length < 8) {

Better use sizeof(*status_output) here.

> +		dev_err(&data->wdev->dev, "WMI battery status call returned a buffer of unexpected length %d\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +	}
> +
> +	status_output = (struct get_battery_health_control_status_output *)obj->buffer.pointer;
> +
> +	if (health_mode) {
> +		if (status_output->uFunctionList & HEALTH_MODE)
> +			*health_mode = status_output->uFunctionStatus[0] > 0;
> +		else
> +			ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int acer_wmi_battery_set_battery_health_control(struct acer_wmi_battery_data *data,
> +						       u8 function, bool function_status)
> +{
> +	struct set_battery_health_control_input args = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionMask = function,
> +		.uFunctionStatus = function_status ? 1 : 0,
> +		.uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
> +	};
> +	struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmidev_evaluate_method(data->wdev, 0, 21, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	obj = output.pointer;
> +
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER)
> +		ret = -EIO;
> +
> +	if (obj->buffer.length < 4) {
> +		dev_err(&data->wdev->dev, "WMI battery status set operation returned a buffer of unexpected length %d\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static int acer_battery_ext_property_get(struct power_supply *psy,
> +					 const struct power_supply_ext *ext,
> +					 void *ext_data,
> +					 enum power_supply_property psp,
> +					 union power_supply_propval *val)
> +{
> +	struct acer_wmi_battery_data *data = ext_data;
> +	bool health_mode;
> +	u32 value;
> +	int ret;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		ret = acer_wmi_battery_get_health_control_status(data, &health_mode);
> +		if (ret)
> +			return ret;
> +
> +		val->intval = health_mode
> +			      ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
> +			      : POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> +		break;
> +	case POWER_SUPPLY_PROP_TEMP:
> +		ret = acer_wmi_battery_get_information(data, 0x8, ACER_BATTERY_INDEX, &value);
> +		if (ret)
> +			return ret;
> +
> +		if (value > U16_MAX)
> +			return -ERANGE;
> +
> +		val->intval = value - 2731;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int acer_battery_ext_property_set(struct power_supply *psy,
> +					 const struct power_supply_ext *ext,
> +					 void *ext_data,
> +					 enum power_supply_property psp,
> +					 const union power_supply_propval *val)
> +{
> +	struct acer_wmi_battery_data *data = ext_data;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		return acer_wmi_battery_set_battery_health_control(data, HEALTH_MODE,
> +				val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int acer_battery_ext_property_is_writeable(struct power_supply *psy,
> +						  const struct power_supply_ext *ext,
> +						  void *ext_data,
> +						  enum power_supply_property psp)
> +{
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_CHARGE_TYPES:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct dmi_system_id acer_wmi_battery_health_mode_table[] = {
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P")
> +		}
> +	},
> +	{}
> +};

I suggest that you perform the DMI check during module initialization. This way
the DMI table can be marked as __initconst and does not consume any memory after
anymore after module initialization is complete.

The result of the DMI check could then be stored inside a global variable.

Other than that, the driver seems fine to me.

Thanks,
Armin Wolf

> +
> +static const enum power_supply_property acer_battery_properties_v1[] = {
> +	POWER_SUPPLY_PROP_TEMP,
> +};
> +
> +static const enum power_supply_property acer_battery_properties_v2[] = {
> +	POWER_SUPPLY_PROP_TEMP,
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
> +};
> +
> +#define DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(_name, _power_supply_props)		\
> +	static const struct power_supply_ext _name = {					\
> +		.name			= DRIVER_NAME,					\
> +		.properties		= _power_supply_props,				\
> +		.num_properties		= ARRAY_SIZE(_power_supply_props),		\
> +		.charge_types           = BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |	\
> +					  BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE),	\
> +		.get_property		= acer_battery_ext_property_get,		\
> +		.set_property		= acer_battery_ext_property_set,		\
> +		.property_is_writeable	= acer_battery_ext_property_is_writeable,	\
> +	}
> +
> +DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(acer_wmi_battery_extension_v1, acer_battery_properties_v1);
> +DEFINE_ACER_WMI_POWER_SUPPLY_EXTENSION(acer_wmi_battery_extension_v2, acer_battery_properties_v2);

Please drop the macro, saving a couple of lines here provides not enough benefit.

> +
> +static int acer_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	struct acer_wmi_battery_data *data = container_of(hook, struct acer_wmi_battery_data, hook);
> +
> +	return power_supply_register_extension(battery, data->battery_ext,
> +					       &data->wdev->dev, data);
> +}
> +
> +static int acer_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	struct acer_wmi_battery_data *data = container_of(hook, struct acer_wmi_battery_data, hook);
> +
> +	power_supply_unregister_extension(battery, data->battery_ext);
> +
> +	return 0;
> +}
> +
> +static int acer_wmi_battery_probe(struct wmi_device *wdev, const void *context)
> +{
> +	struct acer_wmi_battery_data *data;
> +
> +	data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&wdev->dev, data);
> +	data->wdev = wdev;
> +	data->features.health_mode = dmi_check_system(acer_wmi_battery_health_mode_table);
> +	data->battery_ext = data->features.health_mode
> +			    ? &acer_wmi_battery_extension_v2
> +			    : &acer_wmi_battery_extension_v1;
> +	data->hook.name = "Acer Battery Extension";
> +	data->hook.add_battery = acer_battery_add;
> +	data->hook.remove_battery = acer_battery_remove;
> +
> +	return devm_battery_hook_register(&data->wdev->dev, &data->hook);
> +}
> +
> +static const struct wmi_device_id acer_wmi_battery_id_table[] = {
> +	{ ACER_BATTERY_GUID, NULL },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(wmi, acer_wmi_battery_id_table);
> +
> +static struct wmi_driver acer_wmi_battery_driver = {
> +	.driver = {
> +		.name = DRIVER_NAME,
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +	},
> +	.id_table = acer_wmi_battery_id_table,
> +	.probe = acer_wmi_battery_probe,
> +	.no_singleton = true,
> +};
> +module_wmi_driver(acer_wmi_battery_driver);
> +
> +MODULE_AUTHOR("Frederik Harwath <frederik@harwath.name>");
> +MODULE_AUTHOR("Jelle van der Waa <jelle@vdwaa.nl>");
> +MODULE_DESCRIPTION("Acer battery health control WMI driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH v2 1/1] platform/x86: add Acer battery control driver

[Jelle van der Waa]

On 1/31/26 00:24, Armin Wolf wrote:
> Am 25.01.26 um 19:23 schrieb Jelle van der Waa:
> 
>> Some Acer laptops can configure battery related features through Acer
>> Care Center on Windows. This driver uses the power supply extension to
>> set a battery charge limit and exposes the battery
>> temperature.
>>
>> This driver is based on the existing acer-wmi-battery project on GitHub
>> and was tested on an Acer Aspire A315-510P.
>>
>> Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
>>
>> ---
>> v2:
>> - Alphabetically sort linux headers
>> - Include headers for types / _packed
>> - Use cleanup.h instead of goto + label
>> - Add missing prefix for set_battery_health_control
>> - General code formatting fixes
>> - Remove HWMON dependency in Kconfig
>> - Use wmidev_evaluate_method()
>> - Accept oversized ACPI buffers
>> - Use DRIVER_NAME for battery extension name
>> - Set no_singleton = true
>> - Implement DMI matching to support laptops with only battery
>>    temperature support.
>> ---
>>   drivers/platform/x86/Kconfig            |  11 +
>>   drivers/platform/x86/Makefile           |   1 +
>>   drivers/platform/x86/acer-wmi-battery.c | 355 ++++++++++++++++++++++++
>>   3 files changed, 367 insertions(+)
>>   create mode 100644 drivers/platform/x86/acer-wmi-battery.c
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 4cb7d97a9fcc..88c11a698fb9 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -170,6 +170,17 @@ config ACER_WMI
>>         If you have an ACPI-WMI compatible Acer/ Wistron laptop, say Y 
>> or M
>>         here.
>> +config ACER_WMI_BATTERY
>> +    tristate "Acer WMI Battery"
>> +    depends on ACPI_WMI
>> +    depends on ACPI_BATTERY
> 
> Please also depend on DMI so that dmi_check_system() will always be 
> available
> when building this driver.
> 
>> +    help
>> +      This is a driver for Acer laptops with battery health control. It
>> +      adds charge limit control and battery temperature reporting.
>> +
>> +      If you have an ACPI-WMI Battery compatible Acer laptop, say Y or M
>> +      here.
>> +
>>   source "drivers/platform/x86/amd/Kconfig"
>>   config ADV_SWBUTTON
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/ 
>> Makefile
>> index d25762f7114f..9cf28baff3ae 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_GIGABYTE_WMI)        += gigabyte-wmi.o
>>   obj-$(CONFIG_ACERHDF)        += acerhdf.o
>>   obj-$(CONFIG_ACER_WIRELESS)    += acer-wireless.o
>>   obj-$(CONFIG_ACER_WMI)        += acer-wmi.o
>> +obj-$(CONFIG_ACER_WMI_BATTERY)    += acer-wmi-battery.o
>>   # AMD
>>   obj-y                += amd/
>> diff --git a/drivers/platform/x86/acer-wmi-battery.c b/drivers/ 
>> platform/x86/acer-wmi-battery.c
>> new file mode 100644
>> index 000000000000..abb45cfcc6fe
>> --- /dev/null
>> +++ b/drivers/platform/x86/acer-wmi-battery.c
>> @@ -0,0 +1,355 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * acer-wmi-battery.c: Acer battery health control driver
>> + *
>> + * This is a driver for the WMI battery health control interface found
>> + * on some Acer laptops.  This interface allows to enable/disable a
>> + * battery charge limit ("health mode") and exposes the battery 
>> temperature.
>> + *
>> + * Based on acer-wmi-battery https://github.com/frederik-h/acer-wmi- 
>> battery/
>> + *
>> + * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/compiler_attributes.h>
>> +#include <linux/dmi.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/limits.h>
>> +#include <linux/module.h>
>> +#include <linux/power_supply.h>
>> +#include <linux/types.h>
>> +#include <linux/unaligned.h>
>> +#include <linux/version.h>
>> +#include <linux/wmi.h>
>> +
>> +#include <acpi/battery.h>
>> +
>> +#define DRIVER_NAME    "acer-wmi-battery"
>> +
>> +#define ACER_BATTERY_GUID "79772EC5-04B1-4BFD-843C-61E7F77B6CC9"
>> +
>> +/*
>> + * The Acer OEM software seems to always use this battery index,
>> + * so we emulate this behaviour to not confuse the underlying firmware.
>> + *
>> + * However this also means that we only fully support devices with a
>> + * single battery for now.
>> + */
>> +#define ACER_BATTERY_INDEX    0x1
>> +
>> +struct get_battery_health_control_status_input {
>> +    u8 uBatteryNo;
>> +    u8 uFunctionQuery;
>> +    u8 uReserved[2];
>> +} __packed;
>> +
>> +struct get_battery_health_control_status_output {
>> +    u8 uFunctionList;
>> +    u8 uReturn[2];
>> +    u8 uFunctionStatus[5];
>> +} __packed;
>> +
>> +struct set_battery_health_control_input {
>> +    u8 uBatteryNo;
>> +    u8 uFunctionMask;
>> +    u8 uFunctionStatus;
>> +    u8 uReservedIn[5];
>> +} __packed;
>> +
>> +struct set_battery_health_control_output {
>> +    u8 uReturn;
>> +    u8 uReservedOut;
>> +} __packed;
>> +
>> +enum battery_mode {
>> +    HEALTH_MODE = 1,
>> +    CALIBRATION_MODE = 2,
>> +};
>> +
>> +struct acer_wmi_battery_data {
>> +    struct acpi_battery_hook hook;
>> +    struct wmi_device *wdev;
>> +    const struct power_supply_ext *battery_ext;
>> +    struct {
>> +        bool health_mode : 1;
>> +    } features;
>> +};
>> +
>> +static int acer_wmi_battery_get_information(struct 
>> acer_wmi_battery_data *data,
>> +                        u32 index, u32 battery, u32 *result)
>> +{
>> +    u32 args[2] = { index, battery };
>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> +    struct acpi_buffer input = { sizeof(args), args };
>> +    int ret;
>> +
>> +    ret = wmidev_evaluate_method(data->wdev, 0, 19, &input, &output);
>> +    if (ACPI_FAILURE(ret))
>> +        return -EIO;
>> +
>> +    union acpi_object *obj __free(kfree) = output.pointer;
>> +    if (!obj)
>> +        return -EIO;
>> +
>> +    if (obj->type != ACPI_TYPE_BUFFER)
>> +        ret = -EIO;
>> +
>> +    if (obj->buffer.length < sizeof(u32)) {
>> +        dev_err(&data->wdev->dev, "WMI battery information call 
>> returned buffer of unexpected length %u\n",
>> +            obj->buffer.length);
>> +        ret = -EINVAL;
>> +    }
>> +
>> +    *result = get_unaligned_le32(obj->buffer.pointer);
>> +
>> +    return ret;
>> +}
>> +
>> +static int acer_wmi_battery_get_health_control_status(struct 
>> acer_wmi_battery_data *data,
>> +                              bool *health_mode)
>> +{
>> +    /*
>> +     * Acer Care Center seems to always call the WMI method
>> +     * with fixed parameters. This yields information about
>> +     * the availability and state of both health and
>> +     * calibration mode. The modes probably apply to
>> +     * all batteries of the system.
>> +     */
>> +    struct get_battery_health_control_status_input args = {
>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>> +        .uFunctionQuery = 0x1,
>> +        .uReserved = { 0x0, 0x0 }
>> +    };
>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>> +    struct get_battery_health_control_status_output *status_output;
>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> +    int ret;
>> +
>> +    ret = wmidev_evaluate_method(data->wdev, 0, 20, &input, &output);
>> +    if (ACPI_FAILURE(ret))
>> +        return -EIO;
>> +
>> +    union acpi_object *obj __free(kfree) = output.pointer;
>> +    if (!obj)
>> +        return -EIO;
>> +
>> +    if (obj->type != ACPI_TYPE_BUFFER)
>> +        ret = -EIO;
>> +
>> +    if (obj->buffer.length < 8) {
> 
> Better use sizeof(*status_output) here.
> 
>> +        dev_err(&data->wdev->dev, "WMI battery status call returned a 
>> buffer of unexpected length %d\n",
>> +            obj->buffer.length);
>> +        ret = -EINVAL;
>> +    }
>> +
>> +    status_output = (struct get_battery_health_control_status_output 
>> *)obj->buffer.pointer;
>> +
>> +    if (health_mode) {
>> +        if (status_output->uFunctionList & HEALTH_MODE)
>> +            *health_mode = status_output->uFunctionStatus[0] > 0;
>> +        else
>> +            ret = -EINVAL;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static int acer_wmi_battery_set_battery_health_control(struct 
>> acer_wmi_battery_data *data,
>> +                               u8 function, bool function_status)
>> +{
>> +    struct set_battery_health_control_input args = {
>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>> +        .uFunctionMask = function,
>> +        .uFunctionStatus = function_status ? 1 : 0,
>> +        .uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
>> +    };
>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> +    union acpi_object *obj;
>> +    int ret;
>> +
>> +    ret = wmidev_evaluate_method(data->wdev, 0, 21, &input, &output);
>> +    if (ACPI_FAILURE(ret))
>> +        return -EIO;
>> +
>> +    obj = output.pointer;
>> +
>> +    if (!obj)
>> +        return -EIO;
>> +
>> +    if (obj->type != ACPI_TYPE_BUFFER)
>> +        ret = -EIO;
>> +
>> +    if (obj->buffer.length < 4) {
>> +        dev_err(&data->wdev->dev, "WMI battery status set operation 
>> returned a buffer of unexpected length %d\n",
>> +            obj->buffer.length);
>> +        ret = -EINVAL;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static int acer_battery_ext_property_get(struct power_supply *psy,
>> +                     const struct power_supply_ext *ext,
>> +                     void *ext_data,
>> +                     enum power_supply_property psp,
>> +                     union power_supply_propval *val)
>> +{
>> +    struct acer_wmi_battery_data *data = ext_data;
>> +    bool health_mode;
>> +    u32 value;
>> +    int ret;
>> +
>> +    switch (psp) {
>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>> +        ret = acer_wmi_battery_get_health_control_status(data, 
>> &health_mode);
>> +        if (ret)
>> +            return ret;
>> +
>> +        val->intval = health_mode
>> +                  ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
>> +                  : POWER_SUPPLY_CHARGE_TYPE_STANDARD;
>> +        break;
>> +    case POWER_SUPPLY_PROP_TEMP:
>> +        ret = acer_wmi_battery_get_information(data, 0x8, 
>> ACER_BATTERY_INDEX, &value);
>> +        if (ret)
>> +            return ret;
>> +
>> +        if (value > U16_MAX)
>> +            return -ERANGE;
>> +
>> +        val->intval = value - 2731;
>> +        break;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int acer_battery_ext_property_set(struct power_supply *psy,
>> +                     const struct power_supply_ext *ext,
>> +                     void *ext_data,
>> +                     enum power_supply_property psp,
>> +                     const union power_supply_propval *val)
>> +{
>> +    struct acer_wmi_battery_data *data = ext_data;
>> +
>> +    switch (psp) {
>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>> +        return acer_wmi_battery_set_battery_health_control(data, 
>> HEALTH_MODE,
>> +                val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +}
>> +
>> +static int acer_battery_ext_property_is_writeable(struct power_supply 
>> *psy,
>> +                          const struct power_supply_ext *ext,
>> +                          void *ext_data,
>> +                          enum power_supply_property psp)
>> +{
>> +    switch (psp) {
>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>> +        return true;
>> +    default:
>> +        return false;
>> +    }
>> +}
>> +
>> +static const struct dmi_system_id 
>> acer_wmi_battery_health_mode_table[] = {
>> +    {
>> +        .matches = {
>> +            DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
>> +            DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P")
>> +        }
>> +    },
>> +    {}
>> +};
> 
> I suggest that you perform the DMI check during module initialization. 
> This way
> the DMI table can be marked as __initconst and does not consume any 
> memory after
> anymore after module initialization is complete.
> 
> The result of the DMI check could then be stored inside a global variable.
> 
> Other than that, the driver seems fine to me.

Thanks for the review comments, due to some personal circumstances it 
has taken me while to spin up v3.

I've been digging into the WMI dump data looking if I could figure out 
if there was a way to check if health checks are available but only 
stumbled on another battery related WMI method.

Method 0x16: GetBatteryFunctionData
- Input: uFunctionMask (uint8), uReservedIn (uint8 array)
- Output: uReturnCode (uint8 array), uBACStartTime (uint8 array), 
uBACStopTime (uint8 array), uBACStatus (uint8), uReservedOut (uint8 array)
Method 0x17: SetBatteryFunctionData
- Input: uFunctionMask (uint8), uBACSwitch (uint8), uReservedIn (uint8 
array)

But I was wondering why in probe or init, the driver couldn't call 
"acer_wmi_battery_get_health_control_status". Then we wouldn't need to 
add a WMI whitelist for every device that supports this.

If I recall correctly you own an Acer laptop which supports the battery 
temperature but lacks health control? Seeing how many WMI methods are 
exposed on my device I suppose it is also exposed for you, but it should 
hopefully error out?

Thanks,

Jelle van der Waa

Re: [PATCH v2 1/1] platform/x86: add Acer battery control driver

[Armin Wolf]

Am 10.05.26 um 20:48 schrieb Jelle van der Waa:
> On 1/31/26 00:24, Armin Wolf wrote:
>> Am 25.01.26 um 19:23 schrieb Jelle van der Waa:
>>
>>> Some Acer laptops can configure battery related features through Acer
>>> Care Center on Windows. This driver uses the power supply extension to
>>> set a battery charge limit and exposes the battery
>>> temperature.
>>>
>>> This driver is based on the existing acer-wmi-battery project on GitHub
>>> and was tested on an Acer Aspire A315-510P.
>>>
>>> Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
>>>
>>> ---
>>> v2:
>>> - Alphabetically sort linux headers
>>> - Include headers for types / _packed
>>> - Use cleanup.h instead of goto + label
>>> - Add missing prefix for set_battery_health_control
>>> - General code formatting fixes
>>> - Remove HWMON dependency in Kconfig
>>> - Use wmidev_evaluate_method()
>>> - Accept oversized ACPI buffers
>>> - Use DRIVER_NAME for battery extension name
>>> - Set no_singleton = true
>>> - Implement DMI matching to support laptops with only battery
>>>    temperature support.
>>> ---
>>>   drivers/platform/x86/Kconfig            |  11 +
>>>   drivers/platform/x86/Makefile           |   1 +
>>>   drivers/platform/x86/acer-wmi-battery.c | 355 ++++++++++++++++++++++++
>>>   3 files changed, 367 insertions(+)
>>>   create mode 100644 drivers/platform/x86/acer-wmi-battery.c
>>>
>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>> index 4cb7d97a9fcc..88c11a698fb9 100644
>>> --- a/drivers/platform/x86/Kconfig
>>> +++ b/drivers/platform/x86/Kconfig
>>> @@ -170,6 +170,17 @@ config ACER_WMI
>>>         If you have an ACPI-WMI compatible Acer/ Wistron laptop, say 
>>> Y or M
>>>         here.
>>> +config ACER_WMI_BATTERY
>>> +    tristate "Acer WMI Battery"
>>> +    depends on ACPI_WMI
>>> +    depends on ACPI_BATTERY
>>
>> Please also depend on DMI so that dmi_check_system() will always be 
>> available
>> when building this driver.
>>
>>> +    help
>>> +      This is a driver for Acer laptops with battery health control. It
>>> +      adds charge limit control and battery temperature reporting.
>>> +
>>> +      If you have an ACPI-WMI Battery compatible Acer laptop, say Y 
>>> or M
>>> +      here.
>>> +
>>>   source "drivers/platform/x86/amd/Kconfig"
>>>   config ADV_SWBUTTON
>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/ 
>>> Makefile
>>> index d25762f7114f..9cf28baff3ae 100644
>>> --- a/drivers/platform/x86/Makefile
>>> +++ b/drivers/platform/x86/Makefile
>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_GIGABYTE_WMI)        += gigabyte-wmi.o
>>>   obj-$(CONFIG_ACERHDF)        += acerhdf.o
>>>   obj-$(CONFIG_ACER_WIRELESS)    += acer-wireless.o
>>>   obj-$(CONFIG_ACER_WMI)        += acer-wmi.o
>>> +obj-$(CONFIG_ACER_WMI_BATTERY)    += acer-wmi-battery.o
>>>   # AMD
>>>   obj-y                += amd/
>>> diff --git a/drivers/platform/x86/acer-wmi-battery.c b/drivers/ 
>>> platform/x86/acer-wmi-battery.c
>>> new file mode 100644
>>> index 000000000000..abb45cfcc6fe
>>> --- /dev/null
>>> +++ b/drivers/platform/x86/acer-wmi-battery.c
>>> @@ -0,0 +1,355 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * acer-wmi-battery.c: Acer battery health control driver
>>> + *
>>> + * This is a driver for the WMI battery health control interface found
>>> + * on some Acer laptops.  This interface allows to enable/disable a
>>> + * battery charge limit ("health mode") and exposes the battery 
>>> temperature.
>>> + *
>>> + * Based on acer-wmi-battery https://github.com/frederik-h/acer-wmi- 
>>> battery/
>>> + *
>>> + * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
>>> + */
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/cleanup.h>
>>> +#include <linux/compiler_attributes.h>
>>> +#include <linux/dmi.h>
>>> +#include <linux/init.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/limits.h>
>>> +#include <linux/module.h>
>>> +#include <linux/power_supply.h>
>>> +#include <linux/types.h>
>>> +#include <linux/unaligned.h>
>>> +#include <linux/version.h>
>>> +#include <linux/wmi.h>
>>> +
>>> +#include <acpi/battery.h>
>>> +
>>> +#define DRIVER_NAME    "acer-wmi-battery"
>>> +
>>> +#define ACER_BATTERY_GUID "79772EC5-04B1-4BFD-843C-61E7F77B6CC9"
>>> +
>>> +/*
>>> + * The Acer OEM software seems to always use this battery index,
>>> + * so we emulate this behaviour to not confuse the underlying firmware.
>>> + *
>>> + * However this also means that we only fully support devices with a
>>> + * single battery for now.
>>> + */
>>> +#define ACER_BATTERY_INDEX    0x1
>>> +
>>> +struct get_battery_health_control_status_input {
>>> +    u8 uBatteryNo;
>>> +    u8 uFunctionQuery;
>>> +    u8 uReserved[2];
>>> +} __packed;
>>> +
>>> +struct get_battery_health_control_status_output {
>>> +    u8 uFunctionList;
>>> +    u8 uReturn[2];
>>> +    u8 uFunctionStatus[5];
>>> +} __packed;
>>> +
>>> +struct set_battery_health_control_input {
>>> +    u8 uBatteryNo;
>>> +    u8 uFunctionMask;
>>> +    u8 uFunctionStatus;
>>> +    u8 uReservedIn[5];
>>> +} __packed;
>>> +
>>> +struct set_battery_health_control_output {
>>> +    u8 uReturn;
>>> +    u8 uReservedOut;
>>> +} __packed;
>>> +
>>> +enum battery_mode {
>>> +    HEALTH_MODE = 1,
>>> +    CALIBRATION_MODE = 2,
>>> +};
>>> +
>>> +struct acer_wmi_battery_data {
>>> +    struct acpi_battery_hook hook;
>>> +    struct wmi_device *wdev;
>>> +    const struct power_supply_ext *battery_ext;
>>> +    struct {
>>> +        bool health_mode : 1;
>>> +    } features;
>>> +};
>>> +
>>> +static int acer_wmi_battery_get_information(struct 
>>> acer_wmi_battery_data *data,
>>> +                        u32 index, u32 battery, u32 *result)
>>> +{
>>> +    u32 args[2] = { index, battery };
>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +    struct acpi_buffer input = { sizeof(args), args };
>>> +    int ret;
>>> +
>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 19, &input, &output);
>>> +    if (ACPI_FAILURE(ret))
>>> +        return -EIO;
>>> +
>>> +    union acpi_object *obj __free(kfree) = output.pointer;
>>> +    if (!obj)
>>> +        return -EIO;
>>> +
>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>> +        ret = -EIO;
>>> +
>>> +    if (obj->buffer.length < sizeof(u32)) {
>>> +        dev_err(&data->wdev->dev, "WMI battery information call 
>>> returned buffer of unexpected length %u\n",
>>> +            obj->buffer.length);
>>> +        ret = -EINVAL;
>>> +    }
>>> +
>>> +    *result = get_unaligned_le32(obj->buffer.pointer);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int acer_wmi_battery_get_health_control_status(struct 
>>> acer_wmi_battery_data *data,
>>> +                              bool *health_mode)
>>> +{
>>> +    /*
>>> +     * Acer Care Center seems to always call the WMI method
>>> +     * with fixed parameters. This yields information about
>>> +     * the availability and state of both health and
>>> +     * calibration mode. The modes probably apply to
>>> +     * all batteries of the system.
>>> +     */
>>> +    struct get_battery_health_control_status_input args = {
>>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>>> +        .uFunctionQuery = 0x1,
>>> +        .uReserved = { 0x0, 0x0 }
>>> +    };
>>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>>> +    struct get_battery_health_control_status_output *status_output;
>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +    int ret;
>>> +
>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 20, &input, &output);
>>> +    if (ACPI_FAILURE(ret))
>>> +        return -EIO;
>>> +
>>> +    union acpi_object *obj __free(kfree) = output.pointer;
>>> +    if (!obj)
>>> +        return -EIO;
>>> +
>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>> +        ret = -EIO;
>>> +
>>> +    if (obj->buffer.length < 8) {
>>
>> Better use sizeof(*status_output) here.
>>
>>> +        dev_err(&data->wdev->dev, "WMI battery status call returned 
>>> a buffer of unexpected length %d\n",
>>> +            obj->buffer.length);
>>> +        ret = -EINVAL;
>>> +    }
>>> +
>>> +    status_output = (struct get_battery_health_control_status_output 
>>> *)obj->buffer.pointer;
>>> +
>>> +    if (health_mode) {
>>> +        if (status_output->uFunctionList & HEALTH_MODE)
>>> +            *health_mode = status_output->uFunctionStatus[0] > 0;
>>> +        else
>>> +            ret = -EINVAL;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int acer_wmi_battery_set_battery_health_control(struct 
>>> acer_wmi_battery_data *data,
>>> +                               u8 function, bool function_status)
>>> +{
>>> +    struct set_battery_health_control_input args = {
>>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>>> +        .uFunctionMask = function,
>>> +        .uFunctionStatus = function_status ? 1 : 0,
>>> +        .uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
>>> +    };
>>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +    union acpi_object *obj;
>>> +    int ret;
>>> +
>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 21, &input, &output);
>>> +    if (ACPI_FAILURE(ret))
>>> +        return -EIO;
>>> +
>>> +    obj = output.pointer;
>>> +
>>> +    if (!obj)
>>> +        return -EIO;
>>> +
>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>> +        ret = -EIO;
>>> +
>>> +    if (obj->buffer.length < 4) {
>>> +        dev_err(&data->wdev->dev, "WMI battery status set operation 
>>> returned a buffer of unexpected length %d\n",
>>> +            obj->buffer.length);
>>> +        ret = -EINVAL;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int acer_battery_ext_property_get(struct power_supply *psy,
>>> +                     const struct power_supply_ext *ext,
>>> +                     void *ext_data,
>>> +                     enum power_supply_property psp,
>>> +                     union power_supply_propval *val)
>>> +{
>>> +    struct acer_wmi_battery_data *data = ext_data;
>>> +    bool health_mode;
>>> +    u32 value;
>>> +    int ret;
>>> +
>>> +    switch (psp) {
>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>> +        ret = acer_wmi_battery_get_health_control_status(data, 
>>> &health_mode);
>>> +        if (ret)
>>> +            return ret;
>>> +
>>> +        val->intval = health_mode
>>> +                  ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
>>> +                  : POWER_SUPPLY_CHARGE_TYPE_STANDARD;
>>> +        break;
>>> +    case POWER_SUPPLY_PROP_TEMP:
>>> +        ret = acer_wmi_battery_get_information(data, 0x8, 
>>> ACER_BATTERY_INDEX, &value);
>>> +        if (ret)
>>> +            return ret;
>>> +
>>> +        if (value > U16_MAX)
>>> +            return -ERANGE;
>>> +
>>> +        val->intval = value - 2731;
>>> +        break;
>>> +    default:
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int acer_battery_ext_property_set(struct power_supply *psy,
>>> +                     const struct power_supply_ext *ext,
>>> +                     void *ext_data,
>>> +                     enum power_supply_property psp,
>>> +                     const union power_supply_propval *val)
>>> +{
>>> +    struct acer_wmi_battery_data *data = ext_data;
>>> +
>>> +    switch (psp) {
>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>> +        return acer_wmi_battery_set_battery_health_control(data, 
>>> HEALTH_MODE,
>>> +                val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);
>>> +    default:
>>> +        return -EINVAL;
>>> +    }
>>> +}
>>> +
>>> +static int acer_battery_ext_property_is_writeable(struct 
>>> power_supply *psy,
>>> +                          const struct power_supply_ext *ext,
>>> +                          void *ext_data,
>>> +                          enum power_supply_property psp)
>>> +{
>>> +    switch (psp) {
>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>> +        return true;
>>> +    default:
>>> +        return false;
>>> +    }
>>> +}
>>> +
>>> +static const struct dmi_system_id 
>>> acer_wmi_battery_health_mode_table[] = {
>>> +    {
>>> +        .matches = {
>>> +            DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
>>> +            DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P")
>>> +        }
>>> +    },
>>> +    {}
>>> +};
>>
>> I suggest that you perform the DMI check during module initialization. 
>> This way
>> the DMI table can be marked as __initconst and does not consume any 
>> memory after
>> anymore after module initialization is complete.
>>
>> The result of the DMI check could then be stored inside a global 
>> variable.
>>
>> Other than that, the driver seems fine to me.
> 
> Thanks for the review comments, due to some personal circumstances it 
> has taken me while to spin up v3.
> 
> I've been digging into the WMI dump data looking if I could figure out 
> if there was a way to check if health checks are available but only 
> stumbled on another battery related WMI method.
> 
> Method 0x16: GetBatteryFunctionData
> - Input: uFunctionMask (uint8), uReservedIn (uint8 array)
> - Output: uReturnCode (uint8 array), uBACStartTime (uint8 array), 
> uBACStopTime (uint8 array), uBACStatus (uint8), uReservedOut (uint8 array)
> Method 0x17: SetBatteryFunctionData
> - Input: uFunctionMask (uint8), uBACSwitch (uint8), uReservedIn (uint8 
> array)
> 
> But I was wondering why in probe or init, the driver couldn't call 
> "acer_wmi_battery_get_health_control_status". Then we wouldn't need to 
> add a WMI whitelist for every device that supports this.
> 
> If I recall correctly you own an Acer laptop which supports the battery 
> temperature but lacks health control? Seeing how many WMI methods are 
> exposed on my device I suppose it is also exposed for you, but it should 
> hopefully error out?

Calling an non-existing WMI method is undefined behavior and could in 
the worst cast cause the ACPI firmware to crash. Because of this we have 
to use a DMI whitelist for now :(

I am working on a in-kernel BMOF parser, but i am currently occupied by 
other work.

Thanks,
Armin Wolf

> 
> Thanks,
> 
> Jelle van der Waa

Re: [PATCH v2 1/1] platform/x86: add Acer battery control driver

[Jelle van der Waa]

On 5/13/26 11:19, Armin Wolf wrote:
> Am 10.05.26 um 20:48 schrieb Jelle van der Waa:
>> On 1/31/26 00:24, Armin Wolf wrote:
>>> Am 25.01.26 um 19:23 schrieb Jelle van der Waa:
>>>
>>>> Some Acer laptops can configure battery related features through Acer
>>>> Care Center on Windows. This driver uses the power supply extension to
>>>> set a battery charge limit and exposes the battery
>>>> temperature.
>>>>
>>>> This driver is based on the existing acer-wmi-battery project on GitHub
>>>> and was tested on an Acer Aspire A315-510P.
>>>>
>>>> Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
>>>>
>>>> ---
>>>> v2:
>>>> - Alphabetically sort linux headers
>>>> - Include headers for types / _packed
>>>> - Use cleanup.h instead of goto + label
>>>> - Add missing prefix for set_battery_health_control
>>>> - General code formatting fixes
>>>> - Remove HWMON dependency in Kconfig
>>>> - Use wmidev_evaluate_method()
>>>> - Accept oversized ACPI buffers
>>>> - Use DRIVER_NAME for battery extension name
>>>> - Set no_singleton = true
>>>> - Implement DMI matching to support laptops with only battery
>>>>    temperature support.
>>>> ---
>>>>   drivers/platform/x86/Kconfig            |  11 +
>>>>   drivers/platform/x86/Makefile           |   1 +
>>>>   drivers/platform/x86/acer-wmi-battery.c | 355 ++++++++++++++++++++ 
>>>> ++++
>>>>   3 files changed, 367 insertions(+)
>>>>   create mode 100644 drivers/platform/x86/acer-wmi-battery.c
>>>>
>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/ 
>>>> Kconfig
>>>> index 4cb7d97a9fcc..88c11a698fb9 100644
>>>> --- a/drivers/platform/x86/Kconfig
>>>> +++ b/drivers/platform/x86/Kconfig
>>>> @@ -170,6 +170,17 @@ config ACER_WMI
>>>>         If you have an ACPI-WMI compatible Acer/ Wistron laptop, say 
>>>> Y or M
>>>>         here.
>>>> +config ACER_WMI_BATTERY
>>>> +    tristate "Acer WMI Battery"
>>>> +    depends on ACPI_WMI
>>>> +    depends on ACPI_BATTERY
>>>
>>> Please also depend on DMI so that dmi_check_system() will always be 
>>> available
>>> when building this driver.
>>>
>>>> +    help
>>>> +      This is a driver for Acer laptops with battery health 
>>>> control. It
>>>> +      adds charge limit control and battery temperature reporting.
>>>> +
>>>> +      If you have an ACPI-WMI Battery compatible Acer laptop, say Y 
>>>> or M
>>>> +      here.
>>>> +
>>>>   source "drivers/platform/x86/amd/Kconfig"
>>>>   config ADV_SWBUTTON
>>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/ 
>>>> Makefile
>>>> index d25762f7114f..9cf28baff3ae 100644
>>>> --- a/drivers/platform/x86/Makefile
>>>> +++ b/drivers/platform/x86/Makefile
>>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_GIGABYTE_WMI)        += gigabyte-wmi.o
>>>>   obj-$(CONFIG_ACERHDF)        += acerhdf.o
>>>>   obj-$(CONFIG_ACER_WIRELESS)    += acer-wireless.o
>>>>   obj-$(CONFIG_ACER_WMI)        += acer-wmi.o
>>>> +obj-$(CONFIG_ACER_WMI_BATTERY)    += acer-wmi-battery.o
>>>>   # AMD
>>>>   obj-y                += amd/
>>>> diff --git a/drivers/platform/x86/acer-wmi-battery.c b/drivers/ 
>>>> platform/x86/acer-wmi-battery.c
>>>> new file mode 100644
>>>> index 000000000000..abb45cfcc6fe
>>>> --- /dev/null
>>>> +++ b/drivers/platform/x86/acer-wmi-battery.c
>>>> @@ -0,0 +1,355 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>>> +/*
>>>> + * acer-wmi-battery.c: Acer battery health control driver
>>>> + *
>>>> + * This is a driver for the WMI battery health control interface found
>>>> + * on some Acer laptops.  This interface allows to enable/disable a
>>>> + * battery charge limit ("health mode") and exposes the battery 
>>>> temperature.
>>>> + *
>>>> + * Based on acer-wmi-battery https://github.com/frederik-h/acer- 
>>>> wmi- battery/
>>>> + *
>>>> + * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
>>>> + */
>>>> +
>>>> +#include <linux/acpi.h>
>>>> +#include <linux/cleanup.h>
>>>> +#include <linux/compiler_attributes.h>
>>>> +#include <linux/dmi.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/limits.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/power_supply.h>
>>>> +#include <linux/types.h>
>>>> +#include <linux/unaligned.h>
>>>> +#include <linux/version.h>
>>>> +#include <linux/wmi.h>
>>>> +
>>>> +#include <acpi/battery.h>
>>>> +
>>>> +#define DRIVER_NAME    "acer-wmi-battery"
>>>> +
>>>> +#define ACER_BATTERY_GUID "79772EC5-04B1-4BFD-843C-61E7F77B6CC9"
>>>> +
>>>> +/*
>>>> + * The Acer OEM software seems to always use this battery index,
>>>> + * so we emulate this behaviour to not confuse the underlying 
>>>> firmware.
>>>> + *
>>>> + * However this also means that we only fully support devices with a
>>>> + * single battery for now.
>>>> + */
>>>> +#define ACER_BATTERY_INDEX    0x1
>>>> +
>>>> +struct get_battery_health_control_status_input {
>>>> +    u8 uBatteryNo;
>>>> +    u8 uFunctionQuery;
>>>> +    u8 uReserved[2];
>>>> +} __packed;
>>>> +
>>>> +struct get_battery_health_control_status_output {
>>>> +    u8 uFunctionList;
>>>> +    u8 uReturn[2];
>>>> +    u8 uFunctionStatus[5];
>>>> +} __packed;
>>>> +
>>>> +struct set_battery_health_control_input {
>>>> +    u8 uBatteryNo;
>>>> +    u8 uFunctionMask;
>>>> +    u8 uFunctionStatus;
>>>> +    u8 uReservedIn[5];
>>>> +} __packed;
>>>> +
>>>> +struct set_battery_health_control_output {
>>>> +    u8 uReturn;
>>>> +    u8 uReservedOut;
>>>> +} __packed;
>>>> +
>>>> +enum battery_mode {
>>>> +    HEALTH_MODE = 1,
>>>> +    CALIBRATION_MODE = 2,
>>>> +};
>>>> +
>>>> +struct acer_wmi_battery_data {
>>>> +    struct acpi_battery_hook hook;
>>>> +    struct wmi_device *wdev;
>>>> +    const struct power_supply_ext *battery_ext;
>>>> +    struct {
>>>> +        bool health_mode : 1;
>>>> +    } features;
>>>> +};
>>>> +
>>>> +static int acer_wmi_battery_get_information(struct 
>>>> acer_wmi_battery_data *data,
>>>> +                        u32 index, u32 battery, u32 *result)
>>>> +{
>>>> +    u32 args[2] = { index, battery };
>>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>>> +    struct acpi_buffer input = { sizeof(args), args };
>>>> +    int ret;
>>>> +
>>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 19, &input, &output);
>>>> +    if (ACPI_FAILURE(ret))
>>>> +        return -EIO;
>>>> +
>>>> +    union acpi_object *obj __free(kfree) = output.pointer;
>>>> +    if (!obj)
>>>> +        return -EIO;
>>>> +
>>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>>> +        ret = -EIO;
>>>> +
>>>> +    if (obj->buffer.length < sizeof(u32)) {
>>>> +        dev_err(&data->wdev->dev, "WMI battery information call 
>>>> returned buffer of unexpected length %u\n",
>>>> +            obj->buffer.length);
>>>> +        ret = -EINVAL;
>>>> +    }
>>>> +
>>>> +    *result = get_unaligned_le32(obj->buffer.pointer);
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static int acer_wmi_battery_get_health_control_status(struct 
>>>> acer_wmi_battery_data *data,
>>>> +                              bool *health_mode)
>>>> +{
>>>> +    /*
>>>> +     * Acer Care Center seems to always call the WMI method
>>>> +     * with fixed parameters. This yields information about
>>>> +     * the availability and state of both health and
>>>> +     * calibration mode. The modes probably apply to
>>>> +     * all batteries of the system.
>>>> +     */
>>>> +    struct get_battery_health_control_status_input args = {
>>>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>>>> +        .uFunctionQuery = 0x1,
>>>> +        .uReserved = { 0x0, 0x0 }
>>>> +    };
>>>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>>>> +    struct get_battery_health_control_status_output *status_output;
>>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>>> +    int ret;
>>>> +
>>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 20, &input, &output);
>>>> +    if (ACPI_FAILURE(ret))
>>>> +        return -EIO;
>>>> +
>>>> +    union acpi_object *obj __free(kfree) = output.pointer;
>>>> +    if (!obj)
>>>> +        return -EIO;
>>>> +
>>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>>> +        ret = -EIO;
>>>> +
>>>> +    if (obj->buffer.length < 8) {
>>>
>>> Better use sizeof(*status_output) here.
>>>
>>>> +        dev_err(&data->wdev->dev, "WMI battery status call returned 
>>>> a buffer of unexpected length %d\n",
>>>> +            obj->buffer.length);
>>>> +        ret = -EINVAL;
>>>> +    }
>>>> +
>>>> +    status_output = (struct 
>>>> get_battery_health_control_status_output *)obj->buffer.pointer;
>>>> +
>>>> +    if (health_mode) {
>>>> +        if (status_output->uFunctionList & HEALTH_MODE)
>>>> +            *health_mode = status_output->uFunctionStatus[0] > 0;
>>>> +        else
>>>> +            ret = -EINVAL;
>>>> +    }
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static int acer_wmi_battery_set_battery_health_control(struct 
>>>> acer_wmi_battery_data *data,
>>>> +                               u8 function, bool function_status)
>>>> +{
>>>> +    struct set_battery_health_control_input args = {
>>>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>>>> +        .uFunctionMask = function,
>>>> +        .uFunctionStatus = function_status ? 1 : 0,
>>>> +        .uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
>>>> +    };
>>>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>>> +    union acpi_object *obj;
>>>> +    int ret;
>>>> +
>>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 21, &input, &output);
>>>> +    if (ACPI_FAILURE(ret))
>>>> +        return -EIO;
>>>> +
>>>> +    obj = output.pointer;
>>>> +
>>>> +    if (!obj)
>>>> +        return -EIO;
>>>> +
>>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>>> +        ret = -EIO;
>>>> +
>>>> +    if (obj->buffer.length < 4) {
>>>> +        dev_err(&data->wdev->dev, "WMI battery status set operation 
>>>> returned a buffer of unexpected length %d\n",
>>>> +            obj->buffer.length);
>>>> +        ret = -EINVAL;
>>>> +    }
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static int acer_battery_ext_property_get(struct power_supply *psy,
>>>> +                     const struct power_supply_ext *ext,
>>>> +                     void *ext_data,
>>>> +                     enum power_supply_property psp,
>>>> +                     union power_supply_propval *val)
>>>> +{
>>>> +    struct acer_wmi_battery_data *data = ext_data;
>>>> +    bool health_mode;
>>>> +    u32 value;
>>>> +    int ret;
>>>> +
>>>> +    switch (psp) {
>>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>>> +        ret = acer_wmi_battery_get_health_control_status(data, 
>>>> &health_mode);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +
>>>> +        val->intval = health_mode
>>>> +                  ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
>>>> +                  : POWER_SUPPLY_CHARGE_TYPE_STANDARD;
>>>> +        break;
>>>> +    case POWER_SUPPLY_PROP_TEMP:
>>>> +        ret = acer_wmi_battery_get_information(data, 0x8, 
>>>> ACER_BATTERY_INDEX, &value);
>>>> +        if (ret)
>>>> +            return ret;
>>>> +
>>>> +        if (value > U16_MAX)
>>>> +            return -ERANGE;
>>>> +
>>>> +        val->intval = value - 2731;
>>>> +        break;
>>>> +    default:
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int acer_battery_ext_property_set(struct power_supply *psy,
>>>> +                     const struct power_supply_ext *ext,
>>>> +                     void *ext_data,
>>>> +                     enum power_supply_property psp,
>>>> +                     const union power_supply_propval *val)
>>>> +{
>>>> +    struct acer_wmi_battery_data *data = ext_data;
>>>> +
>>>> +    switch (psp) {
>>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>>> +        return acer_wmi_battery_set_battery_health_control(data, 
>>>> HEALTH_MODE,
>>>> +                val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);
>>>> +    default:
>>>> +        return -EINVAL;
>>>> +    }
>>>> +}
>>>> +
>>>> +static int acer_battery_ext_property_is_writeable(struct 
>>>> power_supply *psy,
>>>> +                          const struct power_supply_ext *ext,
>>>> +                          void *ext_data,
>>>> +                          enum power_supply_property psp)
>>>> +{
>>>> +    switch (psp) {
>>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>>> +        return true;
>>>> +    default:
>>>> +        return false;
>>>> +    }
>>>> +}
>>>> +
>>>> +static const struct dmi_system_id 
>>>> acer_wmi_battery_health_mode_table[] = {
>>>> +    {
>>>> +        .matches = {
>>>> +            DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
>>>> +            DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P")
>>>> +        }
>>>> +    },
>>>> +    {}
>>>> +};
>>>
>>> I suggest that you perform the DMI check during module 
>>> initialization. This way
>>> the DMI table can be marked as __initconst and does not consume any 
>>> memory after
>>> anymore after module initialization is complete.
>>>
>>> The result of the DMI check could then be stored inside a global 
>>> variable.
>>>
>>> Other than that, the driver seems fine to me.
>>
>> Thanks for the review comments, due to some personal circumstances it 
>> has taken me while to spin up v3.
>>
>> I've been digging into the WMI dump data looking if I could figure out 
>> if there was a way to check if health checks are available but only 
>> stumbled on another battery related WMI method.
>>
>> Method 0x16: GetBatteryFunctionData
>> - Input: uFunctionMask (uint8), uReservedIn (uint8 array)
>> - Output: uReturnCode (uint8 array), uBACStartTime (uint8 array), 
>> uBACStopTime (uint8 array), uBACStatus (uint8), uReservedOut (uint8 
>> array)
>> Method 0x17: SetBatteryFunctionData
>> - Input: uFunctionMask (uint8), uBACSwitch (uint8), uReservedIn (uint8 
>> array)
>>
>> But I was wondering why in probe or init, the driver couldn't call 
>> "acer_wmi_battery_get_health_control_status". Then we wouldn't need to 
>> add a WMI whitelist for every device that supports this.
>>
>> If I recall correctly you own an Acer laptop which supports the 
>> battery temperature but lacks health control? Seeing how many WMI 
>> methods are exposed on my device I suppose it is also exposed for you, 
>> but it should hopefully error out?
> 
> Calling an non-existing WMI method is undefined behavior and could in 
> the worst cast cause the ACPI firmware to crash. Because of this we have 
> to use a DMI whitelist for now :(

Thanks for the answer, then I'll rework the driver to use the module 
__init for DMI as you suggested.

I suppose it is fine to already add the other tested models by the 
community? [1].
And should I add an module load option to force health mode so users can 
test if their hardware is compatible?

> I am working on a in-kernel BMOF parser, but i am currently occupied by 
> other work.
No worries.

[1] https://github.com/frederik-h/acer-wmi-battery/blob/main/MODELS.md

Re: [PATCH v2 1/1] platform/x86: add Acer battery control driver

[Armin Wolf]

Am 15.05.26 um 11:45 schrieb Jelle van der Waa:

> On 5/13/26 11:19, Armin Wolf wrote:
>> Am 10.05.26 um 20:48 schrieb Jelle van der Waa:
>>> On 1/31/26 00:24, Armin Wolf wrote:
>>>> Am 25.01.26 um 19:23 schrieb Jelle van der Waa:
>>>>
>>>>> Some Acer laptops can configure battery related features through Acer
>>>>> Care Center on Windows. This driver uses the power supply 
>>>>> extension to
>>>>> set a battery charge limit and exposes the battery
>>>>> temperature.
>>>>>
>>>>> This driver is based on the existing acer-wmi-battery project on 
>>>>> GitHub
>>>>> and was tested on an Acer Aspire A315-510P.
>>>>>
>>>>> Signed-off-by: Jelle van der Waa <jelle@vdwaa.nl>
>>>>>
>>>>> ---
>>>>> v2:
>>>>> - Alphabetically sort linux headers
>>>>> - Include headers for types / _packed
>>>>> - Use cleanup.h instead of goto + label
>>>>> - Add missing prefix for set_battery_health_control
>>>>> - General code formatting fixes
>>>>> - Remove HWMON dependency in Kconfig
>>>>> - Use wmidev_evaluate_method()
>>>>> - Accept oversized ACPI buffers
>>>>> - Use DRIVER_NAME for battery extension name
>>>>> - Set no_singleton = true
>>>>> - Implement DMI matching to support laptops with only battery
>>>>>    temperature support.
>>>>> ---
>>>>>   drivers/platform/x86/Kconfig            |  11 +
>>>>>   drivers/platform/x86/Makefile           |   1 +
>>>>>   drivers/platform/x86/acer-wmi-battery.c | 355 
>>>>> ++++++++++++++++++++ ++++
>>>>>   3 files changed, 367 insertions(+)
>>>>>   create mode 100644 drivers/platform/x86/acer-wmi-battery.c
>>>>>
>>>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/ 
>>>>> Kconfig
>>>>> index 4cb7d97a9fcc..88c11a698fb9 100644
>>>>> --- a/drivers/platform/x86/Kconfig
>>>>> +++ b/drivers/platform/x86/Kconfig
>>>>> @@ -170,6 +170,17 @@ config ACER_WMI
>>>>>         If you have an ACPI-WMI compatible Acer/ Wistron laptop, 
>>>>> say Y or M
>>>>>         here.
>>>>> +config ACER_WMI_BATTERY
>>>>> +    tristate "Acer WMI Battery"
>>>>> +    depends on ACPI_WMI
>>>>> +    depends on ACPI_BATTERY
>>>>
>>>> Please also depend on DMI so that dmi_check_system() will always be 
>>>> available
>>>> when building this driver.
>>>>
>>>>> +    help
>>>>> +      This is a driver for Acer laptops with battery health 
>>>>> control. It
>>>>> +      adds charge limit control and battery temperature reporting.
>>>>> +
>>>>> +      If you have an ACPI-WMI Battery compatible Acer laptop, say 
>>>>> Y or M
>>>>> +      here.
>>>>> +
>>>>>   source "drivers/platform/x86/amd/Kconfig"
>>>>>   config ADV_SWBUTTON
>>>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/ 
>>>>> Makefile
>>>>> index d25762f7114f..9cf28baff3ae 100644
>>>>> --- a/drivers/platform/x86/Makefile
>>>>> +++ b/drivers/platform/x86/Makefile
>>>>> @@ -19,6 +19,7 @@ obj-$(CONFIG_GIGABYTE_WMI)        += gigabyte-wmi.o
>>>>>   obj-$(CONFIG_ACERHDF)        += acerhdf.o
>>>>>   obj-$(CONFIG_ACER_WIRELESS)    += acer-wireless.o
>>>>>   obj-$(CONFIG_ACER_WMI)        += acer-wmi.o
>>>>> +obj-$(CONFIG_ACER_WMI_BATTERY)    += acer-wmi-battery.o
>>>>>   # AMD
>>>>>   obj-y                += amd/
>>>>> diff --git a/drivers/platform/x86/acer-wmi-battery.c b/drivers/ 
>>>>> platform/x86/acer-wmi-battery.c
>>>>> new file mode 100644
>>>>> index 000000000000..abb45cfcc6fe
>>>>> --- /dev/null
>>>>> +++ b/drivers/platform/x86/acer-wmi-battery.c
>>>>> @@ -0,0 +1,355 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>>>> +/*
>>>>> + * acer-wmi-battery.c: Acer battery health control driver
>>>>> + *
>>>>> + * This is a driver for the WMI battery health control interface 
>>>>> found
>>>>> + * on some Acer laptops.  This interface allows to enable/disable a
>>>>> + * battery charge limit ("health mode") and exposes the battery 
>>>>> temperature.
>>>>> + *
>>>>> + * Based on acer-wmi-battery https://github.com/frederik-h/acer- 
>>>>> wmi- battery/
>>>>> + *
>>>>> + * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
>>>>> + */
>>>>> +
>>>>> +#include <linux/acpi.h>
>>>>> +#include <linux/cleanup.h>
>>>>> +#include <linux/compiler_attributes.h>
>>>>> +#include <linux/dmi.h>
>>>>> +#include <linux/init.h>
>>>>> +#include <linux/kernel.h>
>>>>> +#include <linux/limits.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/power_supply.h>
>>>>> +#include <linux/types.h>
>>>>> +#include <linux/unaligned.h>
>>>>> +#include <linux/version.h>
>>>>> +#include <linux/wmi.h>
>>>>> +
>>>>> +#include <acpi/battery.h>
>>>>> +
>>>>> +#define DRIVER_NAME    "acer-wmi-battery"
>>>>> +
>>>>> +#define ACER_BATTERY_GUID "79772EC5-04B1-4BFD-843C-61E7F77B6CC9"
>>>>> +
>>>>> +/*
>>>>> + * The Acer OEM software seems to always use this battery index,
>>>>> + * so we emulate this behaviour to not confuse the underlying 
>>>>> firmware.
>>>>> + *
>>>>> + * However this also means that we only fully support devices with a
>>>>> + * single battery for now.
>>>>> + */
>>>>> +#define ACER_BATTERY_INDEX    0x1
>>>>> +
>>>>> +struct get_battery_health_control_status_input {
>>>>> +    u8 uBatteryNo;
>>>>> +    u8 uFunctionQuery;
>>>>> +    u8 uReserved[2];
>>>>> +} __packed;
>>>>> +
>>>>> +struct get_battery_health_control_status_output {
>>>>> +    u8 uFunctionList;
>>>>> +    u8 uReturn[2];
>>>>> +    u8 uFunctionStatus[5];
>>>>> +} __packed;
>>>>> +
>>>>> +struct set_battery_health_control_input {
>>>>> +    u8 uBatteryNo;
>>>>> +    u8 uFunctionMask;
>>>>> +    u8 uFunctionStatus;
>>>>> +    u8 uReservedIn[5];
>>>>> +} __packed;
>>>>> +
>>>>> +struct set_battery_health_control_output {
>>>>> +    u8 uReturn;
>>>>> +    u8 uReservedOut;
>>>>> +} __packed;
>>>>> +
>>>>> +enum battery_mode {
>>>>> +    HEALTH_MODE = 1,
>>>>> +    CALIBRATION_MODE = 2,
>>>>> +};
>>>>> +
>>>>> +struct acer_wmi_battery_data {
>>>>> +    struct acpi_battery_hook hook;
>>>>> +    struct wmi_device *wdev;
>>>>> +    const struct power_supply_ext *battery_ext;
>>>>> +    struct {
>>>>> +        bool health_mode : 1;
>>>>> +    } features;
>>>>> +};
>>>>> +
>>>>> +static int acer_wmi_battery_get_information(struct 
>>>>> acer_wmi_battery_data *data,
>>>>> +                        u32 index, u32 battery, u32 *result)
>>>>> +{
>>>>> +    u32 args[2] = { index, battery };
>>>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>>>> +    struct acpi_buffer input = { sizeof(args), args };
>>>>> +    int ret;
>>>>> +
>>>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 19, &input, 
>>>>> &output);
>>>>> +    if (ACPI_FAILURE(ret))
>>>>> +        return -EIO;
>>>>> +
>>>>> +    union acpi_object *obj __free(kfree) = output.pointer;
>>>>> +    if (!obj)
>>>>> +        return -EIO;
>>>>> +
>>>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>>>> +        ret = -EIO;
>>>>> +
>>>>> +    if (obj->buffer.length < sizeof(u32)) {
>>>>> +        dev_err(&data->wdev->dev, "WMI battery information call 
>>>>> returned buffer of unexpected length %u\n",
>>>>> +            obj->buffer.length);
>>>>> +        ret = -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    *result = get_unaligned_le32(obj->buffer.pointer);
>>>>> +
>>>>> +    return ret;
>>>>> +}
>>>>> +
>>>>> +static int acer_wmi_battery_get_health_control_status(struct 
>>>>> acer_wmi_battery_data *data,
>>>>> +                              bool *health_mode)
>>>>> +{
>>>>> +    /*
>>>>> +     * Acer Care Center seems to always call the WMI method
>>>>> +     * with fixed parameters. This yields information about
>>>>> +     * the availability and state of both health and
>>>>> +     * calibration mode. The modes probably apply to
>>>>> +     * all batteries of the system.
>>>>> +     */
>>>>> +    struct get_battery_health_control_status_input args = {
>>>>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>>>>> +        .uFunctionQuery = 0x1,
>>>>> +        .uReserved = { 0x0, 0x0 }
>>>>> +    };
>>>>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>>>>> +    struct get_battery_health_control_status_output *status_output;
>>>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>>>> +    int ret;
>>>>> +
>>>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 20, &input, 
>>>>> &output);
>>>>> +    if (ACPI_FAILURE(ret))
>>>>> +        return -EIO;
>>>>> +
>>>>> +    union acpi_object *obj __free(kfree) = output.pointer;
>>>>> +    if (!obj)
>>>>> +        return -EIO;
>>>>> +
>>>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>>>> +        ret = -EIO;
>>>>> +
>>>>> +    if (obj->buffer.length < 8) {
>>>>
>>>> Better use sizeof(*status_output) here.
>>>>
>>>>> + dev_err(&data->wdev->dev, "WMI battery status call returned a 
>>>>> buffer of unexpected length %d\n",
>>>>> +            obj->buffer.length);
>>>>> +        ret = -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    status_output = (struct 
>>>>> get_battery_health_control_status_output *)obj->buffer.pointer;
>>>>> +
>>>>> +    if (health_mode) {
>>>>> +        if (status_output->uFunctionList & HEALTH_MODE)
>>>>> +            *health_mode = status_output->uFunctionStatus[0] > 0;
>>>>> +        else
>>>>> +            ret = -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    return ret;
>>>>> +}
>>>>> +
>>>>> +static int acer_wmi_battery_set_battery_health_control(struct 
>>>>> acer_wmi_battery_data *data,
>>>>> +                               u8 function, bool function_status)
>>>>> +{
>>>>> +    struct set_battery_health_control_input args = {
>>>>> +        .uBatteryNo = ACER_BATTERY_INDEX,
>>>>> +        .uFunctionMask = function,
>>>>> +        .uFunctionStatus = function_status ? 1 : 0,
>>>>> +        .uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
>>>>> +    };
>>>>> +    struct acpi_buffer input = { (acpi_size) sizeof(args), &args };
>>>>> +    struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>>>> +    union acpi_object *obj;
>>>>> +    int ret;
>>>>> +
>>>>> +    ret = wmidev_evaluate_method(data->wdev, 0, 21, &input, 
>>>>> &output);
>>>>> +    if (ACPI_FAILURE(ret))
>>>>> +        return -EIO;
>>>>> +
>>>>> +    obj = output.pointer;
>>>>> +
>>>>> +    if (!obj)
>>>>> +        return -EIO;
>>>>> +
>>>>> +    if (obj->type != ACPI_TYPE_BUFFER)
>>>>> +        ret = -EIO;
>>>>> +
>>>>> +    if (obj->buffer.length < 4) {
>>>>> +        dev_err(&data->wdev->dev, "WMI battery status set 
>>>>> operation returned a buffer of unexpected length %d\n",
>>>>> +            obj->buffer.length);
>>>>> +        ret = -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    return ret;
>>>>> +}
>>>>> +
>>>>> +static int acer_battery_ext_property_get(struct power_supply *psy,
>>>>> +                     const struct power_supply_ext *ext,
>>>>> +                     void *ext_data,
>>>>> +                     enum power_supply_property psp,
>>>>> +                     union power_supply_propval *val)
>>>>> +{
>>>>> +    struct acer_wmi_battery_data *data = ext_data;
>>>>> +    bool health_mode;
>>>>> +    u32 value;
>>>>> +    int ret;
>>>>> +
>>>>> +    switch (psp) {
>>>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>>>> +        ret = acer_wmi_battery_get_health_control_status(data, 
>>>>> &health_mode);
>>>>> +        if (ret)
>>>>> +            return ret;
>>>>> +
>>>>> +        val->intval = health_mode
>>>>> +                  ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE
>>>>> +                  : POWER_SUPPLY_CHARGE_TYPE_STANDARD;
>>>>> +        break;
>>>>> +    case POWER_SUPPLY_PROP_TEMP:
>>>>> +        ret = acer_wmi_battery_get_information(data, 0x8, 
>>>>> ACER_BATTERY_INDEX, &value);
>>>>> +        if (ret)
>>>>> +            return ret;
>>>>> +
>>>>> +        if (value > U16_MAX)
>>>>> +            return -ERANGE;
>>>>> +
>>>>> +        val->intval = value - 2731;
>>>>> +        break;
>>>>> +    default:
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static int acer_battery_ext_property_set(struct power_supply *psy,
>>>>> +                     const struct power_supply_ext *ext,
>>>>> +                     void *ext_data,
>>>>> +                     enum power_supply_property psp,
>>>>> +                     const union power_supply_propval *val)
>>>>> +{
>>>>> +    struct acer_wmi_battery_data *data = ext_data;
>>>>> +
>>>>> +    switch (psp) {
>>>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>>>> +        return acer_wmi_battery_set_battery_health_control(data, 
>>>>> HEALTH_MODE,
>>>>> +                val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);
>>>>> +    default:
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static int acer_battery_ext_property_is_writeable(struct 
>>>>> power_supply *psy,
>>>>> +                          const struct power_supply_ext *ext,
>>>>> +                          void *ext_data,
>>>>> +                          enum power_supply_property psp)
>>>>> +{
>>>>> +    switch (psp) {
>>>>> +    case POWER_SUPPLY_PROP_CHARGE_TYPES:
>>>>> +        return true;
>>>>> +    default:
>>>>> +        return false;
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static const struct dmi_system_id 
>>>>> acer_wmi_battery_health_mode_table[] = {
>>>>> +    {
>>>>> +        .matches = {
>>>>> +            DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
>>>>> +            DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P")
>>>>> +        }
>>>>> +    },
>>>>> +    {}
>>>>> +};
>>>>
>>>> I suggest that you perform the DMI check during module 
>>>> initialization. This way
>>>> the DMI table can be marked as __initconst and does not consume any 
>>>> memory after
>>>> anymore after module initialization is complete.
>>>>
>>>> The result of the DMI check could then be stored inside a global 
>>>> variable.
>>>>
>>>> Other than that, the driver seems fine to me.
>>>
>>> Thanks for the review comments, due to some personal circumstances 
>>> it has taken me while to spin up v3.
>>>
>>> I've been digging into the WMI dump data looking if I could figure 
>>> out if there was a way to check if health checks are available but 
>>> only stumbled on another battery related WMI method.
>>>
>>> Method 0x16: GetBatteryFunctionData
>>> - Input: uFunctionMask (uint8), uReservedIn (uint8 array)
>>> - Output: uReturnCode (uint8 array), uBACStartTime (uint8 array), 
>>> uBACStopTime (uint8 array), uBACStatus (uint8), uReservedOut (uint8 
>>> array)
>>> Method 0x17: SetBatteryFunctionData
>>> - Input: uFunctionMask (uint8), uBACSwitch (uint8), uReservedIn 
>>> (uint8 array)
>>>
>>> But I was wondering why in probe or init, the driver couldn't call 
>>> "acer_wmi_battery_get_health_control_status". Then we wouldn't need 
>>> to add a WMI whitelist for every device that supports this.
>>>
>>> If I recall correctly you own an Acer laptop which supports the 
>>> battery temperature but lacks health control? Seeing how many WMI 
>>> methods are exposed on my device I suppose it is also exposed for 
>>> you, but it should hopefully error out?
>>
>> Calling an non-existing WMI method is undefined behavior and could in 
>> the worst cast cause the ACPI firmware to crash. Because of this we 
>> have to use a DMI whitelist for now :(
>
> Thanks for the answer, then I'll rework the driver to use the module 
> __init for DMI as you suggested.
>
> I suppose it is fine to already add the other tested models by the 
> community? [1].
> And should I add an module load option to force health mode so users 
> can test if their hardware is compatible?

Sure.

Thanks,
Armin Wolf

>
>> I am working on a in-kernel BMOF parser, but i am currently occupied 
>> by other work.
> No worries.
>
> [1] https://github.com/frederik-h/acer-wmi-battery/blob/main/MODELS.md
>