Re: [PATCH v3 6/6] platform/x86: lenovo-wmi-other: Add WMI battery charge limiting.

Derek J. Clark
2026-02-25T18:23:42Z

On February 25, 2026 9:55:40 AM PST, Rong Zhang <i@rong.moe> wrote:
>Hi Derek,
>
>There is an extra dot (period) in the title. Please remove it.
>
>On Tue, 2026-02-24 at 04:32 +0000, Derek J. Clark wrote:
>> Add charge-type power supply extension for devices that support WMI based
>> charge enable/disable. Lenovo Legion devices that implement function ID
>> and capdata 00 ID 0x03010001 are able to enable or disable charging
>> through the lenovo-wmi-other interface.
>
>"disable charging"...
>
>I'd expect it pauses charging immediately and unconditionally instead
>of setting a threshold, especially considering the its name
>"INSTANT_MODE". If this is the case, I don't think it fits the
>definition of POWER_SUPPLY_CHARGE_TYPE_LONGLIFE. It'd better to use
>POWER_SUPPLY_CHARGE_TYPE_NONE.

It really just sets a charge threshold of 80% and lowers the charge speed a bit. I'll come up with a better name than the one Lenovo uses in their docs.

>> The ideapad_laptop driver
>> conflicts with this if it can also provide the attribute, so we have to
>> get the acpi_handle and check for the same ACPI methods that enable the
>> feature in that driver. The ACPI method is more reliable from my testing
>> when both are present, so there is no need to modify the ideapad_laptop
>> driver instead.
>
>When ideapad-laptop is blacklisted, there should be no conflict. I
>would suggest adding a module parameter to force wmi-other to register
>psy_ext anyway so that users can have their own choice.
>

That should be fine. I've found the WMI interface is a little buggy on devices that support the ACPI method. I.e. on the Go S and Go 2 it always reports the "Enabled" status, but changing it shows the ACPI method reporting the correct value. (I used a different attr name to test this)

