Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Dominik Brodowski]

Mario,

unfortunately, my Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016 fails to
boot v4.16-rc5. More exactly, I could bisect it down to commit 25d47027e10
("platform/x86: dell-smbios: Link all dell-smbios-* modules together").
Usually, I have enabled

CONFIG_SENSORS_DELL_SMM=y
CONFIG_DELL_SMBIOS=y
CONFIG_DELL_SMBIOS_WMI=y
CONFIG_DELL_SMBIOS_SMM=y
CONFIG_DELL_LAPTOP=y
CONFIG_DELL_WMI=y
CONFIG_DELL_WMI_DESCRIPTOR=y
# CONFIG_DELL_WMI_AIO is not set
# CONFIG_DELL_WMI_LED is not set
# CONFIG_DELL_SMO8800 is not set
# CONFIG_DELL_RBTN is not set
# CONFIG_DELL_RBU is not set

For v4.16-rc5 to work, I need to manually disable DELL_SMBIOS_WMI:

-CONFIG_DELL_SMBIOS_WMI=y
+# CONFIG_DELL_SMBIOS_WMI is not set

Any ideas?

Thanks,
	Dominik

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Darren Hart]

On Mon, Mar 12, 2018 at 07:53:57PM +0100, Dominik Brodowski wrote:
> Mario,
> 
> unfortunately, my Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016 fails to
> boot v4.16-rc5. More exactly, I could bisect it down to commit 25d47027e10
> ("platform/x86: dell-smbios: Link all dell-smbios-* modules together").
> Usually, I have enabled
> 
> CONFIG_SENSORS_DELL_SMM=y
> CONFIG_DELL_SMBIOS=y
> CONFIG_DELL_SMBIOS_WMI=y
> CONFIG_DELL_SMBIOS_SMM=y
> CONFIG_DELL_LAPTOP=y
> CONFIG_DELL_WMI=y
> CONFIG_DELL_WMI_DESCRIPTOR=y
> # CONFIG_DELL_WMI_AIO is not set
> # CONFIG_DELL_WMI_LED is not set
> # CONFIG_DELL_SMO8800 is not set
> # CONFIG_DELL_RBTN is not set
> # CONFIG_DELL_RBU is not set
> 
> For v4.16-rc5 to work, I need to manually disable DELL_SMBIOS_WMI:
> 
> -CONFIG_DELL_SMBIOS_WMI=y
> +# CONFIG_DELL_SMBIOS_WMI is not set
> 
> Any ideas?

We're working on a dependency fix, but I think the problem you have here is
likely to be ACPI_WMI=m instead of =y while you have DELL_SMBIOS=y

The fix won't allow that configuration. I'll include you in Cc of v2 of that
fix which I'll send shortly.

-- 
Darren Hart
VMware Open Source Technology Center

[PATCH v2] platform/x86: dell-smbios: Resolve dependency error on ACPI_WMI

[Darren Hart]

Similarly to DCDBAS for DELL_SMBIOS_SMM, if DELL_SMBIOS_WMI is enabled,
DELL_SMBIOS becomes dependent on ACPI_WMI. Update the depends lines to
prevent a configuration where DELL_SMBIOS=y and either backend
dependency =m. Update the comment accordingly.

Cc: Mario Limonciello <mario.limonciello@dell.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
Since v1: Split depends line into two per Andy Shevchenko's feedback

 drivers/platform/x86/Kconfig | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index e55b008..1868aab 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -106,13 +106,14 @@ config ASUS_LAPTOP
 	  If you have an ACPI-compatible ASUS laptop, say Y or M here.
 
 #
-# If the DELL_SMBIOS_SMM feature is enabled, the DELL_SMBIOS driver
-# becomes dependent on the DCDBAS driver. The "depends" line prevents a
-# configuration where DELL_SMBIOS=y while DCDBAS=m.
+# The DELL_SMBIOS driver depends on ACPI_WMI and/or DCDBAS if those
+# backends are selected. The "depends" line prevents a configuration
+# where DELL_SMBIOS=y while either of those dependencies =m.
 #
 config DELL_SMBIOS
 	tristate "Dell SMBIOS driver"
 	depends on DCDBAS || DCDBAS=n
+	depends on ACPI_WMI || ACPI_WMI=n
 	---help---
 	This provides support for the Dell SMBIOS calling interface.
 	If you have a Dell computer you should enable this option.
-- 
2.9.5


-- 
Darren Hart
VMware Open Source Technology Center

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Dominik Brodowski]

