Re: [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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@archlinux jelle]# cat /sys/class/power_supply/BAT1/charge_types
> > Fast [Standard] Long_Life
> > [root@archlinux jelle]# echo 'Fast' > 
> > /sys/class/power_supply/BAT1/charge_types
> > [root@archlinux 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"))
>