public inbox for linux-hwmon@vger.kernel.org
 help / color / mirror / Atom feed
From: "Kurt Borja" <kuurtb@gmail.com>
To: "Rong Zhang" <i@rong.moe>, "Kurt Borja" <kuurtb@gmail.com>
Cc: "Mark Pearson" <mpearson-lenovo@squebb.ca>,
	"Derek J. Clark" <derekjohn.clark@gmail.com>,
	"Armin Wolf" <W_Armin@gmx.de>, "Hans de Goede" <hansg@kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Guenter Roeck" <linux@roeck-us.net>,
	platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org
Subject: Re: [PATCH v9 7/7] platform/x86: lenovo-wmi-other: Add HWMON for fan reporting/tuning
Date: Thu, 15 Jan 2026 08:58:10 -0500	[thread overview]
Message-ID: <DFP7SAGSD32N.3SIIV8JMYHWRM@gmail.com> (raw)
In-Reply-To: <1a9909f4083d85736a1e28067517ae0899e462f2.camel@rong.moe>

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@rong.moe>
>> > Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.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?

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.

As it seems that you can change the RPM in 100 increments, 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.

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

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

[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


-- 
Thanks,
 ~ Kurt


  reply	other threads:[~2026-01-15 13:58 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-14 12:27 [PATCH v9 0/7] platform/x86: lenovo-wmi-{capdata,other}: Add HWMON for fan speed Rong Zhang
2026-01-14 12:27 ` [PATCH v9 1/7] platform/x86: lenovo-wmi-helpers: Convert returned buffer into u32 Rong Zhang
2026-01-14 12:27 ` [PATCH v9 2/7] platform/x86: Rename lenovo-wmi-capdata01 to lenovo-wmi-capdata Rong Zhang
2026-01-14 12:27 ` [PATCH v9 3/7] platform/x86: lenovo-wmi-{capdata,other}: Support multiple Capability Data Rong Zhang
2026-01-14 12:27 ` [PATCH v9 4/7] platform/x86: lenovo-wmi-capdata: Add support for Capability Data 00 Rong Zhang
2026-01-14 12:27 ` [PATCH v9 5/7] platform/x86: lenovo-wmi-capdata: Add support for Fan Test Data Rong Zhang
2026-01-14 12:27 ` [PATCH v9 6/7] platform/x86: lenovo-wmi-capdata: Wire up " Rong Zhang
2026-01-14 12:27 ` [PATCH v9 7/7] platform/x86: lenovo-wmi-other: Add HWMON for fan reporting/tuning Rong Zhang
2026-01-15  0:16   ` Kurt Borja
2026-01-15 13:03     ` Rong Zhang
2026-01-15 13:58       ` Kurt Borja [this message]
2026-01-15 16:57         ` Rong Zhang
2026-01-15 17:45           ` Derek J. Clark
2026-01-15 18:19             ` Kurt Borja
2026-01-16 14:01             ` Rong Zhang
2026-01-15 19:09           ` Kurt Borja
2026-01-16 14:18             ` Rong Zhang
2026-01-16 17:17               ` Kurt Borja

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DFP7SAGSD32N.3SIIV8JMYHWRM@gmail.com \
    --to=kuurtb@gmail.com \
    --cc=W_Armin@gmx.de \
    --cc=derekjohn.clark@gmail.com \
    --cc=hansg@kernel.org \
    --cc=i@rong.moe \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mpearson-lenovo@squebb.ca \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox