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

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

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: