RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-05 Thread Mario.Limonciello
> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-x86-
> ow...@vger.kernel.org] On Behalf Of Darren Hart
> Sent: Tuesday, March 6, 2018 7:49 AM
> To: Limonciello, Mario <mario_limoncie...@dell.com>
> Cc: andy.shevche...@gmail.com; pali.ro...@gmail.com; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Sat, Mar 03, 2018 at 11:59:55AM +, mario.limoncie...@dell.com wrote:
> > > -Original Message-
> > > From: Darren Hart [mailto:dvh...@infradead.org]
> > > Sent: Friday, March 2, 2018 8:11 PM
> > > To: Limonciello, Mario <mario_limoncie...@dell.com>
> > > Cc: Andy Shevchenko <andy.shevche...@gmail.com>; pali.ro...@gmail.com;
> LKML
> > > <linux-kernel@vger.kernel.org>; platform-driver-...@vger.kernel.org
> > > Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all 
> > > dell-smbios-*
> > > modules together
> > >
> > > On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > > > Some race conditions were raised due to dell-smbios and its backends
> > > > not being ready by the time that a consumer would call one of the
> > > > exported methods.
> > > >
> > > > To avoid this problem, guarantee that all initialization has been
> > > > done by linking them all together and running init for them all.
> > > >
> > > > As part of this change the Kconfig needs to be adjusted so that
> > > > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > > > rather than modules.
> > > >
> > > > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > > > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> > > >
> > > > Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> > > > ---
> > > > Changes from v2:
> > > >  * Move rest of #ifdef out of source files
> > > >  * Only compile units as necessary
> > > >
> > > >  drivers/platform/x86/Kconfig| 11 ---
> > > >  drivers/platform/x86/Makefile   |  4 ++--
> > > >  drivers/platform/x86/dell-smbios-base.c | 21 -
> > > >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> > > >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> > > >  drivers/platform/x86/dell-smbios.h  | 27 
> > > > ++-
> > > >  6 files changed, 64 insertions(+), 31 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > > > index 9a8f964..0c0897e 100644
> > > > --- a/drivers/platform/x86/Kconfig
> > > > +++ b/drivers/platform/x86/Kconfig
> > > > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> > > >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> > > >
> > > >  config DELL_SMBIOS
> > > > -   tristate
> > > > +   tristate "Dell SMBIOS driver"
> > > > +   ---help---
> > > > +   This provides support for the Dell SMBIOS calling interface.
> > > > +   If you have a Dell computer you should enable this option.
> > > > +
> > > > +   Be sure to select at least one backend for it to work properly.
> > > >
> > > >  config DELL_SMBIOS_WMI
> > > > -   tristate "Dell SMBIOS calling interface (WMI implementation)"
> > > > +   bool "Dell SMBIOS driver WMI support"
> > > > depends on ACPI_WMI
> > > > select DELL_WMI_DESCRIPTOR
> > > > select DELL_SMBIOS
> > >
> > > Blarg, this also violates the kconfig-language guidance for using select
> > > only for non-visible symbols with no dependencies.
> > >
> > > I have a patch that works, but I don't love it. We can remove the prompt
> > > from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on"
> > > DELL_SMBIOS.
> > > The options just appear sort of randomly when either DELL_LAPTOP or
> > > DELL_WMI are selected - but they can default to y, which is nice.
> > >
> > > I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
> > > DELL_LAPTOP, which makes this look a lot n

RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-05 Thread Mario.Limonciello
> -Original Message-
> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-x86-
> ow...@vger.kernel.org] On Behalf Of Darren Hart
> Sent: Tuesday, March 6, 2018 7:49 AM
> To: Limonciello, Mario 
> Cc: andy.shevche...@gmail.com; pali.ro...@gmail.com; linux-
> ker...@vger.kernel.org; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Sat, Mar 03, 2018 at 11:59:55AM +, mario.limoncie...@dell.com wrote:
> > > -Original Message-
> > > From: Darren Hart [mailto:dvh...@infradead.org]
> > > Sent: Friday, March 2, 2018 8:11 PM
> > > To: Limonciello, Mario 
> > > Cc: Andy Shevchenko ; pali.ro...@gmail.com;
> LKML
> > > ; platform-driver-...@vger.kernel.org
> > > Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all 
> > > dell-smbios-*
> > > modules together
> > >
> > > On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > > > Some race conditions were raised due to dell-smbios and its backends
> > > > not being ready by the time that a consumer would call one of the
> > > > exported methods.
> > > >
> > > > To avoid this problem, guarantee that all initialization has been
> > > > done by linking them all together and running init for them all.
> > > >
> > > > As part of this change the Kconfig needs to be adjusted so that
> > > > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > > > rather than modules.
> > > >
> > > > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > > > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> > > >
> > > > Signed-off-by: Mario Limonciello 
> > > > ---
> > > > Changes from v2:
> > > >  * Move rest of #ifdef out of source files
> > > >  * Only compile units as necessary
> > > >
> > > >  drivers/platform/x86/Kconfig| 11 ---
> > > >  drivers/platform/x86/Makefile   |  4 ++--
> > > >  drivers/platform/x86/dell-smbios-base.c | 21 -
> > > >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> > > >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> > > >  drivers/platform/x86/dell-smbios.h  | 27 
> > > > ++-
> > > >  6 files changed, 64 insertions(+), 31 deletions(-)
> > > >
> > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > > > index 9a8f964..0c0897e 100644
> > > > --- a/drivers/platform/x86/Kconfig
> > > > +++ b/drivers/platform/x86/Kconfig
> > > > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> > > >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> > > >
> > > >  config DELL_SMBIOS
> > > > -   tristate
> > > > +   tristate "Dell SMBIOS driver"
> > > > +   ---help---
> > > > +   This provides support for the Dell SMBIOS calling interface.
> > > > +   If you have a Dell computer you should enable this option.
> > > > +
> > > > +   Be sure to select at least one backend for it to work properly.
> > > >
> > > >  config DELL_SMBIOS_WMI
> > > > -   tristate "Dell SMBIOS calling interface (WMI implementation)"
> > > > +   bool "Dell SMBIOS driver WMI support"
> > > > depends on ACPI_WMI
> > > > select DELL_WMI_DESCRIPTOR
> > > > select DELL_SMBIOS
> > >
> > > Blarg, this also violates the kconfig-language guidance for using select
> > > only for non-visible symbols with no dependencies.
> > >
> > > I have a patch that works, but I don't love it. We can remove the prompt
> > > from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on"
> > > DELL_SMBIOS.
> > > The options just appear sort of randomly when either DELL_LAPTOP or
> > > DELL_WMI are selected - but they can default to y, which is nice.
> > >
> > > I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
> > > DELL_LAPTOP, which makes this look a lot nicer. These just appear under
> > > DELL_LAPTOP as bool options. The problem with this approach is DELL_WMI
> > > also requires DELL_SMBIOS and DEL

Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-05 Thread Darren Hart
On Sat, Mar 03, 2018 at 11:59:55AM +, mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Darren Hart [mailto:dvh...@infradead.org]
> > Sent: Friday, March 2, 2018 8:11 PM
> > To: Limonciello, Mario <mario_limoncie...@dell.com>
> > Cc: Andy Shevchenko <andy.shevche...@gmail.com>; pali.ro...@gmail.com; LKML
> > <linux-kernel@vger.kernel.org>; platform-driver-...@vger.kernel.org
> > Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all 
> > dell-smbios-*
> > modules together
> > 
> > On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > > Some race conditions were raised due to dell-smbios and its backends
> > > not being ready by the time that a consumer would call one of the
> > > exported methods.
> > >
> > > To avoid this problem, guarantee that all initialization has been
> > > done by linking them all together and running init for them all.
> > >
> > > As part of this change the Kconfig needs to be adjusted so that
> > > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > > rather than modules.
> > >
> > > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> > >
> > > Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> > > ---
> > > Changes from v2:
> > >  * Move rest of #ifdef out of source files
> > >  * Only compile units as necessary
> > >
> > >  drivers/platform/x86/Kconfig| 11 ---
> > >  drivers/platform/x86/Makefile   |  4 ++--
> > >  drivers/platform/x86/dell-smbios-base.c | 21 -
> > >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> > >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> > >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> > >  6 files changed, 64 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > > index 9a8f964..0c0897e 100644
> > > --- a/drivers/platform/x86/Kconfig
> > > +++ b/drivers/platform/x86/Kconfig
> > > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> > > If you have an ACPI-compatible ASUS laptop, say Y or M here.
> > >
> > >  config DELL_SMBIOS
> > > - tristate
> > > + tristate "Dell SMBIOS driver"
> > > + ---help---
> > > + This provides support for the Dell SMBIOS calling interface.
> > > + If you have a Dell computer you should enable this option.
> > > +
> > > + Be sure to select at least one backend for it to work properly.
> > >
> > >  config DELL_SMBIOS_WMI
> > > - tristate "Dell SMBIOS calling interface (WMI implementation)"
> > > + bool "Dell SMBIOS driver WMI support"
> > >   depends on ACPI_WMI
> > >   select DELL_WMI_DESCRIPTOR
> > >   select DELL_SMBIOS
> > 
> > Blarg, this also violates the kconfig-language guidance for using select
> > only for non-visible symbols with no dependencies.
> > 
> > I have a patch that works, but I don't love it. We can remove the prompt
> > from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on"
> > DELL_SMBIOS.
> > The options just appear sort of randomly when either DELL_LAPTOP or
> > DELL_WMI are selected - but they can default to y, which is nice.
> > 
> > I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
> > DELL_LAPTOP, which makes this look a lot nicer. These just appear under
> > DELL_LAPTOP as bool options. The problem with this approach is DELL_WMI
> > also requires DELL_SMBIOS and DELL_WMI doesn't currently depend on
> > DELL_LAPTOP. One option is to make it depend on DELL_LAPTOP as it seems
> > unlikely for them to ever be used separately.  I don't love this either.
> > Thoughts?
> > 
> > Perhaps the compromise is leave the DELL_SMBIOS prompt. This doesn't
> > have the negative consequences the kconfig-languages is trying to guard
> > against, and it makes it obvious what the DELL_SMBIOS_* options are -
> > and we can still default them to y. See below.
> > 
> > This seems like it might be the least bad option at the moment. I'll
> > keep thinking on it a bit...
> > 
> > From: "Darren Hart (VMware)" <dvh...@infradead.org>
> > Date: Fri, 2 Mar 2018 17:40:32 -0800
> > Subject: [PATCH] p

Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-05 Thread Darren Hart
On Sat, Mar 03, 2018 at 11:59:55AM +, mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Darren Hart [mailto:dvh...@infradead.org]
> > Sent: Friday, March 2, 2018 8:11 PM
> > To: Limonciello, Mario 
> > Cc: Andy Shevchenko ; pali.ro...@gmail.com; LKML
> > ; platform-driver-...@vger.kernel.org
> > Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all 
> > dell-smbios-*
> > modules together
> > 
> > On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > > Some race conditions were raised due to dell-smbios and its backends
> > > not being ready by the time that a consumer would call one of the
> > > exported methods.
> > >
> > > To avoid this problem, guarantee that all initialization has been
> > > done by linking them all together and running init for them all.
> > >
> > > As part of this change the Kconfig needs to be adjusted so that
> > > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > > rather than modules.
> > >
> > > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> > >
> > > Signed-off-by: Mario Limonciello 
> > > ---
> > > Changes from v2:
> > >  * Move rest of #ifdef out of source files
> > >  * Only compile units as necessary
> > >
> > >  drivers/platform/x86/Kconfig| 11 ---
> > >  drivers/platform/x86/Makefile   |  4 ++--
> > >  drivers/platform/x86/dell-smbios-base.c | 21 -
> > >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> > >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> > >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> > >  6 files changed, 64 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > > index 9a8f964..0c0897e 100644
> > > --- a/drivers/platform/x86/Kconfig
> > > +++ b/drivers/platform/x86/Kconfig
> > > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> > > If you have an ACPI-compatible ASUS laptop, say Y or M here.
> > >
> > >  config DELL_SMBIOS
> > > - tristate
> > > + tristate "Dell SMBIOS driver"
> > > + ---help---
> > > + This provides support for the Dell SMBIOS calling interface.
> > > + If you have a Dell computer you should enable this option.
> > > +
> > > + Be sure to select at least one backend for it to work properly.
> > >
> > >  config DELL_SMBIOS_WMI
> > > - tristate "Dell SMBIOS calling interface (WMI implementation)"
> > > + bool "Dell SMBIOS driver WMI support"
> > >   depends on ACPI_WMI
> > >   select DELL_WMI_DESCRIPTOR
> > >   select DELL_SMBIOS
> > 
> > Blarg, this also violates the kconfig-language guidance for using select
> > only for non-visible symbols with no dependencies.
> > 
> > I have a patch that works, but I don't love it. We can remove the prompt
> > from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on"
> > DELL_SMBIOS.
> > The options just appear sort of randomly when either DELL_LAPTOP or
> > DELL_WMI are selected - but they can default to y, which is nice.
> > 
> > I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
> > DELL_LAPTOP, which makes this look a lot nicer. These just appear under
> > DELL_LAPTOP as bool options. The problem with this approach is DELL_WMI
> > also requires DELL_SMBIOS and DELL_WMI doesn't currently depend on
> > DELL_LAPTOP. One option is to make it depend on DELL_LAPTOP as it seems
> > unlikely for them to ever be used separately.  I don't love this either.
> > Thoughts?
> > 
> > Perhaps the compromise is leave the DELL_SMBIOS prompt. This doesn't
> > have the negative consequences the kconfig-languages is trying to guard
> > against, and it makes it obvious what the DELL_SMBIOS_* options are -
> > and we can still default them to y. See below.
> > 
> > This seems like it might be the least bad option at the moment. I'll
> > keep thinking on it a bit...
> > 
> > From: "Darren Hart (VMware)" 
> > Date: Fri, 2 Mar 2018 17:40:32 -0800
> > Subject: [PATCH] platform/x86: Allow for SMBIOS backend defaults
> > 
> > Avoid accidental configurations by setting default y for DELL_SMBIOS
> > backends. Avoid th

RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-03 Thread Mario.Limonciello
> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Friday, March 2, 2018 8:11 PM
> To: Limonciello, Mario <mario_limoncie...@dell.com>
> Cc: Andy Shevchenko <andy.shevche...@gmail.com>; pali.ro...@gmail.com; LKML
> <linux-kernel@vger.kernel.org>; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > Some race conditions were raised due to dell-smbios and its backends
> > not being ready by the time that a consumer would call one of the
> > exported methods.
> >
> > To avoid this problem, guarantee that all initialization has been
> > done by linking them all together and running init for them all.
> >
> > As part of this change the Kconfig needs to be adjusted so that
> > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > rather than modules.
> >
> > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> >
> > Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> > ---
> > Changes from v2:
> >  * Move rest of #ifdef out of source files
> >  * Only compile units as necessary
> >
> >  drivers/platform/x86/Kconfig| 11 ---
> >  drivers/platform/x86/Makefile   |  4 ++--
> >  drivers/platform/x86/dell-smbios-base.c | 21 -
> >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> >  6 files changed, 64 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 9a8f964..0c0897e 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> >
> >  config DELL_SMBIOS
> > -   tristate
> > +   tristate "Dell SMBIOS driver"
> > +   ---help---
> > +   This provides support for the Dell SMBIOS calling interface.
> > +   If you have a Dell computer you should enable this option.
> > +
> > +   Be sure to select at least one backend for it to work properly.
> >
> >  config DELL_SMBIOS_WMI
> > -   tristate "Dell SMBIOS calling interface (WMI implementation)"
> > +   bool "Dell SMBIOS driver WMI support"
> > depends on ACPI_WMI
> > select DELL_WMI_DESCRIPTOR
> > select DELL_SMBIOS
> 
> Blarg, this also violates the kconfig-language guidance for using select
> only for non-visible symbols with no dependencies.
> 
> I have a patch that works, but I don't love it. We can remove the prompt
> from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on"
> DELL_SMBIOS.
> The options just appear sort of randomly when either DELL_LAPTOP or
> DELL_WMI are selected - but they can default to y, which is nice.
> 
> I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
> DELL_LAPTOP, which makes this look a lot nicer. These just appear under
> DELL_LAPTOP as bool options. The problem with this approach is DELL_WMI
> also requires DELL_SMBIOS and DELL_WMI doesn't currently depend on
> DELL_LAPTOP. One option is to make it depend on DELL_LAPTOP as it seems
> unlikely for them to ever be used separately.  I don't love this either.
> Thoughts?
> 
> Perhaps the compromise is leave the DELL_SMBIOS prompt. This doesn't
> have the negative consequences the kconfig-languages is trying to guard
> against, and it makes it obvious what the DELL_SMBIOS_* options are -
> and we can still default them to y. See below.
> 
> This seems like it might be the least bad option at the moment. I'll
> keep thinking on it a bit...
> 
> From: "Darren Hart (VMware)" <dvh...@infradead.org>
> Date: Fri, 2 Mar 2018 17:40:32 -0800
> Subject: [PATCH] platform/x86: Allow for SMBIOS backend defaults
> 
> Avoid accidental configurations by setting default y for DELL_SMBIOS
> backends. Avoid this impacting the default build size, by making them
> dependent on DELL_SMBIOS, so they only appear when DELL_SMBIOS is
> manually selected, or by DELL_LAPTOP or DELL_WMI.
> 
> Signed-off-by: Darren Hart (VMware) <dvh...@infradead.org>
> ---
>  drivers/platform/x86/Kconfig | 6 --
>  1 file changed, 4 insertions(+), 2 deleti

RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-03 Thread Mario.Limonciello
> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Friday, March 2, 2018 8:11 PM
> To: Limonciello, Mario 
> Cc: Andy Shevchenko ; pali.ro...@gmail.com; LKML
> ; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > Some race conditions were raised due to dell-smbios and its backends
> > not being ready by the time that a consumer would call one of the
> > exported methods.
> >
> > To avoid this problem, guarantee that all initialization has been
> > done by linking them all together and running init for them all.
> >
> > As part of this change the Kconfig needs to be adjusted so that
> > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > rather than modules.
> >
> > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> >
> > Signed-off-by: Mario Limonciello 
> > ---
> > Changes from v2:
> >  * Move rest of #ifdef out of source files
> >  * Only compile units as necessary
> >
> >  drivers/platform/x86/Kconfig| 11 ---
> >  drivers/platform/x86/Makefile   |  4 ++--
> >  drivers/platform/x86/dell-smbios-base.c | 21 -
> >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> >  6 files changed, 64 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 9a8f964..0c0897e 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> >
> >  config DELL_SMBIOS
> > -   tristate
> > +   tristate "Dell SMBIOS driver"
> > +   ---help---
> > +   This provides support for the Dell SMBIOS calling interface.
> > +   If you have a Dell computer you should enable this option.
> > +
> > +   Be sure to select at least one backend for it to work properly.
> >
> >  config DELL_SMBIOS_WMI
> > -   tristate "Dell SMBIOS calling interface (WMI implementation)"
> > +   bool "Dell SMBIOS driver WMI support"
> > depends on ACPI_WMI
> > select DELL_WMI_DESCRIPTOR
> > select DELL_SMBIOS
> 
> Blarg, this also violates the kconfig-language guidance for using select
> only for non-visible symbols with no dependencies.
> 
> I have a patch that works, but I don't love it. We can remove the prompt
> from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on"
> DELL_SMBIOS.
> The options just appear sort of randomly when either DELL_LAPTOP or
> DELL_WMI are selected - but they can default to y, which is nice.
> 
> I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
> DELL_LAPTOP, which makes this look a lot nicer. These just appear under
> DELL_LAPTOP as bool options. The problem with this approach is DELL_WMI
> also requires DELL_SMBIOS and DELL_WMI doesn't currently depend on
> DELL_LAPTOP. One option is to make it depend on DELL_LAPTOP as it seems
> unlikely for them to ever be used separately.  I don't love this either.
> Thoughts?
> 
> Perhaps the compromise is leave the DELL_SMBIOS prompt. This doesn't
> have the negative consequences the kconfig-languages is trying to guard
> against, and it makes it obvious what the DELL_SMBIOS_* options are -
> and we can still default them to y. See below.
> 
> This seems like it might be the least bad option at the moment. I'll
> keep thinking on it a bit...
> 
> From: "Darren Hart (VMware)" 
> Date: Fri, 2 Mar 2018 17:40:32 -0800
> Subject: [PATCH] platform/x86: Allow for SMBIOS backend defaults
> 
> Avoid accidental configurations by setting default y for DELL_SMBIOS
> backends. Avoid this impacting the default build size, by making them
> dependent on DELL_SMBIOS, so they only appear when DELL_SMBIOS is
> manually selected, or by DELL_LAPTOP or DELL_WMI.
> 
> Signed-off-by: Darren Hart (VMware) 
> ---
>  drivers/platform/x86/Kconfig | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index e3f682c..022c837 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platfo

Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.
>  
>  config DELL_SMBIOS_WMI
> - tristate "Dell SMBIOS calling interface (WMI implementation)"
> + bool "Dell SMBIOS driver WMI support"
>   depends on ACPI_WMI
>   select DELL_WMI_DESCRIPTOR
>   select DELL_SMBIOS

Blarg, this also violates the kconfig-language guidance for using select
only for non-visible symbols with no dependencies.

I have a patch that works, but I don't love it. We can remove the prompt
from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on" DELL_SMBIOS.
The options just appear sort of randomly when either DELL_LAPTOP or
DELL_WMI are selected - but they can default to y, which is nice.

I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
DELL_LAPTOP, which makes this look a lot nicer. These just appear under
DELL_LAPTOP as bool options. The problem with this approach is DELL_WMI
also requires DELL_SMBIOS and DELL_WMI doesn't currently depend on
DELL_LAPTOP. One option is to make it depend on DELL_LAPTOP as it seems
unlikely for them to ever be used separately.  I don't love this either.
Thoughts?

Perhaps the compromise is leave the DELL_SMBIOS prompt. This doesn't
have the negative consequences the kconfig-languages is trying to guard
against, and it makes it obvious what the DELL_SMBIOS_* options are -
and we can still default them to y. See below.

This seems like it might be the least bad option at the moment. I'll
keep thinking on it a bit...

From: "Darren Hart (VMware)" 
Date: Fri, 2 Mar 2018 17:40:32 -0800
Subject: [PATCH] platform/x86: Allow for SMBIOS backend defaults

Avoid accidental configurations by setting default y for DELL_SMBIOS
backends. Avoid this impacting the default build size, by making them
dependent on DELL_SMBIOS, so they only appear when DELL_SMBIOS is
manually selected, or by DELL_LAPTOP or DELL_WMI.

Signed-off-by: Darren Hart (VMware) 
---
 drivers/platform/x86/Kconfig | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index e3f682c..022c837 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -115,9 +115,10 @@ config DELL_SMBIOS
 
 config DELL_SMBIOS_WMI
bool "Dell SMBIOS driver WMI backend"
+   default y
depends on ACPI_WMI
select DELL_WMI_DESCRIPTOR
-   select DELL_SMBIOS
+   depends on DELL_SMBIOS
---help---
This provides an implementation for the Dell SMBIOS calling interface
communicated over ACPI-WMI.
@@ -128,8 +129,9 @@ config DELL_SMBIOS_WMI
 
 config DELL_SMBIOS_SMM
bool "Dell SMBIOS driver SMM backend"
+   default y
depends on DCDBAS
-   select DELL_SMBIOS
+   depends on DELL_SMBIOS
---help---
This provides an implementation for the Dell SMBIOS calling interface
communicated over SMI/SMM.
-- 
2.9.5


-- 
Darren Hart
VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.
>  
>  config DELL_SMBIOS_WMI
> - tristate "Dell SMBIOS calling interface (WMI implementation)"
> + bool "Dell SMBIOS driver WMI support"
>   depends on ACPI_WMI
>   select DELL_WMI_DESCRIPTOR
>   select DELL_SMBIOS

