[PATCH 0/1] add Acer battery control driver

[PATCH 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.

[1] https://github.com/frederik-h/acer-wmi-battery

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

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

-- 
2.52.0

[PATCH 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>
---
 drivers/platform/x86/Kconfig            |  12 +
 drivers/platform/x86/Makefile           |   1 +
 drivers/platform/x86/acer-wmi-battery.c | 345 ++++++++++++++++++++++++
 3 files changed, 358 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..b3de6a2827e8 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -170,6 +170,18 @@ 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
+	depends on HWMON
+	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..fffa521cde27
--- /dev/null
+++ b/drivers/platform/x86/acer-wmi-battery.c
@@ -0,0 +1,345 @@
+// 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/init.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/power_supply.h>
+#include <linux/version.h>
+#include <linux/wmi.h>
+#include <linux/unaligned.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;
+};
+
+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 };
+	union acpi_object *obj;
+	int ret;
+
+	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 0, 19, &input, &output);
+	if (ACPI_FAILURE(ret))
+		return -EIO;
+
+	obj = output.pointer;
+	if (!obj)
+		return -EIO;
+
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		ret = -EIO;
+		goto out_free_obj;
+	}
+
+	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;
+		goto out_free_obj;
+	}
+
+	*result = get_unaligned_le32(obj->buffer.pointer);
+
+out_free_obj:
+	kfree(obj);
+	return ret;
+}
+
+static int acer_wmi_battery_get_health_control_status(struct acer_wmi_battery_data *data,
+							      s8 *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 params = {
+		.uBatteryNo = ACER_BATTERY_INDEX,
+		.uFunctionQuery = 0x1,
+		.uReserved = { 0x0, 0x0 }
+	};
+	struct acpi_buffer input = {
+		sizeof(struct get_battery_health_control_status_input), &params
+	};
+	struct get_battery_health_control_status_output status_output;
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	int ret;
+
+	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 0, 20, &input, &output);
+	if (ACPI_FAILURE(ret))
+		return -EIO;
+
+	obj = output.pointer;
+	if (!obj)
+		return -EIO;
+	else if (obj->type != ACPI_TYPE_BUFFER) {
+		ret = -EIO;
+		goto out_free_obj;
+	}
+
+	status_output = *((struct get_battery_health_control_status_output *)
+			obj->buffer.pointer);
+	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;
+		goto out_free_obj;
+	}
+
+	if (health_mode)
+		*health_mode = status_output.uFunctionList & HEALTH_MODE ?
+					  status_output.uFunctionStatus[0] > 0 :
+					  -1;
+
+out_free_obj:
+	kfree(obj);
+	return ret;
+}
+
+static int set_battery_health_control(struct acer_wmi_battery_data *data,
+					      u8 function, bool function_status)
+{
+	struct set_battery_health_control_input params = {
+		.uBatteryNo = ACER_BATTERY_INDEX,
+		.uFunctionMask = function,
+		.uFunctionStatus = (u8)function_status,
+		.uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
+	};
+	struct acpi_buffer input = {
+		sizeof(struct set_battery_health_control_input),
+		&params,
+	};
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	int ret;
+
+	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 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;
+		goto out_free_obj;
+	}
+
+	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;
+		goto out_free_obj;
+	}
+
+out_free_obj:
+	kfree(obj);
+	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;
+	s8 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;
+
+		if (health_mode < 0)
+			return -EINVAL;
+
+		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 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_TEMP:
+		return false;
+	default:
+		return true;
+	}
+}
+
+static const enum power_supply_property acer_battery_properties[] = {
+	POWER_SUPPLY_PROP_CHARGE_TYPES,
+	POWER_SUPPLY_PROP_TEMP,
+};
+
+static const struct power_supply_ext acer_wmi_battery_extension = {
+	.name			= "acer_laptop",
+	.properties		= acer_battery_properties,
+	.num_properties		= ARRAY_SIZE(acer_battery_properties),
+	.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,
+};
+
+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, &acer_wmi_battery_extension,
+					       &data->wdev->dev, data);
+}
+
+static int acer_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+	power_supply_unregister_extension(battery, &acer_wmi_battery_extension);
+
+	return 0;
+}
+
+static int acer_wmi_battery_battery_add(struct acer_wmi_battery_data *data)
+{
+	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 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;
+
+	return acer_wmi_battery_battery_add(data);
+}
+
+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,
+};
+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 1/1] platform/x86: add Acer battery control driver

[Ilpo Järvinen]

