| 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: <ea57d978-3fd3-fd86-aec7-e814359e3e02@linux.intel.com>
Date: Mon, 22 Sep 2025 15:49:42 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Daniel <dany97@...e.ca>
cc: Markus.Elfring@....de, hansg@...nel.org,
LKML <linux-kernel@...r.kernel.org>, matan@...alib.org,
platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v3] platform/x86: lg-laptop: Fix WMAB call in
fan_mode_store()
On Fri, 19 Sep 2025, Daniel wrote:
> > Is it good to reuse the input side define here for response side or
> > should you have another with a more specific name? (This is put to
> > status named variable so my natural expectation is that the field's
> > name is somehow related to that.)
>
> The fan mode really is sent back to us in *_LOWER and *_UPPER form,
> exactly like how we send the fan mode. (Only, the lower and upper
> portions are guaranteed to be identical, so we just read the lower.)
> Hence why I did not add a new define for the response side.
>
> Should I add another define FAN_MODE_RESPONSE?
Ah okay so the variable name is misleading then as it's really "mode"
that is returned, not really "status"? I think this is fine then although
I'd prefer to have also another patch on top of this which would rename
that variable to mode.
> -- >8 --
> Subject: [PATCH v4] platform/x86: lg-laptop: Fix WMAB call in fan_mode_store()
>
> On my LG Gram 16Z95P-K.AA75A8 (2022), writes to
> /sys/devices/platform/lg-laptop/fan_mode have no effect and reads always
> report a status of 0.
>
> Disassembling the relevant ACPI tables reveals that in the WMAB call to
> set the fan mode, the new mode is read either from bits 0,1 or bits 4,5
> (depending on the value of some other EC register). Thus when we call
> WMAB twice, first with bits 4,5 zero, then bits 0,1 zero, the second
> call undoes the effect of the first call.
>
> 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.
>
> I can confirm with this patch reading/writing the fan mode now works
> as expected on my laptop, although I have not tested it on any other
> LG laptop.
>
> 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: dbf0c5a6b1f8e7bec5e17baa60a1e04c28d90f9b ("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
>
> .../admin-guide/laptops/lg-laptop.rst | 4 +--
> drivers/platform/x86/lg-laptop.c | 30 ++++++++-----------
> 2 files changed, 14 insertions(+), 20 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..0ef179f7a 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;
> @@ -317,7 +311,7 @@ static ssize_t fan_mode_show(struct device *dev,
> return -EIO;
> }
>
> - status = r->integer.value & 0x01;
> + status = FIELD_GET(FAN_MODE_LOWER, r->integer.value);
> kfree(r);
>
> return sysfs_emit(buffer, "%d\n", status);
>
--
i.
Powered by blists - more mailing lists