Re: [PATCH v3 07/12] platform/x86: lenovo-wmi-capdata: Adopt new WMI API

[Mark Pearson]

On Tue, Oct 13, 2026, at 3:21 PM, Rong Zhang wrote:
> The new WMI API supports multiple ACPI types by converting them into a
> unified buffer that satisfies alignment and size requirements.
>
> Adopt it to make our life easier.
>
> Note that the new WMI API only accepts a few ACPI types to conform to
> the behavior of the Windows WMI-ACPI driver. By adopting the new API, we
> intentionally rejects improper ACPI types instead of silently ignoring
> them.
>
> Meanwhile, considering that `struct_size(block, data, count * 3)' may
> overflow when calculating `count * 3', ignore Fan Test Data with count >
> U8_MAX instead of caping `count'.
>

Minor typo: caping -> capping

> Reviewed-by: Armin Wolf <W_Armin@xxxxxx>
> Tested-by: Navon John Lukose <navonjohnlukose@xxxxxxxxx>
> Reviewed-by: Derek J. Clark <derekjohn.clark@xxxxxxxxx>
> Signed-off-by: Rong Zhang <i@xxxxxxxx>
> ---
> Changes in v3:
> - Leave struct cd_fan_block's __packed attribute as is (thanks Armin
>   Wolf)
> ---
>  drivers/platform/x86/lenovo/wmi-capdata.c | 70 ++++++++++++++-----------------
>  1 file changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/wmi-capdata.c 
> b/drivers/platform/x86/lenovo/wmi-capdata.c
> index 3c09bb9e3d46..abf5ffda7163 100644
> --- a/drivers/platform/x86/lenovo/wmi-capdata.c
> +++ b/drivers/platform/x86/lenovo/wmi-capdata.c
> @@ -626,17 +626,19 @@ static int __lwmi_cd_cache(struct lwmi_cd_priv 
> *priv)
>  	}
> 
>  	for (idx = 0; idx < priv->list->count; idx++, p += size) {
> -		union acpi_object *ret_obj __free(kfree) = NULL;
> +		struct wmi_buffer wbuf;
> +		int ret;
> 
> -		ret_obj = wmidev_block_query(priv->wdev, idx);
> -		if (!ret_obj)
> -			return -ENODEV;
> -
> -		if (ret_obj->type != ACPI_TYPE_BUFFER ||
> -		    ret_obj->buffer.length < size)
> +		ret = wmidev_query_block(priv->wdev, idx, &wbuf, size);
> +		if (ret == -ENODATA) /* The block is too short, probably stubbed. */
>  			continue;
> +		if (ret)
> +			return ret;
> +
> +		/* Capdata 01 is an extension to capdata 00. */
> +		struct capdata00 *capdata __free(kfree) = wbuf.data;
> 
> -		memcpy(p, ret_obj->buffer.pointer, size);
> +		memcpy(p, capdata, size);
>  	}
> 
>  	return 0;
> @@ -678,43 +680,34 @@ static int lwmi_cd_cache(struct lwmi_cd_priv *priv)
>   */
>  static int lwmi_cd_fan_list_alloc_cache(struct lwmi_cd_priv *priv)
>  {
> +	struct wmi_buffer wbuf;
>  	struct cd_list *list;
> -	size_t size;
> +	int ret, idx;
>  	u32 count;
> -	int idx;
> 
> -	/* Emit unaligned access to u8 buffer with __packed. */
>  	struct cd_fan_block {
>  		u32 nr;
>  		u32 data[]; /* id[nr], max_rpm[nr], min_rpm[nr] */
> -	} __packed * block;
> +	} __packed;
> 
> -	union acpi_object *ret_obj __free(kfree) = wmidev_block_query(priv->wdev, 0);
> -	if (!ret_obj)
> -		return -ENODEV;
> +	ret = wmidev_query_block(priv->wdev, 0, &wbuf, sizeof(struct cd_fan_block));
> +	if (ret == -ENODATA) /* The block is too short, probably stubbed. */
> +		return 0;
> +	if (ret)
> +		return ret;
> 
> -	if (ret_obj->type == ACPI_TYPE_BUFFER) {
> -		block = (struct cd_fan_block *)ret_obj->buffer.pointer;
> -		size = ret_obj->buffer.length;
> +	struct cd_fan_block *block __free(kfree) = wbuf.data;
> 
> -		count = size >= sizeof(*block) ? block->nr : 0;
> -		if (size < struct_size(block, data, count * 3)) {
> -			dev_warn(&priv->wdev->dev,
> -				 "incomplete fan test data block: %zu < %zu, ignoring\n",
> -				 size, struct_size(block, data, count * 3));
> -			count = 0;
> -		} else if (count > U8_MAX) {
> -			dev_warn(&priv->wdev->dev,
> -				 "too many fans reported: %u > %u, truncating\n",
> -				 count, U8_MAX);
> -			count = U8_MAX;
> -		}
> -	} else {
> -		/*
> -		 * This is usually caused by a dummy ACPI method. Do not return an error
> -		 * as failing to probe this device will result in sub-master device being
> -		 * unbound. This behavior aligns with lwmi_cd_cache().
> -		 */
> +	count = block->nr;
> +
> +	if (count > U8_MAX) {
> +		dev_warn(&priv->wdev->dev,
> +			 "too many fans reported: %u > %u, ignoring\n", count, U8_MAX);
> +		count = 0;
> +	} else if (wbuf.length < struct_size(block, data, count * 3)) {
> +		dev_warn(&priv->wdev->dev,
> +			 "incomplete fan test data block: %zu < %zu (%u fans), ignoring\n",
> +			 wbuf.length, struct_size(block, data, count * 3), count);
>  		count = 0;
>  	}
> 
> @@ -729,11 +722,10 @@ static int lwmi_cd_fan_list_alloc_cache(struct 
> lwmi_cd_priv *priv)
>  	priv->list = list;
> 
>  	for (idx = 0; idx < count; idx++) {
> -		/* Do not calculate array index using count, as it may be truncated. */
>  		list->cd_fan[idx] = (struct capdata_fan) {
>  			.id      = block->data[idx],
> -			.max_rpm = block->data[idx + block->nr],
> -			.min_rpm = block->data[idx + (2 * block->nr)],
> +			.max_rpm = block->data[idx + count],
> +			.min_rpm = block->data[idx + (2 * count)],
>  		};
>  	}
> 
>
> -- 
> 2.55.0

Reviewed-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Mark