From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Rong Zhang <i@rong.moe>
Cc: Ike Panhc <ikepanhc@gmail.com>,
Mark Pearson <mpearson-lenovo@squebb.ca>,
"Derek J. Clark" <derekjohn.clark@gmail.com>,
Hans de Goede <hansg@kernel.org>,
platform-driver-x86@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] platform/x86: ideapad-laptop: Protect GBMD/SBMC calls with mutex
Date: Thu, 30 Oct 2025 18:40:12 +0200 (EET) [thread overview]
Message-ID: <2bae2ea7-2ef9-0cfa-0c2c-39a7043b2aa5@linux.intel.com> (raw)
In-Reply-To: <20251020192443.33088-2-i@rong.moe>
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;
>
next prev parent reply other threads:[~2025-10-30 16:40 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2bae2ea7-2ef9-0cfa-0c2c-39a7043b2aa5@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=derekjohn.clark@gmail.com \
--cc=hansg@kernel.org \
--cc=i@rong.moe \
--cc=ikepanhc@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpearson-lenovo@squebb.ca \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.