Re: [PATCH v6 05/13] platform/x86: lenovo-wmi-other: Add lwmi_attr_id() function

["Derek J. Clark" <derekjohn.clark@gmail.com>]

On April 1, 2026 11:29:25 AM PDT, Rong Zhang <i@rong.moe> wrote:
>Hi Derek,
>
>On Tue, 2026-03-31 at 18:12 +0000, Derek J. Clark wrote:
>> Adds lwmi_attr_id() function. In the same vein as LWMI_ATTR_ID_FAN_RPM(),
>> but as a generic, to de-duplicate attribute_id assignment biolerplate.
>> 
>> Reviewed-by: Rong Zhang <i@rong.moe>
>> Tested-by: Rong Zhang <i@rong.moe>
>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
>> ---
>> v6:
>>   - Move lwmi_attr_id to wmi-capdata.h as static inline.
>> v5:
>>   - Move references to cv/cd_mode_id to patch 4/8.
>>   - Move lwmi_attr_id to wmi-capdata.c and export with namespace.
>> v4:
>>   - Switch from macro to static inline to preserve types.
>> ---
>>  drivers/platform/x86/lenovo/wmi-capdata.c |  8 ++---
>>  drivers/platform/x86/lenovo/wmi-capdata.h | 20 ++++++++++++
>>  drivers/platform/x86/lenovo/wmi-other.c   | 39 ++++++-----------------
>>  3 files changed, 34 insertions(+), 33 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c b/drivers/platform/x86/lenovo/wmi-capdata.c
>> index ee1fb02d8e31..169665be4dcf 100644
>> --- a/drivers/platform/x86/lenovo/wmi-capdata.c
>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
>> @@ -27,7 +27,6 @@
>>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>  
>>  #include <linux/acpi.h>
>> -#include <linux/bitfield.h>
>>  #include <linux/bug.h>
>>  #include <linux/cleanup.h>
>>  #include <linux/component.h>
>> @@ -48,6 +47,7 @@
>>  #include <linux/wmi.h>
>>  
>>  #include "wmi-capdata.h"
>> +#include "wmi-helpers.h"
>
>Bad rebase? IIUC it is unneeded until patch 13.
>
Hi Rong.

No, it is needed.

>Thanks,
>Rong
>
>>  
>>  #define LENOVO_CAPABILITY_DATA_00_GUID "362A3AFE-3D96-4665-8530-96DAD5BB300E"
>>  #define LENOVO_CAPABILITY_DATA_01_GUID "7A8F5407-CB67-4D6E-B547-39B3BE018154"
>> @@ -58,9 +58,9 @@
>>  
>>  #define LWMI_FEATURE_ID_FAN_TEST 0x05
>>  
>> -#define LWMI_ATTR_ID_FAN_TEST							\
>> -	(FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, LWMI_DEVICE_ID_FAN) |		\
>> -	 FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, LWMI_FEATURE_ID_FAN_TEST))
>> +#define LWMI_ATTR_ID_FAN_TEST                                      \
>> +	lwmi_attr_id(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_TEST, \
>> +		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_TYPE_ID_NONE)
>>  

Thermal mode none is referenced here, and now lives in helpers.h

- Derek

