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

Ilpo Järvinen
2026-04-01T10:06:32Z

[-- Attachment #1: Type: text/plain, Size: 4142 bytes --]

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?

> 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.