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

Armin Wolf posted 9 patches 1 month ago
There is a newer version of this series
[PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
Posted by Armin Wolf 1 month ago
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   | 96 ++++++++++---------
 1 file changed, 49 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..621502368895 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 {
+	__le32 vendor_signature;
+	__le32 object_signature;
+	__le32 interface_version;
+	__le32 buffer_length;
+	__le32 hotfix_number;
+} __packed;
+
 struct descriptor_priv {
 	struct list_head list;
 	u32 interface_version;
@@ -88,76 +106,60 @@ 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)) {
+		dev_err(&wdev->dev,
+			"Dell descriptor buffer contains not enough data (%zu)\n",
+			buffer.length);
 		ret = -EINVAL;
 		descriptor_valid = ret;
 		goto out;
 	}
 
-	/* Although it's not technically a failure, this would lead to
-	 * unexpected behavior
-	 */
-	if (obj->buffer.length != 128) {
-		dev_err(&wdev->dev,
-			"Dell descriptor buffer has unexpected length (%d)\n",
-			obj->buffer.length);
-		ret = -EINVAL;
+	desc = buffer.data;
+
+	/* "DELL" */
+	if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%u)\n",
+			le32_to_cpu(desc->vendor_signature));
+		ret = -ENOMSG;
 		descriptor_valid = ret;
 		goto out;
 	}
 
-	buffer = (u32 *)obj->buffer.pointer;
-
-	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
-		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
-			buffer);
-		ret = -EINVAL;
+	/* " WMI" */
+	if (le32_to_cpu(desc->object_signature) != 0x494D5720) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%u)\n",
+			le32_to_cpu(desc->object_signature));
+		ret = -ENOMSG;
 		descriptor_valid = ret;
 		goto out;
 	}
 	descriptor_valid = 0;
 
-	if (buffer[2] != 0 && buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
-			(unsigned long) buffer[2]);
-
-	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
-	GFP_KERNEL);
+	if (le32_to_cpu(desc->interface_version) > 2)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
+			 le32_to_cpu(desc->interface_version));
 
+	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv) {
 		ret = -ENOMEM;
 		goto out;
 	}
 
-	priv->interface_version = buffer[2];
-	priv->size = buffer[3];
-	priv->hotfix = buffer[4];
+	priv->interface_version = le32_to_cpu(desc->interface_version);
+	priv->size = le32_to_cpu(desc->buffer_length);
+	priv->hotfix = le32_to_cpu(desc->hotfix_number);
 	ret = 0;
 	dev_set_drvdata(&wdev->dev, priv);
 	mutex_lock(&list_mutex);
@@ -170,7 +172,7 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
 		(unsigned long) priv->hotfix);
 
 out:
-	kfree(obj);
+	kfree(buffer.data);
 	return ret;
 }
 
