Re: [PATCH v5 0/7] platform/x86: lenovo-wmi-{capdata,other}: Add HWMON for fan speed

[Rong Zhang]

Hi Armin,

On Mon, 2025-11-17 at 07:38 +0100, Armin Wolf wrote:
> Am 14.11.25 um 18:59 schrieb Rong Zhang:
> 
> > Lenovo WMI Other Mode interface also supports querying or setting fan
> > speed RPM. This capability is decribed by LENOVO_CAPABILITY_DATA_00.
> > Besides, LENOVO_FAN_TEST_DATA provides reference data for self-test of
> > cooling fans, including minimum and maximum fan speed RPM.
> > 
> > This patchset turns lenovo-wmi-capdata01 into a unified driver (now
> > named lenovo-wmi-capdata) for LENOVO_CAPABILITY_DATA_{00,01} and
> > LENOVO_FAN_TEST_DATA; then adds HWMON support for lenovo-wmi-other:
> > 
> >   - fanX_enable: enable/disable the fan (tunable)
> >   - fanX_input: current RPM
> >   - fanX_max: maximum RPM
> >   - fanX_min: minimum RPM
> >   - fanX_target: target RPM (tunable)
> > 
> > LENOVO_CAPABILITY_DATA_{00,01} presents on all devices, so
> > both binds to lenovo-wmi-other. However, some device does not have
> > LENOVO_FAN_TEST_DATA and its presence is described by
> > LENOVO_CAPABILITY_DATA_00; hence, the former binds to the latter and a
> > callback is used to pass the data to lenovo-wmi-other.
> > 
> > The implementation does not rely on a specific binding sequence. This
> > has been fuzz-tested using:
> > 
> > 	#!/bin/bash
> > 
> > 	DRV_DIR=/sys/bus/wmi/drivers/lenovo_wmi_cd
> > 	CAPDATA_GUIDS=(
> > 		$(find "$DRV_DIR"/ -name '*-*-*-*-*-*' -printf "%f ")
> > 	)
> > 
> > 	b() { sudo tee "$DRV_DIR"/bind <<<"$1"; }
> > 	u() { sudo tee "$DRV_DIR"/unbind <<<"$1"; }
> > 
> > 	for guid in "${CAPDATA_GUIDS[@]}"; do
> > 		u "$guid"
> > 	done
> > 
> > 	while read -rsa perm; do
> > 		for guid in "${perm[@]}"; do
> > 			b "$guid"
> > 		done
> > 		sensors | grep -A3 lenovo_wmi_other || true
> > 		for guid in "${perm[@]}"; do
> > 			u "$guid"
> > 		done
> > 	done < <(python3 -c "
> > 	from itertools import permutations
> > 	ps = permutations('${CAPDATA_GUIDS[*]}'.split())
> > 	for p in ps: print(' '.join(p))")
> > 
> > 	for guid in "${CAPDATA_GUIDS[@]}"; do
> > 		b "$guid"
> > 	done
> > 
> > Tested on ThinkBook 14 G7+ ASP.
> 
> Sorry for taking a bit to respond to your question regarding the previous
> patch series. Your idea with the submaster seems good to me.

Thanks!

> Your script
> for testing this is a very cool idea, are you OK with me adding  a
> similar script to the tools/wmi/?

Sure ;)

Feel free to add Co-developed-by: Rong Zhang <i@xxxxxxxx> if you
prefer.

> Thanks,
> Armin Wolf

Thanks,
Rong

> 
> > Changes in v5:
> > - Do not cast pointer to non-pointer or vice versa (thanks kernel test
> >    robot)
> > - Fix missing include (ditto)
> > - Link to v4: https://lore.kernel.org/r/20251113191152.96076-1-i@xxxxxxxx/
> > 
> > Changes in v4:
> > - Get rid of wmi_has_guid() (thanks Armin Wolf's inspiration)
> >    - Add [PATCH v4 6/7], please review & test
> >    - Rework HWMON registration
> >      - Collect fan into from capdata00 and capdata_fan separately
> >      - Use a callback to collect fan info from capdata_fan
> >      - Trigger HWMON registration only if all fan info is collected
> >      - Do not check 0x04050000.supported, implied by the presense of
> >        capdata_fan
> > - Drop Reviewed-by & Tested-by from [PATCH v4 7/7] due to the changes,
> >    please review & test
> > - Link to v3: https://lore.kernel.org/r/20251031155349.24693-1-i@xxxxxxxx/
> > 
> > Changes in v3:
> > - Fix grammar (thanks Derek J. Clark)
> > - Link to v2: https://lore.kernel.org/r/20251030193955.107148-1-i@xxxxxxxx/
> > 
> > Changes in v2:
> > - Add a workaround for ACPI methods that return a 4B buffer for u32
> >    (thanks Armin Wolf)
> > - Fix function documentation (thanks kernel test bot)
> > - Reword documentation (thanks Derek J. Clark)
> > - Squash min/max reporting patch into the initial HWMON one (ditto)
> > - Query 0x04050000 for interface availability (ditto)
> >    - New parameter "expose_all_fans" to skip this check
> > - Enforce min/max RPM constraint on set (ditto)
> >    - New parameter "relax_fan_constraint" to disable this behavior
> >    - Drop parameter "ignore_fan_cap", superseded by the next one
> >    - New parameter "expose_all_fans" to expose fans w/o such data
> > - Assume auto mode on probe (ditto)
> > - Do not register HWMON device if no fan can be exposed
> > - fanX_target: Return -EBUSY instead of raw target value when fan stops
> > - Link to v1: https://lore.kernel.org/r/20251019210450.88830-1-i@xxxxxxxx/
> > 
> > Rong Zhang (7):
> >    platform/x86: lenovo-wmi-helpers: Convert returned buffer into u32
> >    platform/x86: Rename lenovo-wmi-capdata01 to lenovo-wmi-capdata
> >    platform/x86: lenovo-wmi-{capdata,other}: Support multiple Capability
> >      Data
> >    platform/x86: lenovo-wmi-capdata: Add support for Capability Data 00
> >    platform/x86: lenovo-wmi-capdata: Add support for Fan Test Data
> >    platform/x86: lenovo-wmi-capdata: Wire up Fan Test Data
> >    platform/x86: lenovo-wmi-other: Add HWMON for fan reporting/tuning
> > 
> >   .../wmi/devices/lenovo-wmi-other.rst          |  43 +-
> >   drivers/platform/x86/lenovo/Kconfig           |   5 +-
> >   drivers/platform/x86/lenovo/Makefile          |   2 +-
> >   drivers/platform/x86/lenovo/wmi-capdata.c     | 808 ++++++++++++++++++
> >   drivers/platform/x86/lenovo/wmi-capdata.h     |  65 ++
> >   drivers/platform/x86/lenovo/wmi-capdata01.c   | 302 -------
> >   drivers/platform/x86/lenovo/wmi-capdata01.h   |  25 -
> >   drivers/platform/x86/lenovo/wmi-helpers.c     |  22 +-
> >   drivers/platform/x86/lenovo/wmi-other.c       | 511 ++++++++++-
> >   9 files changed, 1426 insertions(+), 357 deletions(-)
> >   create mode 100644 drivers/platform/x86/lenovo/wmi-capdata.c
> >   create mode 100644 drivers/platform/x86/lenovo/wmi-capdata.h
> >   delete mode 100644 drivers/platform/x86/lenovo/wmi-capdata01.c
> >   delete mode 100644 drivers/platform/x86/lenovo/wmi-capdata01.h
> > 
> > 
> > base-commit: 6da43bbeb6918164f7287269881a5f861ae09d7e