On Mon, Mar 12, 2018 at 01:23:19PM -0700, Darren Hart wrote:
> On Mon, Mar 12, 2018 at 07:53:57PM +0100, Dominik Brodowski wrote:
> > Mario,
> > 
> > unfortunately, my Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016 fails to
> > boot v4.16-rc5. More exactly, I could bisect it down to commit 25d47027e10
> > ("platform/x86: dell-smbios: Link all dell-smbios-* modules together").
> > Usually, I have enabled
> > 
> > CONFIG_SENSORS_DELL_SMM=y
> > CONFIG_DELL_SMBIOS=y
> > CONFIG_DELL_SMBIOS_WMI=y
> > CONFIG_DELL_SMBIOS_SMM=y
> > CONFIG_DELL_LAPTOP=y
> > CONFIG_DELL_WMI=y
> > CONFIG_DELL_WMI_DESCRIPTOR=y
> > # CONFIG_DELL_WMI_AIO is not set
> > # CONFIG_DELL_WMI_LED is not set
> > # CONFIG_DELL_SMO8800 is not set
> > # CONFIG_DELL_RBTN is not set
> > # CONFIG_DELL_RBU is not set
> > 
> > For v4.16-rc5 to work, I need to manually disable DELL_SMBIOS_WMI:
> > 
> > -CONFIG_DELL_SMBIOS_WMI=y
> > +# CONFIG_DELL_SMBIOS_WMI is not set
> > 
> > Any ideas?
> 
> We're working on a dependency fix, but I think the problem you have here is
> likely to be ACPI_WMI=m instead of =y while you have DELL_SMBIOS=y
> 
> The fix won't allow that configuration. I'll include you in Cc of v2 of that
> fix which I'll send shortly.

On Mon, Mar 12, 2018 at 01:36:06PM -0700, Darren Hart wrote:
> Similarly to DCDBAS for DELL_SMBIOS_SMM, if DELL_SMBIOS_WMI is enabled,
> DELL_SMBIOS becomes dependent on ACPI_WMI. Update the depends lines to
> prevent a configuration where DELL_SMBIOS=y and either backend
> dependency =m. Update the comment accordingly.

I'm awfully sorry, but this patch doesn't solve the issue.
CONFIG_ACPI_WMI=y was/is set, but still enabling CONFIG_DELL_SMBIOS_WMI=y
causes a very-early crash of v4.16-rc5. In fact, so early that the normal
boot messages never show up on the screen...

Thanks,
	Dominik

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Linus Torvalds]

On Mon, Mar 12, 2018 at 3:07 PM, Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
>
> I'm awfully sorry, but this patch doesn't solve the issue.
> CONFIG_ACPI_WMI=y was/is set, but still enabling CONFIG_DELL_SMBIOS_WMI=y
> causes a very-early crash of v4.16-rc5. In fact, so early that the normal
> boot messages never show up on the screen...

Hmm. My xps13 works fine, but it's the 9350 version so not the same machine.

But it does sound like that commit 25d47027e10 ("platform/x86:
dell-smbios: Link all dell-smbios-* modules together") should just be
reverted. It has clearly caused a lot more pain than it fixed.

                Linus

RE: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Mario.Limonciello]

> -----Original Message-----
> From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> Sent: Tuesday, March 13, 2018 2:54 AM
> To: dvhart@infradead.org; Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> 
> Mario,
> 
> unfortunately, my Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016 fails to
> boot v4.16-rc5. More exactly, I could bisect it down to commit 25d47027e10
> ("platform/x86: dell-smbios: Link all dell-smbios-* modules together").
> Usually, I have enabled
> 
> CONFIG_SENSORS_DELL_SMM=y
> CONFIG_DELL_SMBIOS=y
> CONFIG_DELL_SMBIOS_WMI=y
> CONFIG_DELL_SMBIOS_SMM=y
> CONFIG_DELL_LAPTOP=y
> CONFIG_DELL_WMI=y
> CONFIG_DELL_WMI_DESCRIPTOR=y
> # CONFIG_DELL_WMI_AIO is not set
> # CONFIG_DELL_WMI_LED is not set
> # CONFIG_DELL_SMO8800 is not set
> # CONFIG_DELL_RBTN is not set
> # CONFIG_DELL_RBU is not set
> 
> For v4.16-rc5 to work, I need to manually disable DELL_SMBIOS_WMI:
> 
> -CONFIG_DELL_SMBIOS_WMI=y
> +# CONFIG_DELL_SMBIOS_WMI is not set
> 
> Any ideas?
> 
Dominick,

Interesting.  Can you please change CONFIG_DELL_SMBIOS to a module
and see if that behavior persists?  If it does, can you please blacklist it on
the kernel command line and try to load it manually and share any
backtrace?

Thanks,

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Darren Hart]

On Mon, Mar 12, 2018 at 03:17:57PM -0700, Linus Torvalds wrote:
> On Mon, Mar 12, 2018 at 3:07 PM, Dominik Brodowski
> <linux@dominikbrodowski.net> wrote:
> >
> > I'm awfully sorry, but this patch doesn't solve the issue.
> > CONFIG_ACPI_WMI=y was/is set, but still enabling CONFIG_DELL_SMBIOS_WMI=y
> > causes a very-early crash of v4.16-rc5. In fact, so early that the normal
> > boot messages never show up on the screen...
> 
> Hmm. My xps13 works fine, but it's the 9350 version so not the same machine.
> 
> But it does sound like that commit 25d47027e10 ("platform/x86:
> dell-smbios: Link all dell-smbios-* modules together") should just be
> reverted. It has clearly caused a lot more pain than it fixed.

Unfortunately, yes. This will re-instate the race condition it "fixed".
I'll work with Mario to fix this properly in 4.17, preferably in a way
that we can apply to stable. Pending an "oops, nevermind, the patch
works" from Dominik by tomorrow morning, I'll submit the pull request to
revert dell-smbios changes back to and including 25d47027e10.

Apologies for the noise on this one Linus, it got away from me.

-- 
Darren Hart
VMware Open Source Technology Center

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Dominik Brodowski]

