* Re: [PATCH v2] platform/x86: lenovo-wmi-other: Deconflict battery with thinkpad_acpi
2026-11-12 7:48 [PATCH v2] platform/x86: lenovo-wmi-other: Deconflict battery with thinkpad_acpi Aaron Ma
@ 2026-11-12 14:46 ` Derek J. Clark
2026-11-12 15:13 ` Rong Zhang
1 sibling, 0 replies; 3+ messages in thread
From: Derek J. Clark @ 2026-11-12 14:46 UTC (permalink / raw)
To: Aaron Ma, Mark Pearson, Rong Zhang, Hans de Goede,
Ilpo Järvinen, platform-driver-x86, linux-kernel
On November 11, 2026 11:48:23 PM PST, Aaron Ma <aaron.ma@canonical.com> wrote:
>ThinkPads expose battery charge thresholds and charge_behaviour through
>thinkpad_acpi's ACPI battery hook using raw sysfs attribute groups. The
>Lenovo WMI "Other Mode" driver also registers a power_supply_ext for
>charge_behaviour and charge_types on systems advertising those WMI
>capabilities.
>
>When lenovo-wmi-other registers first, thinkpad_acpi fails to create a
>duplicate charge_behaviour sysfs attribute and unregisters its battery
>extension. Conversely, registering or updating any power_supply_ext on
>the same battery runs sysfs_update_groups(), which removes standard
>power-supply attributes not owned by power_supply_core and wipes out
>thinkpad_acpi's charge_control_start_threshold,
>charge_control_end_threshold, and charge_behaviour files.
>
>Deconflict both ideapad_laptop (VPC2004 with GBMD and SBMC) and
>thinkpad_acpi (LEN0268, LEN0068, or IBM0068 with BDSG and BICG) via a
>table check when the corresponding driver is enabled in Kconfig, and
>update the force_load_psy_ext description accordingly.
>
>Fixes: 9ca8fc065b88b ("platform/x86: lenovo-wmi-other: Add WMI battery charge limiting")
>Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
Hi Aaron,
Looks good, thanks. I'll test it in a couple of days. I have one question below but it's not blocking for me, I just want to check my assumption is correct.
Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
>---
>Changes in v2:
>- Consolidate IdeaPad and ThinkPad deconfliction into a table of ACPI
> device IDs, method pairs, and driver configuration checks, as suggested
> by Derek J. Clark.
>- Check GBMD and SBMC for IdeaPads, and BDSG and BICG for ThinkPads,
> instead of relying only on ThinkPad ACPI device presence.
>- Check whether the corresponding driver is enabled before looking up
> its ACPI device, including for ideapad_laptop.
>- Update the force_load_psy_ext description to cover both drivers.
>
>v1: https://lore.kernel.org/platform-driver-x86/20261111083529.727285-1-aaron.ma@canonical.com/
>
> drivers/platform/x86/lenovo/wmi-other.c | 51 ++++++++++++++++++-------
> 1 file changed, 38 insertions(+), 13 deletions(-)
>
>diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>index bd64e7b44669d..14b2c3f4cc7ae 100644
>--- a/drivers/platform/x86/lenovo/wmi-other.c
>+++ b/drivers/platform/x86/lenovo/wmi-other.c
>@@ -189,10 +189,10 @@ MODULE_PARM_DESC(relax_fan_constraint,
> static bool force_load_psy_ext;
> module_param(force_load_psy_ext, bool, 0444);
> MODULE_PARM_DESC(force_load_psy_ext,
>- "This option will skip checking if the ideapad_laptop driver will conflict "
>+ "This option will skip checking if the ideapad_laptop or thinkpad_acpi driver will conflict "
> "with adding an extension to set the battery charge behavior and battery charge "
> "control end threshold. It will also skip checking if the BIOS reports that "
>- "those features are fully supported. It is recommended to blacklist the ideapad "
>+ "those features are fully supported. It is recommended to blacklist the conflicting "
> "driver before using this option.");
>
> /* ======== HWMON (component: lenovo-wmi-capdata 00 & fan) ======== */
>@@ -905,17 +905,32 @@ static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
> return AE_CTRL_TERMINATE;
> }
>
>+struct lwmi_psy_deconflict {
>+ const char *hid;
>+ char * const *methods;
>+ bool driver_enabled;
>+};
>+
>+static char * const lwmi_ideapad_psy_methods[] = { "GBMD", "SBMC" };
>+static char * const lwmi_thinkpad_psy_methods[] = { "BDSG", "BICG" };
>+
>+static const struct lwmi_psy_deconflict lwmi_psy_deconflict_list[] = {
>+ { "VPC2004", lwmi_ideapad_psy_methods, IS_ENABLED(CONFIG_IDEAPAD_LAPTOP) },
>+ { "LEN0268", lwmi_thinkpad_psy_methods, IS_ENABLED(CONFIG_THINKPAD_ACPI) },
>+ { "LEN0068", lwmi_thinkpad_psy_methods, IS_ENABLED(CONFIG_THINKPAD_ACPI) },
>+ { "IBM0068", lwmi_thinkpad_psy_methods, IS_ENABLED(CONFIG_THINKPAD_ACPI) },
>+};
>+
> /**
> * lwmi_om_psy_ext_init() - Hooks power supply extension to device battery
> * @priv: Pointer to the lwmi_om_priv drvdata.
> *
>- * Checks if the ideapad_laptop driver is going to manage charge attributes first,
>- * then if not, hooks the battery to our WMI methods if they are supported.
>+ * Checks if the ideapad_laptop or thinkpad_acpi driver is going to manage
>+ * charge attributes first, then if not, hooks the battery to our WMI methods
>+ * if they are supported.
> */
> static void lwmi_om_psy_ext_init(struct lwmi_om_priv *priv)
> {
>- static const char * const ideapad_hid = "VPC2004";
>- acpi_handle handle = NULL;
I'm assuming this was moved to avoid handling cleanup each loop?
Thanks,
- Derek
> unsigned int props = 0;
> int ret;
>
>@@ -928,14 +943,24 @@ static void lwmi_om_psy_ext_init(struct lwmi_om_priv *priv)
> if (force_load_psy_ext)
> goto load_psy_ext;
>
>- /* Deconflict ideapad_laptop driver */
>- ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
>- if (ret)
>- return;
>+ for (unsigned int i = 0; i < ARRAY_SIZE(lwmi_psy_deconflict_list); i++) {
>+ const struct lwmi_psy_deconflict *entry = &lwmi_psy_deconflict_list[i];
>+ acpi_handle handle = NULL;
>
>- if (handle && acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
>- dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device\n");
>- return;
>+ if (!entry->driver_enabled)
>+ continue;
>+
>+ ret = acpi_get_devices(entry->hid, lwmi_acpi_match, &handle, NULL);
>+ if (ret || !handle)
>+ continue;
>+
>+ if (acpi_has_method(handle, entry->methods[0]) &&
>+ acpi_has_method(handle, entry->methods[1])) {
>+ dev_dbg(&priv->wdev->dev,
>+ "ACPI device %s manages battery for device\n",
>+ entry->hid);
>+ return;
>+ }
> }
>
> load_psy_ext:
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] platform/x86: lenovo-wmi-other: Deconflict battery with thinkpad_acpi
2026-11-12 7:48 [PATCH v2] platform/x86: lenovo-wmi-other: Deconflict battery with thinkpad_acpi Aaron Ma
2026-11-12 14:46 ` Derek J. Clark
@ 2026-11-12 15:13 ` Rong Zhang
1 sibling, 0 replies; 3+ messages in thread
From: Rong Zhang @ 2026-11-12 15:13 UTC (permalink / raw)
To: Aaron Ma, Mark Pearson, Derek J. Clark, Hans de Goede,
Ilpo Järvinen, platform-driver-x86, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6296 bytes --]
Hi Aaron,
Thanks for working on this.
On Thu, 2026-11-12 at 15:48 +0800, Aaron Ma wrote:
> ThinkPads expose battery charge thresholds and charge_behaviour through
> thinkpad_acpi's ACPI battery hook using raw sysfs attribute groups. The
> Lenovo WMI "Other Mode" driver also registers a power_supply_ext for
> charge_behaviour and charge_types on systems advertising those WMI
> capabilities.
>
> When lenovo-wmi-other registers first, thinkpad_acpi fails to create a
> duplicate charge_behaviour sysfs attribute and unregisters its battery
> extension. Conversely, registering or updating any power_supply_ext on
> the same battery runs sysfs_update_groups(), which removes standard
> power-supply attributes not owned by power_supply_core and wipes out
> thinkpad_acpi's charge_control_start_threshold,
> charge_control_end_threshold, and charge_behaviour files.
>
> Deconflict both ideapad_laptop (VPC2004 with GBMD and SBMC) and
> thinkpad_acpi (LEN0268, LEN0068, or IBM0068 with BDSG and BICG) via a
> table check when the corresponding driver is enabled in Kconfig, and
> update the force_load_psy_ext description accordingly.
>
> Fixes: 9ca8fc065b88b ("platform/x86: lenovo-wmi-other: Add WMI battery charge limiting")
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> Changes in v2:
> - Consolidate IdeaPad and ThinkPad deconfliction into a table of ACPI
> device IDs, method pairs, and driver configuration checks, as suggested
> by Derek J. Clark.
> - Check GBMD and SBMC for IdeaPads, and BDSG and BICG for ThinkPads,
> instead of relying only on ThinkPad ACPI device presence.
> - Check whether the corresponding driver is enabled before looking up
> its ACPI device, including for ideapad_laptop.
> - Update the force_load_psy_ext description to cover both drivers.
>
> v1: https://lore.kernel.org/platform-driver-x86/20261111083529.727285-1-aaron.ma@canonical.com/
>
> drivers/platform/x86/lenovo/wmi-other.c | 51 ++++++++++++++++++-------
> 1 file changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
> index bd64e7b44669d..14b2c3f4cc7ae 100644
> --- a/drivers/platform/x86/lenovo/wmi-other.c
> +++ b/drivers/platform/x86/lenovo/wmi-other.c
> @@ -189,10 +189,10 @@ MODULE_PARM_DESC(relax_fan_constraint,
> static bool force_load_psy_ext;
> module_param(force_load_psy_ext, bool, 0444);
> MODULE_PARM_DESC(force_load_psy_ext,
> - "This option will skip checking if the ideapad_laptop driver will conflict "
> + "This option will skip checking if the ideapad_laptop or thinkpad_acpi driver will conflict "
> "with adding an extension to set the battery charge behavior and battery charge "
> "control end threshold. It will also skip checking if the BIOS reports that "
> - "those features are fully supported. It is recommended to blacklist the ideapad "
> + "those features are fully supported. It is recommended to blacklist the conflicting "
> "driver before using this option.");
>
> /* ======== HWMON (component: lenovo-wmi-capdata 00 & fan) ======== */
> @@ -905,17 +905,32 @@ static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
> return AE_CTRL_TERMINATE;
> }
>
> +struct lwmi_psy_deconflict {
> + const char *hid;
> + char * const *methods;
> + bool driver_enabled;
Why is this required?
IMO the relevant items should be wrapped by #if IS_ENABLED(CONFIG_*). In
this manner, lwmi_om_psy_ext_init() doesn't need to iterate through
useless items in vain.
> +};
> +
> +static char * const lwmi_ideapad_psy_methods[] = { "GBMD", "SBMC" };
> +static char * const lwmi_thinkpad_psy_methods[] = { "BDSG", "BICG" };
> +
> +static const struct lwmi_psy_deconflict lwmi_psy_deconflict_list[] = {
> + { "VPC2004", lwmi_ideapad_psy_methods, IS_ENABLED(CONFIG_IDEAPAD_LAPTOP) },
> + { "LEN0268", lwmi_thinkpad_psy_methods, IS_ENABLED(CONFIG_THINKPAD_ACPI) },
> + { "LEN0068", lwmi_thinkpad_psy_methods, IS_ENABLED(CONFIG_THINKPAD_ACPI) },
> + { "IBM0068", lwmi_thinkpad_psy_methods, IS_ENABLED(CONFIG_THINKPAD_ACPI) },
> +};
It could be flattened and simplified into a 2-items array. See the
attached patch (untested, may be completely nonsense).
If you adopt my approach, feel free to add
Co-developed-by: Rong Zhang <i@rong.moe>
Signed-off-by: Rong Zhang <i@rong.moe>
Thanks,
Rong
> +
> /**
> * lwmi_om_psy_ext_init() - Hooks power supply extension to device battery
> * @priv: Pointer to the lwmi_om_priv drvdata.
> *
> - * Checks if the ideapad_laptop driver is going to manage charge attributes first,
> - * then if not, hooks the battery to our WMI methods if they are supported.
> + * Checks if the ideapad_laptop or thinkpad_acpi driver is going to manage
> + * charge attributes first, then if not, hooks the battery to our WMI methods
> + * if they are supported.
> */
> static void lwmi_om_psy_ext_init(struct lwmi_om_priv *priv)
> {
> - static const char * const ideapad_hid = "VPC2004";
> - acpi_handle handle = NULL;
> unsigned int props = 0;
> int ret;
>
> @@ -928,14 +943,24 @@ static void lwmi_om_psy_ext_init(struct lwmi_om_priv *priv)
> if (force_load_psy_ext)
> goto load_psy_ext;
>
> - /* Deconflict ideapad_laptop driver */
> - ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
> - if (ret)
> - return;
> + for (unsigned int i = 0; i < ARRAY_SIZE(lwmi_psy_deconflict_list); i++) {
> + const struct lwmi_psy_deconflict *entry = &lwmi_psy_deconflict_list[i];
> + acpi_handle handle = NULL;
>
> - if (handle && acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
> - dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device\n");
> - return;
> + if (!entry->driver_enabled)
> + continue;
> +
> + ret = acpi_get_devices(entry->hid, lwmi_acpi_match, &handle, NULL);
> + if (ret || !handle)
> + continue;
> +
> + if (acpi_has_method(handle, entry->methods[0]) &&
> + acpi_has_method(handle, entry->methods[1])) {
> + dev_dbg(&priv->wdev->dev,
> + "ACPI device %s manages battery for device\n",
> + entry->hid);
> + return;
> + }
> }
>
> load_psy_ext:
[-- Attachment #2: flattened-charpp.patch --]
[-- Type: text/x-patch, Size: 3680 bytes --]
diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
index bd64e7b44669..0172c4ceed58 100644
--- a/drivers/platform/x86/lenovo/wmi-other.c
+++ b/drivers/platform/x86/lenovo/wmi-other.c
@@ -189,10 +189,10 @@ MODULE_PARM_DESC(relax_fan_constraint,
static bool force_load_psy_ext;
module_param(force_load_psy_ext, bool, 0444);
MODULE_PARM_DESC(force_load_psy_ext,
- "This option will skip checking if the ideapad_laptop driver will conflict "
+ "This option will skip checking if the ideapad_laptop or thinkpad_acpi driver will conflict "
"with adding an extension to set the battery charge behavior and battery charge "
"control end threshold. It will also skip checking if the BIOS reports that "
- "those features are fully supported. It is recommended to blacklist the ideapad "
+ "those features are fully supported. It is recommended to blacklist the conflicting "
"driver before using this option.");
/* ======== HWMON (component: lenovo-wmi-capdata 00 & fan) ======== */
@@ -905,17 +905,39 @@ static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
return AE_CTRL_TERMINATE;
}
+struct lwmi_psy_deconflict {
+ const char *const *hids;
+ const char *const *methods;
+};
+
+#define LWMI_PSY_DECONFLICT_STRS(...) \
+ (const char *[]){ __VA_ARGS__, NULL }
+
+static const struct lwmi_psy_deconflict lwmi_psy_deconflict_list[] = {
+#if IS_ENABLED(CONFIG_IDEAPAD_LAPTOP)
+ {
+ .hids = LWMI_PSY_DECONFLICT_STRS("VPC2004"),
+ .methods = LWMI_PSY_DECONFLICT_STRS("GBMD", "SBMC"),
+ },
+#endif /* IS_ENABLED(CONFIG_IDEAPAD_LAPTOP) */
+#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
+ {
+ .hids = LWMI_PSY_DECONFLICT_STRS("LEN0268", "LEN0068", "IBM0068"),
+ .methods = LWMI_PSY_DECONFLICT_STRS("BDSG", "BICG"),
+ },
+#endif /* IS_ENABLED(CONFIG_THINKPAD_ACPI) */
+};
+
/**
* lwmi_om_psy_ext_init() - Hooks power supply extension to device battery
* @priv: Pointer to the lwmi_om_priv drvdata.
*
- * Checks if the ideapad_laptop driver is going to manage charge attributes first,
- * then if not, hooks the battery to our WMI methods if they are supported.
+ * Checks if the ideapad_laptop or thinkpad_acpi driver is going to manage
+ * charge attributes first, then if not, hooks the battery to our WMI methods
+ * if they are supported.
*/
static void lwmi_om_psy_ext_init(struct lwmi_om_priv *priv)
{
- static const char * const ideapad_hid = "VPC2004";
- acpi_handle handle = NULL;
unsigned int props = 0;
int ret;
@@ -928,14 +950,30 @@ static void lwmi_om_psy_ext_init(struct lwmi_om_priv *priv)
if (force_load_psy_ext)
goto load_psy_ext;
- /* Deconflict ideapad_laptop driver */
- ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
- if (ret)
- return;
-
- if (handle && acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
- dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device\n");
- return;
+ for (unsigned int i = 0; i < ARRAY_SIZE(lwmi_psy_deconflict_list); i++) {
+ const struct lwmi_psy_deconflict *entry = &lwmi_psy_deconflict_list[i];
+ acpi_handle handle = NULL;
+
+ for (unsigned int j = 0; entry->hids[j]; j++) {
+ const char *hid = entry->hids[j];
+ bool has_methods = true;
+
+ ret = acpi_get_devices(hid, lwmi_acpi_match, &handle, NULL);
+ if (ret || !handle)
+ continue;
+
+ for (unsigned int k = 0; entry->methods[k]; k++) {
+ has_methods = acpi_has_method(handle, (char *)entry->methods[k]);
+ if (!has_methods)
+ break;
+ }
+ if (!has_methods)
+ continue;
+
+ dev_dbg(&priv->wdev->dev, "ACPI device %s manages battery for device\n",
+ hid);
+ return;
+ }
}
load_psy_ext:
^ permalink raw reply [flat|nested] 3+ messages in thread