On Mon, 5 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>
> ---
>  drivers/platform/x86/Kconfig            |  12 +
>  drivers/platform/x86/Makefile           |   1 +
>  drivers/platform/x86/acer-wmi-battery.c | 345 ++++++++++++++++++++++++
>  3 files changed, 358 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..b3de6a2827e8 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -170,6 +170,18 @@ 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
> +	depends on HWMON
> +	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..fffa521cde27
> --- /dev/null
> +++ b/drivers/platform/x86/acer-wmi-battery.c
> @@ -0,0 +1,345 @@
> +// 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/

Add an empty line here.

> + * Copyright (C) 2022-2025  Frederik Harwath <frederik@harwath.name>
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/power_supply.h>
> +#include <linux/version.h>
> +#include <linux/wmi.h>
> +#include <linux/unaligned.h>

Always use alphabetical order within each block of includes (such as 
those under linux/).

> +#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];

+ types.h

> +} __packed;

__packed needs #include.

> +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 };

Do we expect to ever extend this list? If yes, put them on separate lines 
and add the trailing comma.

Are these BIT() or values of a field, you seem to use HEALTH_MODE with & 
down below which sounds like BIT()?

> +
> +struct acer_wmi_battery_data {
> +	struct acpi_battery_hook hook;
> +	struct wmi_device *wdev;
> +};
> +
> +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 };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 0, 19, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	obj = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		ret = -EIO;
> +		goto out_free_obj;
> +	}
> +
> +	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;
> +		goto out_free_obj;
> +	}
> +
> +	*result = get_unaligned_le32(obj->buffer.pointer);
> +
> +out_free_obj:
> +	kfree(obj);

Please use cleanup.h instead of goto + label. Remember to declare var at 
the site, not in the variable declarations block.

> +	return ret;
> +}
> +
> +static int acer_wmi_battery_get_health_control_status(struct acer_wmi_battery_data *data,
> +							      s8 *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 params = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionQuery = 0x1,
> +		.uReserved = { 0x0, 0x0 }
> +	};
> +	struct acpi_buffer input = {
> +		sizeof(struct get_battery_health_control_status_input), &params
> +	};
> +	struct get_battery_health_control_status_output status_output;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 0, 20, &input, &output);
> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	obj = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +	else if (obj->type != ACPI_TYPE_BUFFER) {
> +		ret = -EIO;
> +		goto out_free_obj;
> +	}
> +
> +	status_output = *((struct get_battery_health_control_status_output *)
> +			obj->buffer.pointer);
> +	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;
> +		goto out_free_obj;
> +	}
> +
> +	if (health_mode)
> +		*health_mode = status_output.uFunctionList & HEALTH_MODE ?
> +					  status_output.uFunctionStatus[0] > 0 :
> +					  -1;

Please use braces for multiline blocks.

> +
> +out_free_obj:
> +	kfree(obj);

cleanup.h, please change all of these.

> +	return ret;
> +}
> +
> +static int set_battery_health_control(struct acer_wmi_battery_data *data,

Missing prefix.

> +					      u8 function, bool function_status)
> +{
> +	struct set_battery_health_control_input params = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionMask = function,
> +		.uFunctionStatus = (u8)function_status,

Use ? : instead of relying C's bool -> u8 conversion.

> +		.uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
> +	};
> +	struct acpi_buffer input = {
> +		sizeof(struct set_battery_health_control_input),
> +		&params,

Previouslly you had these on the same line, it would be nice to use 
consistent style.

> +	};
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 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;
> +		goto out_free_obj;
> +	}
> +
> +	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;
> +		goto out_free_obj;
> +	}
> +
> +out_free_obj:
> +	kfree(obj);
> +	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;
> +	s8 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;
> +
> +		if (health_mode < 0)
> +			return -EINVAL;

Why doesn't acer_wmi_battery_get_health_control_status() return -EINVAL 
directlu but does this odd s8 trickery?

> +
> +		val->intval = health_mode ? POWER_SUPPLY_CHARGE_TYPE_LONGLIFE :
> +				POWER_SUPPLY_CHARGE_TYPE_STANDARD;

Please align this properly.

> +		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 set_battery_health_control(data, HEALTH_MODE,
> +				val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE);

There's quite big leap from POWER_SUPPLY_CHARGE_TYPE_LONGLIFE to something 
called "function_status". Is this perhaps a (argument) naming issue?