On Mon, Mar 12, 2018 at 10:42:01PM +0000, Mario.Limonciello@dell.com wrote:
> 
> 
> > -----Original Message-----
> > From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> > Sent: Tuesday, March 13, 2018 2:54 AM
> > To: dvhart@infradead.org; Limonciello, Mario <Mario_Limonciello@Dell.com>
> > Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> > 
> > Mario,
> > 
> > unfortunately, my Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016 fails to
> > boot v4.16-rc5. More exactly, I could bisect it down to commit 25d47027e10
> > ("platform/x86: dell-smbios: Link all dell-smbios-* modules together").
> > Usually, I have enabled
> > 
> > CONFIG_SENSORS_DELL_SMM=y
> > CONFIG_DELL_SMBIOS=y
> > CONFIG_DELL_SMBIOS_WMI=y
> > CONFIG_DELL_SMBIOS_SMM=y
> > CONFIG_DELL_LAPTOP=y
> > CONFIG_DELL_WMI=y
> > CONFIG_DELL_WMI_DESCRIPTOR=y
> > # CONFIG_DELL_WMI_AIO is not set
> > # CONFIG_DELL_WMI_LED is not set
> > # CONFIG_DELL_SMO8800 is not set
> > # CONFIG_DELL_RBTN is not set
> > # CONFIG_DELL_RBU is not set
> > 
> > For v4.16-rc5 to work, I need to manually disable DELL_SMBIOS_WMI:
> > 
> > -CONFIG_DELL_SMBIOS_WMI=y
> > +# CONFIG_DELL_SMBIOS_WMI is not set
> > 
> > Any ideas?
> > 
> Dominick,
> 
> Interesting.  Can you please change CONFIG_DELL_SMBIOS to a module
> and see if that behavior persists?  If it does, can you please blacklist it on
> the kernel command line and try to load it manually and share any
> backtrace?

Mario,

building and running it as a *module* works flawlessly. But that was
actually expected after a 'grep "initcall"' in drivers/platform/x86:

As Darren pointed out, DELL_SMBIOS_WMI depends on ACPI_WMI, so probably
ACPI_WMI needs to be initialized first. However, the all-in-one
dell-smbios.o is run as subsys_initcall(), same as wmi.o
(subsys_initcall_sync() there).

If both are built-ins, that means that dell-smbios.o is run first, and wmi.o
second. Changing dell-smbios.o to run at the later fs_initcall() level
instead lets me boot the kernel. HOWEVER:

1) Is there a reason why both the core and the dell-smbios-smm driver have
   to run already at subsys_initcall() time? They did so previous to your
   patch. Is it OK to defer these parts opf the all-in-one dell-smbios.o
   to fs_initcall(), or even to the default device_initcall()?

2) dell-smbios-wmi depends on (well, selects) DELL_WMI_DESCRIPTOR. The
   dell-smbios-wmi is running at the default device_initcall() time, but
   (AFAICS) probably later than the initialization of dell-smbios-wmi.o.
   May I presume that this poses no additional problem?

Thanks,
	Dominik

RE: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Mario.Limonciello]

> -----Original Message-----
> From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> Sent: Tuesday, March 13, 2018 1:51 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; platform-driver-x86@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> 
> On Mon, Mar 12, 2018 at 10:42:01PM +0000, Mario.Limonciello@dell.com wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> > > Sent: Tuesday, March 13, 2018 2:54 AM
> > > To: dvhart@infradead.org; Limonciello, Mario <Mario_Limonciello@Dell.com>
> > > Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> > >
> > > Mario,
> > >
> > > unfortunately, my Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016 fails to
> > > boot v4.16-rc5. More exactly, I could bisect it down to commit 25d47027e10
> > > ("platform/x86: dell-smbios: Link all dell-smbios-* modules together").
> > > Usually, I have enabled
> > >
> > > CONFIG_SENSORS_DELL_SMM=y
> > > CONFIG_DELL_SMBIOS=y
> > > CONFIG_DELL_SMBIOS_WMI=y
> > > CONFIG_DELL_SMBIOS_SMM=y
> > > CONFIG_DELL_LAPTOP=y
> > > CONFIG_DELL_WMI=y
> > > CONFIG_DELL_WMI_DESCRIPTOR=y
> > > # CONFIG_DELL_WMI_AIO is not set
> > > # CONFIG_DELL_WMI_LED is not set
> > > # CONFIG_DELL_SMO8800 is not set
> > > # CONFIG_DELL_RBTN is not set
> > > # CONFIG_DELL_RBU is not set
> > >
> > > For v4.16-rc5 to work, I need to manually disable DELL_SMBIOS_WMI:
> > >
> > > -CONFIG_DELL_SMBIOS_WMI=y
> > > +# CONFIG_DELL_SMBIOS_WMI is not set
> > >
> > > Any ideas?
> > >
> > Dominick,
> >
> > Interesting.  Can you please change CONFIG_DELL_SMBIOS to a module
> > and see if that behavior persists?  If it does, can you please blacklist it on
> > the kernel command line and try to load it manually and share any
> > backtrace?
> 
> Mario,
> 
> building and running it as a *module* works flawlessly. But that was
> actually expected after a 'grep "initcall"' in drivers/platform/x86:

