[PATCH] platform/x86: lg-laptop: Fix WMAB call in fan_mode_store

[PATCH] platform/x86: lg-laptop: Fix WMAB call in fan_mode_store

[Daniel]

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 call

	lg_wmab(dev, WM_FAN_MODE, WM_SET, x)

the new mode is read from either the upper nibble or the lower nibble of x,
depending on the value of some other EC register.  Crucially, when WMAB
is called twice (once with the fan mode in the upper nibble, once with
it in the lower nibble), the result of the second call can overwrite
the first call.

Fix this by calling WMAB once, with the fan mode set in both nibbles.
As a bonus, the driver now supports the "Performance" mode seen in
the Windows LG Control Center app (less aggressive CPU throttling, but
louder fan noise and shorter battery life).  I can confirm that with
this patch writing/reading the fan mode works as expected on my laptop,
although I haven't tested it on any other LG laptops.

Also, correct the documentation to reflect that a value of 0 corresponds
to the default mode (what the LG app calls "Optimal") and a value of 1
corresponds to the silent mode.

Tested-by: Daniel <dany97@live.ca>
Signed-off-by: Daniel <dany97@live.ca>
---
 .../admin-guide/laptops/lg-laptop.rst         |  4 ++--
 drivers/platform/x86/lg-laptop.c              | 22 +++++--------------
 2 files changed, 8 insertions(+), 18 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..b8de6e568 100644
--- a/drivers/platform/x86/lg-laptop.c
+++ b/drivers/platform/x86/lg-laptop.c
@@ -274,29 +274,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);
+	r = lg_wmab(dev, WM_FAN_MODE, WM_SET, (value << 4) | value);
 	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);
 	kfree(r);
 
 	return count;
@@ -317,7 +307,7 @@ static ssize_t fan_mode_show(struct device *dev,
 		return -EIO;
 	}
 
-	status = r->integer.value & 0x01;
+	status = r->integer.value & 0x03;
 	kfree(r);
 
 	return sysfs_emit(buffer, "%d\n", status);
-- 
2.50.1

Re: [PATCH] platform/x86: lg-laptop: Fix WMAB call in fan_mode_store

[Ilpo Järvinen]

On Fri, 22 Aug 2025, Daniel wrote:

> 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 call
> 
> 	lg_wmab(dev, WM_FAN_MODE, WM_SET, x)
> 
> the new mode is read from either the upper nibble or the lower nibble of x,
> depending on the value of some other EC register.  Crucially, when WMAB
> is called twice (once with the fan mode in the upper nibble, once with
> it in the lower nibble), the result of the second call can overwrite
> the first call.
> 
> Fix this by calling WMAB once, with the fan mode set in both nibbles.
> As a bonus, the driver now supports the "Performance" mode seen in
> the Windows LG Control Center app (less aggressive CPU throttling, but
> louder fan noise and shorter battery life).  I can confirm that with
> this patch writing/reading the fan mode works as expected on my laptop,
> although I haven't tested it on any other LG laptops.
> 
> Also, correct the documentation to reflect that a value of 0 corresponds
> to the default mode (what the LG app calls "Optimal") and a value of 1
> corresponds to the silent mode.
> 
> Tested-by: Daniel <dany97@live.ca>
> Signed-off-by: Daniel <dany97@live.ca>
> ---
>  .../admin-guide/laptops/lg-laptop.rst         |  4 ++--
>  drivers/platform/x86/lg-laptop.c              | 22 +++++--------------
>  2 files changed, 8 insertions(+), 18 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..b8de6e568 100644
> --- a/drivers/platform/x86/lg-laptop.c
> +++ b/drivers/platform/x86/lg-laptop.c
> @@ -274,29 +274,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);
> +	r = lg_wmab(dev, WM_FAN_MODE, WM_SET, (value << 4) | value);

Is it okay to remove preserving the other bits?

Please name these field with defined GENMASK() and then use FIELD_PREP() 
here for both fields.

>  	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);
>  	kfree(r);
>  
>  	return count;
> @@ -317,7 +307,7 @@ static ssize_t fan_mode_show(struct device *dev,
>  		return -EIO;
>  	}
>  
> -	status = r->integer.value & 0x01;
> +	status = r->integer.value & 0x03;

This looks also like a field so should be named with a define?

>  	kfree(r);
>  
>  	return sysfs_emit(buffer, "%d\n", status);
> 

-- 
 i.

Re: [PATCH] platform/x86: lg-laptop: Fix WMAB call in fan_mode_store

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4106 bytes --]

On Thu, 28 Aug 2025, Ilpo Järvinen wrote:

> On Fri, 22 Aug 2025, Daniel wrote:
> 
> > 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 call
> > 
> > 	lg_wmab(dev, WM_FAN_MODE, WM_SET, x)
> > 
> > the new mode is read from either the upper nibble or the lower nibble of x,
> > depending on the value of some other EC register.  Crucially, when WMAB
> > is called twice (once with the fan mode in the upper nibble, once with
> > it in the lower nibble), the result of the second call can overwrite
> > the first call.

Oh, one more thing. I think this would warrant a Fixes tag pointing to the 
original commit.

--
 i.

> > Fix this by calling WMAB once, with the fan mode set in both nibbles.
> > As a bonus, the driver now supports the "Performance" mode seen in
> > the Windows LG Control Center app (less aggressive CPU throttling, but
> > louder fan noise and shorter battery life).  I can confirm that with
> > this patch writing/reading the fan mode works as expected on my laptop,
> > although I haven't tested it on any other LG laptops.
> > 
> > Also, correct the documentation to reflect that a value of 0 corresponds
> > to the default mode (what the LG app calls "Optimal") and a value of 1
> > corresponds to the silent mode.
> >
> > Tested-by: Daniel <dany97@live.ca>
> > Signed-off-by: Daniel <dany97@live.ca>
> > ---
> >  .../admin-guide/laptops/lg-laptop.rst         |  4 ++--
> >  drivers/platform/x86/lg-laptop.c              | 22 +++++--------------
> >  2 files changed, 8 insertions(+), 18 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..b8de6e568 100644
> > --- a/drivers/platform/x86/lg-laptop.c
> > +++ b/drivers/platform/x86/lg-laptop.c
> > @@ -274,29 +274,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);
> > +	r = lg_wmab(dev, WM_FAN_MODE, WM_SET, (value << 4) | value);
> 
> Is it okay to remove preserving the other bits?
> 
> Please name these field with defined GENMASK() and then use FIELD_PREP() 
> here for both fields.
> 
> >  	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);
> >  	kfree(r);
> >  
> >  	return count;
> > @@ -317,7 +307,7 @@ static ssize_t fan_mode_show(struct device *dev,
> >  		return -EIO;
> >  	}
> >  
> > -	status = r->integer.value & 0x01;
> > +	status = r->integer.value & 0x03;
> 
> This looks also like a field so should be named with a define?
> 
> >  	kfree(r);
> >  
> >  	return sysfs_emit(buffer, "%d\n", status);
> > 
> 
>