mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Rong Zhang <i@rong.moe>
Cc: Mark Pearson <mpearson-lenovo@squebb.ca>,
	 "Derek J. Clark" <derekjohn.clark@gmail.com>,
	 Hans de Goede <hansg@kernel.org>, Armin Wolf <W_Armin@gmx.de>,
	 Charles <hanker007@gmail.com>,
	 Navon John Lukose <navonjohnlukose@gmail.com>,
	 platform-driver-x86@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 07/12] platform/x86: lenovo-wmi-capdata: Adopt new WMI API
Date: Mon, 19 Oct 2026 09:46:45 +0300 (EEST)	[thread overview]
Message-ID: <e67534db-c29c-4f89-9b7d-cf9ff5704674@linux.intel.com> (raw)
In-Reply-To: <20261018-lwmi-wmi-new-api-v4-7-aafef7956c4a@rong.moe>

On Sun, 18 Oct 2026, 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 capping `count'.
> 
> Reviewed-by: Armin Wolf <W_Armin@gmx.de>
> Tested-by: Navon John Lukose <navonjohnlukose@gmail.com>
> Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com>
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> Changes in v4:
> - Fix typo (thanks Mark Pearson)
> 
> 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;

Could this just be moved outside of the function?

> -	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)],
>  		};
>  	}
>  
> 
> 

-- 
 i.


  reply	other threads:[~2026-10-19  6:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-17 18:42 [PATCH v4 00/12] platform/x86: lenovo-wmi-{other,capdata,helpers}: Improve robustness on faulty firmware Rong Zhang
2026-10-17 18:42 ` [PATCH v4 01/12] platform/wmi: Introduce wmidev_exists() Rong Zhang
2026-10-17 18:42 ` [PATCH v4 02/12] platform/x86: lenovo-wmi-capdata: Do not stop the AC notifier chain on error Rong Zhang
2026-10-17 18:42 ` [PATCH v4 03/12] platform/x86: lenovo-wmi-capdata: Only allocate sub-master info when necessary Rong Zhang
2026-10-17 18:42 ` [PATCH v4 04/12] platform/x86: lenovo-wmi-capdata: Store a pointer to component info Rong Zhang
2026-10-17 18:42 ` [PATCH v4 05/12] platform/x86: lenovo-wmi-capdata: Defer mutex initialization Rong Zhang
2026-10-17 18:42 ` [PATCH v4 06/12] platform/x86: lenovo-wmi-{capdata,other}: Only allocate capdata list when necessary Rong Zhang
2026-10-17 18:42 ` [PATCH v4 07/12] platform/x86: lenovo-wmi-capdata: Adopt new WMI API Rong Zhang
2026-10-19  6:46   ` Ilpo Järvinen [this message]
2026-10-19 11:54     ` Rong Zhang
2026-10-17 18:42 ` [PATCH v4 08/12] platform/x86: lenovo-wmi-capdata: Register component even on WMI error Rong Zhang
2026-10-17 18:42 ` [PATCH v4 09/12] platform/x86: lenovo-wmi-capdata: Detect stubbed capdata device Rong Zhang
2026-10-17 18:42 ` [PATCH v4 10/12] platform/x86: lenovo-wmi-capdata: Do not match missing components Rong Zhang
2026-10-17 18:42 ` [PATCH v4 11/12] platform/x86: lenovo-wmi-helpers: Adopt new WMI API Rong Zhang
2026-10-17 18:42 ` [PATCH v4 12/12] MAINTAINERS: Add myself as a LENOVO drivers co-maintainer Rong Zhang

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=e67534db-c29c-4f89-9b7d-cf9ff5704674@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=W_Armin@gmx.de \
    --cc=derekjohn.clark@gmail.com \
    --cc=hanker007@gmail.com \
    --cc=hansg@kernel.org \
    --cc=i@rong.moe \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpearson-lenovo@squebb.ca \
    --cc=navonjohnlukose@gmail.com \
    --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

all inboxes | Powered by JetHome®