Dominck,

Thanks for your checking and explanation of the situation.

> 
> As Darren pointed out, DELL_SMBIOS_WMI depends on ACPI_WMI, so probably
> ACPI_WMI needs to be initialized first. However, the all-in-one
> dell-smbios.o is run as subsys_initcall(), same as wmi.o
> (subsys_initcall_sync() there).
> 
> If both are built-ins, that means that dell-smbios.o is run first, and wmi.o
> second. Changing dell-smbios.o to run at the later fs_initcall() level
> instead lets me boot the kernel. HOWEVER:
> 
> 1) Is there a reason why both the core and the dell-smbios-smm driver have
>    to run already at subsys_initcall() time? They did so previous to your
>    patch. Is it OK to defer these parts opf the all-in-one dell-smbios.o
>    to fs_initcall(), or even to the default device_initcall()?

As long as they're ready before dell-laptop's initialization which uses
late_initcall that should be fine.

Am I correct to presume you're going to propose a patch you can test and
confirm your hypothesis rather than Darren reverting my patch to bring
them together?

> 
> 2) dell-smbios-wmi depends on (well, selects) DELL_WMI_DESCRIPTOR. The
>    dell-smbios-wmi is running at the default device_initcall() time, but
>    (AFAICS) probably later than the initialization of dell-smbios-wmi.o.
>    May I presume that this poses no additional problem?
> 

I "think" that should actually be fine.  The driver will use deferred probing
until it's ready meaning that dell-smbios-wmi should wait until dell-wmi-descriptor
is done with it's probing routine.
 

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Darren Hart]

On Tue, Mar 13, 2018 at 06:51:09AM +0100, Dominik Brodowski wrote:
> On Mon, Mar 12, 2018 at 10:42:01PM +0000, Mario.Limonciello@dell.com wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> > > Sent: Tuesday, March 13, 2018 2:54 AM
> > > To: dvhart@infradead.org; Limonciello, Mario <Mario_Limonciello@Dell.com>
> > > Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> > > 
> > > Mario,
> > > 
> > > unfortunately, my Dell Inc. XPS 13 9343/0TM99H, BIOS A11 12/08/2016 fails to
> > > boot v4.16-rc5. More exactly, I could bisect it down to commit 25d47027e10
> > > ("platform/x86: dell-smbios: Link all dell-smbios-* modules together").
> > > Usually, I have enabled
> > > 
> > > CONFIG_SENSORS_DELL_SMM=y
> > > CONFIG_DELL_SMBIOS=y
> > > CONFIG_DELL_SMBIOS_WMI=y
> > > CONFIG_DELL_SMBIOS_SMM=y
> > > CONFIG_DELL_LAPTOP=y
> > > CONFIG_DELL_WMI=y
> > > CONFIG_DELL_WMI_DESCRIPTOR=y
> > > # CONFIG_DELL_WMI_AIO is not set
> > > # CONFIG_DELL_WMI_LED is not set
> > > # CONFIG_DELL_SMO8800 is not set
> > > # CONFIG_DELL_RBTN is not set
> > > # CONFIG_DELL_RBU is not set
> > > 
> > > For v4.16-rc5 to work, I need to manually disable DELL_SMBIOS_WMI:
> > > 
> > > -CONFIG_DELL_SMBIOS_WMI=y
> > > +# CONFIG_DELL_SMBIOS_WMI is not set
> > > 
> > > Any ideas?
> > > 
> > Dominick,
> > 
> > Interesting.  Can you please change CONFIG_DELL_SMBIOS to a module
> > and see if that behavior persists?  If it does, can you please blacklist it on
> > the kernel command line and try to load it manually and share any
> > backtrace?
> 
> Mario,
> 
> building and running it as a *module* works flawlessly. But that was
> actually expected after a 'grep "initcall"' in drivers/platform/x86:
> 
> As Darren pointed out, DELL_SMBIOS_WMI depends on ACPI_WMI, so probably
> ACPI_WMI needs to be initialized first. However, the all-in-one
> dell-smbios.o is run as subsys_initcall(), same as wmi.o
> (subsys_initcall_sync() there).
> 
> If both are built-ins, that means that dell-smbios.o is run first, and wmi.o
> second. Changing dell-smbios.o to run at the later fs_initcall() level
> instead lets me boot the kernel. HOWEVER:
> 
> 1) Is there a reason why both the core and the dell-smbios-smm driver have
>    to run already at subsys_initcall() time? They did so previous to your
>    patch. Is it OK to defer these parts opf the all-in-one dell-smbios.o
>    to fs_initcall(), or even to the default device_initcall()?
> 
> 2) dell-smbios-wmi depends on (well, selects) DELL_WMI_DESCRIPTOR. The
>    dell-smbios-wmi is running at the default device_initcall() time, but
>    (AFAICS) probably later than the initialization of dell-smbios-wmi.o.
>    May I presume that this poses no additional problem?

Nice catch Dominik,

