[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f2885370-f466-6bf2-70d1-e7f3177111db@linux.intel.com>
Date: Wed, 5 Nov 2025 10:59:03 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Rong Zhang <i@...g.moe>
cc: Jelle van der Waa <jelle@...aa.nl>, Ike Panhc <ikepanhc@...il.com>,
Mark Pearson <mpearson-lenovo@...ebb.ca>,
"Derek J. Clark" <derekjohn.clark@...il.com>,
Hans de Goede <hansg@...nel.org>, platform-driver-x86@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast
(Rapid Charge)
On Mon, 3 Nov 2025, Rong Zhang wrote:
> Hi Jelle,
>
> On Sun, 2025-11-02 at 17:09 +0100, Jelle van der Waa wrote:
> > On 10/20/25 21:24, Rong Zhang wrote:
> > > The GBMD/SBMC interface on IdeaPad/ThinkBook supports Rapid Charge mode
> > > (charge_types: Fast) in addition to Conservation Mode (charge_types:
> > > Long_Life).
> > >
> > > This patchset exposes these two modes while carefully maintaining their
> > > mutually exclusive state, which aligns with the behavior of manufacturer
> > > utilities on Windows.
> > >
> > > Tested on ThinkBook 14 G7+ ASP.
> >
> > Tested this patch on my Lenovo Ideapad U330p, it now advertises that
> > `Fast` is a supported charge_type although my laptop does not seem to
> > support it:
> >
> > [root@...hlinux jelle]# cat /sys/class/power_supply/BAT1/charge_types
> > Fast [Standard] Long_Life
> > [root@...hlinux jelle]# echo 'Fast' >
> > /sys/class/power_supply/BAT1/charge_types
> > [root@...hlinux jelle]# cat /sys/class/power_supply/BAT1/charge_types
> > Fast [Standard] Long_Life
>
> Ahh, then we need an approach to determine if it is supported on a
> specific device.
>
> Glancing at the disassembled DSDT.dsl of my device, I found:
>
> Method (GBMD, 0, NotSerialized)
> {
> [...]
> If ((One == QCGS))
> {
> Local0 |= 0x00020000
> }
> [...]
> }
>
> BIT(17) of GBMD is 1 on my device. Maybe QCGS means "Quick CharGe
> Supported?"
>
> With this assumption, I did some random Internet digging. The same bit
> on other devices is called QKSP ("QuicK charge SuPported?"), SQCG
> ("Support Quick CharGe?"), or QCBX (see below).
>
> Method (GBMD, 0, NotSerialized)
> {
> [...]
> If ((One == QCBX))
> {
> If ((One == QCHO))
> {
> Local0 |= 0x04
> }
> }
> [...]
> If ((One == QCBX))
> {
> Local0 |= 0x00020000
> }
> [...]
> }
>
> https://badland.io/static/acpidump.txt
>
> 0x04 is BIT(2)/GBMD_RAPID_CHARGE_STATE_BIT. With all these pieces of
> information, I presume BIT(17) of GBMD is what we are searching for.
>
> > I'm wondering if the battery extension API allows to not advertise a
> > property if it isn't supported or if it should at least return -EINVAL.
>
> We can achieve this by defining multiple struct power_supply_ext. See
> drivers/power/supply/cros_charge-control.c.
>
> Could you test the patch below (based on "review-ilpo-next")?
>
> @Ilpo:
>
> This patch series has been merge into your "review-ilpo-next" branch.
>
> Should I reorganize the series and send a [PATCH v2]? Or should I just
> send the patch below (after adding a commit message, ofc)?
Hi Rong,
I've dropped those two v1 patches from the review-ilpo-next branch.
(They are still in for-next but will be gone from there as well on the
next review-ilpo-next -> for-next propagation.)
So please send the full series with this fixed as v2. You can include the
str_on_off() change also within the v2 series.
--
i.
> > Greetings,
> >
> > Jelle van der Waa
>
> Thanks,
> Rong
>
> ---
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 931a72a2a487..b9927493cb93 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -75,6 +75,7 @@ enum {
> enum {
> GBMD_RAPID_CHARGE_STATE_BIT = 2,
> GBMD_CONSERVATION_STATE_BIT = 5,
> + GBMD_RAPID_CHARGE_SUPPORTED_BIT = 17,
> };
>
> enum {
> @@ -180,6 +181,7 @@ struct ideapad_private {
> struct ideapad_dytc_priv *dytc;
> struct dentry *debug;
> struct acpi_battery_hook battery_hook;
> + const struct power_supply_ext *battery_ext;
> unsigned long cfg;
> unsigned long r_touchpad_val;
> struct {
> @@ -2119,30 +2121,42 @@ static const enum power_supply_property ideapad_power_supply_props[] = {
> POWER_SUPPLY_PROP_CHARGE_TYPES,
> };
>
> -static const struct power_supply_ext ideapad_battery_ext = {
> - .name = "ideapad_laptop",
> - .properties = ideapad_power_supply_props,
> - .num_properties = ARRAY_SIZE(ideapad_power_supply_props),
> - .charge_types = (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> - BIT(POWER_SUPPLY_CHARGE_TYPE_FAST) |
> - BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE)),
> - .get_property = ideapad_psy_ext_get_prop,
> - .set_property = ideapad_psy_ext_set_prop,
> - .property_is_writeable = ideapad_psy_prop_is_writeable,
> -};
> +#define DEFINE_IDEAPAD_POWER_SUPPLY_EXTENSION(_name, _charge_types) \
> + static const struct power_supply_ext _name = { \
> + .name = "ideapad_laptop", \
> + .properties = ideapad_power_supply_props, \
> + .num_properties = ARRAY_SIZE(ideapad_power_supply_props), \
> + .charge_types = _charge_types, \
> + .get_property = ideapad_psy_ext_get_prop, \
> + .set_property = ideapad_psy_ext_set_prop, \
> + .property_is_writeable = ideapad_psy_prop_is_writeable, \
> + }
> +
> +DEFINE_IDEAPAD_POWER_SUPPLY_EXTENSION(ideapad_battery_ext_v1,
> + (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> + BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE))
> +);
> +
> +DEFINE_IDEAPAD_POWER_SUPPLY_EXTENSION(ideapad_battery_ext_v2,
> + (BIT(POWER_SUPPLY_CHARGE_TYPE_STANDARD) |
> + BIT(POWER_SUPPLY_CHARGE_TYPE_FAST) |
> + BIT(POWER_SUPPLY_CHARGE_TYPE_LONGLIFE))
> +);
>
> static int ideapad_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook)
> {
> struct ideapad_private *priv = container_of(hook, struct ideapad_private, battery_hook);
>
> - return power_supply_register_extension(battery, &ideapad_battery_ext,
> + return power_supply_register_extension(battery, priv->battery_ext,
> &priv->platform_device->dev, priv);
> }
>
> static int ideapad_battery_remove(struct power_supply *battery,
> struct acpi_battery_hook *hook)
> {
> - power_supply_unregister_extension(battery, &ideapad_battery_ext);
> + struct ideapad_private *priv = container_of(hook, struct ideapad_private, battery_hook);
> +
> + power_supply_unregister_extension(battery, priv->battery_ext);
>
> return 0;
> }
> @@ -2167,14 +2181,22 @@ static int ideapad_check_features(struct ideapad_private *priv)
> priv->features.fan_mode = true;
>
> if (acpi_has_method(handle, "GBMD") && acpi_has_method(handle, "SBMC")) {
> - priv->features.conservation_mode = true;
> - priv->battery_hook.add_battery = ideapad_battery_add;
> - priv->battery_hook.remove_battery = ideapad_battery_remove;
> - priv->battery_hook.name = "Ideapad Battery Extension";
> -
> - err = devm_battery_hook_register(&priv->platform_device->dev, &priv->battery_hook);
> - if (err)
> - return err;
> + /* Not acquiring gbmd_sbmc_mutex as race condition is impossible on init */
> + if (!eval_gbmd(handle, &val)) {
> + priv->features.conservation_mode = true;
> + priv->battery_ext = test_bit(GBMD_RAPID_CHARGE_SUPPORTED_BIT, &val)
> + ? &ideapad_battery_ext_v2
> + : &ideapad_battery_ext_v1;
> +
> + priv->battery_hook.add_battery = ideapad_battery_add;
> + priv->battery_hook.remove_battery = ideapad_battery_remove;
> + priv->battery_hook.name = "Ideapad Battery Extension";
> +
> + err = devm_battery_hook_register(&priv->platform_device->dev,
> + &priv->battery_hook);
> + if (err)
> + return err;
> + }
> }
>
> if (acpi_has_method(handle, "DYTC"))
>
Powered by blists - more mailing lists