Blarg, this also violates the kconfig-language guidance for using select
only for non-visible symbols with no dependencies.

I have a patch that works, but I don't love it. We can remove the prompt
from DELL_SMBIOS, and make DELL_SMBIOS_WMI|SMM "depend on" DELL_SMBIOS.
The options just appear sort of randomly when either DELL_LAPTOP or
DELL_WMI are selected - but they can default to y, which is nice.

I have a subsequent patch which makes DELL_SMBIOS_WMI|SMM "depend on"
DELL_LAPTOP, which makes this look a lot nicer. These just appear under
DELL_LAPTOP as bool options. The problem with this approach is DELL_WMI
also requires DELL_SMBIOS and DELL_WMI doesn't currently depend on
DELL_LAPTOP. One option is to make it depend on DELL_LAPTOP as it seems
unlikely for them to ever be used separately.  I don't love this either.
Thoughts?

Perhaps the compromise is leave the DELL_SMBIOS prompt. This doesn't
have the negative consequences the kconfig-languages is trying to guard
against, and it makes it obvious what the DELL_SMBIOS_* options are -
and we can still default them to y. See below.

This seems like it might be the least bad option at the moment. I'll
keep thinking on it a bit...

From: "Darren Hart (VMware)" 
Date: Fri, 2 Mar 2018 17:40:32 -0800
Subject: [PATCH] platform/x86: Allow for SMBIOS backend defaults

Avoid accidental configurations by setting default y for DELL_SMBIOS
backends. Avoid this impacting the default build size, by making them
dependent on DELL_SMBIOS, so they only appear when DELL_SMBIOS is
manually selected, or by DELL_LAPTOP or DELL_WMI.

Signed-off-by: Darren Hart (VMware) 
---
 drivers/platform/x86/Kconfig | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index e3f682c..022c837 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -115,9 +115,10 @@ config DELL_SMBIOS
 
 config DELL_SMBIOS_WMI
bool "Dell SMBIOS driver WMI backend"
+   default y
depends on ACPI_WMI
select DELL_WMI_DESCRIPTOR
-   select DELL_SMBIOS
+   depends on DELL_SMBIOS
---help---
This provides an implementation for the Dell SMBIOS calling interface
communicated over ACPI-WMI.
@@ -128,8 +129,9 @@ config DELL_SMBIOS_WMI
 
 config DELL_SMBIOS_SMM
bool "Dell SMBIOS driver SMM backend"
+   default y
depends on DCDBAS
-   select DELL_SMBIOS
+   depends on DELL_SMBIOS
---help---
This provides an implementation for the Dell SMBIOS calling interface
communicated over SMI/SMM.
-- 
2.9.5


-- 
Darren Hart
VMware Open Source Technology Center


RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Mario.Limonciello


> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Friday, March 2, 2018 6:22 PM
> To: Limonciello, Mario <mario_limoncie...@dell.com>
> Cc: Andy Shevchenko <andy.shevche...@gmail.com>; pali.ro...@gmail.com; LKML
> <linux-kernel@vger.kernel.org>; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > Some race conditions were raised due to dell-smbios and its backends
> > not being ready by the time that a consumer would call one of the
> > exported methods.
> >
> > To avoid this problem, guarantee that all initialization has been
> > done by linking them all together and running init for them all.
> >
> > As part of this change the Kconfig needs to be adjusted so that
> > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > rather than modules.
> >
> > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> >
> > Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> > ---
> > Changes from v2:
> >  * Move rest of #ifdef out of source files
> >  * Only compile units as necessary
> >
> >  drivers/platform/x86/Kconfig| 11 ---
> >  drivers/platform/x86/Makefile   |  4 ++--
> >  drivers/platform/x86/dell-smbios-base.c | 21 -
> >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> >  6 files changed, 64 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 9a8f964..0c0897e 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> >
> >  config DELL_SMBIOS
> > -   tristate
> > +   tristate "Dell SMBIOS driver"
> > +   ---help---
> > +   This provides support for the Dell SMBIOS calling interface.
> > +   If you have a Dell computer you should enable this option.
> > +
> > +   Be sure to select at least one backend for it to work properly.
> >
> >  config DELL_SMBIOS_WMI
> > -   tristate "Dell SMBIOS calling interface (WMI implementation)"
> > +   bool "Dell SMBIOS driver WMI support"
> 
> Would you be happy with using "backend" here and for SMM?
> 
>   bool "Dell SMBIOS driver WMI backend"
> 
> It makes it obvious what the help text is referring to by "select at
> least one backend".
> 
> Happy to just make this change locally, no need to resubmit.
> 

Yeah, fine by me.

Thanks,



RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Mario.Limonciello


> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Friday, March 2, 2018 6:22 PM
> To: Limonciello, Mario 
> Cc: Andy Shevchenko ; pali.ro...@gmail.com; LKML
> ; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > Some race conditions were raised due to dell-smbios and its backends
> > not being ready by the time that a consumer would call one of the
> > exported methods.
> >
> > To avoid this problem, guarantee that all initialization has been
> > done by linking them all together and running init for them all.
> >
> > As part of this change the Kconfig needs to be adjusted so that
> > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > rather than modules.
> >
> > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> >
> > Signed-off-by: Mario Limonciello 
> > ---
> > Changes from v2:
> >  * Move rest of #ifdef out of source files
> >  * Only compile units as necessary
> >
> >  drivers/platform/x86/Kconfig| 11 ---
> >  drivers/platform/x86/Makefile   |  4 ++--
> >  drivers/platform/x86/dell-smbios-base.c | 21 -
> >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> >  6 files changed, 64 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 9a8f964..0c0897e 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> >
> >  config DELL_SMBIOS
> > -   tristate
> > +   tristate "Dell SMBIOS driver"
> > +   ---help---
> > +   This provides support for the Dell SMBIOS calling interface.
> > +   If you have a Dell computer you should enable this option.
> > +
> > +   Be sure to select at least one backend for it to work properly.
> >
> >  config DELL_SMBIOS_WMI
> > -   tristate "Dell SMBIOS calling interface (WMI implementation)"
> > +   bool "Dell SMBIOS driver WMI support"
> 
> Would you be happy with using "backend" here and for SMM?
> 
>   bool "Dell SMBIOS driver WMI backend"
> 
> It makes it obvious what the help text is referring to by "select at
> least one backend".
> 
> Happy to just make this change locally, no need to resubmit.
> 