There is one other caveat, which you'll find documented in
dell-laptop.c, namely that dell-laptop needs to init after dell-rbtn
(I'm starting to appreciate the monolithic thinkpad-acpi driver).

We need things to init in this order (items on the same line have no
dependency):

1. DCDBAS, ACPI_WMI
2. DELL_SMBIOS, DELL_RBTN
3. DELL_LAPTOP, DELL_WMI

Currently:
subsys_initcall: ACPI_WMI, DELL_SMBIOS
module_init: DCDBAS, DELL_WMI
late_initcall: DELL_LAPTOP

>From a quick naive glance, it appears as though we might be able to
address this as follows:

subsys_initcall: DCDBAS, ACPI_WMI
module_init: DELL_SMBIOS, DELL_RBTN
late_initcall: DELL_LAPTOP, DELL_WMI

e.g. Totally untested...

>From 2dd1191593a46bf8fd1e5be8bbfb98b797e041f8 Mon Sep 17 00:00:00 2001
Message-Id: <2dd1191593a46bf8fd1e5be8bbfb98b797e041f8.1520922688.git.dvhart@infradead.org>
From: "Darren Hart (VMware)" <dvhart@infradead.org>
Date: Mon, 12 Mar 2018 23:28:00 -0700
Subject: [PATCH] platform/x86: Fix dell driver init order

Update the initcall ordering to satisfy the following dependency
ordering:

1. DCDBAS, ACPI_WMI
2. DELL_SMBIOS, DELL_RBTN
3. DELL_LAPTOP, DELL_WMI

By assigning them to the following initcall levels:

subsys_initcall: DCDBAS, ACPI_WMI
module_init: DELL_SMBIOS, DELL_RBTN
late_initcall: DELL_LAPTOP, DELL_WMI

Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Mario.Limonciello@dell.com
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
 drivers/firmware/dcdbas.c               | 2 +-
 drivers/platform/x86/dell-smbios-base.c | 2 +-
 drivers/platform/x86/dell-wmi.c         | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/dcdbas.c b/drivers/firmware/dcdbas.c
index c16600f..0bdea60 100644
--- a/drivers/firmware/dcdbas.c
+++ b/drivers/firmware/dcdbas.c
@@ -639,7 +639,7 @@ static void __exit dcdbas_exit(void)
 	platform_driver_unregister(&dcdbas_driver);
 }
 
-module_init(dcdbas_init);
+subsys_initcall_sync(dcdbas_init);
 module_exit(dcdbas_exit);
 
 MODULE_DESCRIPTION(DRIVER_DESCRIPTION " (version " DRIVER_VERSION ")");
diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
index 5bcf8a1..2485c80 100644
--- a/drivers/platform/x86/dell-smbios-base.c
+++ b/drivers/platform/x86/dell-smbios-base.c
@@ -637,7 +637,7 @@ static void __exit dell_smbios_exit(void)
 	mutex_unlock(&smbios_mutex);
 }
 
-subsys_initcall(dell_smbios_init);
+module_init(dell_smbios_init);
 module_exit(dell_smbios_exit);
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 2c99274..8d10219 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -714,7 +714,7 @@ static int __init dell_wmi_init(void)
 
 	return wmi_driver_register(&dell_wmi_driver);
 }
-module_init(dell_wmi_init);
+late_initcall(dell_wmi_init);
 
 static void __exit dell_wmi_exit(void)
 {
-- 
2.9.5


-- 
Darren Hart
VMware Open Source Technology Center

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Dominik Brodowski]

On Tue, Mar 13, 2018 at 06:12:04AM +0000, Mario.Limonciello@dell.com wrote:
> As long as they're ready before dell-laptop's initialization which uses
> late_initcall that should be fine.
> 
> Am I correct to presume you're going to propose a patch you can test and
> confirm your hypothesis rather than Darren reverting my patch to bring
> them together?

Thanks for the input; a draft patch (which works fine on my system) is
attached below.

On Mon, Mar 12, 2018 at 11:32:13PM -0700, Darren Hart wrote:
> There is one other caveat, which you'll find documented in
> dell-laptop.c, namely that dell-laptop needs to init after dell-rbtn
> (I'm starting to appreciate the monolithic thinkpad-acpi driver).
> 
> We need things to init in this order (items on the same line have no
> dependency):
> 
> 1. DCDBAS, ACPI_WMI
> 2. DELL_SMBIOS, DELL_RBTN
> 3. DELL_LAPTOP, DELL_WMI
> 
> Currently:
> subsys_initcall: ACPI_WMI, DELL_SMBIOS
> module_init: DCDBAS, DELL_WMI
> late_initcall: DELL_LAPTOP
> 
> From a quick naive glance, it appears as though we might be able to
> address this as follows:
> 
> subsys_initcall: DCDBAS, ACPI_WMI
> module_init: DELL_SMBIOS, DELL_RBTN
> late_initcall: DELL_LAPTOP, DELL_WMI

Hmmm. I do not yet understand why you propose to

a) advance the DCDBAS initialization to subsys_initcall, as only DELL_LAPTOP
   (running as a late_initcall) requires it to be up and running, and

