From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 371833B27E8 for ; Mon, 8 Jun 2026 08:36:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.14 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780907817; cv=none; b=Ihh175FRk9Xx6jBJjPYkCEy1sqkTTt7VEL+zADv9J0M42lyHW5cteKXdVSa9spaGK6YR/v6fJTgxAoFONsdYWopBSW7DqVubDXwATTFjB8lGIk0iRDhaGtgYLk4+5hnK7ikKLSEHeYmba0WbpX/rFA6jI5DNZweiGY91WTDoxsI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780907817; c=relaxed/simple; bh=+eFmC4yx2Bx5WIBE71AgsmKk5L1vOfdZAha8CntGGIA=; h=From:Date:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=K3qYXGxW3kh/K5pEVJZtM3DLPFqvd1rDKAxOiXbQEpSXQgxGjX0yIlsbip+B0DHB+rilDDnvLMyMtsOxIDQoxnxF06jFuWC5QRxMSpBbuLIqPMe8/Qew/e6CNcXayi/YsV8gVm8Eu+BvglCy3U/NWLxr1D5P1AQiahYNLcfLITQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=pass smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=Vpku6OMF; arc=none smtp.client-ip=192.198.163.14 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="Vpku6OMF" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1780907809; x=1812443809; h=from:date:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=+eFmC4yx2Bx5WIBE71AgsmKk5L1vOfdZAha8CntGGIA=; b=Vpku6OMFShgBRNYgbV1rML/csvvysT+oegwPSaYOw3PVdZWziF+1mq4d 1gJryB00qz77Lr+lBIGQkTg/wngy7VlWMihuF7bxovyShwdCcMKfagciW aM/Ad5DnvvmQ6HAxTcyKQJg+CCgQpJAb6K1sAVNnG5cNb2SbQF4d2S5j4 SFKfBV7/gTpVC8zB3u6IrB5Tzt8kykdMMmFl6IPZTyDr4tho1HTgEv22o SGeC1c148IH4gGC8Ncr8sQHCGMu+1pZBDiYEtDDvku7ZxXMMq3QCUbuBn K8XVsNh6SpC19D3tjXEPDWEHamXLDlJKgBSJ5becFAmZMcM/txaS3rBXt A==; X-CSE-ConnectionGUID: +6ODajz+SLC9bki+7QSO2w== X-CSE-MsgGUID: CI1sI5EcTUObWaFRgPYn/w== X-IronPort-AV: E=McAfee;i="6800,10657,11810"; a="81683576" X-IronPort-AV: E=Sophos;i="6.24,194,1774335600"; d="scan'208";a="81683576" Received: from orviesa005.jf.intel.com ([10.64.159.145]) by fmvoesa108.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jun 2026 01:36:48 -0700 X-CSE-ConnectionGUID: BuIj5WQaRrmfx/cCaJybvQ== X-CSE-MsgGUID: JLIz1BZSQVqE7hSfn2a6Kw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.24,194,1774335600"; d="scan'208";a="250401217" Received: from ijarvine-mobl1.ger.corp.intel.com (HELO localhost) ([10.245.244.182]) by orviesa005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Jun 2026 01:36:46 -0700 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 8 Jun 2026 11:36:42 +0300 (EEST) To: Jelle van der Waa cc: Hans de Goede , platform-driver-x86@vger.kernel.org, Frederik Harwath Subject: Re: [PATCH v4 1/1] platform/x86: add Acer battery control driver In-Reply-To: <20260531090505.156394-2-jelle@vdwaa.nl> Message-ID: References: <20260531090505.156394-1-jelle@vdwaa.nl> <20260531090505.156394-2-jelle@vdwaa.nl> Precedence: bulk X-Mailing-List: platform-driver-x86@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Sun, 31 May 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 > > --- > v4: > - Add additional DMI ids from https://github.com/frederik-h/acer-wmi-battery/blob/main/MODELS.md > cross checked with DMI ids found in https://github.com/linuxhw/DMI > - Move DMI matching to __init and make the DMI table __initconst > - Port to the new wmidev_invoke_* API Thanks for the update. Some small things still noted below. > v3: > - Add depends on DMI > - Rename CamelCase struct member names > - Simplify returning errors > - Add comma to non-terminating entries > - Simplified acer_wmi_battery_set_battery_health_control to acer_wmi_set_battery_health_control > - Use sizeof for acpi_object buffer > - Drop POWER_SUPPLY_EXTENSION macro > 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 | 12 + > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/acer-wmi-battery.c | 445 ++++++++++++++++++++++++ > 3 files changed, 458 insertions(+) > create mode 100644 drivers/platform/x86/acer-wmi-battery.c > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 7a4956088300..ad7474c35d8e 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -189,6 +189,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 DMI > + 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 872ac3842391..a877acd937cd 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -20,6 +20,7 @@ obj-$(CONFIG_BITLAND_MIFS_WMI) += bitland-mifs-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..903a6e762ebd > --- /dev/null > +++ b/drivers/platform/x86/acer-wmi-battery.c > @@ -0,0 +1,445 @@ > +// 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 > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#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 battery_no; > + u8 function_query; > + u8 reserved[2]; > +} __packed; > + > +struct get_battery_health_control_status_output { > + u8 function_list; > + u8 ret[2]; > + u8 function_status[5]; > +} __packed; > + > +struct set_battery_health_control_input { > + u8 battery_no; > + u8 function_mask; > + u8 function_status; > + u8 reserved_in[5]; > +} __packed; > + > +struct set_battery_health_control_output { > + u8 ret; > + u8 reserved_out; > +} __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; > +}; > + > +bool health_mode; > + > +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 wmi_buffer input = { .length = sizeof(args), .data = args }; > + struct wmi_buffer output = { 0 }; = {} would be enough to initialize it but I see Armin noted this is unnecesary so please follow his advice. > + int ret; > + > + ret = wmidev_invoke_method(data->wdev, 0, 19, &input, &output, sizeof(u32)); > + if (ret) > + return ret; > + > + *result = get_unaligned_le32(output.data); My impression was that the data is always aligned by wmi core: "Said data is guaranteed to be aligned on a 8-byte boundary." You probably don't need the related header afterwards either. > + kfree(output.data); > + > + return 0; > +} > + > +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 = { > + .battery_no = ACER_BATTERY_INDEX, > + .function_query = 0x1, > + .reserved = { 0x0, 0x0 }, > + }; > + struct wmi_buffer input = { .length = sizeof(args), .data = &args }; > + struct wmi_buffer output = { 0 }; Unnecessary init here as well. > + int ret; > + > + ret = wmidev_invoke_method(data->wdev, 0, 20, &input, &output, > + sizeof(struct get_battery_health_control_status_output)); > + if (ret) > + return ret; > + > + struct get_battery_health_control_status_output *status_output __free(kfree) = output.data; > + > + if (health_mode) { > + if (!(status_output->function_list & HEALTH_MODE)) > + return -EINVAL; > + > + *health_mode = status_output->function_status[0] > 0; > + } > + > + return 0; > +} > + > +static int acer_wmi_battery_set_health_control(struct acer_wmi_battery_data *data, > + u8 function, bool function_status) > +{ > + struct set_battery_health_control_input args = { > + .battery_no = ACER_BATTERY_INDEX, > + .function_mask = function, > + .function_status = function_status ? 1 : 0, > + .reserved_in = { 0x0, 0x0, 0x0, 0x0, 0x0 }, > + }; > + struct wmi_buffer input = { .length = sizeof(args), .data = &args }; > + int ret; > + > + ret = wmidev_invoke_procedure(data->wdev, 0, 21, &input); > + if (ret) > + return ret; > + > + return 0; > +} > + > +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; We can derive that literal from constant defs: ABSOLUTE_ZERO_MILLICELSIUS / MILLIDEGREE_PER_DECIDEGREE Whether you want to comment about losing the 50 millidegrees from ABSOLUTE_ZERO_MILLICELSIUS or not, I leave up to you. > + 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_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[] __initconst = { > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-24PT") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-44P") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-59") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-59") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A315-510P") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Aspire A715-42G") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Nitro ANV15-51") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Nitro AN515-57") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Nitro AN515-58") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Nitro AN517-54") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Nitro AN517-54") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Predator PHN16-71") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Swift SF314-34") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Swift SF314-43") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Swift SFE16-44") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Swift SFG16-72") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Swift SFX14-71G") > + } > + }, > + { > + .matches = { > + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), > + DMI_MATCH(DMI_PRODUCT_NAME, "Swift SFX16-61G") Pleaese add the trailing commas to any non-terminating entry. > + } > + }, > + {} > +}; > + > +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, > +}; > + > +static const struct power_supply_ext acer_wmi_battery_extension_v1 = { > + .name = DRIVER_NAME, > + .properties = acer_battery_properties_v1, > + .num_properties = ARRAY_SIZE(acer_battery_properties_v1), > + .get_property = acer_battery_ext_property_get, > + .set_property = acer_battery_ext_property_set, > + .property_is_writeable = acer_battery_ext_property_is_writeable, > +}; > + > +static const struct power_supply_ext acer_wmi_battery_extension_v2 = { > + .name = DRIVER_NAME, > + .properties = acer_battery_properties_v2, > + .num_properties = ARRAY_SIZE(acer_battery_properties_v2), > + .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, 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->battery_ext = 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, > +}; > + > +static int __init acer_wmi_battery_module_init(void) > +{ > + int r; > + > + if (dmi_check_system(acer_wmi_battery_health_mode_table)) > + health_mode = true; > + else > + health_mode = false; > + > + r = wmi_driver_register(&acer_wmi_battery_driver); > + if (r) > + return r; > + > + return r; > +} > + > +static void __exit acer_wmi_battery_module_exit(void) > +{ > + wmi_driver_unregister(&acer_wmi_battery_driver); > +} > + > +module_init(acer_wmi_battery_module_init); > +module_exit(acer_wmi_battery_module_exit); > + > +MODULE_AUTHOR("Frederik Harwath "); > +MODULE_AUTHOR("Jelle van der Waa "); > +MODULE_DESCRIPTION("Acer battery health control WMI driver"); > +MODULE_LICENSE("GPL"); > -- i.