Yeah, fine by me.

Thanks,



Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.
>  
>  config DELL_SMBIOS_WMI
> - tristate "Dell SMBIOS calling interface (WMI implementation)"
> + bool "Dell SMBIOS driver WMI support"

Oops, the help text still says Y or M for this and SMM - I'll correct locally.

-- 
Darren Hart
VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.
>  
>  config DELL_SMBIOS_WMI
> - tristate "Dell SMBIOS calling interface (WMI implementation)"
> + bool "Dell SMBIOS driver WMI support"

Oops, the help text still says Y or M for this and SMM - I'll correct locally.

-- 
Darren Hart
VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.
>  
>  config DELL_SMBIOS_WMI
> - tristate "Dell SMBIOS calling interface (WMI implementation)"
> + bool "Dell SMBIOS driver WMI support"

Would you be happy with using "backend" here and for SMM?

bool "Dell SMBIOS driver WMI backend"

It makes it obvious what the help text is referring to by "select at
least one backend".

Happy to just make this change locally, no need to resubmit.

-- 
Darren Hart
VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-02 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.
>  
>  config DELL_SMBIOS_WMI
> - tristate "Dell SMBIOS calling interface (WMI implementation)"
> + bool "Dell SMBIOS driver WMI support"

Would you be happy with using "backend" here and for SMM?

bool "Dell SMBIOS driver WMI backend"

It makes it obvious what the help text is referring to by "select at
least one backend".

Happy to just make this change locally, no need to resubmit.

-- 
Darren Hart
VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-01 Thread Darren Hart
On Thu, Mar 01, 2018 at 08:54:31PM +, mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Darren Hart [mailto:dvh...@infradead.org]
> > Sent: Thursday, March 1, 2018 11:49 AM
> > To: Limonciello, Mario <mario_limoncie...@dell.com>
> > Cc: Andy Shevchenko <andy.shevche...@gmail.com>; pali.ro...@gmail.com;
> > LKML <linux-kernel@vger.kernel.org>; platform-driver-...@vger.kernel.org
> > Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all 
> > dell-smbios-*
> > modules together
> > 
> > On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > > Some race conditions were raised due to dell-smbios and its backends
> > > not being ready by the time that a consumer would call one of the
> > > exported methods.
> > >
> > > To avoid this problem, guarantee that all initialization has been
> > > done by linking them all together and running init for them all.
> > >
> > > As part of this change the Kconfig needs to be adjusted so that
> > > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > > rather than modules.
> > >
> > > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> > >
> > > Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> > > ---
> > > Changes from v2:
> > >  * Move rest of #ifdef out of source files
> > >  * Only compile units as necessary
> > >
> > >  drivers/platform/x86/Kconfig| 11 ---
> > >  drivers/platform/x86/Makefile   |  4 ++--
> > >  drivers/platform/x86/dell-smbios-base.c | 21 -
> > >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> > >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> > >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> > >  6 files changed, 64 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > > index 9a8f964..0c0897e 100644
> > > --- a/drivers/platform/x86/Kconfig
> > > +++ b/drivers/platform/x86/Kconfig
> > > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> > > If you have an ACPI-compatible ASUS laptop, say Y or M here.
> > >
> > >  config DELL_SMBIOS
> > > - tristate
> > > + tristate "Dell SMBIOS driver"
> > > + ---help---
> > > + This provides support for the Dell SMBIOS calling interface.
> > > + If you have a Dell computer you should enable this option.
> > > +
> > > + Be sure to select at least one backend for it to work properly.
> > 
> > In order to make the default sane, I would suggest making the above a menu
> > (default n), nesting the two options below, and making WMI a default=y. This
> > satisfies Linus' requirement by defaulting everything to n, but enables a 
> > sane
> > default IFF the DELL_SMBIOS menu is enabled.
> > 
> > Alternatively, all of the above can be put under the "Dell Extras" menu I
> > proposed, and DELL_SMBIOS can default to m and DELL_SMBIOS_WMI can default
> > to y.
> 
> I'm on board with moving everything to a Dell extras menu.  There are so many
> drivers now, it really seems like the best way to make sure that people get 
> them.
> 
> You'll have to actively go out of your way to turn off one of the sub-drivers 
> then
> to get into the situation that was described.
> 
> Since you did the original patch for dell extras nested in a mail would you 
> mind 
> sending that as a follow up second patch on top of my patch?
> 
> I think it's probably too much to do it all in this same patch itself.

Yup, easier for me to wrangle that all on my end. I'll look at your 3 again with
that in mind and let you know if I think any changes are needed, or just update
mine to apply on top.