b) delay DELL_WMI to late_initcall, as it can safely be initialized as long
   as ACPI_WMI is ready.

What do I miss?

Attached patch *seems* to work fine on my system.

Thanks,
	Dominik

----
From: Dominik Brodowski <linux@dominikbrodowski.net>
Date: Tue, 13 Mar 2018 07:27:41 +0100
Subject: [PATCH] platform/x86: dell-smbios: wait for WMI initialization

If DELL_SMBIOS_WMI is enabled, the combined dell-smbios driver
needs to wait for ACPI_WMI to be initialized, which occurs at the
subsys_initcall() level. As the dell-smbios driver only needs to
be up and running by the time the dell-laptop module is initialized,
which occurs at the late_initcall() level, it is sufficient to
initialize the dell-smbios driver at the default device_initcall()
level.

Fixes: 25d47027e100 ("platform/x86: dell-smbios: Link all dell-smbios-* modules together")
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-smbios-base.c
index 5bcf8a18f785..2485c80a9fdd 100644
--- a/drivers/platform/x86/dell-smbios-base.c
+++ b/drivers/platform/x86/dell-smbios-base.c
@@ -637,7 +637,7 @@ static void __exit dell_smbios_exit(void)
 	mutex_unlock(&smbios_mutex);
 }
 
-subsys_initcall(dell_smbios_init);
+module_init(dell_smbios_init);
 module_exit(dell_smbios_exit);
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");

RE: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Mario.Limonciello]

> -----Original Message-----
> From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> Sent: Tuesday, March 13, 2018 2:44 PM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>; Darren Hart
> <dvhart@infradead.org>
> Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> 
> On Tue, Mar 13, 2018 at 06:12:04AM +0000, Mario.Limonciello@dell.com wrote:
> > As long as they're ready before dell-laptop's initialization which uses
> > late_initcall that should be fine.
> >
> > Am I correct to presume you're going to propose a patch you can test and
> > confirm your hypothesis rather than Darren reverting my patch to bring
> > them together?
> 
> Thanks for the input; a draft patch (which works fine on my system) is
> attached below.
> 
> On Mon, Mar 12, 2018 at 11:32:13PM -0700, Darren Hart wrote:
> > There is one other caveat, which you'll find documented in
> > dell-laptop.c, namely that dell-laptop needs to init after dell-rbtn
> > (I'm starting to appreciate the monolithic thinkpad-acpi driver).
> >
> > We need things to init in this order (items on the same line have no
> > dependency):
> >
> > 1. DCDBAS, ACPI_WMI
> > 2. DELL_SMBIOS, DELL_RBTN
> > 3. DELL_LAPTOP, DELL_WMI
> >
> > Currently:
> > subsys_initcall: ACPI_WMI, DELL_SMBIOS
> > module_init: DCDBAS, DELL_WMI
> > late_initcall: DELL_LAPTOP
> >
> > From a quick naive glance, it appears as though we might be able to
> > address this as follows:
> >
> > subsys_initcall: DCDBAS, ACPI_WMI
> > module_init: DELL_SMBIOS, DELL_RBTN
> > late_initcall: DELL_LAPTOP, DELL_WMI
> 
> Hmmm. I do not yet understand why you propose to
> 
> a) advance the DCDBAS initialization to subsys_initcall, as only DELL_LAPTOP
>    (running as a late_initcall) requires it to be up and running, and

Actually dell-smbios itself should require this too.  The SMM backend will use
it during initialization to determine if WSMT is enabled.  If it's not operational
yet then we may get invalid results.

So considering this I think Darren's proposal is good to move DCDBAS to earlier.

> 
> b) delay DELL_WMI to late_initcall, as it can safely be initialized as long
>    as ACPI_WMI is ready.

Maybe Darren meant dell-wmi-descriptor not dell-wmi?
Otherwise I would agree that part isn't needed.

> 
> What do I miss?
> 
> Attached patch *seems* to work fine on my system.

At least to me, I think it's better to see 
> 
> Thanks,
> 	Dominik
> 
> ----
> From: Dominik Brodowski <linux@dominikbrodowski.net>
> Date: Tue, 13 Mar 2018 07:27:41 +0100
> Subject: [PATCH] platform/x86: dell-smbios: wait for WMI initialization
> 
> If DELL_SMBIOS_WMI is enabled, the combined dell-smbios driver
> needs to wait for ACPI_WMI to be initialized, which occurs at the
> subsys_initcall() level. As the dell-smbios driver only needs to
> be up and running by the time the dell-laptop module is initialized,
> which occurs at the late_initcall() level, it is sufficient to
> initialize the dell-smbios driver at the default device_initcall()
> level.
> 
> Fixes: 25d47027e100 ("platform/x86: dell-smbios: Link all dell-smbios-* modules
> together")
> Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
> 
> diff --git a/drivers/platform/x86/dell-smbios-base.c b/drivers/platform/x86/dell-
> smbios-base.c
> index 5bcf8a18f785..2485c80a9fdd 100644
> --- a/drivers/platform/x86/dell-smbios-base.c
> +++ b/drivers/platform/x86/dell-smbios-base.c
> @@ -637,7 +637,7 @@ static void __exit dell_smbios_exit(void)
>  	mutex_unlock(&smbios_mutex);
>  }
> 
> -subsys_initcall(dell_smbios_init);
> +module_init(dell_smbios_init);
>  module_exit(dell_smbios_exit);
> 
>  MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Darren Hart]

