All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
@ 2025-10-20 19:24 Rong Zhang
  2025-10-20 19:24 ` [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex Rong Zhang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Rong Zhang @ 2025-10-20 19:24 UTC (permalink / raw)
  To: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	Ilpo Järvinen
  Cc: Rong Zhang, platform-driver-x86, linux-kernel

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.

Rong Zhang (2):
  platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex
  platform/x86: ideapad-laptop: Add charge_types [Fast] (Rapid Charge)

 drivers/platform/x86/lenovo/ideapad-laptop.c | 144 ++++++++++++++-----
 1 file changed, 108 insertions(+), 36 deletions(-)


base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
-- 
2.51.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex
  2025-10-20 19:24 [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
@ 2025-10-20 19:24 ` Rong Zhang
  2025-10-30 16:40   ` Ilpo Järvinen
  2025-10-20 19:24 ` [PATCH 2/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
  2025-11-02 16:09 ` [PATCH 0/2] " Jelle van der Waa
  2 siblings, 1 reply; 10+ messages in thread
From: Rong Zhang @ 2025-10-20 19:24 UTC (permalink / raw)
  To: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	Ilpo Järvinen
  Cc: Rong Zhang, platform-driver-x86, linux-kernel

The upcoming changes for Rapid Charge support require two consecutive
SBMC calls to switch charge_types. Hence, a mutex is required.

The reason for not using rw_semaphore for this purpose is that allowing
simultaneous GBMD calls is not really useful and doesn't deserve the
overhead.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/ideapad-laptop.c | 91 ++++++++++++--------
 1 file changed, 56 insertions(+), 35 deletions(-)

diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
index fcebfbaf04605..9f956f51ec8db 100644
--- a/drivers/platform/x86/lenovo/ideapad-laptop.c
+++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
@@ -158,6 +158,7 @@ struct ideapad_rfk_priv {
 struct ideapad_private {
 	struct acpi_device *adev;
 	struct mutex vpc_mutex; /* protects the VPC calls */
+	struct mutex gbmd_sbmc_mutex; /* protects GBMD/SBMC calls */
 	struct rfkill *rfk[IDEAPAD_RFKILL_DEV_NUM];
 	struct ideapad_rfk_priv rfk_priv[IDEAPAD_RFKILL_DEV_NUM];
 	struct platform_device *platform_device;
@@ -455,37 +456,40 @@ static int debugfs_status_show(struct seq_file *s, void *data)
 	struct ideapad_private *priv = s->private;
 	unsigned long value;
 
-	guard(mutex)(&priv->vpc_mutex);
-
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
-		seq_printf(s, "Backlight max:  %lu\n", value);
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
-		seq_printf(s, "Backlight now:  %lu\n", value);
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
-		seq_printf(s, "BL power value: %s (%lu)\n", value ? "on" : "off", value);
-
-	seq_puts(s, "=====================\n");
-
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
-		seq_printf(s, "Radio status: %s (%lu)\n", value ? "on" : "off", value);
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
-		seq_printf(s, "Wifi status:  %s (%lu)\n", value ? "on" : "off", value);
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
-		seq_printf(s, "BT status:    %s (%lu)\n", value ? "on" : "off", value);
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
-		seq_printf(s, "3G status:    %s (%lu)\n", value ? "on" : "off", value);
+	scoped_guard(mutex, &priv->vpc_mutex) {
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
+			seq_printf(s, "Backlight max:  %lu\n", value);
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
+			seq_printf(s, "Backlight now:  %lu\n", value);
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
+			seq_printf(s, "BL power value: %s (%lu)\n", value ? "on" : "off", value);
+
+		seq_puts(s, "=====================\n");
+
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
+			seq_printf(s, "Radio status: %s (%lu)\n", value ? "on" : "off", value);
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
+			seq_printf(s, "Wifi status:  %s (%lu)\n", value ? "on" : "off", value);
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
+			seq_printf(s, "BT status:    %s (%lu)\n", value ? "on" : "off", value);
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
+			seq_printf(s, "3G status:    %s (%lu)\n", value ? "on" : "off", value);
+
+		seq_puts(s, "=====================\n");
+
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
+			seq_printf(s, "Touchpad status: %s (%lu)\n", value ? "on" : "off", value);
+		if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
+			seq_printf(s, "Camera status:   %s (%lu)\n", value ? "on" : "off", value);
+	}
 
 	seq_puts(s, "=====================\n");
 
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
-		seq_printf(s, "Touchpad status: %s (%lu)\n", value ? "on" : "off", value);
-	if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
-		seq_printf(s, "Camera status:   %s (%lu)\n", value ? "on" : "off", value);
-
-	seq_puts(s, "=====================\n");
+	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
+		if (!eval_gbmd(priv->adev->handle, &value))
+			seq_printf(s, "GBMD: %#010lx\n", value);
+	}
 
-	if (!eval_gbmd(priv->adev->handle, &value))
-		seq_printf(s, "GBMD: %#010lx\n", value);
 	if (!eval_hals(priv->adev->handle, &value))
 		seq_printf(s, "HALS: %#010lx\n", value);
 
@@ -622,9 +626,11 @@ static ssize_t conservation_mode_show(struct device *dev,
 
 	show_conservation_mode_deprecation_warning(dev);
 
-	err = eval_gbmd(priv->adev->handle, &result);
-	if (err)
-		return err;
+	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
+		err = eval_gbmd(priv->adev->handle, &result);
+		if (err)
+			return err;
+	}
 
 	return sysfs_emit(buf, "%d\n", !!test_bit(GBMD_CONSERVATION_STATE_BIT, &result));
 }
@@ -643,6 +649,8 @@ static ssize_t conservation_mode_store(struct device *dev,
 	if (err)
 		return err;
 
+	guard(mutex)(&priv->gbmd_sbmc_mutex);
+
 	err = exec_sbmc(priv->adev->handle, state ? SBMC_CONSERVATION_ON : SBMC_CONSERVATION_OFF);
 	if (err)
 		return err;
@@ -2007,15 +2015,22 @@ static int ideapad_psy_ext_set_prop(struct power_supply *psy,
 				    const union power_supply_propval *val)
 {
 	struct ideapad_private *priv = ext_data;
+	unsigned long op;
 
 	switch (val->intval) {
 	case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE:
-		return exec_sbmc(priv->adev->handle, SBMC_CONSERVATION_ON);
+		op = SBMC_CONSERVATION_ON;
+		break;
 	case POWER_SUPPLY_CHARGE_TYPE_STANDARD:
-		return exec_sbmc(priv->adev->handle, SBMC_CONSERVATION_OFF);
+		op = SBMC_CONSERVATION_OFF;
+		break;
 	default:
 		return -EINVAL;
 	}
+
+	guard(mutex)(&priv->gbmd_sbmc_mutex);
+
+	return exec_sbmc(priv->adev->handle, op);
 }
 
 static int ideapad_psy_ext_get_prop(struct power_supply *psy,
@@ -2028,9 +2043,11 @@ static int ideapad_psy_ext_get_prop(struct power_supply *psy,
 	unsigned long result;
 	int err;
 
-	err = eval_gbmd(priv->adev->handle, &result);
-	if (err)
-		return err;
+	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
+		err = eval_gbmd(priv->adev->handle, &result);
+		if (err)
+			return err;
+	}
 
 	if (test_bit(GBMD_CONSERVATION_STATE_BIT, &result))
 		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
@@ -2292,6 +2309,10 @@ static int ideapad_acpi_add(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	err = devm_mutex_init(&pdev->dev, &priv->gbmd_sbmc_mutex);
+	if (err)
+		return err;
+
 	err = ideapad_check_features(priv);
 	if (err)
 		return err;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  2025-10-20 19:24 [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
  2025-10-20 19:24 ` [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex Rong Zhang
@ 2025-10-20 19:24 ` Rong Zhang
  2025-10-22 18:43   ` Mark Pearson
  2025-11-02 16:09 ` [PATCH 0/2] " Jelle van der Waa
  2 siblings, 1 reply; 10+ messages in thread
From: Rong Zhang @ 2025-10-20 19:24 UTC (permalink / raw)
  To: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	Ilpo Järvinen
  Cc: Rong Zhang, platform-driver-x86, linux-kernel

The GBMD/SBMC interface supports Rapid Charge mode (charge_types: Fast)
in addition to Conservation Mode (charge_types: Long_Life).

Expose these two modes while carefully maintaining their mutually
exclusive state, which aligns with the behavior of manufacturer
utilities on Windows.

Signed-off-by: Rong Zhang <i@rong.moe>
---
 drivers/platform/x86/lenovo/ideapad-laptop.c | 61 ++++++++++++++++++--
 1 file changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
index 9f956f51ec8db..d2bfaa532020a 100644
--- a/drivers/platform/x86/lenovo/ideapad-laptop.c
+++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
@@ -62,13 +62,26 @@ enum {
 	CFG_OSD_CAM_BIT      = 31,
 };
 
+/*
+ * There are two charge modes supported by the GBMD/SBMC interface:
+ * - "Rapid Charge": increase power to speed up charging
+ * - "Conservation Mode": stop charging at 60-80% (depends on model)
+ *
+ * The interface doesn't prohibit enabling both modes at the same time.
+ * However, doing so is essentially meaningless, and the manufacturer utilities
+ * on Windows always make them mutually exclusive.
+ */
+
 enum {
+	GBMD_RAPID_CHARGE_STATE_BIT = 2,
 	GBMD_CONSERVATION_STATE_BIT = 5,
 };
 
 enum {
 	SBMC_CONSERVATION_ON  = 3,
 	SBMC_CONSERVATION_OFF = 5,
+	SBMC_RAPID_CHARGE_ON  = 7,
+	SBMC_RAPID_CHARGE_OFF = 8,
 };
 
 enum {
@@ -632,6 +645,10 @@ static ssize_t conservation_mode_show(struct device *dev,
 			return err;
 	}
 
+	/*
+	 * For backward compatibility, ignore Rapid Charge while reporting the
+	 * state of Conservation Mode.
+	 */
 	return sysfs_emit(buf, "%d\n", !!test_bit(GBMD_CONSERVATION_STATE_BIT, &result));
 }
 
@@ -651,6 +668,16 @@ static ssize_t conservation_mode_store(struct device *dev,
 
 	guard(mutex)(&priv->gbmd_sbmc_mutex);
 
+	/*
+	 * Prevent mutually exclusive modes from being set at the same time,
+	 * but do not disable Rapid Charge while disabling Conservation Mode.
+	 */
+	if (state) {
+		err = exec_sbmc(priv->adev->handle, SBMC_RAPID_CHARGE_OFF);
+		if (err)
+			return err;
+	}
+
 	err = exec_sbmc(priv->adev->handle, state ? SBMC_CONSERVATION_ON : SBMC_CONSERVATION_OFF);
 	if (err)
 		return err;
@@ -2015,14 +2042,21 @@ static int ideapad_psy_ext_set_prop(struct power_supply *psy,
 				    const union power_supply_propval *val)
 {
 	struct ideapad_private *priv = ext_data;
-	unsigned long op;
+	unsigned long op1, op2;
+	int err;
 
 	switch (val->intval) {
+	case POWER_SUPPLY_CHARGE_TYPE_FAST:
+		op1 = SBMC_CONSERVATION_OFF;
+		op2 = SBMC_RAPID_CHARGE_ON;
+		break;
 	case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE:
-		op = SBMC_CONSERVATION_ON;
+		op1 = SBMC_RAPID_CHARGE_OFF;
+		op2 = SBMC_CONSERVATION_ON;
 		break;
 	case POWER_SUPPLY_CHARGE_TYPE_STANDARD:
-		op = SBMC_CONSERVATION_OFF;
+		op1 = SBMC_RAPID_CHARGE_OFF;
+		op2 = SBMC_CONSERVATION_OFF;
 		break;
 	default:
 		return -EINVAL;
@@ -2030,7 +2064,11 @@ static int ideapad_psy_ext_set_prop(struct power_supply *psy,
 
 	guard(mutex)(&priv->gbmd_sbmc_mutex);
 
-	return exec_sbmc(priv->adev->handle, op);
+	err = exec_sbmc(priv->adev->handle, op1);
+	if (err)
+		return err;
+
+	return exec_sbmc(priv->adev->handle, op2);
 }
 
 static int ideapad_psy_ext_get_prop(struct power_supply *psy,
@@ -2042,6 +2080,7 @@ static int ideapad_psy_ext_get_prop(struct power_supply *psy,
 	struct ideapad_private *priv = ext_data;
 	unsigned long result;
 	int err;
+	bool is_rapid_charge, is_conservation;
 
 	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
 		err = eval_gbmd(priv->adev->handle, &result);
@@ -2049,7 +2088,18 @@ static int ideapad_psy_ext_get_prop(struct power_supply *psy,
 			return err;
 	}
 
-	if (test_bit(GBMD_CONSERVATION_STATE_BIT, &result))
+	is_rapid_charge = test_bit(GBMD_RAPID_CHARGE_STATE_BIT, &result);
+	is_conservation = test_bit(GBMD_CONSERVATION_STATE_BIT, &result);
+
+	if (unlikely(is_rapid_charge && is_conservation)) {
+		dev_err(&priv->platform_device->dev,
+			"unexpected charge_types: both [Fast] and [Long_Life] are enabled\n");
+		return -EINVAL;
+	}
+
+	if (is_rapid_charge)
+		val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
+	else if (is_conservation)
 		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
 	else
 		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
@@ -2074,6 +2124,7 @@ static const struct power_supply_ext ideapad_battery_ext = {
 	.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,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  2025-10-20 19:24 ` [PATCH 2/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
@ 2025-10-22 18:43   ` Mark Pearson
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Pearson @ 2025-10-22 18:43 UTC (permalink / raw)
  To: Rong Zhang, Ike Panhc, Derek J . Clark, Hans de Goede,
	Ilpo Järvinen
  Cc: platform-driver-x86@vger.kernel.org, linux-kernel

Hi Rong

On Mon, Oct 20, 2025, at 9:24 PM, Rong Zhang wrote:
> The GBMD/SBMC interface supports Rapid Charge mode (charge_types: Fast)
> in addition to Conservation Mode (charge_types: Long_Life).
>
> Expose these two modes while carefully maintaining their mutually
> exclusive state, which aligns with the behavior of manufacturer
> utilities on Windows.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/platform/x86/lenovo/ideapad-laptop.c | 61 ++++++++++++++++++--
>  1 file changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c 
> b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 9f956f51ec8db..d2bfaa532020a 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -62,13 +62,26 @@ enum {
>  	CFG_OSD_CAM_BIT      = 31,
>  };
> 
> +/*
> + * There are two charge modes supported by the GBMD/SBMC interface:
> + * - "Rapid Charge": increase power to speed up charging
> + * - "Conservation Mode": stop charging at 60-80% (depends on model)
> + *
> + * The interface doesn't prohibit enabling both modes at the same time.
> + * However, doing so is essentially meaningless, and the manufacturer utilities
> + * on Windows always make them mutually exclusive.
> + */
> +
>  enum {
> +	GBMD_RAPID_CHARGE_STATE_BIT = 2,
>  	GBMD_CONSERVATION_STATE_BIT = 5,
>  };
> 
>  enum {
>  	SBMC_CONSERVATION_ON  = 3,
>  	SBMC_CONSERVATION_OFF = 5,
> +	SBMC_RAPID_CHARGE_ON  = 7,
> +	SBMC_RAPID_CHARGE_OFF = 8,
>  };
> 
>  enum {
> @@ -632,6 +645,10 @@ static ssize_t conservation_mode_show(struct device *dev,
>  			return err;
>  	}
> 
> +	/*
> +	 * For backward compatibility, ignore Rapid Charge while reporting the
> +	 * state of Conservation Mode.
> +	 */
>  	return sysfs_emit(buf, "%d\n", 
> !!test_bit(GBMD_CONSERVATION_STATE_BIT, &result));
>  }
> 
> @@ -651,6 +668,16 @@ static ssize_t conservation_mode_store(struct device *dev,
> 
>  	guard(mutex)(&priv->gbmd_sbmc_mutex);
> 
> +	/*
> +	 * Prevent mutually exclusive modes from being set at the same time,
> +	 * but do not disable Rapid Charge while disabling Conservation Mode.
> +	 */
> +	if (state) {
> +		err = exec_sbmc(priv->adev->handle, SBMC_RAPID_CHARGE_OFF);
> +		if (err)
> +			return err;
> +	}
> +
>  	err = exec_sbmc(priv->adev->handle, state ? SBMC_CONSERVATION_ON : 
> SBMC_CONSERVATION_OFF);
>  	if (err)
>  		return err;
> @@ -2015,14 +2042,21 @@ static int ideapad_psy_ext_set_prop(struct 
> power_supply *psy,
>  				    const union power_supply_propval *val)
>  {
>  	struct ideapad_private *priv = ext_data;
> -	unsigned long op;
> +	unsigned long op1, op2;
> +	int err;
> 
>  	switch (val->intval) {
> +	case POWER_SUPPLY_CHARGE_TYPE_FAST:
> +		op1 = SBMC_CONSERVATION_OFF;
> +		op2 = SBMC_RAPID_CHARGE_ON;
> +		break;
>  	case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE:
> -		op = SBMC_CONSERVATION_ON;
> +		op1 = SBMC_RAPID_CHARGE_OFF;
> +		op2 = SBMC_CONSERVATION_ON;
>  		break;
>  	case POWER_SUPPLY_CHARGE_TYPE_STANDARD:
> -		op = SBMC_CONSERVATION_OFF;
> +		op1 = SBMC_RAPID_CHARGE_OFF;
> +		op2 = SBMC_CONSERVATION_OFF;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -2030,7 +2064,11 @@ static int ideapad_psy_ext_set_prop(struct 
> power_supply *psy,
> 
>  	guard(mutex)(&priv->gbmd_sbmc_mutex);
> 
> -	return exec_sbmc(priv->adev->handle, op);
> +	err = exec_sbmc(priv->adev->handle, op1);
> +	if (err)
> +		return err;
> +
> +	return exec_sbmc(priv->adev->handle, op2);
>  }
> 
>  static int ideapad_psy_ext_get_prop(struct power_supply *psy,
> @@ -2042,6 +2080,7 @@ static int ideapad_psy_ext_get_prop(struct 
> power_supply *psy,
>  	struct ideapad_private *priv = ext_data;
>  	unsigned long result;
>  	int err;
> +	bool is_rapid_charge, is_conservation;
> 
>  	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
>  		err = eval_gbmd(priv->adev->handle, &result);
> @@ -2049,7 +2088,18 @@ static int ideapad_psy_ext_get_prop(struct 
> power_supply *psy,
>  			return err;
>  	}
> 
> -	if (test_bit(GBMD_CONSERVATION_STATE_BIT, &result))
> +	is_rapid_charge = test_bit(GBMD_RAPID_CHARGE_STATE_BIT, &result);
> +	is_conservation = test_bit(GBMD_CONSERVATION_STATE_BIT, &result);
> +
> +	if (unlikely(is_rapid_charge && is_conservation)) {
> +		dev_err(&priv->platform_device->dev,
> +			"unexpected charge_types: both [Fast] and [Long_Life] are 
> enabled\n");
> +		return -EINVAL;
> +	}
> +
> +	if (is_rapid_charge)
> +		val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST;
> +	else if (is_conservation)
>  		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
>  	else
>  		val->intval = POWER_SUPPLY_CHARGE_TYPE_STANDARD;
> @@ -2074,6 +2124,7 @@ static const struct power_supply_ext 
> ideapad_battery_ext = {
>  	.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,
> -- 
> 2.51.0

I can't comment on the functionality as I don't have hooks into the Ideapad team to confirm - but the implementation looks good to me.

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex
  2025-10-20 19:24 ` [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex Rong Zhang
@ 2025-10-30 16:40   ` Ilpo Järvinen
  0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2025-10-30 16:40 UTC (permalink / raw)
  To: Rong Zhang
  Cc: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	platform-driver-x86, LKML

On Tue, 21 Oct 2025, Rong Zhang wrote:

> The upcoming changes for Rapid Charge support require two consecutive
> SBMC calls to switch charge_types. Hence, a mutex is required.
> 
> The reason for not using rw_semaphore for this purpose is that allowing
> simultaneous GBMD calls is not really useful and doesn't deserve the
> overhead.
> 
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
>  drivers/platform/x86/lenovo/ideapad-laptop.c | 91 ++++++++++++--------
>  1 file changed, 56 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index fcebfbaf04605..9f956f51ec8db 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -158,6 +158,7 @@ struct ideapad_rfk_priv {
>  struct ideapad_private {
>  	struct acpi_device *adev;
>  	struct mutex vpc_mutex; /* protects the VPC calls */
> +	struct mutex gbmd_sbmc_mutex; /* protects GBMD/SBMC calls */
>  	struct rfkill *rfk[IDEAPAD_RFKILL_DEV_NUM];
>  	struct ideapad_rfk_priv rfk_priv[IDEAPAD_RFKILL_DEV_NUM];
>  	struct platform_device *platform_device;
> @@ -455,37 +456,40 @@ static int debugfs_status_show(struct seq_file *s, void *data)
>  	struct ideapad_private *priv = s->private;
>  	unsigned long value;
>  
> -	guard(mutex)(&priv->vpc_mutex);
> -
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
> -		seq_printf(s, "Backlight max:  %lu\n", value);
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
> -		seq_printf(s, "Backlight now:  %lu\n", value);
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
> -		seq_printf(s, "BL power value: %s (%lu)\n", value ? "on" : "off", value);
> -
> -	seq_puts(s, "=====================\n");
> -
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
> -		seq_printf(s, "Radio status: %s (%lu)\n", value ? "on" : "off", value);
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
> -		seq_printf(s, "Wifi status:  %s (%lu)\n", value ? "on" : "off", value);
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
> -		seq_printf(s, "BT status:    %s (%lu)\n", value ? "on" : "off", value);
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
> -		seq_printf(s, "3G status:    %s (%lu)\n", value ? "on" : "off", value);
> +	scoped_guard(mutex, &priv->vpc_mutex) {
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_MAX, &value))
> +			seq_printf(s, "Backlight max:  %lu\n", value);
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL, &value))
> +			seq_printf(s, "Backlight now:  %lu\n", value);
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BL_POWER, &value))
> +			seq_printf(s, "BL power value: %s (%lu)\n", value ? "on" : "off", value);

Thanks for the patches. I've taken them into the review-ilpo-next branch.

Unrelated to this series itself, these ? "on" : "off" constructs could be 
changed to use str_on_off().

--
 i.

> +
> +		seq_puts(s, "=====================\n");
> +
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_RF, &value))
> +			seq_printf(s, "Radio status: %s (%lu)\n", value ? "on" : "off", value);
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_WIFI, &value))
> +			seq_printf(s, "Wifi status:  %s (%lu)\n", value ? "on" : "off", value);
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_BT, &value))
> +			seq_printf(s, "BT status:    %s (%lu)\n", value ? "on" : "off", value);
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_3G, &value))
> +			seq_printf(s, "3G status:    %s (%lu)\n", value ? "on" : "off", value);
> +
> +		seq_puts(s, "=====================\n");
> +
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
> +			seq_printf(s, "Touchpad status: %s (%lu)\n", value ? "on" : "off", value);
> +		if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
> +			seq_printf(s, "Camera status:   %s (%lu)\n", value ? "on" : "off", value);
> +	}
>  
>  	seq_puts(s, "=====================\n");
>  
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value))
> -		seq_printf(s, "Touchpad status: %s (%lu)\n", value ? "on" : "off", value);
> -	if (!read_ec_data(priv->adev->handle, VPCCMD_R_CAMERA, &value))
> -		seq_printf(s, "Camera status:   %s (%lu)\n", value ? "on" : "off", value);
> -
> -	seq_puts(s, "=====================\n");
> +	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
> +		if (!eval_gbmd(priv->adev->handle, &value))
> +			seq_printf(s, "GBMD: %#010lx\n", value);
> +	}
>  
> -	if (!eval_gbmd(priv->adev->handle, &value))
> -		seq_printf(s, "GBMD: %#010lx\n", value);
>  	if (!eval_hals(priv->adev->handle, &value))
>  		seq_printf(s, "HALS: %#010lx\n", value);
>  
> @@ -622,9 +626,11 @@ static ssize_t conservation_mode_show(struct device *dev,
>  
>  	show_conservation_mode_deprecation_warning(dev);
>  
> -	err = eval_gbmd(priv->adev->handle, &result);
> -	if (err)
> -		return err;
> +	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
> +		err = eval_gbmd(priv->adev->handle, &result);
> +		if (err)
> +			return err;
> +	}
>  
>  	return sysfs_emit(buf, "%d\n", !!test_bit(GBMD_CONSERVATION_STATE_BIT, &result));
>  }
> @@ -643,6 +649,8 @@ static ssize_t conservation_mode_store(struct device *dev,
>  	if (err)
>  		return err;
>  
> +	guard(mutex)(&priv->gbmd_sbmc_mutex);
> +
>  	err = exec_sbmc(priv->adev->handle, state ? SBMC_CONSERVATION_ON : SBMC_CONSERVATION_OFF);
>  	if (err)
>  		return err;
> @@ -2007,15 +2015,22 @@ static int ideapad_psy_ext_set_prop(struct power_supply *psy,
>  				    const union power_supply_propval *val)
>  {
>  	struct ideapad_private *priv = ext_data;
> +	unsigned long op;
>  
>  	switch (val->intval) {
>  	case POWER_SUPPLY_CHARGE_TYPE_LONGLIFE:
> -		return exec_sbmc(priv->adev->handle, SBMC_CONSERVATION_ON);
> +		op = SBMC_CONSERVATION_ON;
> +		break;
>  	case POWER_SUPPLY_CHARGE_TYPE_STANDARD:
> -		return exec_sbmc(priv->adev->handle, SBMC_CONSERVATION_OFF);
> +		op = SBMC_CONSERVATION_OFF;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> +
> +	guard(mutex)(&priv->gbmd_sbmc_mutex);
> +
> +	return exec_sbmc(priv->adev->handle, op);
>  }
>  
>  static int ideapad_psy_ext_get_prop(struct power_supply *psy,
> @@ -2028,9 +2043,11 @@ static int ideapad_psy_ext_get_prop(struct power_supply *psy,
>  	unsigned long result;
>  	int err;
>  
> -	err = eval_gbmd(priv->adev->handle, &result);
> -	if (err)
> -		return err;
> +	scoped_guard(mutex, &priv->gbmd_sbmc_mutex) {
> +		err = eval_gbmd(priv->adev->handle, &result);
> +		if (err)
> +			return err;
> +	}
>  
>  	if (test_bit(GBMD_CONSERVATION_STATE_BIT, &result))
>  		val->intval = POWER_SUPPLY_CHARGE_TYPE_LONGLIFE;
> @@ -2292,6 +2309,10 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> +	err = devm_mutex_init(&pdev->dev, &priv->gbmd_sbmc_mutex);
> +	if (err)
> +		return err;
> +
>  	err = ideapad_check_features(priv);
>  	if (err)
>  		return err;
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  2025-10-20 19:24 [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
  2025-10-20 19:24 ` [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex Rong Zhang
  2025-10-20 19:24 ` [PATCH 2/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
@ 2025-11-02 16:09 ` Jelle van der Waa
  2025-11-02 18:57   ` Rong Zhang
  2 siblings, 1 reply; 10+ messages in thread
From: Jelle van der Waa @ 2025-11-02 16:09 UTC (permalink / raw)
  To: Rong Zhang, Ike Panhc, Mark Pearson, Derek J. Clark,
	Hans de Goede, Ilpo Järvinen
  Cc: platform-driver-x86, linux-kernel

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

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.

Greetings,

Jelle van der Waa

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  2025-11-02 16:09 ` [PATCH 0/2] " Jelle van der Waa
@ 2025-11-02 18:57   ` Rong Zhang
  2025-11-02 19:24     ` Rong Zhang
  2025-11-05  8:59     ` Ilpo Järvinen
  0 siblings, 2 replies; 10+ messages in thread
From: Rong Zhang @ 2025-11-02 18:57 UTC (permalink / raw)
  To: Jelle van der Waa, Ilpo Järvinen
  Cc: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	platform-driver-x86, linux-kernel

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)?

> 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"))

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  2025-11-02 18:57   ` Rong Zhang
@ 2025-11-02 19:24     ` Rong Zhang
  2025-11-03 21:31       ` Jelle van der Waa
  2025-11-05  8:59     ` Ilpo Järvinen
  1 sibling, 1 reply; 10+ messages in thread
From: Rong Zhang @ 2025-11-02 19:24 UTC (permalink / raw)
  To: Jelle van der Waa, Ilpo Järvinen
  Cc: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	platform-driver-x86, linux-kernel

On Mon, 2025-11-03 at 02:57 +0800, 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")?

Note: this patch is just a quick PoC (I am going to sleep now, zzz...).
ideapad_psy_ext_{get,set}_prop need to be reorganized to properly
support your device. If `cat charge_types' doesn't show `Fast', we're
in the right direction.

Thanks,
Rong

> @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)?
> 
> > 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"))

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  2025-11-02 19:24     ` Rong Zhang
@ 2025-11-03 21:31       ` Jelle van der Waa
  0 siblings, 0 replies; 10+ messages in thread
From: Jelle van der Waa @ 2025-11-03 21:31 UTC (permalink / raw)
  To: Rong Zhang, Ilpo Järvinen
  Cc: Ike Panhc, Mark Pearson, Derek J. Clark, Hans de Goede,
	platform-driver-x86, linux-kernel

On 11/2/25 20:24, Rong Zhang wrote:
> On Mon, 2025-11-03 at 02:57 +0800, 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")?
> 
> Note: this patch is just a quick PoC (I am going to sleep now, zzz...).
> ideapad_psy_ext_{get,set}_prop need to be reorganized to properly
> support your device. If `cat charge_types' doesn't show `Fast', we're
> in the right direction.

Thanks for the quick patch!

Tested it on my Ideapad U330p:

[root@archlinux ~]# cat /sys/class/power_supply/BAT1/charge_types
[Standard] Long_Life
[root@archlinux ~]# echo Long_Life > 
/sys/class/power_supply/BAT1/charge_types
[root@archlinux ~]# cat /sys/class/power_supply/BAT1/charge_types
Standard [Long_Life]

I'm however still waiting on the laptop to slowly charge to 80% to 
confirm that charge limits are applied.

Greetings,

Jelle van der Waa

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  2025-11-02 18:57   ` Rong Zhang
  2025-11-02 19:24     ` Rong Zhang
@ 2025-11-05  8:59     ` Ilpo Järvinen
  1 sibling, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2025-11-05  8:59 UTC (permalink / raw)
  To: Rong Zhang
  Cc: Jelle van der Waa, Ike Panhc, Mark Pearson, Derek J. Clark,
	Hans de Goede, platform-driver-x86, LKML

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"))
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-11-05  8:59 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-20 19:24 [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
2025-10-20 19:24 ` [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex Rong Zhang
2025-10-30 16:40   ` Ilpo Järvinen
2025-10-20 19:24 ` [PATCH 2/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge) Rong Zhang
2025-10-22 18:43   ` Mark Pearson
2025-11-02 16:09 ` [PATCH 0/2] " Jelle van der Waa
2025-11-02 18:57   ` Rong Zhang
2025-11-02 19:24     ` Rong Zhang
2025-11-03 21:31       ` Jelle van der Waa
2025-11-05  8:59     ` Ilpo Järvinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.