-- 
2.39.5
Re: [PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
Posted by Mario Limonciello 1 month ago

On 3/7/2026 6:25 PM, 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   | 96 ++++++++++---------
>   1 file changed, 49 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..621502368895 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 {
> +	__le32 vendor_signature;
> +	__le32 object_signature;
> +	__le32 interface_version;
> +	__le32 buffer_length;
> +	__le32 hotfix_number;
> +} __packed;
> +
>   struct descriptor_priv {
>   	struct list_head list;
>   	u32 interface_version;
> @@ -88,76 +106,60 @@ 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)) {
> +		dev_err(&wdev->dev,
> +			"Dell descriptor buffer contains not enough data (%zu)\n",
> +			buffer.length);
>   		ret = -EINVAL;
>   		descriptor_valid = ret;
>   		goto out;
>   	}
>   
> -	/* Although it's not technically a failure, this would lead to
> -	 * unexpected behavior
> -	 */
> -	if (obj->buffer.length != 128) {
> -		dev_err(&wdev->dev,
> -			"Dell descriptor buffer has unexpected length (%d)\n",
> -			obj->buffer.length);
> -		ret = -EINVAL;
> +	desc = buffer.data;
> +
> +	/* "DELL" */
> +	if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {

Do you think you could come up with a helper for matching an expected 
"string" like this?  I /suspect/ it's going to be a common pattern that 
vendors use and it will increase code readability going forward if a 
helper is possible.  I see it at least twice in this patch alone.

Something like this prototype:

bool wmi_valid_signature(u32 field, const char* expected_str);


> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%u)\n",
> +			le32_to_cpu(desc->vendor_signature));
> +		ret = -ENOMSG;
>   		descriptor_valid = ret;
>   		goto out;
>   	}
>   
> -	buffer = (u32 *)obj->buffer.pointer;
> -
> -	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
> -		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
> -			buffer);
> -		ret = -EINVAL;
> +	/* " WMI" */
> +	if (le32_to_cpu(desc->object_signature) != 0x494D5720) {
> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%u)\n",
> +			le32_to_cpu(desc->object_signature));
> +		ret = -ENOMSG;
>   		descriptor_valid = ret;
>   		goto out;
>   	}
>   	descriptor_valid = 0;
>   
> -	if (buffer[2] != 0 && buffer[2] != 1)
> -		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
> -			(unsigned long) buffer[2]);
> -
> -	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
> -	GFP_KERNEL);
> +	if (le32_to_cpu(desc->interface_version) > 2)
> +		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
> +			 le32_to_cpu(desc->interface_version));
>   
> +	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
>   	if (!priv) {
>   		ret = -ENOMEM;
>   		goto out;
>   	}
>   
> -	priv->interface_version = buffer[2];
> -	priv->size = buffer[3];
> -	priv->hotfix = buffer[4];
> +	priv->interface_version = le32_to_cpu(desc->interface_version);
> +	priv->size = le32_to_cpu(desc->buffer_length);
> +	priv->hotfix = le32_to_cpu(desc->hotfix_number);
>   	ret = 0;
>   	dev_set_drvdata(&wdev->dev, priv);
>   	mutex_lock(&list_mutex);
> @@ -170,7 +172,7 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
>   		(unsigned long) priv->hotfix);
>   
>   out:
> -	kfree(obj);
> +	kfree(buffer.data);
>   	return ret;
>   }
>
Re: [PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
Posted by Pali Rohár 1 month ago
On Monday 09 March 2026 10:41:20 Mario Limonciello wrote:
> On 3/7/2026 6:25 PM, Armin Wolf wrote:
> > +	/* "DELL" */
> > +	if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {
> 
> Do you think you could come up with a helper for matching an expected
> "string" like this?  I /suspect/ it's going to be a common pattern that
> vendors use and it will increase code readability going forward if a helper
> is possible.  I see it at least twice in this patch alone.
> 
> Something like this prototype:
> 
> bool wmi_valid_signature(u32 field, const char* expected_str);

When I read your comment, I come to another idea. What about introducing
a macro which will convert 4-byte string to u32 number? For example:

  #define str_to_u32(str) ({ _Static_assert(__builtin_types_compatible_p(__typeof__(str), char[5]), "wrong type"); (u32)(u8)(str)[0] | ((u32)((u8)(str)[1]) << 8) | ((u32)((u8)(str)[2]) << 16) | ((u32)((u8)(str)[3]) << 24); })

The whole conversion would be done in compile time (with -O2) so should
not cause any possible performance issues.

With it, the condition could be written as:

  if (le32_to_cpu(desc->vendor_signature) != str_to_u32("DELL")) {

and no need to use magic number 0x4C4C4544 and write comment that this
number in ASCII is the "DELL" string.

> 
> > +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%u)\n",
> > +			le32_to_cpu(desc->vendor_signature));
> > +		ret = -ENOMSG;
> >   		descriptor_valid = ret;
> >   		goto out;
> >   	}
> > -	buffer = (u32 *)obj->buffer.pointer;
> > -
> > -	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
> > -		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
> > -			buffer);
> > -		ret = -EINVAL;
> > +	/* " WMI" */
> > +	if (le32_to_cpu(desc->object_signature) != 0x494D5720) {
> > +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%u)\n",
> > +			le32_to_cpu(desc->object_signature));
> > +		ret = -ENOMSG;
> >   		descriptor_valid = ret;
> >   		goto out;
> >   	}
Re: [PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
Posted by Armin Wolf 1 month ago
Am 09.03.26 um 18:23 schrieb Pali Rohár:

> On Monday 09 March 2026 10:41:20 Mario Limonciello wrote:
>> On 3/7/2026 6:25 PM, Armin Wolf wrote:
>>> +	/* "DELL" */
>>> +	if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {
>> Do you think you could come up with a helper for matching an expected
>> "string" like this?  I /suspect/ it's going to be a common pattern that
>> vendors use and it will increase code readability going forward if a helper
>> is possible.  I see it at least twice in this patch alone.
>>
>> Something like this prototype:
>>
>> bool wmi_valid_signature(u32 field, const char* expected_str);
> When I read your comment, I come to another idea. What about introducing
> a macro which will convert 4-byte string to u32 number? For example:
>
>    #define str_to_u32(str) ({ _Static_assert(__builtin_types_compatible_p(__typeof__(str), char[5]), "wrong type"); (u32)(u8)(str)[0] | ((u32)((u8)(str)[1]) << 8) | ((u32)((u8)(str)[2]) << 16) | ((u32)((u8)(str)[3]) << 24); })
>
> The whole conversion would be done in compile time (with -O2) so should
> not cause any possible performance issues.
>
> With it, the condition could be written as:
>
>    if (le32_to_cpu(desc->vendor_signature) != str_to_u32("DELL")) {
>
> and no need to use magic number 0x4C4C4544 and write comment that this
> number in ASCII is the "DELL" string.

To be honest i do nothing think that having a helper function for this inside the WMI driver core
is useful, especially since most vendors other than Dell do not use such magic numbers.

 From my perspective assembling the two constants ourself is fine here.

Thanks,
Armin Wolf

>
>>> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%u)\n",
>>> +			le32_to_cpu(desc->vendor_signature));
>>> +		ret = -ENOMSG;
>>>    		descriptor_valid = ret;
>>>    		goto out;
>>>    	}
>>> -	buffer = (u32 *)obj->buffer.pointer;
>>> -
>>> -	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
>>> -		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
>>> -			buffer);
>>> -		ret = -EINVAL;
>>> +	/* " WMI" */
>>> +	if (le32_to_cpu(desc->object_signature) != 0x494D5720) {
>>> +		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%u)\n",
>>> +			le32_to_cpu(desc->object_signature));
>>> +		ret = -ENOMSG;
>>>    		descriptor_valid = ret;
>>>    		goto out;
>>>    	}
Re: [PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
Posted by Gergo Koteles 1 month ago
Hi Armin,

On Mon, 2026-03-09 at 20:45 +0100, Armin Wolf wrote:
> Am 09.03.26 um 18:23 schrieb Pali Rohár:
> 
> > On Monday 09 March 2026 10:41:20 Mario Limonciello wrote:
> > > On 3/7/2026 6:25 PM, Armin Wolf wrote:
> > > > +	/* "DELL" */
> > > > +	if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {
> > > Do you think you could come up with a helper for matching an expected
> > > "string" like this?  I /suspect/ it's going to be a common pattern that
> > > vendors use and it will increase code readability going forward if a helper
> > > is possible.  I see it at least twice in this patch alone.
> > > 
> > > Something like this prototype:
> > > 
> > > bool wmi_valid_signature(u32 field, const char* expected_str);
> > When I read your comment, I come to another idea. What about introducing
> > a macro which will convert 4-byte string to u32 number? For example:
> > 
> >    #define str_to_u32(str) ({ _Static_assert(__builtin_types_compatible_p(__typeof__(str), char[5]), "wrong type"); (u32)(u8)(str)[0] | ((u32)((u8)(str)[1]) << 8) | ((u32)((u8)(str)[2]) << 16) | ((u32)((u8)(str)[3]) << 24); })
> > 
> > The whole conversion would be done in compile time (with -O2) so should
> > not cause any possible performance issues.
> > 
> > With it, the condition could be written as:
> > 
> >    if (le32_to_cpu(desc->vendor_signature) != str_to_u32("DELL")) {
> > 
> > and no need to use magic number 0x4C4C4544 and write comment that this
> > number in ASCII is the "DELL" string.
> 
> To be honest i do nothing think that having a helper function for this inside the WMI driver core
> is useful, especially since most vendors other than Dell do not use such magic numbers.
> 
>  From my perspective assembling the two constants ourself is fine here.
> 

But what about the other readers? :)

Why don't you use a char array for the descriptors?

struct descriptor {
	char vendor_signature[4];
	char object_signature[4];
	__le32 interface_version;
	__le32 buffer_length;
	__le32 hotfix_number;
} __packed;

Best regards,
Gergo Koteles
Re: [PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
Posted by Armin Wolf 3 weeks, 4 days ago
Am 10.03.26 um 11:46 schrieb Gergo Koteles:

> Hi Armin,
>
> On Mon, 2026-03-09 at 20:45 +0100, Armin Wolf wrote:
>> Am 09.03.26 um 18:23 schrieb Pali Rohár:
>>
>>> On Monday 09 March 2026 10:41:20 Mario Limonciello wrote:
>>>> On 3/7/2026 6:25 PM, Armin Wolf wrote:
>>>>> +	/* "DELL" */
>>>>> +	if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {
>>>> Do you think you could come up with a helper for matching an expected
>>>> "string" like this?  I /suspect/ it's going to be a common pattern that
>>>> vendors use and it will increase code readability going forward if a helper
>>>> is possible.  I see it at least twice in this patch alone.
>>>>
>>>> Something like this prototype:
>>>>
>>>> bool wmi_valid_signature(u32 field, const char* expected_str);
>>> When I read your comment, I come to another idea. What about introducing
>>> a macro which will convert 4-byte string to u32 number? For example:
>>>
>>>     #define str_to_u32(str) ({ _Static_assert(__builtin_types_compatible_p(__typeof__(str), char[5]), "wrong type"); (u32)(u8)(str)[0] | ((u32)((u8)(str)[1]) << 8) | ((u32)((u8)(str)[2]) << 16) | ((u32)((u8)(str)[3]) << 24); })
>>>
>>> The whole conversion would be done in compile time (with -O2) so should
>>> not cause any possible performance issues.
>>>
>>> With it, the condition could be written as:
>>>
>>>     if (le32_to_cpu(desc->vendor_signature) != str_to_u32("DELL")) {
>>>
>>> and no need to use magic number 0x4C4C4544 and write comment that this
>>> number in ASCII is the "DELL" string.
>> To be honest i do nothing think that having a helper function for this inside the WMI driver core
>> is useful, especially since most vendors other than Dell do not use such magic numbers.
>>
>>   From my perspective assembling the two constants ourself is fine here.
>>
> But what about the other readers? :)
>
> Why don't you use a char array for the descriptors?
>
> struct descriptor {
> 	char vendor_signature[4];
> 	char object_signature[4];
> 	__le32 interface_version;
> 	__le32 buffer_length;
> 	__le32 hotfix_number;
> } __packed;
>
> Best regards,
> Gergo Koteles
>
Fine, i will model the vendor and object signatures using char arrays.

Thanks,
Armin Wolf
Re: [PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
Posted by Mario Limonciello 1 month ago

On 3/9/2026 2:45 PM, Armin Wolf wrote:
> Am 09.03.26 um 18:23 schrieb Pali Rohár:
> 
>> On Monday 09 March 2026 10:41:20 Mario Limonciello wrote:
>>> On 3/7/2026 6:25 PM, Armin Wolf wrote:
>>>> +    /* "DELL" */
>>>> +    if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {
>>> Do you think you could come up with a helper for matching an expected
>>> "string" like this?  I /suspect/ it's going to be a common pattern that
>>> vendors use and it will increase code readability going forward if a 
>>> helper
>>> is possible.  I see it at least twice in this patch alone.
>>>
>>> Something like this prototype:
>>>
>>> bool wmi_valid_signature(u32 field, const char* expected_str);
>> When I read your comment, I come to another idea. What about introducing
>> a macro which will convert 4-byte string to u32 number? For example:
>>
>>    #define str_to_u32(str) 
>> ({ _Static_assert(__builtin_types_compatible_p(__typeof__(str), 
>> char[5]), "wrong type"); (u32)(u8)(str)[0] | ((u32)((u8)(str)[1]) << 
>> 8) | ((u32)((u8)(str)[2]) << 16) | ((u32)((u8)(str)[3]) << 24); })
>>
>> The whole conversion would be done in compile time (with -O2) so should
>> not cause any possible performance issues.
>>
>> With it, the condition could be written as:
>>
>>    if (le32_to_cpu(desc->vendor_signature) != str_to_u32("DELL")) {
>>
>> and no need to use magic number 0x4C4C4544 and write comment that this
>> number in ASCII is the "DELL" string.
> 
> To be honest i do nothing think that having a helper function for this 
> inside the WMI driver core
> is useful, especially since most vendors other than Dell do not use such 
> magic numbers.
> 
>  From my perspective assembling the two constants ourself is fine here.
> 
> Thanks,
> Armin Wolf

Oh I didn't realize no one else is doing this.  Yeah if it's Dell only, 
agree - meh.


> 
>>
>>>> +        dev_err(&wdev->dev, "Dell descriptor buffer has invalid 
>>>> vendor signature (%u)\n",
>>>> +            le32_to_cpu(desc->vendor_signature));
>>>> +        ret = -ENOMSG;
>>>>            descriptor_valid = ret;
>>>>            goto out;
>>>>        }
>>>> -    buffer = (u32 *)obj->buffer.pointer;
>>>> -
>>>> -    if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
>>>> -        dev_err(&wdev->dev, "Dell descriptor buffer has invalid 
>>>> signature (%8ph)\n",
>>>> -            buffer);
>>>> -        ret = -EINVAL;
>>>> +    /* " WMI" */
>>>> +    if (le32_to_cpu(desc->object_signature) != 0x494D5720) {
>>>> +        dev_err(&wdev->dev, "Dell descriptor buffer has invalid 
>>>> object signature (%u)\n",
>>>> +            le32_to_cpu(desc->object_signature));
>>>> +        ret = -ENOMSG;
>>>>            descriptor_valid = ret;
>>>>            goto out;
>>>>        }
>