On Tue, Mar 13, 2018 at 07:07:26AM +0000, Mario.Limonciello@dell.com wrote:
> > -----Original Message-----
> > From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> > Sent: Tuesday, March 13, 2018 2:44 PM
> > To: Limonciello, Mario <Mario_Limonciello@Dell.com>; Darren Hart
> > <dvhart@infradead.org>
> > Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> > 
> > On Tue, Mar 13, 2018 at 06:12:04AM +0000, Mario.Limonciello@dell.com wrote:
> > > As long as they're ready before dell-laptop's initialization which uses
> > > late_initcall that should be fine.
> > >
> > > Am I correct to presume you're going to propose a patch you can test and
> > > confirm your hypothesis rather than Darren reverting my patch to bring
> > > them together?
> > 
> > Thanks for the input; a draft patch (which works fine on my system) is
> > attached below.
> > 
> > On Mon, Mar 12, 2018 at 11:32:13PM -0700, Darren Hart wrote:
> > > There is one other caveat, which you'll find documented in
> > > dell-laptop.c, namely that dell-laptop needs to init after dell-rbtn
> > > (I'm starting to appreciate the monolithic thinkpad-acpi driver).
> > >
> > > We need things to init in this order (items on the same line have no
> > > dependency):
> > >
> > > 1. DCDBAS, ACPI_WMI
> > > 2. DELL_SMBIOS, DELL_RBTN
> > > 3. DELL_LAPTOP, DELL_WMI
> > >
> > > Currently:
> > > subsys_initcall: ACPI_WMI, DELL_SMBIOS
> > > module_init: DCDBAS, DELL_WMI
> > > late_initcall: DELL_LAPTOP
> > >
> > > From a quick naive glance, it appears as though we might be able to
> > > address this as follows:
> > >
> > > subsys_initcall: DCDBAS, ACPI_WMI
> > > module_init: DELL_SMBIOS, DELL_RBTN
> > > late_initcall: DELL_LAPTOP, DELL_WMI
> > 
> > Hmmm. I do not yet understand why you propose to
> > 
> > a) advance the DCDBAS initialization to subsys_initcall, as only DELL_LAPTOP
> >    (running as a late_initcall) requires it to be up and running, and
> 
> Actually dell-smbios itself should require this too.  The SMM backend will use
> it during initialization to determine if WSMT is enabled.  If it's not operational
> yet then we may get invalid results.
>

Exactly.

> So considering this I think Darren's proposal is good to move DCDBAS to earlier.
> 
> > 
> > b) delay DELL_WMI to late_initcall, as it can safely be initialized as long
> >    as ACPI_WMI is ready.
> 
> Maybe Darren meant dell-wmi-descriptor not dell-wmi?
> Otherwise I would agree that part isn't needed.

Like DELL_LAPTOP, DELL_WMI depends on smbios being ready, so needs to
init after DELL_SMBIOS as well.

-- 
Darren Hart
VMware Open Source Technology Center

RE: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Mario.Limonciello]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Wednesday, March 14, 2018 5:43 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: linux@dominikbrodowski.net; platform-driver-x86@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> 
> On Tue, Mar 13, 2018 at 07:07:26AM +0000, Mario.Limonciello@dell.com wrote:
> > > -----Original Message-----
> > > From: Dominik Brodowski [mailto:linux@dominikbrodowski.net]
> > > Sent: Tuesday, March 13, 2018 2:44 PM
> > > To: Limonciello, Mario <Mario_Limonciello@Dell.com>; Darren Hart
> > > <dvhart@infradead.org>
> > > Cc: platform-driver-x86@vger.kernel.org; linux-kernel@vger.kernel.org
> > > Subject: Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5
> > >
> > > On Tue, Mar 13, 2018 at 06:12:04AM +0000, Mario.Limonciello@dell.com wrote:
> > > > As long as they're ready before dell-laptop's initialization which uses
> > > > late_initcall that should be fine.
> > > >
> > > > Am I correct to presume you're going to propose a patch you can test and
> > > > confirm your hypothesis rather than Darren reverting my patch to bring
> > > > them together?
> > >
> > > Thanks for the input; a draft patch (which works fine on my system) is
> > > attached below.
> > >
> > > On Mon, Mar 12, 2018 at 11:32:13PM -0700, Darren Hart wrote:
> > > > There is one other caveat, which you'll find documented in
> > > > dell-laptop.c, namely that dell-laptop needs to init after dell-rbtn
> > > > (I'm starting to appreciate the monolithic thinkpad-acpi driver).
> > > >
> > > > We need things to init in this order (items on the same line have no
> > > > dependency):
> > > >
> > > > 1. DCDBAS, ACPI_WMI
> > > > 2. DELL_SMBIOS, DELL_RBTN
> > > > 3. DELL_LAPTOP, DELL_WMI
> > > >
> > > > Currently:
> > > > subsys_initcall: ACPI_WMI, DELL_SMBIOS
> > > > module_init: DCDBAS, DELL_WMI
> > > > late_initcall: DELL_LAPTOP
> > > >
> > > > From a quick naive glance, it appears as though we might be able to
> > > > address this as follows:
> > > >
> > > > subsys_initcall: DCDBAS, ACPI_WMI
> > > > module_init: DELL_SMBIOS, DELL_RBTN
> > > > late_initcall: DELL_LAPTOP, DELL_WMI
> > >
> > > Hmmm. I do not yet understand why you propose to
> > >
> > > a) advance the DCDBAS initialization to subsys_initcall, as only DELL_LAPTOP
> > >    (running as a late_initcall) requires it to be up and running, and
> >
> > Actually dell-smbios itself should require this too.  The SMM backend will use
> > it during initialization to determine if WSMT is enabled.  If it's not operational
> > yet then we may get invalid results.
> >
> 
> Exactly.
> 
> > So considering this I think Darren's proposal is good to move DCDBAS to earlier.
> >
> > >
> > > b) delay DELL_WMI to late_initcall, as it can safely be initialized as long
> > >    as ACPI_WMI is ready.
> >
> > Maybe Darren meant dell-wmi-descriptor not dell-wmi?
> > Otherwise I would agree that part isn't needed.
> 
> Like DELL_LAPTOP, DELL_WMI depends on smbios being ready, so needs to
> init after DELL_SMBIOS as well.
> 
> --