>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>> drivers/platform/x86/lenovo/wmi-capdata.h | 1 +
>> drivers/platform/x86/lenovo/wmi-other.c | 230 ++++++++++++++++++++++
>> 2 files changed, 231 insertions(+)
>>
>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
>> index b7f9ee7b301a..00471551e7d6 100644
>> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
>> @@ -26,6 +26,7 @@
>> enum lwmi_device_id {
>> LWMI_DEVICE_ID_CPU = 0x01,
>> LWMI_DEVICE_ID_GPU = 0x02,
>> + LWMI_DEVICE_ID_PSU = 0x03,
>> LWMI_DEVICE_ID_FAN = 0x04,
>> };
>>
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index 7f0d5a17b44f..b2daff1b45c2 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -42,9 +42,12 @@
>> #include <linux/module.h>
>> #include <linux/notifier.h>
>> #include <linux/platform_profile.h>
>> +#include <linux/power_supply.h>
>> #include <linux/types.h>
>> #include <linux/wmi.h>
>>
>> +#include <acpi/battery.h>
>> +
>> #include "wmi-capdata.h"
>> #include "wmi-events.h"
>> #include "wmi-gamezone.h"
>> @@ -78,10 +81,17 @@ enum lwmi_feature_id_gpu {
>> LWMI_FEATURE_ID_GPU_NV_CPU_BOOST = 0x0b,
>> };
>>
>> +enum lwmi_feature_id_psu {
>> + LWMI_FEATURE_ID_PSU_INSTANT_MODE = 0x01,
>> + LWMI_FEATURE_ID_PSU_CHARGE_MODE = 0x02,
>
>I see no code referencing LWMI_FEATURE_ID_PSU_CHARGE_MODE. What's the
>mode for?

It's another option exposed by some BIOS that is supposed to have similar functionality. None of my devices support it properly so I removed it but forgot to clean up this define.

>> +};
>> +
>> #define LWMI_FEATURE_ID_FAN_RPM 0x03
>>
>> #define LWMI_TYPE_ID_NONE 0x00
>> #define LWMI_TYPE_ID_CROSSLOAD 0x01
>> +#define LWMI_TYPE_ID_PSU_AC 0x01
>> +#define LWMI_TYPE_ID_PSU_PD 0x02
>>
>> #define LWMI_FEATURE_VALUE_GET 17
>> #define LWMI_FEATURE_VALUE_SET 18
>> @@ -92,10 +102,17 @@ enum lwmi_feature_id_gpu {
>>
>> #define LWMI_FAN_DIV 100
>>
>> +#define LWMI_CHARGE_MODE_ENABLED 0x00
>> +#define LWMI_CHARGE_MODE_DISABLED 0x01
>> +
>> #define LWMI_ATTR_ID_FAN_RPM(x) \
>> LWMI_ATTR_ID(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>> LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>>
>> +#define LWMI_ATTR_ID_PSU(feat, type) \
>> + LWMI_ATTR_ID(LWMI_DEVICE_ID_PSU, feat, \
>> + LWMI_GZ_THERMAL_MODE_NONE, type)
>> +
>
>Unaligned backslashes.
>
>> #define LWMI_OM_SYSFS_NAME "lenovo-wmi-other"
>> #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>>
>> @@ -137,6 +154,8 @@ struct lwmi_om_priv {
>> bool capdata00_collected : 1;
>> bool capdata_fan_collected : 1;
>> } fan_flags;
>> +
>> + struct acpi_battery_hook battery_hook;
>> };
>>
>> /*
>> @@ -561,6 +580,216 @@ static void lwmi_om_fan_info_collect_cd_fan(struct device *dev, struct cd_list *
>> lwmi_om_hwmon_add(priv);
>> }
>>
>> +/* ======== Power Supply Extension (component: lenovo-wmi-capdata 00) ======== */
>> +
>> +/**
>> + * lwmi_psy_ext_get_prop() - Get a power_supply_ext property
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to read
>> + * @val: The value to return
>> + *
>> + * Writes the given value to the power_supply_ext property
>> + *
>> + * Return: 0 on success, or an error
>> + */
>> +static int lwmi_psy_ext_get_prop(struct power_supply *ps,
>> + const struct power_supply_ext *ext,
>> + void *ext_data,
>> + enum power_supply_property prop,
>> + union power_supply_propval *val)
>> +{
>> + struct lwmi_om_priv *priv = ext_data;
>> + struct wmi_method_args_32 args;
>> + u32 retval;
>> + int ret;
>> +
>> + args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> +
>> + ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>> + (unsigned char *)&args, sizeof(args),
>> + &retval);
>> + if (ret)
>> + return ret;
>> +
>> + dev_dbg(&priv->wdev->dev, "Got return value %x for property %x\n", retval, prop);
>> +
>> + if (retval == LWMI_CHARGE_MODE_DISABLED)
>> + val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
>> + else
>> + val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * lwmi_psy_ext_set_prop() - Set a power_supply_ext property
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to write
>> + * @val: The value to write
>> + *
>> + * Writes the given value to the power_supply_ext property
>> + *
>> + * Return: 0 on success, or an error
>> + */
>> +static int lwmi_psy_ext_set_prop(struct power_supply *ps,
>> + const struct power_supply_ext *ext,
>> + void *ext_data,
>> + enum power_supply_property prop,
>> + const union power_supply_propval *val)
>> +{
>> + struct lwmi_om_priv *priv = ext_data;
>> + struct wmi_method_args_32 args;
>> +
>> + args.arg0 = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> + if (val->intval == POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)
>> + args.arg1 = LWMI_CHARGE_MODE_DISABLED;
>> + else
>> + args.arg1 = LWMI_CHARGE_MODE_ENABLED;
>> +
>> + dev_dbg(&priv->wdev->dev, "Attempting to set %#08x for property %x to %x\n",
>> + args.arg0, prop, args.arg1);
>> +
>> + return lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
>> + (unsigned char *)&args, sizeof(args), NULL);
>> +}
>> +
>> +/**
>> + * lwmi_psy_prop_is_writeable() - Determine if the property is supported
>> + * @ps: The battery that was extended
>> + * @ext: The extension
>> + * @ext_data: Pointer the lwmi_om_priv drvdata
>> + * @prop: The property to check
>> + *
>> + * Checks capdata 00 to determine if the property is supported.
>> + *
>> + * Return: Support level, or false
>> + */
>> +static int lwmi_psy_prop_is_writeable(struct power_supply *ps,
>> + const struct power_supply_ext *ext,
>> + void *ext_data,
>> + enum power_supply_property prop)
>> +{
>> + struct lwmi_om_priv *priv = ext_data;
>> + struct capdata00 capdata;
>> + u32 attribute_id = LWMI_ATTR_ID_PSU(LWMI_FEATURE_ID_PSU_INSTANT_MODE, LWMI_TYPE_ID_PSU_AC);
>> + int ret;
>> +
>> + ret = lwmi_cd00_get_data(priv->cd00_list, attribute_id, &capdata);
>> + if (ret)
>> + return false;
>> +
>> + dev_dbg(&priv->wdev->dev, "Battery charge mode (%#08x) support level: %x\n",
>> + attribute_id, capdata.supported);
>> +
>> + return capdata.supported;
>> +}
>> +
>> +static const enum power_supply_property lwmi_psy_ext_props[] = {
>> + POWER_SUPPLY_PROP_CHARGE_TYPES,
>> +};
>> +
>> +static const struct power_supply_ext lwmi_psy_ext = {
>> + .name = LWMI_OM_SYSFS_NAME,
>> + .properties = lwmi_psy_ext_props,
>> + .num_properties = ARRAY_SIZE(lwmi_psy_ext_props),
>> + .charge_types = (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
>> + BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
>> + .get_property = lwmi_psy_ext_get_prop,
>> + .set_property = lwmi_psy_ext_set_prop,
>> + .property_is_writeable = lwmi_psy_prop_is_writeable,
>> +};
>> +
>> +/**
>> + * lwmi_add_battery() - Connect the power_supply_ext
>> + * @battery: The battery to extend
>> + * @hook: The driver hook used to extend the battery
>> + *
>> + * Return: 0 on success, or an error.
>> + */
>> +static int lwmi_add_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
>> +{
>> + struct lwmi_om_priv *priv = container_of(hook, struct lwmi_om_priv, battery_hook);
>> +
>> + return power_supply_register_extension(battery, &lwmi_psy_ext, &priv->wdev->dev, priv);
>> +}
>> +
>> +/**
>> + * lwmi_remove_battery() - Disconnect the power_supply_ext
>> + * @battery: The battery that was extended
>> + * @hook: The driver hook used to extend the battery
>> + *
>> + * Return: 0 on success, or an error.
>> + */
>> +static int lwmi_remove_battery(struct power_supply *battery, struct acpi_battery_hook *hook)
>> +{
>> + power_supply_unregister_extension(battery, &lwmi_psy_ext);
>> + return 0;
>> +}
>> +
>> +/**
>> + * lwmi_acpi_match() - Attempts to return the ideapad acpi handle
>> + * @handle: The ACPI handle that manages battery charging
>> + * @lvl: Unused
>> + * @context: Void pointer to the acpi_handle object to return
>> + * @retval: Unused
>> + *
>> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
>> + * then if not, hooks the battery to our WMI methods.
>> + *
>> + * Return: AE_CTRL_TERMINATE if found, AE_OK if not found.
>> + */
>> +static acpi_status lwmi_acpi_match(acpi_handle handle, u32 lvl,
>> + void *context, void **retval)
>> +{
>> + if (!handle)
>> + return AE_OK;
>> +
>> + acpi_handle *ahand = context;
>> + *ahand = handle;
>> +
>> + return AE_CTRL_TERMINATE;
>> +}
>> +
>> +/**
>> + * lwmi_om_ps_ext_init() - Hooks power supply extension to device battery
>> + * @priv: Driver private data
>> + *
>> + * Checks if the ideapad_laptop driver is going to manage charge_type first,
>> + * then if not, hooks the battery to our WMI methods.
>> + */
>> +static void lwmi_om_ps_ext_init(struct lwmi_om_priv *priv)
>> +{
>> + static const char * const ideapad_hid = "VPC2004";
>> + acpi_handle handle = NULL;
>> + int ret;
>> +
>> + /* Deconflict ideapad_laptop driver */
>> + ret = acpi_get_devices(ideapad_hid, lwmi_acpi_match, &handle, NULL);
>> + if (ret)
>> + return;
>> +
>> + if (!handle)
>> + return;
>
>I am a bit confused here. If VPC2004 is not found, handle should remain
>NULL. In this case we have no conflict, right?
>
>Thanks,
>Rong
>
>> +
>> + if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
>> + dev_dbg(&priv->wdev->dev, "ideapad_laptop driver manages battery for device.\n");
>> + return;
>> + }
>> +
>> + /* Add battery hooks */
>> + priv->battery_hook.add_battery = lwmi_add_battery,
>> + priv->battery_hook.remove_battery = lwmi_remove_battery,
>> + priv->battery_hook.name = "Lenovo WMI Other Battery Extension",
>> +
>> + ret = devm_battery_hook_register(&priv->wdev->dev, &priv->battery_hook);
>> + if (ret)
>> + dev_err(&priv->wdev->dev, "Error during battery hook: %i\n", ret);
>> +}
>> +
>> /* ======== fw_attributes (component: lenovo-wmi-capdata 01) ======== */
>>
>> struct tunable_attr_01 {
>> @@ -1325,6 +1554,7 @@ static int lwmi_om_master_bind(struct device *dev)
>> return -ENODEV;
>>
>> lwmi_om_fan_info_collect_cd00(priv);
>> + lwmi_om_ps_ext_init(priv);
>>
>> return lwmi_om_fw_attr_add(priv);
>> }