Re: [PATCH 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API
From: Mario Limonciello
Date: Mon Mar 09 2026 - 21:56:34 EST
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:When I read your comment, I come to another idea. What about introducing
+ /* "DELL" */Do you think you could come up with a helper for matching an expected
+ if (le32_to_cpu(desc->vendor_signature) != 0x4C4C4544) {
"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);
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;
}