-- 
Darren Hart
VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-01 Thread Darren Hart
On Thu, Mar 01, 2018 at 08:54:31PM +, mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Darren Hart [mailto:dvh...@infradead.org]
> > Sent: Thursday, March 1, 2018 11:49 AM
> > To: Limonciello, Mario 
> > Cc: Andy Shevchenko ; pali.ro...@gmail.com;
> > LKML ; platform-driver-...@vger.kernel.org
> > Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all 
> > dell-smbios-*
> > modules together
> > 
> > On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > > Some race conditions were raised due to dell-smbios and its backends
> > > not being ready by the time that a consumer would call one of the
> > > exported methods.
> > >
> > > To avoid this problem, guarantee that all initialization has been
> > > done by linking them all together and running init for them all.
> > >
> > > As part of this change the Kconfig needs to be adjusted so that
> > > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > > rather than modules.
> > >
> > > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> > >
> > > Signed-off-by: Mario Limonciello 
> > > ---
> > > Changes from v2:
> > >  * Move rest of #ifdef out of source files
> > >  * Only compile units as necessary
> > >
> > >  drivers/platform/x86/Kconfig| 11 ---
> > >  drivers/platform/x86/Makefile   |  4 ++--
> > >  drivers/platform/x86/dell-smbios-base.c | 21 -
> > >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> > >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> > >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> > >  6 files changed, 64 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > > index 9a8f964..0c0897e 100644
> > > --- a/drivers/platform/x86/Kconfig
> > > +++ b/drivers/platform/x86/Kconfig
> > > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> > > If you have an ACPI-compatible ASUS laptop, say Y or M here.
> > >
> > >  config DELL_SMBIOS
> > > - tristate
> > > + tristate "Dell SMBIOS driver"
> > > + ---help---
> > > + This provides support for the Dell SMBIOS calling interface.
> > > + If you have a Dell computer you should enable this option.
> > > +
> > > + Be sure to select at least one backend for it to work properly.
> > 
> > In order to make the default sane, I would suggest making the above a menu
> > (default n), nesting the two options below, and making WMI a default=y. This
> > satisfies Linus' requirement by defaulting everything to n, but enables a 
> > sane
> > default IFF the DELL_SMBIOS menu is enabled.
> > 
> > Alternatively, all of the above can be put under the "Dell Extras" menu I
> > proposed, and DELL_SMBIOS can default to m and DELL_SMBIOS_WMI can default
> > to y.
> 
> I'm on board with moving everything to a Dell extras menu.  There are so many
> drivers now, it really seems like the best way to make sure that people get 
> them.
> 
> You'll have to actively go out of your way to turn off one of the sub-drivers 
> then
> to get into the situation that was described.
> 
> Since you did the original patch for dell extras nested in a mail would you 
> mind 
> sending that as a follow up second patch on top of my patch?
> 
> I think it's probably too much to do it all in this same patch itself.

Yup, easier for me to wrangle that all on my end. I'll look at your 3 again with
that in mind and let you know if I think any changes are needed, or just update
mine to apply on top.

-- 
Darren Hart
VMware Open Source Technology Center


RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-01 Thread Mario.Limonciello
> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Thursday, March 1, 2018 11:49 AM
> To: Limonciello, Mario <mario_limoncie...@dell.com>
> Cc: Andy Shevchenko <andy.shevche...@gmail.com>; pali.ro...@gmail.com;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > Some race conditions were raised due to dell-smbios and its backends
> > not being ready by the time that a consumer would call one of the
> > exported methods.
> >
> > To avoid this problem, guarantee that all initialization has been
> > done by linking them all together and running init for them all.
> >
> > As part of this change the Kconfig needs to be adjusted so that
> > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > rather than modules.
> >
> > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> >
> > Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> > ---
> > Changes from v2:
> >  * Move rest of #ifdef out of source files
> >  * Only compile units as necessary
> >
> >  drivers/platform/x86/Kconfig| 11 ---
> >  drivers/platform/x86/Makefile   |  4 ++--
> >  drivers/platform/x86/dell-smbios-base.c | 21 -
> >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> >  6 files changed, 64 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 9a8f964..0c0897e 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> >
> >  config DELL_SMBIOS
> > -   tristate
> > +   tristate "Dell SMBIOS driver"
> > +   ---help---
> > +   This provides support for the Dell SMBIOS calling interface.
> > +   If you have a Dell computer you should enable this option.
> > +
> > +   Be sure to select at least one backend for it to work properly.
> 
> In order to make the default sane, I would suggest making the above a menu
> (default n), nesting the two options below, and making WMI a default=y. This
> satisfies Linus' requirement by defaulting everything to n, but enables a sane
> default IFF the DELL_SMBIOS menu is enabled.
> 
> Alternatively, all of the above can be put under the "Dell Extras" menu I
> proposed, and DELL_SMBIOS can default to m and DELL_SMBIOS_WMI can default
> to y.

I'm on board with moving everything to a Dell extras menu.  There are so many
drivers now, it really seems like the best way to make sure that people get 
them.

You'll have to actively go out of your way to turn off one of the sub-drivers 
then
to get into the situation that was described.

Since you did the original patch for dell extras nested in a mail would you 
mind 
sending that as a follow up second patch on top of my patch?

I think it's probably too much to do it all in this same patch itself.
Alternatively I can re-send my series with it before my 3/3, up to you.

> 
> >
> > +   /* register backends */
> > +   wmi = init_dell_smbios_wmi();
> > +   if (wmi)
> > +   pr_debug("Failed to initialize WMI backend: %d\n", wmi);
> > +   smm = init_dell_smbios_smm();
> > +   if (smm)
> > +   pr_debug("Failed to initialize SMM backend: %d\n", smm);
> > +   if (wmi && smm) {
> > +   pr_err("No SMBIOS backends available (wmi: %d, smm: %d)\n",
> > +   wmi, smm);
> > +   goto fail_sysfs;
> > +   }
> > +
> 
> I like the approach of building a library instead of a bunch of separate
> modules.
> 
> --
> Darren Hart
> VMware Open Source Technology Center


RE: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-01 Thread Mario.Limonciello
> -Original Message-
> From: Darren Hart [mailto:dvh...@infradead.org]
> Sent: Thursday, March 1, 2018 11:49 AM
> To: Limonciello, Mario 
> Cc: Andy Shevchenko ; pali.ro...@gmail.com;
> LKML ; platform-driver-...@vger.kernel.org
> Subject: Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-*
> modules together
> 
> On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> > Some race conditions were raised due to dell-smbios and its backends
> > not being ready by the time that a consumer would call one of the
> > exported methods.
> >
> > To avoid this problem, guarantee that all initialization has been
> > done by linking them all together and running init for them all.
> >
> > As part of this change the Kconfig needs to be adjusted so that
> > CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> > rather than modules.
> >
> > CONFIG_DELL_SMBIOS is a visually selectable option again and both
> > CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> >
> > Signed-off-by: Mario Limonciello 
> > ---
> > Changes from v2:
> >  * Move rest of #ifdef out of source files
> >  * Only compile units as necessary
> >
> >  drivers/platform/x86/Kconfig| 11 ---
> >  drivers/platform/x86/Makefile   |  4 ++--
> >  drivers/platform/x86/dell-smbios-base.c | 21 -
> >  drivers/platform/x86/dell-smbios-smm.c  | 18 --
> >  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
> >  drivers/platform/x86/dell-smbios.h  | 27 ++-
> >  6 files changed, 64 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 9a8f964..0c0897e 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> >   If you have an ACPI-compatible ASUS laptop, say Y or M here.
> >
> >  config DELL_SMBIOS
> > -   tristate
> > +   tristate "Dell SMBIOS driver"
> > +   ---help---
> > +   This provides support for the Dell SMBIOS calling interface.
> > +   If you have a Dell computer you should enable this option.
> > +
> > +   Be sure to select at least one backend for it to work properly.
> 
> In order to make the default sane, I would suggest making the above a menu
> (default n), nesting the two options below, and making WMI a default=y. This
> satisfies Linus' requirement by defaulting everything to n, but enables a sane
> default IFF the DELL_SMBIOS menu is enabled.
> 
> Alternatively, all of the above can be put under the "Dell Extras" menu I
> proposed, and DELL_SMBIOS can default to m and DELL_SMBIOS_WMI can default
> to y.