> +	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_TEMP:
> +		return false;
> +	default:
> +		return true;
> +	}
> +}
> +
> +static const enum power_supply_property acer_battery_properties[] = {
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,
> +	POWER_SUPPLY_PROP_TEMP,
> +};
> +
> +static const struct power_supply_ext acer_wmi_battery_extension = {
> +	.name			= "acer_laptop",
> +	.properties		= acer_battery_properties,
> +	.num_properties		= ARRAY_SIZE(acer_battery_properties),
> +	.charge_types           = (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> +				   BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),

Aren't parenthesis unnecessary in this contruct?

> +	.get_property		= acer_battery_ext_property_get,
> +	.set_property		= acer_battery_ext_property_set,
> +	.property_is_writeable	= acer_battery_ext_property_is_writeable,
> +};
> +
> +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, &acer_wmi_battery_extension,
> +					       &data->wdev->dev, data);
> +}
> +
> +static int acer_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	power_supply_unregister_extension(battery, &acer_wmi_battery_extension);
> +
> +	return 0;
> +}
> +
> +static int acer_wmi_battery_battery_add(struct acer_wmi_battery_data *data)
> +{
> +	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 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;
> +
> +	return acer_wmi_battery_battery_add(data);
> +}
> +
> +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,
> +};
> +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 1/1] platform/x86: add Acer battery control driver

[Armin Wolf]

Am 05.01.26 um 18:10 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>
> ---
>   drivers/platform/x86/Kconfig            |  12 +
>   drivers/platform/x86/Makefile           |   1 +
>   drivers/platform/x86/acer-wmi-battery.c | 345 ++++++++++++++++++++++++
>   3 files changed, 358 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..b3de6a2827e8 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -170,6 +170,18 @@ 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
> +	depends on HWMON

Hi,

why do you need this HWMON dependency here? AFAIK you are not using any parts
of the hwmon API, so please remove this.

> +	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..fffa521cde27
> --- /dev/null
> +++ b/drivers/platform/x86/acer-wmi-battery.c
> @@ -0,0 +1,345 @@
> +// 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/init.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/power_supply.h>
> +#include <linux/version.h>
> +#include <linux/wmi.h>
> +#include <linux/unaligned.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;

Please use underscores instead of camel case.

> +	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;
> +};
> +
> +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 };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 0, 19, &input, &output);

Please use wmidev_evaluate_method() instead of the decprecated GUID-based interface.

> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	obj = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		ret = -EIO;
> +		goto out_free_obj;
> +	}
> +
> +	if (obj->buffer.length != sizeof(u32)) {

Please also accept oversized ACPI buffers to emulate the behavior of the Windows WMI-ACPI driver.

> +		dev_err(&data->wdev->dev, "WMI battery information call returned buffer of unexpected length %u\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +		goto out_free_obj;
> +	}
> +
> +	*result = get_unaligned_le32(obj->buffer.pointer);
> +
> +out_free_obj:
> +	kfree(obj);
> +	return ret;
> +}
> +
> +static int acer_wmi_battery_get_health_control_status(struct acer_wmi_battery_data *data,
> +							      s8 *health_mode)

Please run checkpatch --strict over this source file and fix any issues.

