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

[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