I'm on board with moving everything to a Dell extras menu.  There are so many
drivers now, it really seems like the best way to make sure that people get 
them.

You'll have to actively go out of your way to turn off one of the sub-drivers 
then
to get into the situation that was described.

Since you did the original patch for dell extras nested in a mail would you 
mind 
sending that as a follow up second patch on top of my patch?

I think it's probably too much to do it all in this same patch itself.
Alternatively I can re-send my series with it before my 3/3, up to you.

> 
> >
> > +   /* register backends */
> > +   wmi = init_dell_smbios_wmi();
> > +   if (wmi)
> > +   pr_debug("Failed to initialize WMI backend: %d\n", wmi);
> > +   smm = init_dell_smbios_smm();
> > +   if (smm)
> > +   pr_debug("Failed to initialize SMM backend: %d\n", smm);
> > +   if (wmi && smm) {
> > +   pr_err("No SMBIOS backends available (wmi: %d, smm: %d)\n",
> > +   wmi, smm);
> > +   goto fail_sysfs;
> > +   }
> > +
> 
> I like the approach of building a library instead of a bunch of separate
> modules.
> 
> --
> Darren Hart
> VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-01 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.

In order to make the default sane, I would suggest making the above a menu
(default n), nesting the two options below, and making WMI a default=y. This
satisfies Linus' requirement by defaulting everything to n, but enables a sane
default IFF the DELL_SMBIOS menu is enabled.

Alternatively, all of the above can be put under the "Dell Extras" menu I
proposed, and DELL_SMBIOS can default to m and DELL_SMBIOS_WMI can default to y.

>  
> + /* register backends */
> + wmi = init_dell_smbios_wmi();
> + if (wmi)
> + pr_debug("Failed to initialize WMI backend: %d\n", wmi);
> + smm = init_dell_smbios_smm();
> + if (smm)
> + pr_debug("Failed to initialize SMM backend: %d\n", smm);
> + if (wmi && smm) {
> + pr_err("No SMBIOS backends available (wmi: %d, smm: %d)\n",
> + wmi, smm);
> + goto fail_sysfs;
> + }
> +

I like the approach of building a library instead of a bunch of separate
modules.

-- 
Darren Hart
VMware Open Source Technology Center


Re: [PATCH v3 3/3] platform/x86: dell-smbios: Link all dell-smbios-* modules together

2018-03-01 Thread Darren Hart
On Tue, Feb 27, 2018 at 12:23:04PM -0600, Mario Limonciello wrote:
> Some race conditions were raised due to dell-smbios and its backends
> not being ready by the time that a consumer would call one of the
> exported methods.
> 
> To avoid this problem, guarantee that all initialization has been
> done by linking them all together and running init for them all.
> 
> As part of this change the Kconfig needs to be adjusted so that
> CONFIG_DELL_SMBIOS_SMM and CONFIG_DELL_SMBIOS_WMI are boolean
> rather than modules.
> 
> CONFIG_DELL_SMBIOS is a visually selectable option again and both
> CONFIG_DELL_SMBIOS_WMI and CONFIG_DELL_SMBIOS_SMM are optional.
> 
> Signed-off-by: Mario Limonciello 
> ---
> Changes from v2:
>  * Move rest of #ifdef out of source files
>  * Only compile units as necessary
> 
>  drivers/platform/x86/Kconfig| 11 ---
>  drivers/platform/x86/Makefile   |  4 ++--
>  drivers/platform/x86/dell-smbios-base.c | 21 -
>  drivers/platform/x86/dell-smbios-smm.c  | 18 --
>  drivers/platform/x86/dell-smbios-wmi.c  | 14 --
>  drivers/platform/x86/dell-smbios.h  | 27 ++-
>  6 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 9a8f964..0c0897e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -106,10 +106,15 @@ config ASUS_LAPTOP
> If you have an ACPI-compatible ASUS laptop, say Y or M here.
>  
>  config DELL_SMBIOS
> - tristate
> + tristate "Dell SMBIOS driver"
> + ---help---
> + This provides support for the Dell SMBIOS calling interface.
> + If you have a Dell computer you should enable this option.
> +
> + Be sure to select at least one backend for it to work properly.

In order to make the default sane, I would suggest making the above a menu
(default n), nesting the two options below, and making WMI a default=y. This
satisfies Linus' requirement by defaulting everything to n, but enables a sane
default IFF the DELL_SMBIOS menu is enabled.

Alternatively, all of the above can be put under the "Dell Extras" menu I
proposed, and DELL_SMBIOS can default to m and DELL_SMBIOS_WMI can default to y.

>  
> + /* register backends */
> + wmi = init_dell_smbios_wmi();
> + if (wmi)
> + pr_debug("Failed to initialize WMI backend: %d\n", wmi);
> + smm = init_dell_smbios_smm();
> + if (smm)
> + pr_debug("Failed to initialize SMM backend: %d\n", smm);
> + if (wmi && smm) {
> + pr_err("No SMBIOS backends available (wmi: %d, smm: %d)\n",
> + wmi, smm);
> + goto fail_sysfs;
> + }
> +

I like the approach of building a library instead of a bunch of separate
modules.

-- 
Darren Hart
VMware Open Source Technology Center