>>  enum lwmi_cd_type {
>>  	LENOVO_CAPABILITY_DATA_00,
>> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.h b/drivers/platform/x86/lenovo/wmi-capdata.h
>> index 8c1df3efcc55..1388eaf4ab4a 100644
>> --- a/drivers/platform/x86/lenovo/wmi-capdata.h
>> +++ b/drivers/platform/x86/lenovo/wmi-capdata.h
>> @@ -6,6 +6,7 @@
>>  #define _LENOVO_WMI_CAPDATA_H_
>>  
>>  #include <linux/bits.h>
>> +#include <linux/bitfield.h>
>>  #include <linux/types.h>
>>  
>>  #define LWMI_SUPP_VALID		BIT(0)
>> @@ -19,6 +20,8 @@
>>  
>>  #define LWMI_DEVICE_ID_FAN	0x04
>>  
>> +#define LWMI_TYPE_ID_NONE 0x00
>> +
>>  struct component_match;
>>  struct device;
>>  struct cd_list;
>> @@ -57,6 +60,23 @@ struct lwmi_cd_binder {
>>  	cd_list_cb_t cd_fan_list_cb;
>>  };
>>  
>> +/**
>> + * lwmi_attr_id() - Formats a capability data attribute ID
>> + * @dev_id: The u8 corresponding to the device ID.
>> + * @feat_id: The u8 corresponding to the feature ID on the device.
>> + * @mode_id: The u8 corresponding to the wmi-gamezone mode for set/get.
>> + * @type_id: The u8 corresponding to the sub-device.
>> + *
>> + * Return: u32.
>> + */
>> +static inline u32 lwmi_attr_id(u8 dev_id, u8 feat_id, u8 mode_id, u8 type_id)
>> +{
>> +	return (FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, dev_id)   |
>> +		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, feat_id) |
>> +		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode_id) |
>> +		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, type_id));
>> +}
>> +
>>  void lwmi_cd_match_add_all(struct device *master, struct component_match **matchptr);
>>  int lwmi_cd00_get_data(struct cd_list *list, u32 attribute_id, struct capdata00 *output);
>>  int lwmi_cd01_get_data(struct cd_list *list, u32 attribute_id, struct capdata01 *output);
>> diff --git a/drivers/platform/x86/lenovo/wmi-other.c b/drivers/platform/x86/lenovo/wmi-other.c
>> index b24bf98c19ad..d2838ae5934f 100644
>> --- a/drivers/platform/x86/lenovo/wmi-other.c
>> +++ b/drivers/platform/x86/lenovo/wmi-other.c
>> @@ -27,7 +27,6 @@
>>   */
>>  
>>  #include <linux/acpi.h>
>> -#include <linux/bitfield.h>
>>  #include <linux/cleanup.h>
>>  #include <linux/component.h>
>>  #include <linux/container_of.h>
>> @@ -61,8 +60,6 @@
>>  
>>  #define LWMI_FEATURE_ID_FAN_RPM 0x03
>>  
>> -#define LWMI_TYPE_ID_NONE 0x00
>> -
>>  #define LWMI_FEATURE_VALUE_GET 17
>>  #define LWMI_FEATURE_VALUE_SET 18
>>  
>> @@ -72,10 +69,9 @@
>>  
>>  #define LWMI_FAN_DIV 100
>>  
>> -#define LWMI_ATTR_ID_FAN_RPM(x)						\
>> -	(FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, LWMI_DEVICE_ID_FAN) |	\
>> -	 FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, LWMI_FEATURE_ID_FAN_RPM) |	\
>> -	 FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, LWMI_FAN_ID(x)))
>> +#define LWMI_ATTR_ID_FAN_RPM(x)                                   \
>> +	lwmi_attr_id(LWMI_DEVICE_ID_FAN, LWMI_FEATURE_ID_FAN_RPM, \
>> +		     LWMI_GZ_THERMAL_MODE_NONE, LWMI_FAN_ID(x))
>>  
>>  #define LWMI_OM_FW_ATTR_BASE_PATH "lenovo-wmi-other"
>>  #define LWMI_OM_HWMON_NAME "lenovo_wmi_other"
>> @@ -714,12 +710,8 @@ static ssize_t attr_capdata01_show(struct kobject *kobj,
>>  	u32 attribute_id;
>>  	int value, ret;
>>  
>> -	attribute_id =
>> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
>> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
>> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK,
>> -			   LWMI_GZ_THERMAL_MODE_CUSTOM) |
>> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
>> +	attribute_id = lwmi_attr_id(tunable_attr->device_id, tunable_attr->feature_id,
>> +				    LWMI_GZ_THERMAL_MODE_CUSTOM, tunable_attr->type_id);
>>  
>>  	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>>  	if (ret)
>> @@ -774,7 +766,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	struct wmi_method_args_32 args = { 0x0, 0x0 };
>>  	struct capdata01 capdata;
>>  	enum thermal_mode mode;
>> -	u32 attribute_id;
>>  	u32 value;
>>  	int ret;
>>  
>> @@ -785,13 +776,10 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	if (mode != LWMI_GZ_THERMAL_MODE_CUSTOM)
>>  		return -EBUSY;
>>  
>> -	attribute_id =
>> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
>> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
>> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
>> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
>> +	args.arg0 = lwmi_attr_id(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>>  
>> -	ret = lwmi_cd01_get_data(priv->cd01_list, attribute_id, &capdata);
>> +	ret = lwmi_cd01_get_data(priv->cd01_list, args.arg0, &capdata);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -802,7 +790,6 @@ static ssize_t attr_current_value_store(struct kobject *kobj,
>>  	if (value < capdata.min_value || value > capdata.max_value)
>>  		return -EINVAL;
>>  
>> -	args.arg0 = attribute_id;
>>  	args.arg1 = value;
>>  
>>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_SET,
>> @@ -836,7 +823,6 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	struct lwmi_om_priv *priv = dev_get_drvdata(tunable_attr->dev);
>>  	struct wmi_method_args_32 args = { 0x0, 0x0 };
>>  	enum thermal_mode mode;
>> -	u32 attribute_id;
>>  	int retval;
>>  	int ret;
>>  
>> @@ -844,13 +830,8 @@ static ssize_t attr_current_value_show(struct kobject *kobj,
>>  	if (ret)
>>  		return ret;
>>  
>> -	attribute_id =
>> -		FIELD_PREP(LWMI_ATTR_DEV_ID_MASK, tunable_attr->device_id) |
>> -		FIELD_PREP(LWMI_ATTR_FEAT_ID_MASK, tunable_attr->feature_id) |
>> -		FIELD_PREP(LWMI_ATTR_MODE_ID_MASK, mode) |
>> -		FIELD_PREP(LWMI_ATTR_TYPE_ID_MASK, tunable_attr->type_id);
>> -
>> -	args.arg0 = attribute_id;
>> +	args.arg0 = lwmi_attr_id(tunable_attr->device_id, tunable_attr->feature_id,
>> +				 mode, tunable_attr->type_id);
>>  
>>  	ret = lwmi_dev_evaluate_int(priv->wdev, 0x0, LWMI_FEATURE_VALUE_GET,
>>  				    (unsigned char *)&args, sizeof(args),