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

[Armin Wolf]

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@xxxxxx>
> > ---
> >   .../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)?

Personally i see little benefit in having the assignment handled by a macro.

Thanks,
Armin Wolf