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

Commit Message

Darren Hart March 13, 2018, 6:32 a.m. UTC

On Tue, Mar 13, 2018 at 06:51:09AM +0100, Dominik Brodowski wrote:
> On Mon, Mar 12, 2018 at 10:42:01PM +0000, [e-mail] wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Dominik Brodowski [mailto:[e-mail]]
> > > Sent: Tuesday, March 13, 2018 2:54 AM
> > > To: [e-mail]; Limonciello, Mario <[e-mail]>
> > > Cc: [e-mail]; [e-mail]
> > > 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: <[e-mail]>
From: "Darren Hart (VMware)" <[e-mail]>
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 <[e-mail]>
Cc: [e-mail]
Signed-off-by: Darren Hart (VMware) <[e-mail]>
---
 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(-)

Patch

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 <[e-mail]>");
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)
 {