diff mbox

dell_smbios in 4.15 and keyboard backlight

Message ID 4882553904ef48da9b12799ed7a7100c@ausx13mpc120.AMER.DELL.COM (mailing list archive)
State Changes Requested, archived
Delegated to: Darren Hart
Headers show

Commit Message

Limonciello, Mario Feb. 23, 2018, 7:53 p.m. UTC
> -----Original Message-----

> From: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com]

> Sent: Friday, February 23, 2018 1:41 PM

> To: Limonciello, Mario <Mario_Limonciello@Dell.com>; md@linux.it;

> dvhart@infradead.org

> Cc: pali.rohar@gmail.com; platform-driver-x86@vger.kernel.org

> Subject: Re: dell_smbios in 4.15 and keyboard backlight

> 

> On Fri, 2018-02-23 at 19:19 +0000, Mario.Limonciello@dell.com wrote:

> > > -----Original Message-----

> > > From: Marco d'Itri [mailto:md@linux.it]

> > > Sent: Friday, February 23, 2018 1:06 PM

> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>

> > > Cc: pali.rohar@gmail.com

> > > Subject: Re: dell_smbios in 4.15 and keyboard backlight

> > >

> > > On Feb 23, Mario Limonciello <superm1@gmail.com> wrote:

> > >

> > > > Can you please email me on mario.limonciello@dell.com for this

> > > > topic?

> > > >

> > > > I would be interested to know do you have dell-smbios-smm or

> > > > dell-smbios-wmi not loading or compiling?

> > >

> > > Oops... The new modules have not been enabled yet by the Debian

> > > kernel

> > > team, so this explains why I could not find them.

> > > Thank you for your help, I will get back to you if I will still have

> > > troubles with the new kernel.

> > >

> >

> > If you don't mind I'm going to CC this to the mailing list for

> > discussion.  I think

> > you identified a valid config problem.

> >

> > Andy, Darren,

> >

