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

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

> 
> > +		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;
> >   	}