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 > Tested-by: Daniel Lee > 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 > +#include > #include > #include > #include > @@ -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, >