OK thanks for explaining.

Dominik,

Can you please verify if Darren's patch works for you too?

Darren,

If that patch works for Dominik, I'm fine with that for 4.16 if you keep
this series, otherwise we can retry for 4.17 and squash the various patches
together.

Thanks,

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Darren Hart]

On Mon, Mar 12, 2018 at 04:33:11PM -0700, Darren Hart wrote:
> On Mon, Mar 12, 2018 at 03:17:57PM -0700, Linus Torvalds wrote:
> > On Mon, Mar 12, 2018 at 3:07 PM, Dominik Brodowski
> > <linux@dominikbrodowski.net> wrote:
> > >
> > > I'm awfully sorry, but this patch doesn't solve the issue.
> > > CONFIG_ACPI_WMI=y was/is set, but still enabling CONFIG_DELL_SMBIOS_WMI=y
> > > causes a very-early crash of v4.16-rc5. In fact, so early that the normal
> > > boot messages never show up on the screen...
> > 
> > Hmm. My xps13 works fine, but it's the 9350 version so not the same machine.
> > 
> > But it does sound like that commit 25d47027e10 ("platform/x86:
> > dell-smbios: Link all dell-smbios-* modules together") should just be
> > reverted. It has clearly caused a lot more pain than it fixed.
> 
> Unfortunately, yes. This will re-instate the race condition it "fixed".
> I'll work with Mario to fix this properly in 4.17, preferably in a way
> that we can apply to stable. Pending an "oops, nevermind, the patch
> works" from Dominik by tomorrow morning, I'll submit the pull request to
> revert dell-smbios changes back to and including 25d47027e10.
> 
> Apologies for the noise on this one Linus, it got away from me.

Hi Linus,

OK, I think we've identified the ordering issues and I have a patch out
pending testing from Dominik (hopefully tonight). I'd like to hold off
one more day on reverting if you're OK with that.

At RC5, we're certainly further into the RC cycle than I want to see
these changes. So if you just want to call it now, I understand and I'll
send you the revert pull request. If you can give me another day, I
think we may finally have found the end of this loose string.

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Dominik Brodowski]

On Wed, Mar 14, 2018 at 01:28:54AM +0000, Mario.Limonciello@dell.com wrote:
> Dominik,
> 
> Can you please verify if Darren's patch works for you too?

Yes, it does.

Thanks,
	Dominik

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Linus Torvalds]

On Tue, Mar 13, 2018 at 8:34 PM, Darren Hart <dvhart@infradead.org> wrote:
>
> At RC5, we're certainly further into the RC cycle than I want to see
> these changes. So if you just want to call it now, I understand and I'll
> send you the revert pull request. If you can give me another day, I
> think we may finally have found the end of this loose string.

Since it works for Dominik, and we understand what went wrong, I'm ok
with the fix rather than the revert.

But if somebody else ends up finding another problem later, we really
need to revert it all basically immediately. No more time for games.

                  Linus

Re: Dell Inc. XPS 13 9343/0TM99H fails to boot v4.16-rc5

[Darren Hart]

On Wed, Mar 14, 2018 at 10:23:16AM -0700, Linus Torvalds wrote:
> On Tue, Mar 13, 2018 at 8:34 PM, Darren Hart <dvhart@infradead.org> wrote:
> >
> > At RC5, we're certainly further into the RC cycle than I want to see
> > these changes. So if you just want to call it now, I understand and I'll
> > send you the revert pull request. If you can give me another day, I
> > think we may finally have found the end of this loose string.
> 
> Since it works for Dominik, and we understand what went wrong, I'm ok
> with the fix rather than the revert.
> 
> But if somebody else ends up finding another problem later, we really
> need to revert it all basically immediately. No more time for games.

On it. Branches are pushed and tagged, PR is written. Waiting for a
couple final validation builds and will send the PR.

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center