Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid OEM String

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 10/7/20 9:58 PM, Limonciello, Mario wrote:
>> -----Original Message-----
>> From: Gerardo Esteban Malazdrewicz <gerardo@malazdrewicz.com.ar>
>> Sent: Wednesday, October 7, 2020 14:55
>> To: Limonciello, Mario; Pali Rohár; Hans de Goede
>> Subject: Re: [ PATCH v2 1/1] dell smbios driver : Consider Alienware a valid
>> OEM String
>>
>>
>> [EXTERNAL EMAIL]
>>
>> El mié, 07-10-2020 a las 15:53 +0000, Limonciello, Mario escribió:
>>>> Hans, there are more drivers which checks for Dell DMI strings.
>>>> Probably
>>>> it would be needed to update Alienware on more places, not only in
>>>> dell-smbios-base.c driver.
>>>
>>> I would prefer that each of those be checked on a case by case basis
>>> and only
>>> added if actually necessary.  Gerardo if you can please check any
>>> other drivers
>>> that should need this string added to their allow list.
>>
>> I didn't find other instances of that string in this subsystem, but see
>> below.
>>
>> There is one in pci, another in hotplug.
>>
>> However, this is an extract from kernel logs:
>>
>> [  138.093686] dell-smbios A80593CE-A997-11DA-B012-B622A1EF5492: WMI
>> SMBIOS userspace interface not supported(0), try upgrading to a newer
>> BIOS
> 
> Considering that messaging - does the non-WMI interface actually work?
> dell-smbios has two backends available.

Yes that is a very good question.

Gerardo, I guess you started looking into this because of the:

	pr_err("Unable to run on non-Dell system\n");

In dell-smbios-base.c triggering on your system?

As Pali mentioned in another mail, you probably should
be looking at the dell-laptop code, which also has a
has a DMI string check and which uses the dell-smbios code,
another consumer of the dell-smbios code is the dell-wmi
driver.

If neither of those drivers add additional functionality
(e.g. extra hotkey events, being able the control the kbd
backlight), then the right fix might be to silence the
error you see being thrown by dell-smbios-base.c, rather
then allowing it to load.

For now I'll drop your patch from my review-hans branch,
as we first need to clear this up.

> The SMI based backend you can check by using dcdbas.
> 
> I had presumed from your patch that it actually worked.
> 
>> [ 1275.987716] dell_smm_hwmon: not running on a supported Dell system.
>> [ 1275.987734] dell_smm_hwmon: vendor=Alienware, model=Alienware Area-
>> 51m R2, version=1.3.0
>>
>>
>> dell_smm_hwmon ignore_dmi=1
>>
>> /sys/class/hwmon/hwmonX/pwm{1,3} access correctly the left and right
>> fan, respectively

Ok, so that looks good and fixing the DMI check there probably
makes sense. Note that this code is independent from the
dell-smbios code from drivers/platform/x86 so you can
do another patch to fix the DMI check there independent of the
dell-smbios discussion.

Regards,

Hans