Re: [PATCH] platform/x86: lenovo-wmi-other: Deconflict battery with thinkpad_acpi

["Derek J. Clark" <derekjohn.clark@gmail.com>]

On November 11, 2026 12:35:29 AM 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.
>
>Match the existing ideapad_laptop deconfliction and skip registering
>the WMI battery extension when CONFIG_THINKPAD_ACPI is enabled and a
>ThinkPad ACPI hotkey device (LEN0268, LEN0068, or IBM0068) is present,
>unless overridden by force_load_psy_ext.
>
>Fixes: 9ca8fc065b88b ("platform/x86: lenovo-wmi-other: Add WMI battery charge limiting")
>Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>---
> drivers/platform/x86/lenovo/wmi-other.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>index bd64e7b44669d..6175f1e6c51a7 100644
>--- a/drivers/platform/x86/lenovo/wmi-other.c
>+++ b/drivers/platform/x86/lenovo/wmi-other.c
>@@ -909,8 +909,9 @@ static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
>  * 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 thinkpad_acpi or ideapad_laptop 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)
> {
>@@ -928,6 +929,15 @@ static void lwmi_om_psy_ext_init(struct lwmi_om_priv *priv)
> 	if (force_load_psy_ext)
> 		goto load_psy_ext;
> 
>+	/* Deconflict thinkpad_acpi driver */
>+	if (IS_ENABLED(CONFIG_THINKPAD_ACPI) &&
>+	    (acpi_dev_present("LEN0268", NULL, -1) ||
>+	     acpi_dev_present("LEN0068", NULL, -1) ||
>+	     acpi_dev_present("IBM0068", NULL, -1))) {
>+		dev_dbg(&priv->wdev->dev, "thinkpad_acpi driver manages battery for device\n");
>+		return;
>+	}
>+

Hi Aaron,

This doesn't validate that the methods exist on the Thinkpad device. That could have us deferring to a driver that doesn't fully expose the battery on some devices. After looking at tpacpi_battery_probe(), I think we can rewrite this function to validate both the ideapad and thinkpad dicovery.

- Add a for loop to find a matching handle, include the existing ideapad handle and the 3 thinkpad handles.
- Use acpi_has_method() to find if the feature is supported. If it's an ideapad handle, check for "GBMD" and "SBMC". If it is a thinkpad handle, check for "GET_DISHCHARGE" and "GET_INHIBIT".

For this I think we can make two const, over for each method pair, then make a const struct list of method pairs and handles. That will allow you to loop over the list to quickly find a matching handle, then use that index to access the method names without looping a second time. The struct declaration for this would keep it tidy and expandable.

Additionally, the comment & description for force_load_psy_ext needs to be updated to include the thinkpad case with this change.

I do like the fact that you check for the config to be set first. That is something the current implementation lacks. Perhaps a driver_config field on the same struct could hold that data. Then each check becomes: do you have the handle -> do you have its methods -> is it configured.

Thanks,
- Derek

> 	/* Deconflict ideapad_laptop driver */
> 	ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
> 	if (ret)