1. Patchew
  2. linux
  3. [PATCH 0/2] platform/x86: ideapad-laptop: Add charge_types:Fast (Rapid Charge)
  4. View patch

[PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex

Rong Zhang posted 2 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex
Posted by Rong Zhang 3 months, 3 weeks ago 
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
Re: [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex
Posted by Ilpo Järvinen 3 months, 1 week ago 
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;
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.