On Tue, 25 Nov 2025, Rong Zhang wrote: > On Mon, 2025-11-24 at 18:45 +0200, Ilpo Järvinen wrote: > > On Sun, 23 Nov 2025, Rong Zhang wrote: > > > > > Add support for LENOVO_FAN_TEST_DATA WMI data block. Provides an > > > interface for querying the min/max fan speed RPM (reference data) of a > > > given fan ID. > > > > > > This interface is optional. Hence, it does not bind to lenovo-wmi-other > > > and is not registered as a component for the moment. Appropriate binding > > > will be implemented in the subsequent patch. > > > > > > Signed-off-by: Rong Zhang > > > Reviewed-by: Derek J. Clark > > > Tested-by: Derek J. Clark > > > --- > > > Changes in v4: > > > - Rebase on top of changes made to [PATCH v4 3/7] > > > - Do not register it as a component until [PATCH v4 6/7] > > > > > > Changes in v2: > > > - Reword documentation > > > --- > > > .../wmi/devices/lenovo-wmi-other.rst | 17 +++ > > > drivers/platform/x86/lenovo/wmi-capdata.c | 102 ++++++++++++++++++ > > > drivers/platform/x86/lenovo/wmi-capdata.h | 7 ++ > > > 3 files changed, 126 insertions(+) > > > > > > diff --git a/Documentation/wmi/devices/lenovo-wmi-other.rst b/Documentation/wmi/devices/lenovo-wmi-other.rst > > > index fcad595d49af..821282e07d93 100644 > > > --- a/Documentation/wmi/devices/lenovo-wmi-other.rst > > > +++ b/Documentation/wmi/devices/lenovo-wmi-other.rst > > > @@ -62,6 +62,13 @@ The following firmware-attributes are implemented: > > > - ppt_pl2_sppt: Platform Profile Tracking Slow Package Power Tracking > > > - ppt_pl3_fppt: Platform Profile Tracking Fast Package Power Tracking > > > > > > +LENOVO_FAN_TEST_DATA > > > +------------------------- > > > + > > > +WMI GUID ``B642801B-3D21-45DE-90AE-6E86F164FB21`` > > > + > > > +The LENOVO_FAN_TEST_DATA interface provides reference data for self-test of > > > +cooling fans. > > > > > > WMI interface description > > > ========================= > > > @@ -115,3 +122,13 @@ data using the `bmfdec `_ utility: > > > [WmiDataId(3), read, Description("Data Size.")] uint32 DataSize; > > > [WmiDataId(4), read, Description("Default Value"), WmiSizeIs("DataSize")] uint8 DefaultValue[]; > > > }; > > > + > > > + [WMI, Dynamic, Provider("WmiProv"), Locale("MS\\0x409"), Description("Definition of Fan Test Data"), guid("{B642801B-3D21-45DE-90AE-6E86F164FB21}")] > > > + class LENOVO_FAN_TEST_DATA { > > > + [key, read] string InstanceName; > > > + [read] boolean Active; > > > + [WmiDataId(1), read, Description("Mode.")] uint32 NumOfFans; > > > + [WmiDataId(2), read, Description("Fan ID."), WmiSizeIs("NumOfFans")] uint32 FanId[]; > > > + [WmiDataId(3), read, Description("Maximum Fan Speed."), WmiSizeIs("NumOfFans")] uint32 FanMaxSpeed[]; > > > + [WmiDataId(4), read, Description("Minumum Fan Speed."), WmiSizeIs("NumOfFans")] uint32 FanMinSpeed[]; > > > + }; > > > diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c > > > index 29267c373ab3..e6392357395c 100644 > > > --- a/drivers/platform/x86/lenovo/wmi-capdata.c > > > +++ b/drivers/platform/x86/lenovo/wmi-capdata.c > > > @@ -13,6 +13,10 @@ > > > * attribute has multiple pages, one for each of the thermal modes managed by > > > * the Gamezone interface. > > > * > > > + * Fan Test Data includes the max/min fan speed RPM for each fan. This is > > > + * reference data for self-test. If the fan is in good condition, it is capable > > > + * to spin faster than max RPM or slower than min RPM. > > > + * > > > * Copyright (C) 2025 Derek J. Clark > > > * - Initial implementation (formerly named lenovo-wmi-capdata01) > > > * > > > @@ -41,6 +45,7 @@ > > > > > > #define LENOVO_CAPABILITY_DATA_00_GUID "362A3AFE-3D96-4665-8530-96DAD5BB300E" > > > #define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154" > > > +#define LENOVO_FAN_TEST_DATA_GUID "B642801B-3D21-45DE-90AE-6E86F164FB21" > > > > > > #define ACPI_AC_CLASS "ac_adapter" > > > #define ACPI_AC_NOTIFY_STATUS 0x80 > > > @@ -48,6 +53,7 @@ > > > enum lwmi_cd_type { > > > LENOVO_CAPABILITY_DATA_00, > > > LENOVO_CAPABILITY_DATA_01, > > > + LENOVO_FAN_TEST_DATA, > > > }; > > > > > > #define LWMI_CD_TABLE_ITEM(_type) \ > > > @@ -62,6 +68,7 @@ static const struct lwmi_cd_info { > > > } lwmi_cd_table[] = { > > > LWMI_CD_TABLE_ITEM(LENOVO_CAPABILITY_DATA_00), > > > LWMI_CD_TABLE_ITEM(LENOVO_CAPABILITY_DATA_01), > > > + LWMI_CD_TABLE_ITEM(LENOVO_FAN_TEST_DATA), > > > }; > > > > > > struct lwmi_cd_priv { > > > @@ -78,6 +85,7 @@ struct cd_list { > > > union { > > > DECLARE_FLEX_ARRAY(struct capdata00, cd00); > > > DECLARE_FLEX_ARRAY(struct capdata01, cd01); > > > + DECLARE_FLEX_ARRAY(struct capdata_fan, cd_fan); > > > }; > > > }; > > > > > > @@ -117,6 +125,10 @@ void lwmi_cd_match_add_all(struct device *master, struct component_match **match > > > return; > > > > > > for (i = 0; i < ARRAY_SIZE(lwmi_cd_table); i++) { > > > + /* Skip optional interfaces. */ > > > + if (lwmi_cd_table[i].type == LENOVO_FAN_TEST_DATA) > > > + continue; > > > + > > > component_match_add(master, matchptr, lwmi_cd_match, > > > (void *)&lwmi_cd_table[i].type); > > > if (IS_ERR(*matchptr)) > > > @@ -194,6 +206,9 @@ EXPORT_SYMBOL_NS_GPL(lwmi_cd00_get_data, "LENOVO_WMI_CD"); > > > DEF_LWMI_CDXX_GET_DATA(cd01, LENOVO_CAPABILITY_DATA_01, struct capdata01); > > > EXPORT_SYMBOL_NS_GPL(lwmi_cd01_get_data, "LENOVO_WMI_CD"); > > > > > > +DEF_LWMI_CDXX_GET_DATA(cd_fan, LENOVO_FAN_TEST_DATA, struct capdata_fan); > > > +EXPORT_SYMBOL_NS_GPL(lwmi_cd_fan_get_data, "LENOVO_WMI_CD"); > > > + > > > /** > > > * lwmi_cd_cache() - Cache all WMI data block information > > > * @priv: lenovo-wmi-capdata driver data. > > > @@ -217,6 +232,9 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv) > > > p = &priv->list->cd01[0]; > > > size = sizeof(priv->list->cd01[0]); > > > break; > > > + case LENOVO_FAN_TEST_DATA: > > > + /* Done by lwmi_cd_alloc() => lwmi_cd_fan_list_alloc_cache(). */ > > > + return 0; > > > default: > > > return -EINVAL; > > > } > > > @@ -239,6 +257,78 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv) > > > return 0; > > > } > > > > > > +/** > > > + * lwmi_cd_fan_list_alloc_cache() - Alloc and cache Fan Test Data list > > > + * @priv: lenovo-wmi-capdata driver data. > > > + * @listptr: Pointer to returned cd_list pointer. > > > + * > > > + * Return: count of fans found, or an error. > > > + */ > > > +static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv, struct cd_list **listptr) > > > +{ > > > + u32 count, *fan_ids, *fan_min_rpms, *fan_max_rpms; > > > + union acpi_object *ret_obj __free(kfree) = NULL; > > > > Since you're using __free(), please move this to where you assign the > > value. This is to create a pattern with cleanup helpers. The cleanup > > order depends on the order the variables are introduced which in some > > other cases may be significant. > > Make sense. Will move it to the last declaration lines with its > assignment. Thanks. > > > > + struct block { u32 nr; u32 data[]; } *block; > > > > This is the first time I see this style anywhere in the kernel's context, > > has there been some general discussion about this style somewhere? > > > > At least it seems immediately obvious to me that this style will have a > > negative impact on documentability due to (too) concise use of space. > > Make sense. Will break it into multiple lines. Thanks. > > > > + struct cd_list *list; > > > + size_t size; > > > + int idx; > > > + > > > + ret_obj = wmidev_block_query(priv->wdev, 0); > > > + if (!ret_obj) > > > + return -ENODEV; > > > + > > > + /* > > > + * This is usually caused by a dummy ACPI method. Do not return an error > > > + * as failing to probe this device will result in master driver being > > > + * unbound - this behavior aligns with lwmi_cd_cache(). > > > + */ > > > + if (ret_obj->type != ACPI_TYPE_BUFFER) { > > > + count = 0; > > > + goto alloc; > > > + } > > > + > > > + size = ret_obj->buffer.length; > > > + block = (struct block *)ret_obj->buffer.pointer; > > > > void * can be cast implicitly. > > `ret_obj->buffer.pointer' is a `u8 *' pointer so the cast is mandatory. Ah, right. I think I even tried to check it but probably picked up the void *pointer from struct acpi_buffer instead of the correct one. > Hmmm, this reminds me that `struct block' probably needs a `__packed' > to generate unaligned access to it. In all u32 struct, I don't think __packed is required. You can use pahole tool to check the layout if you want, but I don't think compiler adds any gaps with only u32s. -- i.