> > Marco (CC'ed) privately emailed Pali and I to discuss an issue that

> > dell-laptop

> > wasn't working properly for him and dell-smbios couldn't find a

> > backend.

> >

> > I thought at first it was an issue of the race condition recently

> > discussed but

> > it's actually a case that the distro kernel he's using compiled:

> >

> > DELL_SMBIOS

> > DELL_LAPTOP

> >

> > But didn't select DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.

> 

> Distros have to enable whatever they want to.

At least in this instance I'd hypothesize it's because these are new config
options that default to off.

They probably had DELL_SMBIOS enabled before and carried that forward
But there was nothing to transition them to make them turn on 
DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.

> 

> Can it be the Dell model, that survives w/o one of above or even both?


The design as it exists to day is that dell-laptop needs dell-smbios but
dell-smbios won't run unless it has a backend selected.

> 

> > I remember hypothesizing about this with one of the earlier versions

> > of the

> > patch series but the decision was to just hide config DELL_SMBIOS as

> > an invisible

> > tristate.

> 

> 

> > Because this is a real problem happening I think we need something

> > like this:

> >

> > diff --git a/drivers/platform/x86/Kconfig

> > b/drivers/platform/x86/Kconfig

> > index 9a8f964..f2f548b 100644

> > --- a/drivers/platform/x86/Kconfig

> > +++ b/drivers/platform/x86/Kconfig

> > @@ -107,6 +107,7 @@ config ASUS_LAPTOP

> >

> >  config DELL_SMBIOS

> >         tristate

> > +       depends on DELL_SMBIOS_WMI || DELL_SMBIOS_SMM

> 

> This will simple not work.

You're right, I tried it and got this:
warning: (DELL_SMBIOS_WMI && DELL_SMBIOS_SMM && DELL_LAPTOP && DELL_WMI) selects DELL_SMBIOS which has unmet direct dependencies (X86 && X86_PLATFORM_DEVICES && (DELL_SMBIOS_WMI || DELL_SMBIOS_SMM))
warning: (DELL_SMBIOS_WMI && DELL_SMBIOS_SMM && DELL_LAPTOP && DELL_WMI) selects DELL_SMBIOS which has unmet direct dependencies (X86 && X86_PLATFORM_DEVICES && (DELL_SMBIOS_WMI || DELL_SMBIOS_SMM))

but it still let me go forward.

Something like this maybe then to not let them even try to run dell-smbios?

Comments

Andy Shevchenko Feb. 23, 2018, 8:17 p.m. UTC | #1
On Fri, 2018-02-23 at 19:53 +0000, Mario.Limonciello@dell.com wrote:

> > > Marco (CC'ed) privately emailed Pali and I to discuss an issue
> > > that
> > > dell-laptop
> > > wasn't working properly for him and dell-smbios couldn't find a
> > > backend.
> > > 
> > > I thought at first it was an issue of the race condition recently
> > > discussed but
> > > it's actually a case that the distro kernel he's using compiled:
> > > 
> > > DELL_SMBIOS
> > > DELL_LAPTOP
> > > 
> > > But didn't select DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.
> > 
> > Distros have to enable whatever they want to.
> 
> At least in this instance I'd hypothesize it's because these are new
> config
> options that default to off.
> 
> They probably had DELL_SMBIOS enabled before and carried that forward
> But there was nothing to transition them to make them turn on 
> DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.

So, the driver requires now _at least_ one of the backend enabled?

> > Can it be the Dell model, that survives w/o one of above or even
> > both?
> 
> The design as it exists to day is that dell-laptop needs dell-smbios
> but
> dell-smbios won't run unless it has a backend selected.

How was it before?

> Something like this maybe then to not let them even try to run dell-
> smbios?

Wouldn't be a regression still?

P.S. See also the link I answered with to Pali.
Pali Rohár Feb. 23, 2018, 8:39 p.m. UTC | #2
On Friday 23 February 2018 22:17:30 Andy Shevchenko wrote:
> On Fri, 2018-02-23 at 19:53 +0000, Mario.Limonciello@dell.com wrote:
> 
> > > > Marco (CC'ed) privately emailed Pali and I to discuss an issue
> > > > that
> > > > dell-laptop
> > > > wasn't working properly for him and dell-smbios couldn't find a
> > > > backend.
> > > > 
> > > > I thought at first it was an issue of the race condition recently
> > > > discussed but
> > > > it's actually a case that the distro kernel he's using compiled:
> > > > 
> > > > DELL_SMBIOS
> > > > DELL_LAPTOP
> > > > 
> > > > But didn't select DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.
> > > 
> > > Distros have to enable whatever they want to.
> > 
> > At least in this instance I'd hypothesize it's because these are new
> > config
> > options that default to off.
> > 
> > They probably had DELL_SMBIOS enabled before and carried that forward
> > But there was nothing to transition them to make them turn on 
> > DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.
> 
> So, the driver requires now _at least_ one of the backend enabled?

Basically driver can be compiled with zero backends. But then every one
request would fail and I do not see any reason why somebody want such
combination on real hardware.

To make dell_smbios.ko work we need *correct* backend to be loaded. For
laptops >= 2017 it is needed dell_smbios_wmi.ko, For laptops <= 2007 it
is needed dell_smbios_smm.ko and for remaining both should work.

> > > Can it be the Dell model, that survives w/o one of above or even
> > > both?
> > 
> > The design as it exists to day is that dell-laptop needs dell-smbios
> > but
> > dell-smbios won't run unless it has a backend selected.
> 
> How was it before?

Before there was only dell_smbios_smm.ko and was part of dell_smbios.ko.
Now when SMM API started to be deprecated and stopped to work on new
laptops, it was needed to write new backend driver.

> > Something like this maybe then to not let them even try to run dell-
> > smbios?
> 
> Wouldn't be a regression still?
> 
> P.S. See also the link I answered with to Pali.
>
Limonciello, Mario Feb. 23, 2018, 8:41 p.m. UTC | #3
> -----Original Message-----

> From: Andy Shevchenko [mailto:andriy.shevchenko@linux.intel.com]

> Sent: Friday, February 23, 2018 2:18 PM

> To: Limonciello, Mario <Mario_Limonciello@Dell.com>; md@linux.it;

> dvhart@infradead.org

> Cc: pali.rohar@gmail.com; platform-driver-x86@vger.kernel.org

> Subject: Re: dell_smbios in 4.15 and keyboard backlight

> 

> On Fri, 2018-02-23 at 19:53 +0000, Mario.Limonciello@dell.com wrote:

> 

> > > > Marco (CC'ed) privately emailed Pali and I to discuss an issue

> > > > that

> > > > dell-laptop

> > > > wasn't working properly for him and dell-smbios couldn't find a

> > > > backend.

> > > >

> > > > I thought at first it was an issue of the race condition recently

> > > > discussed but

> > > > it's actually a case that the distro kernel he's using compiled:

> > > >

> > > > DELL_SMBIOS

> > > > DELL_LAPTOP

> > > >

> > > > But didn't select DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.

> > >

> > > Distros have to enable whatever they want to.

> >

> > At least in this instance I'd hypothesize it's because these are new

> > config

> > options that default to off.

> >

> > They probably had DELL_SMBIOS enabled before and carried that forward

> > But there was nothing to transition them to make them turn on

> > DELL_SMBIOS_WMI or DELL_SMBIOS_SMM.

> 

> So, the driver requires now _at least_ one of the backend enabled?


Yes.

> 

> > > Can it be the Dell model, that survives w/o one of above or even

> > > both?

> >

> > The design as it exists to day is that dell-laptop needs dell-smbios

> > but

> > dell-smbios won't run unless it has a backend selected.

> 

> How was it before?


The code that's in dell-smbios-smm used to be directly in dell-smbios.
It was split into backend approach so that you can choose that or choose
WMI backend or both.

For a true transition to how things were pre-4.15 it should have been
that CONFIG_DELL_SMBIOS selected CONFIG_DELL_SMBIOS_SMM.

You would have had an identical experience then after upgrade.

> 

> > Something like this maybe then to not let them even try to run dell-

> > smbios?

> 

> Wouldn't be a regression still?

> 

> P.S. See also the link I answered with to Pali.

>
diff mbox

Patch

diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 8541cde..71489fe 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -556,6 +556,11 @@  static int __init dell_smbios_init(void)
        const struct dmi_device *valid;
        int ret;
 
+#if !defined(CONFIG_DELL_SMBIOS_WMI) && !defined(CONFIG_DELL_SMBIOS_SMM)
+       pr_err("Missing SMBIOS backend.");
+       return -ENODEV;
+#endif
+
        valid = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, "Dell System", NULL);
        if (!valid) {
                pr_err("Unable to run on non-Dell system\n");