> +{
> +	/*
> +	 * 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 params = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionQuery = 0x1,

Maybe use a define for that?

> +		.uReserved = { 0x0, 0x0 }
> +	};
> +	struct acpi_buffer input = {
> +		sizeof(struct get_battery_health_control_status_input), &params

Please use sizeof(params).

> +	};
> +	struct get_battery_health_control_status_output status_output;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 0, 20, &input, &output);

Please use wmidev_evaluate_method().

> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	obj = output.pointer;
> +	if (!obj)
> +		return -EIO;
> +	else if (obj->type != ACPI_TYPE_BUFFER) {

The "else" is pointless here, please remove.

> +		ret = -EIO;
> +		goto out_free_obj;
> +	}
> +
> +	status_output = *((struct get_battery_health_control_status_output *)
> +			obj->buffer.pointer);
> +	if (obj->buffer.length != 8) {

Please do the size check first before assigning status_output (keep the previous comment regarding oversized buffers in mind).
Also please turn status_output into a pointer so that you can avoid a copy operation.

> +		dev_err(&data->wdev->dev, "WMI battery status call returned a buffer of unexpected length %d\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +		goto out_free_obj;
> +	}
> +
> +	if (health_mode)
> +		*health_mode = status_output.uFunctionList & HEALTH_MODE ?
> +					  status_output.uFunctionStatus[0] > 0 :
> +					  -1;
> +
> +out_free_obj:
> +	kfree(obj);
> +	return ret;
> +}
> +
> +static int set_battery_health_control(struct acer_wmi_battery_data *data,
> +					      u8 function, bool function_status)
> +{
> +	struct set_battery_health_control_input params = {
> +		.uBatteryNo = ACER_BATTERY_INDEX,
> +		.uFunctionMask = function,
> +		.uFunctionStatus = (u8)function_status,
> +		.uReservedIn = { 0x0, 0x0, 0x0, 0x0, 0x0 }
> +	};
> +	struct acpi_buffer input = {
> +		sizeof(struct set_battery_health_control_input),
> +		&params,
> +	};
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret;
> +
> +	ret = wmi_evaluate_method(ACER_BATTERY_GUID, 0, 21, &input, &output);

Please use wmidev_evaluate_method().

> +	if (ACPI_FAILURE(ret))
> +		return -EIO;
> +
> +	obj = output.pointer;
> +
> +	if (!obj)
> +		return -EIO;
> +
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		ret = -EIO;
> +		goto out_free_obj;
> +	}
> +
> +	if (obj->buffer.length != 4) {

Please also accept oversized buffers. By the way, what is the usage of the return value?

> +		dev_err(&data->wdev->dev, "WMI battery status set operation returned a buffer of unexpected length %d\n",
> +			obj->buffer.length);
> +		ret = -EINVAL;
> +		goto out_free_obj;
> +	}
> +
> +out_free_obj:
> +	kfree(obj);
> +	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;
> +	s8 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;
> +
> +		if (health_mode < 0)
> +			return -EINVAL;
> +
> +		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 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_TEMP:
> +		return false;

Nitpick: Only return true for charge_types and return false for everything else.

> +	default:
> +		return true;
> +	}
> +}
> +
> +static const enum power_supply_property acer_battery_properties[] = {
> +	POWER_SUPPLY_PROP_CHARGE_TYPES,

I own a Acer notebook that only supports the temperature sensor, not the health mode feature.
You can detect this by looking at the embedded WMI binary MOF data, but we currently have no
in-kernel parser for this.

I am working on adding support for the binary MOF data, but for now i suggest that you use a
DMI whitelist before enabling access to the health mode feature.

> +	POWER_SUPPLY_PROP_TEMP,
> +};
> +
> +static const struct power_supply_ext acer_wmi_battery_extension = {
> +	.name			= "acer_laptop",

Could you please use "acer_wmi_battery" (or DRIVER_NAME) instead? This would prevent any future name conflicts
with other drivers for Acer notebooks.

> +	.properties		= acer_battery_properties,
> +	.num_properties		= ARRAY_SIZE(acer_battery_properties),
> +	.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,
> +};
> +
> +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, &acer_wmi_battery_extension,
> +					       &data->wdev->dev, data);
> +}
> +
> +static int acer_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
> +{
> +	power_supply_unregister_extension(battery, &acer_wmi_battery_extension);
> +
> +	return 0;
> +}
> +
> +static int acer_wmi_battery_battery_add(struct acer_wmi_battery_data *data)
> +{
> +	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 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;
> +
> +	return acer_wmi_battery_battery_add(data);

Nitpick: Please fold this function into the caller.

> +}
> +
> +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,

Please also set .no_singleton to signal that this driver can be instantiated multiple times.

Thanks,
Armin Wolf

> +};
> +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 0/1] add Acer battery control driver

[Armin Wolf]

Am 05.01.26 um 18:10 schrieb 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.

Nice work. There are some issues with this patch but nothing too serious.
Can you tell me more about the calibration mode issue on your device?

Thanks,
Armin Wolf

>
> [1] https://github.com/frederik-h/acer-wmi-battery
>
> Jelle van der Waa (1):
>    platform/x86: add Acer battery control driver
>
>   drivers/platform/x86/Kconfig            |  12 +
>   drivers/platform/x86/Makefile           |   1 +
>   drivers/platform/x86/acer-wmi-battery.c | 345 ++++++++++++++++++++++++
>   3 files changed, 358 insertions(+)
>   create mode 100644 drivers/platform/x86/acer-wmi-battery.c
>

Re: [PATCH 0/1] add Acer battery control driver

[Jelle van der Waa]

On 1/8/26 14:52, Armin Wolf wrote:
> Am 05.01.26 um 18:10 schrieb 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.
> 
> Nice work. There are some issues with this patch but nothing too serious.
> Can you tell me more about the calibration mode issue on your device?

Thanks for the review and sorry for the slow response. When I enabled 
the calibration mode on my device the device started to charge but never 
discharged successfully. I want to take a bit more time to investigate 
this properly. On my thinkpad device enabling calibration mode 
discharges the laptop till ~ 0% and then switches over to AC power.

Greetings,

Jelle van der Waa

Re: [PATCH 0/1] add Acer battery control driver

[Armin Wolf]

Am 19.01.26 um 21:23 schrieb Jelle van der Waa:

> On 1/8/26 14:52, Armin Wolf wrote:
>> Am 05.01.26 um 18:10 schrieb 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.
>>
>> Nice work. There are some issues with this patch but nothing too 
>> serious.
>> Can you tell me more about the calibration mode issue on your device?
>
> Thanks for the review and sorry for the slow response. When I enabled 
> the calibration mode on my device the device started to charge but 
> never discharged successfully. I want to take a bit more time to 
> investigate this properly. On my thinkpad device enabling calibration 
> mode discharges the laptop till ~ 0% and then switches over to AC power.

I see. Maybe the battery is first trying to reach 100% charge before starting to discharge:

https://community.acer.com/en/discussion/688985/how-to-perform-battery-calibration-from-the-acer-care-center-in-predator-helios-neo-16

But if you want to do a bit more research before adding support for battery calibration then this is totally fine.

Thanks,
Armin Wolf

>
> Greetings,
>
> Jelle van der Waa
>

Re: [PATCH 0/1] add Acer battery control driver

[Jelle van der Waa]

On 1/19/26 22:19, Armin Wolf wrote:
> Am 19.01.26 um 21:23 schrieb Jelle van der Waa:
> 
>> On 1/8/26 14:52, Armin Wolf wrote:
>>> Am 05.01.26 um 18:10 schrieb 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.
>>>
>>> Nice work. There are some issues with this patch but nothing too 
>>> serious.
>>> Can you tell me more about the calibration mode issue on your device?
>>
>> Thanks for the review and sorry for the slow response. When I enabled 
>> the calibration mode on my device the device started to charge but 
>> never discharged successfully. I want to take a bit more time to 
>> investigate this properly. On my thinkpad device enabling calibration 
>> mode discharges the laptop till ~ 0% and then switches over to AC power.
> 
> I see. Maybe the battery is first trying to reach 100% charge before 
> starting to discharge:
> 
> https://community.acer.com/en/discussion/688985/how-to-perform-battery- 
> calibration-from-the-acer-care-center-in-predator-helios-neo-16
> 
> But if you want to do a bit more research before adding support for 
> battery calibration then this is totally fine.

Thanks for this pointer, I tested this again this week and it indeed 
charges, discharges and charges again. I haven't verified if the bios 
shows any difference.

So my plan is to send v2 with calibration support included.

Thanks,

Jelle van der Waa

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

[Jelle van der Waa]

On 1/8/26 14:50, Armin Wolf wrote:
> Am 05.01.26 um 18:10 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>
>> ---
>>   drivers/platform/x86/Kconfig            |  12 +
>>   drivers/platform/x86/Makefile           |   1 +
>>   drivers/platform/x86/acer-wmi-battery.c | 345 ++++++++++++++++++++++++
>>   3 files changed, 358 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..b3de6a2827e8 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -170,6 +170,18 @@ 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
>> +    depends on HWMON
> 
> Hi,
> 
> why do you need this HWMON dependency here? AFAIK you are not using any 
> parts
> of the hwmon API, so please remove this.
> 

Ah, since the battery temperature is exposed under hwmon in sysfs I 
thought the driver would need a dependency on it.

<snip>

>> +static int acer_wmi_battery_get_health_control_status(struct 
>> acer_wmi_battery_data *data,
>> +                                  s8 *health_mode)
> 
> Please run checkpatch --strict over this source file and fix any issues.
>

Thanks for this suggestion, I wasn't aware of the --strict option.

<snip>

>> +static const enum power_supply_property acer_battery_properties[] = {
>> +    POWER_SUPPLY_PROP_CHARGE_TYPES,
> 
> I own a Acer notebook that only supports the temperature sensor, not the 
> health mode feature.
> You can detect this by looking at the embedded WMI binary MOF data, but 
> we currently have no
> in-kernel parser for this.
> 
> I am working on adding support for the binary MOF data, but for now i 
> suggest that you use a
> DMI whitelist before enabling access to the health mode feature.

I'l look into this when sending v2, the GitHub project has a list of 
models which work with the driver so I'll include those in the DMI table.

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

Thanks,

Jelle van der Waa