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.
next prev parent 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®