| Message ID | 20180307021606.GB124624@localhost.localdomain (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
> -----Original Message----- > From: Darren Hart [mailto:dvhart@infradead.org] > Sent: Wednesday, March 7, 2018 10:16 AM > To: Limonciello, Mario <Mario_Limonciello@Dell.com> > Cc: kbuild-all@01.org; platform-driver-x86@vger.kernel.org; > andriy.shevchenko@linux.intel.com; fengguang.wu@intel.com > Subject: Re: [platform-drivers-x86:testing 4/5] drivers/platform/x86/dell-smbios- > smm.c:99: undefined reference to `dcdbas_smi_request' > > On Tue, Mar 06, 2018 at 05:52:57PM -0800, Darren Hart wrote: > > On Tue, Mar 06, 2018 at 05:53:16AM +0000, Mario.Limonciello@dell.com wrote: > > > Darren, > > > > > > I looked through this and I think this is caused by dell-smbios-smm > > > no longer being mandated as it's own module but dcdbas being > > > allowed to be a module. > > > > > > The config that was selected had dcdbas as a module and dell-smbios > > > compiled in. That particular configuration should be prevented > > > > > > > This is a weird corner case, I had to dig up the syntax: > > > > config DELL_SMBIOS > > depends on DCDBAS || DCDBAS=n > > > > Is what we want. BUT. We only want that if DELL_SMBIOS_SMM, which > > unforunately causes a circular dependency (sort of, but it complains, so > > it won't fly) > > > > I'm working on this between conference sessions. Will report back when I > > sort it out. > > OK, this takes advantage of "select" not doing dependency checking to avoid the > recursion. Yuuuuuuck, but it works. Well that's pretty ugly, but yeah I guess if it works it works.. > > > From fe0c6c32fe4616343af8799f0d3c86d7b12d21a6 Mon Sep 17 00:00:00 2001 > Message-Id: > <fe0c6c32fe4616343af8799f0d3c86d7b12d21a6.1520388937.git.dvhart@infradead. > org> > From: "Darren Hart (VMware)" <dvhart@infradead.org> > Date: Tue, 6 Mar 2018 18:01:04 -0800 > Subject: [PATCH] platform/x86: dell-smbios: Resolve dependency error on DCDBAS > > When the DELL_SMBIOS_SMM backend is enabled, the DELL_SMBIOS symbol > needs to depend on DELL_DCDBAS - and specifically avoid the situation > where DELL_SMBIOS=y and DCDBAS=m. > > The intermediary DELL_SMBIOS_USING_DCDBAS avoids what would otherwise be > reported as a recursive dependency between DELL_SMBIOS and > DELL_SMBIOS_SMM (thanks to Steven Rostedt for the idea). > > Cc: Mario.Limonciello@dell.com > Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org> > --- > drivers/platform/x86/Kconfig | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 022c837..0b3d330 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -107,12 +107,21 @@ config ASUS_LAPTOP > > config DELL_SMBIOS > tristate "Dell SMBIOS driver" > + depends on !DELL_SMBIOS_USING_DCDBAS || (DCDBAS || DCDBAS=n) > ---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. > > +# > +# This decouples what would otherwise be a recursive dependency between > +# DELL_SMBIOS and DELL_SMBIOS_SMM. If DELL_SMBIOS_SMM is selected, then > +# DELL_SMBIOS needs to depend on DCDBAS. > +# > +config DELL_SMBIOS_USING_DCDBAS > + bool > + > config DELL_SMBIOS_WMI > bool "Dell SMBIOS driver WMI backend" > default y > @@ -132,6 +141,7 @@ config DELL_SMBIOS_SMM > default y > depends on DCDBAS > depends on DELL_SMBIOS > + select DELL_SMBIOS_USING_DCDBAS > ---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
On Tue, 2018-03-06 at 18:16 -0800, Darren Hart wrote: > On Tue, Mar 06, 2018 at 05:52:57PM -0800, Darren Hart wrote: > > On Tue, Mar 06, 2018 at 05:53:16AM +0000, Mario.Limonciello@dell.com > > wrote: > > > > config DELL_SMBIOS > tristate "Dell SMBIOS driver" > + depends on !DELL_SMBIOS_USING_DCDBAS || (DCDBAS || DCDBAS=n) > One question, can't we put select to different place and option with some other name to avoid ! in above conditional?
On Wed, Mar 07, 2018 at 02:18:25PM +0200, Andy Shevchenko wrote: > On Tue, 2018-03-06 at 18:16 -0800, Darren Hart wrote: > > On Tue, Mar 06, 2018 at 05:52:57PM -0800, Darren Hart wrote: > > > On Tue, Mar 06, 2018 at 05:53:16AM +0000, Mario.Limonciello@dell.com > > > wrote: > > > > > > > > config DELL_SMBIOS > > tristate "Dell SMBIOS driver" > > + depends on !DELL_SMBIOS_USING_DCDBAS || (DCDBAS || DCDBAS=n) > > > > One question, can't we put select to different place and option with > some other name to avoid ! in above conditional? Hi Andy, DELL_SMBIOS_SMM is the right place to set the flag for a dependency on DCDBAS. It *works* to change the above to: depends on !DELL_SMBIOS_SMM || (DCDBAS || DCDBAS=n) But the Kconfig tooling complains about a circular dependency (even thought it isn't really). The intermediate DELL_SMBIOS_USING_DCDBAS just works around that. Your asking if we can do something to avoid the "!". Why should we try to do that? What's the problem with the "!"? (Noting that yes, this is ugly, but it's a minimal change that is functional within the context of rather messy design and interdependencies that we have to work with between these modules). Thanks,
On Wed, 2018-03-07 at 09:50 -0800, Darren Hart wrote: > On Wed, Mar 07, 2018 at 02:18:25PM +0200, Andy Shevchenko wrote: > > On Tue, 2018-03-06 at 18:16 -0800, Darren Hart wrote: > > > On Tue, Mar 06, 2018 at 05:52:57PM -0800, Darren Hart wrote: > > > > On Tue, Mar 06, 2018 at 05:53:16AM +0000, Mario.Limonciello@dell > > > > .com > > > > wrote: > > > > > > > > > One question, can't we put select to different place and option with > > some other name to avoid ! in above conditional? > > Hi Andy, > > DELL_SMBIOS_SMM is the right place to set the flag for a dependency on > DCDBAS. > It *works* to change the above to: > > depends on !DELL_SMBIOS_SMM || (DCDBAS || DCDBAS=n) > > But the Kconfig tooling complains about a circular dependency (even > thought it > isn't really). The intermediate DELL_SMBIOS_USING_DCDBAS just works > around that. > > Your asking if we can do something to avoid the "!". Why should we try > to do > that? What's the problem with the "!"? No problem. > (Noting that yes, this is ugly, but it's > a minimal change that is functional within the context of rather messy > design > and interdependencies that we have to work with between these > modules). I'm fine with it, I was rather wondering.
On Thu, Mar 08, 2018 at 04:27:38PM +0200, Andy Shevchenko wrote: > On Wed, 2018-03-07 at 09:50 -0800, Darren Hart wrote: > > On Wed, Mar 07, 2018 at 02:18:25PM +0200, Andy Shevchenko wrote: > > > On Tue, 2018-03-06 at 18:16 -0800, Darren Hart wrote: > > > > On Tue, Mar 06, 2018 at 05:52:57PM -0800, Darren Hart wrote: > > > > > On Tue, Mar 06, 2018 at 05:53:16AM +0000, Mario.Limonciello@dell > > > > > .com > > > > > wrote: > > > > > > > > > > > > > One question, can't we put select to different place and option with > > > some other name to avoid ! in above conditional? > > > > Hi Andy, > > > > DELL_SMBIOS_SMM is the right place to set the flag for a dependency on > > DCDBAS. > > It *works* to change the above to: > > > > depends on !DELL_SMBIOS_SMM || (DCDBAS || DCDBAS=n) > > > > But the Kconfig tooling complains about a circular dependency (even > > thought it > > isn't really). The intermediate DELL_SMBIOS_USING_DCDBAS just works > > around that. > > > > Your asking if we can do something to avoid the "!". Why should we try > > to do > > that? What's the problem with the "!"? > > No problem. > > > (Noting that yes, this is ugly, but it's > > a minimal change that is functional within the context of rather messy > > design > > and interdependencies that we have to work with between these > > modules). > > I'm fine with it, I was rather wondering. OK, I've pushed this to testing. Will propogate through to for-next and the dell-smbios series to fixes for another RC PR to Linus. I don't think this is the final solution. I'd like us to think about these modules and how we can simplify these interdependencies. Mario, I'd especially appreciate your thoughts with respect to which of these are current/future, and which are legacy, as this will impact the design.
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 022c837..0b3d330 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -107,12 +107,21 @@ config ASUS_LAPTOP config DELL_SMBIOS tristate "Dell SMBIOS driver" + depends on !DELL_SMBIOS_USING_DCDBAS || (DCDBAS || DCDBAS=n) ---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. +# +# This decouples what would otherwise be a recursive dependency between +# DELL_SMBIOS and DELL_SMBIOS_SMM. If DELL_SMBIOS_SMM is selected, then +# DELL_SMBIOS needs to depend on DCDBAS. +# +config DELL_SMBIOS_USING_DCDBAS + bool + config DELL_SMBIOS_WMI bool "Dell SMBIOS driver WMI backend" default y @@ -132,6 +141,7 @@ config DELL_SMBIOS_SMM default y depends on DCDBAS depends on DELL_SMBIOS + select DELL_SMBIOS_USING_DCDBAS ---help--- This provides an implementation for the Dell SMBIOS calling interface communicated over SMI/SMM.