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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On 10/7/20 9:58 PM, Limonciello, Mario wrote:
> >> -----Original Message-----
> >> From: Gerardo Esteban Malazdrewicz <gerardo@xxxxxxxxxxxxxxxxxxx>
> >> 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?

If that's the case, I would ask why this driver even auto-loaded?
The module load table is very prescriptive.
https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-smbios-wmi.c#L277
https://github.com/torvalds/linux/blob/master/drivers/platform/x86/dell-smbios-smm.c#L56
Was it because it was compiled into the kernel?

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

I have no opposition to dropping that pr_err or at least downgrading
it to pr_debug.

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





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux