| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <98afb5a6-c47f-23c8-84c7-a494a086df27@linux.intel.com>
Date: Wed, 24 Sep 2025 20:58:33 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Daniel <dany97@...e.ca>
cc: Markus.Elfring@....de, Hans de Goede <hansg@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, matan@...alib.org,
platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v5] platform/x86: lg-laptop: Fix WMAB call in
fan_mode_store()
On Wed, 24 Sep 2025, Daniel wrote:
> On 2025-09-24 09:13, Ilpo Järvinen wrote:
>
> > Thank you for your contribution, it has been applied to my local
> > review-ilpo-fixes branch. Note it will show up in the public
> > platform-drivers-x86/review-ilpo-fixes branch only once I've pushed my
> > local branch there, which might take a while.
>
> Thanks a lot, but I was actually thinking about rewording the body of the
> commit message. Could I ask that this version be the one to eventually be
> pushed to the public repo?
I can replace it but please send the new version into a new thread as
in the same thread won't necessarily end up properly into patchwork and
may confuse other tools as well.
> Also, this patch fixes an issue raised in this comment on the kernel bugzilla:
> (https://bugzilla.kernel.org/show_bug.cgi?id=204913#c4), but crucially not
> the issue itself. Do I mention this anywhere in the commit message?
Just use a Link: tag, preferably directly to the comment.
--
i.
> -- >8 --
> Subject: [PATCH v6] platform/x86: lg-laptop: Fix WMAB call in fan_mode_store()
>
> When WMAB is called to set the fan mode, the new mode is read from either
> bits 0-1 or bits 4-5 (depending on the value of some other EC register).
> Thus when WMAB is called with bits 4-5 zeroed and called again with
> bits 0-1 zeroed, the second call undoes the effect of the first call.
> This causes writes to /sys/devices/platform/lg-laptop/fan_mode to have
> no effect (and causes reads to always report a status of zero).
>
> Fix this by calling WMAB once, with the mode set in bits 0,1 and 4,5.
> When the fan mode is returned from WMAB it always has this form, so
> there is no need to preserve the other bits. As a bonus, the driver
> now supports the "Performance" fan mode seen in the LG-provided Windows
> control app, which provides less aggressive CPU throttling but louder
> fan noise and shorter battery life.
>
> Also correct the documentation to reflect that 0 corresponds to the
> default mode (what the Windows app calls "Optimal") and 1 corresponds
> to the silent mode.
>
> Signed-off-by: Daniel Lee <dany97@...e.ca>
> Tested-by: Daniel Lee <dany97@...e.ca>
> Fixes: dbf0c5a6b1f8 ("platform/x86: Add LG Gram laptop special features driver")
> ---
> V1 -> V2: Replace bitops with GENMASK() and FIELD_PREP()
> V2 -> V3: Add parentheses next to function name in summary line
> Use full name in signoff
> V3 -> V4: Add include for linux/bitfield.h
> Remove "FIELD" from bitmask macro names
> V4 -> V5: Rename `status` to `mode` in fan_mode_show()
> V5 -> V6: Reword commit message body
>
> .../admin-guide/laptops/lg-laptop.rst | 4 +--
> drivers/platform/x86/lg-laptop.c | 34 ++++++++-----------
> 2 files changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/Documentation/admin-guide/laptops/lg-laptop.rst b/Documentation/admin-guide/laptops/lg-laptop.rst
> index 67fd6932c..c4dd534f9 100644
> --- a/Documentation/admin-guide/laptops/lg-laptop.rst
> +++ b/Documentation/admin-guide/laptops/lg-laptop.rst
> @@ -48,8 +48,8 @@ This value is reset to 100 when the kernel boots.
> Fan mode
> --------
>
> -Writing 1/0 to /sys/devices/platform/lg-laptop/fan_mode disables/enables
> -the fan silent mode.
> +Writing 0/1/2 to /sys/devices/platform/lg-laptop/fan_mode sets fan mode to
> +Optimal/Silent/Performance respectively.
>
>
> USB charge
> diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
> index 4b57102c7..6af6cf477 100644
> --- a/drivers/platform/x86/lg-laptop.c
> +++ b/drivers/platform/x86/lg-laptop.c
> @@ -8,6 +8,7 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/acpi.h>
> +#include <linux/bitfield.h>
> #include <linux/bits.h>
> #include <linux/device.h>
> #include <linux/dev_printk.h>
> @@ -75,6 +76,9 @@ MODULE_PARM_DESC(fw_debug, "Enable printing of firmware debug messages");
> #define WMBB_USB_CHARGE 0x10B
> #define WMBB_BATT_LIMIT 0x10C
>
> +#define FAN_MODE_LOWER GENMASK(1, 0)
> +#define FAN_MODE_UPPER GENMASK(5, 4)
> +
> #define PLATFORM_NAME "lg-laptop"
>
> MODULE_ALIAS("wmi:" WMI_EVENT_GUID0);
> @@ -274,29 +278,19 @@ static ssize_t fan_mode_store(struct device *dev,
> struct device_attribute *attr,
> const char *buffer, size_t count)
> {
> - bool value;
> + unsigned long value;
> union acpi_object *r;
> - u32 m;
> int ret;
>
> - ret = kstrtobool(buffer, &value);
> + ret = kstrtoul(buffer, 10, &value);
> if (ret)
> return ret;
> + if (value >= 3)
> + return -EINVAL;
>
> - r = lg_wmab(dev, WM_FAN_MODE, WM_GET, 0);
> - if (!r)
> - return -EIO;
> -
> - if (r->type != ACPI_TYPE_INTEGER) {
> - kfree(r);
> - return -EIO;
> - }
> -
> - m = r->integer.value;
> - kfree(r);
> - r = lg_wmab(dev, WM_FAN_MODE, WM_SET, (m & 0xffffff0f) | (value << 4));
> - kfree(r);
> - r = lg_wmab(dev, WM_FAN_MODE, WM_SET, (m & 0xfffffff0) | value);
> + r = lg_wmab(dev, WM_FAN_MODE, WM_SET,
> + FIELD_PREP(FAN_MODE_LOWER, value) |
> + FIELD_PREP(FAN_MODE_UPPER, value));
> kfree(r);
>
> return count;
> @@ -305,7 +299,7 @@ static ssize_t fan_mode_store(struct device *dev,
> static ssize_t fan_mode_show(struct device *dev,
> struct device_attribute *attr, char *buffer)
> {
> - unsigned int status;
> + unsigned int mode;
> union acpi_object *r;
>
> r = lg_wmab(dev, WM_FAN_MODE, WM_GET, 0);
> @@ -317,10 +311,10 @@ static ssize_t fan_mode_show(struct device *dev,
> return -EIO;
> }
>
> - status = r->integer.value & 0x01;
> + mode = FIELD_GET(FAN_MODE_LOWER, r->integer.value);
> kfree(r);
>
> - return sysfs_emit(buffer, "%d\n", status);
> + return sysfs_emit(buffer, "%d\n", mode);
> }
>
> static ssize_t usb_charge_store(struct device *dev,
>
Powered by blists - more mailing lists