Message-ID: <9ff2b73e30f06be69b6c0b72b91a19d766310db7.camel@rong.moe>
Date: Fri, 16 Jan 2026 00:57:42 +0800
From: Rong Zhang <i@...g.moe>
To: Kurt Borja <kuurtb@...il.com>, "Derek J. Clark"
 <derekjohn.clark@...il.com>
Cc: Mark Pearson <mpearson-lenovo@...ebb.ca>, Armin Wolf <W_Armin@....de>, 
 Hans de Goede <hansg@...nel.org>, Ilpo Järvinen
 <ilpo.jarvinen@...ux.intel.com>,  Guenter Roeck <linux@...ck-us.net>,
 platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v9 7/7] platform/x86: lenovo-wmi-other: Add HWMON for
 fan reporting/tuning

Hi Kurt,

On Thu, 2026-01-15 at 08:58 -0500, Kurt Borja wrote:
> On Thu Jan 15, 2026 at 8:03 AM -05, Rong Zhang wrote:
> > Hi Kurt,
> > 
> > On Wed, 2026-01-14 at 19:16 -0500, Kurt Borja wrote:
> > > Hi Rong,
> > > 
> > > On Wed Jan 14, 2026 at 7:27 AM -05, Rong Zhang wrote:
> > > > Register an HWMON device for fan reporting/tuning according to
> > > > Capability Data 00 (capdata00) and Fan Test Data (capdata_fan) provided
> > > > by lenovo-wmi-capdata. The corresponding HWMON nodes are:
> > > > 
> > > >  - fanX_enable: enable/disable the fan (tunable)
> > > >  - fanX_input: current RPM
> > > >  - fanX_max: maximum RPM
> > > >  - fanX_min: minimum RPM
> > > >  - fanX_target: target RPM (tunable)
> > > > 
> > > > Information from capdata00 and capdata_fan are used to control the
> > > > visibility and constraints of HWMON attributes. Fan info from capdata00
> > > > is collected on bind, while fan info from capdata_fan is collected in a
> > > > callback. Once all fan info is collected, register the HWMON device.
> > > > 
> > > > Signed-off-by: Rong Zhang <i@...g.moe>
> > > > Reviewed-by: Derek J. Clark <derekjohn.clark@...il.com>
> > > > ---
> > > 
> > > ...
> > > 
> > > > diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst
> > > > index 821282e07d93c..bd1d733ff286d 100644
> > > > --- a/Documentation/wmi/devices/lenovo-wmi-other.rst
> > > > +++ b/Documentation/wmi/devices/lenovo-wmi-other.rst
> > > > @@ -31,6 +31,8 @@ under the following path:
> > > >  
> > > >    /sys/class/firmware-attributes/lenovo-wmi-other/attributes/<attribute>/
> > > >  
> > > > +Additionally, this driver also exports attributes to HWMON.
> > > > +
> > > >  LENOVO_CAPABILITY_DATA_00
> > > >  -------------------------
> > > >  
> > > > @@ -39,6 +41,11 @@ WMI GUID ``362A3AFE-3D96-4665-8530-96DAD5BB300E``
> > > >  The LENOVO_CAPABILITY_DATA_00 interface provides various information that
> > > >  does not rely on the gamezone thermal mode.
> > > >  
> > > > +The following HWMON attributes are implemented:
> > > > + - fanX_enable: enable/disable the fan (tunable)
> > > 
> > > I was testing this series and I'm a bit confused about fanX_enable.
> > 
> > Thanks for testing!
> 
> Thanks for working on this!
> 
> > 
> > > Judging by this comment and also by taking a quick look at the code, it
> > > looks like writting 0 to this attribute disables the fan.
> > > 
> > > This is however (per hwmon ABI documentation [1]) not how this attribute
> > > should work. IIUC, it is intended for devices which can disable the fan
> > > sensor, not the actual fan.
> > 
> > Hmm, what a confusing name :-/
> > 
> > > I fail to see how this feature is useful and I also find it dangerous
> > > for this to be exposed by default, considering the same could be
> > > achieved with the relaxed module parameter, which at least tells the
> > > user to be careful.
> > 
> > Makes sense. I will remove the attribute and mention such behavior in
> > the module parameter.
> 
> Also, it would be worth to mention that writting 0 to the fanY_target
> attribute is auto mode, right?

Ah, yes.

> I was testing the fanX_target attribute and it does work as intended,
> i.e. the fan speed changes to the desired speed. However, every time I
> write to this attribute I'm getting -EIO error and it always reads 0.
> 
> For example:
> 
> 	$ echo 3550 | sudo tee fan*_target
> 	3550
> 	tee: fan1_target: Input/output error
> 	tee: fan2_target: Input/output error
> 	$ cat fan*_target
> 	0
> 	0
> 
> But as I said, the fans do reach the desired speed (an approximation of
> it):
> 
> 	$ cat fan*_input
> 	3500
> 	3500
> 
> This is a bit weird, but I haven't look in depth into it. I will find
> some time to do it later. This happens on a 83KY (Legion 7 16IAX1)
> laptop.

I'd like to fix it in the next revision. Looking forward to your
progress in debugging :-)

It seems to me that the corresponding ACPI method did not return 1 on
success. There should be some clues in the decompiled ASL code. Could
you attach it in your reply? The HWMON implementation was developed
mostly based on my understanding on the decompiled ASL code of my
device. I'd like to check the one of your device as a cross-reference
to see if there are more potential bugs.

> As it seems that you can change the RPM in 100 increments,

Yeah. That's also true on my device, but I am not sure if all devices
with the interface behave like this. I'd prefer not making such an
assumption.

> maybe you
> could look into the pwmY attributes [1]. I think it is a better fit for
> this feature because pwmY_enable allows you to select between manual and
> auto fan control [2], and I believe some user-space tools already use
> this attribute.

For pwmY, I don't see the point why the kernel should adapt to
userspace tools... If we don't have to, don't be overfit.

For pwmY_enable, it is only a good choice if we adopt pwmY. It's weird
to mix pwmY_enable and fanY_target.

@Derek, may I ask for your opinion here? Should we adopt pwmY*?

> On the implementation you can use fixp_linear_interpolate() [3] to
> convert between and from pwm duty cycle.

If we adopt this, I could imagine three ways to create a PWM-RPM curve:
(1) PWM[0,255] => RPM[min,max]
(2) PWM[0,255] => RPM[0,max]
(3) PWM[0,255] => RPM[0,a_large_number]

relax_fan_constraint=1 definitely needs (3) as the fan can spin
slower/faster than min/max_rpm. I don't want to create scale mismatches
between relax_fan_constraint=1 and =0, so only (3) looks viable to me
in any case. Hmm, we are creating a weird PWM-RPM curve with two
plateau [1].

> This is just a suggestion though, I know I came in too late to the
> discussion but I just got this laptop :P

Happy hacking :P

ThinkBook (my device) only implements few Legion interfaces, so your
information here is very valuable.

> [1] https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/ABI/testing/sysfs-class-hwmon#L297
> [2] https://elixir.bootlin.com/linux/v6.19-rc5/source/Documentation/ABI/testing/sysfs-class-hwmon#L312
> [3] https://elixir.bootlin.com/linux/v6.19-rc5/source/include/linux/fixp-arith.h#L145

[1]: Typical curves look like
https://www.overclock.net/threads/are-voltage-controllers-for-fans-bad-and-pwm-controlled-fans-better.1627409/#post-25993378

Thanks,
Rong

Powered by blists - more mailing lists