Re: [PATCH v3 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

Armin Wolf
2026-04-01T17:31:50Z

Am 01.04.26 um 12:06 schrieb Ilpo Järvinen:

> On Tue, 31 Mar 2026, Armin Wolf wrote:
>
>> Am 31.03.26 um 11:44 schrieb Ilpo Järvinen:
>>
>>> On Sat, 14 Mar 2026, Armin Wolf wrote:
>>>
>>>> Use the new buffer-based WMI API to also support ACPI firmware
>>>> implementations that do not use ACPI buffers for the descriptor.
>>>>
>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>> ---
>>>> .../platform/x86/dell/dell-wmi-descriptor.c | 94 +++++++++----------
>>>> 1 file changed, 47 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> index c2a180202719..fe42eb8bbd79 100644
>>>> --- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> +++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
>>>> @@ -7,7 +7,7 @@
>>>> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>>> -#include <linux/acpi.h>
>>>> +#include <linux/compiler_attributes.h>
>>>> #include <linux/list.h>
>>>> #include <linux/module.h>
>>>> #include <linux/wmi.h>
>>>> @@ -15,6 +15,24 @@
>>>> #define DELL_WMI_DESCRIPTOR_GUID
>>>> "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
>>>> +/*
>>>> + * Descriptor buffer is 128 byte long and contains:
>>>> + *
>>>> + * Name Offset Length Value
>>>> + * Vendor Signature 0 4 "DELL"
>>>> + * Object Signature 4 4 " WMI"
>>>> + * WMI Interface Version 8 4 <version>
>>>> + * WMI buffer length 12 4 <length>
>>>> + * WMI hotfix number 16 4 <hotfix>
>>>> + */
>>>> +struct descriptor {
>>>> + char vendor_signature[4];
>>>> + char object_signature[4];
>>>> + __le32 interface_version;
>>>> + __le32 buffer_length;
>>>> + __le32 hotfix_number;
>>>> +} __packed;
>>>> +
>>>> struct descriptor_priv {
>>>> struct list_head list;
>>>> u32 interface_version;
>>>> @@ -88,76 +106,58 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
>>>> }
>>>> EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
>>>> -/*
>>>> - * Descriptor buffer is 128 byte long and contains:
>>>> - *
>>>> - * Name Offset Length Value
>>>> - * Vendor Signature 0 4 "DELL"
>>>> - * Object Signature 4 4 " WMI"
>>>> - * WMI Interface Version 8 4 <version>
>>>> - * WMI buffer length 12 4 <length>
>>>> - * WMI hotfix number 16 4 <hotfix>
>>>> - */
>>>> -static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>>>> - const void *context)
>>>> +static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void
>>>> *context)
>>>> {
>>>> - union acpi_object *obj = NULL;
>>>> struct descriptor_priv *priv;
>>>> - u32 *buffer;
>>>> + struct wmi_buffer buffer;
>>>> + struct descriptor *desc;
>>>> int ret;
>>>> - obj = wmidev_block_query(wdev, 0);
>>>> - if (!obj) {
>>>> - dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
>>>> - ret = -EIO;
>>>> - goto out;
>>>> - }
>>>> + ret = wmidev_query_block(wdev, 0, &buffer);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> - if (obj->type != ACPI_TYPE_BUFFER) {
>>>> - dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
>>>> + if (buffer.length < sizeof(*desc)) {
>>> Hi again,
>>>
>>> Thinking some more of this...
>>>
>>> Since many of these drivers will need to do this size check and assign
>>> into some variable that is different from the one passed to
>>> wmidev_query_block(), would it make sense to move those into generic code?
>>> It probably requires a macro so you'd have access to type information.
>> You mean something like wmidev_block_query_sized(wdev, instance, size)?
> So this would return ERR_PTR() as I see no out param given to it?

My mistake, i meant something like wmidev_block_query_sized(wdev, instance, buffer, size).
This has the same purpose as acpi_evaluate_object_typed(), where the error handling can
often be simplified.

>
>> Personally i see little benefit in having the assignment handled by a macro.
> I was not so interested in assignment itself but getting the size
> information from the output type directly but if you prefer writing
> sizeof(*xx), not a big deal to me.

I would prefer writing sizeof(*var). I will resend this series with the new *_sized()
WMI helper functions and the __free()-related changes.

Thanks,
Armin Wolf