[PATCH 0/4] x86: Trust firmware a bit more about presence of 8042

[PATCH 0/4] x86: Trust firmware a bit more about presence of 8042

[Dmitry Torokhov]

Hi,

Historically we did not trust PNP data regarding keyboard controllers on
X86, but more and more boards get upset with us if they try to tell us that
there is no keyboard controller and we still go and try to poke at where we
think it might be. To work around this issue let's have a bit more faith in
BIOS data, and if [lack] of PNP devices for mouse and keyboard matches whet
firmware (basically ACPI FADT) tells us, let's abort i8042 probe.

We add a new flag (enum) to x86_platform.legacy structure so we can
distinguish between cases where platform/subarch never has 8042 (such as
MID platform) and cases where firmware says that it is not there, so that
i8042 driver can either abort immediately or go and check for presence of
PNP devices. We also remove x86_platform.i8042_detect() as it is no longer
used (platforms can set value of x86_platform.legacy.i8042 as needed in
quirks).

If you are OK with arch/x86 changes please apply together with the input
part.

Thanks,
Dmitry

Dmitry Torokhov (4):
  x86/init: add i8042 state to the platform data
  Input: i8042 - trust firmware a bit more when probing on X86
  x86/init: remove i8042_detect() form platform ops
  x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h

 arch/x86/include/asm/x86_init.h         | 26 +++++++++++++++++++++-----
 arch/x86/kernel/acpi/boot.c             |  7 +++++++
 arch/x86/kernel/platform-quirks.c       |  5 +++++
 arch/x86/kernel/x86_init.c              |  2 --
 arch/x86/platform/ce4100/ce4100.c       |  6 ------
 arch/x86/platform/intel-mid/intel-mid.c |  7 -------
 drivers/input/serio/i8042-x86ia64io.h   | 10 +++++++---
 7 files changed, 40 insertions(+), 23 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[PATCH 1/4] x86/init: add i8042 state to the platform data

[Dmitry Torokhov]

Add i8042 state to the platform data to help i8042 driver make decision
whether to probe for i8042 or not. We recognize 3 states: platform/subarch
ca not possible have i8042 (as is the case with Inrel MID platform),
firmware (such as ACPI) reports that i8042 is absent from the device,
or i8042 may be present and the driver should probe for it.

The intent is to allow i8042 driver abort initialization on x86 if PNP data
(absence of both keyboard and mouse PNP devices) agrees with firmware data.

It will also allow us to remove i8042_detect later.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/x86/include/asm/x86_init.h   | 18 ++++++++++++++++++
 arch/x86/kernel/acpi/boot.c       |  7 +++++++
 arch/x86/kernel/platform-quirks.c |  5 +++++
 3 files changed, 30 insertions(+)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 6ba7931..c4d09c7 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -165,8 +165,25 @@ struct x86_legacy_devices {
 };
 
 /**
+ * enum x86_legacy_i8042_state - i8042 keyboard controller state
+ * @X86_LEGACY_I8042_PLATFORM_ABSENT: the controller is always absent on
+ *	given platform/subarch.
+ * @X86_LEGACY_I8042_FIRMWARE_ABSENT: firmware reports that the controller
+ *	is absent.
+ * @X86_LEGACY_i8042_EXPECTED_PRESENT: the controller is likely to be
+ *	present, the i8042 driver should probe for controller existence.
+ */
+enum x86_legacy_i8042_state {
+	X86_LEGACY_I8042_PLATFORM_ABSENT,
+	X86_LEGACY_I8042_FIRMWARE_ABSENT,
+	X86_LEGACY_I8042_EXPECTED_PRESENT,
+};
+
+/**
  * struct x86_legacy_features - legacy x86 features
  *
+ * @i8042: indicated if we expect the device to have i8042 controller
+ *	present.
  * @rtc: this device has a CMOS real-time clock present
  * @reserve_bios_regions: boot code will search for the EBDA address and the
  * 	start of the 640k - 1M BIOS region.  If false, the platform must
@@ -175,6 +192,7 @@ struct x86_legacy_devices {
  * 	documentation for further details.
  */
 struct x86_legacy_features {
+	enum x86_legacy_i8042_state i8042;
 	int rtc;
 	int reserve_bios_regions;
 	struct x86_legacy_devices devices;
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 90d84c3..db15c77 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -925,6 +925,13 @@ static int __init acpi_parse_fadt(struct acpi_table_header *table)
 		x86_platform.legacy.devices.pnpbios = 0;
 	}
 
+	if (acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID &&
+	    !(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042) &&
+	    x86_platform.legacy.i8042 != X86_LEGACY_I8042_PLATFORM_ABSENT) {
+		pr_debug("ACPI: i8042 controller is absent\n");
+		x86_platform.legacy.i8042 = X86_LEGACY_I8042_FIRMWARE_ABSENT;
+	}
+
 	if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
 		pr_debug("ACPI: not registering RTC platform device\n");
 		x86_platform.legacy.rtc = 0;
diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c
index 24a5030..9127112 100644
--- a/arch/x86/kernel/platform-quirks.c
+++ b/arch/x86/kernel/platform-quirks.c
@@ -6,6 +6,7 @@
 
 void __init x86_early_init_platform_quirks(void)
 {
+	x86_platform.legacy.i8042 = X86_LEGACY_I8042_EXPECTED_PRESENT;
 	x86_platform.legacy.rtc = 1;
 	x86_platform.legacy.reserve_bios_regions = 0;
 	x86_platform.legacy.devices.pnpbios = 1;
@@ -16,10 +17,14 @@ void __init x86_early_init_platform_quirks(void)
 		break;
 	case X86_SUBARCH_XEN:
 	case X86_SUBARCH_LGUEST:
+		x86_platform.legacy.devices.pnpbios = 0;
+		x86_platform.legacy.rtc = 0;
+		break;
 	case X86_SUBARCH_INTEL_MID:
 	case X86_SUBARCH_CE4100:
 		x86_platform.legacy.devices.pnpbios = 0;
 		x86_platform.legacy.rtc = 0;
+		x86_platform.legacy.i8042 = X86_LEGACY_I8042_PLATFORM_ABSENT;
 		break;
 	}
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH 2/4] Input: i8042 - trust firmware a bit more when probing on X86

[Dmitry Torokhov]

The error message "Can't read CTR while initializing i8042" appears on
Cherry Trail-based devices at each boot time:

  i8042: PNP: No PS/2 controller found. Probing ports directly.
  i8042: Can't read CTR while initializing i8042
  i8042: probe of i8042 failed with error -5

This happens because we historically do not trust firmware on X86 and,
while noting that PNP does not show keyboard or mouse devices, we still
charge ahead and try to probe the controller. Let's relax this a bit and if
results of PNP probe agree with the results of platform
initialization/quirks conclude that there is, in fact, no i8042.

While at it, let's avoid using x86_platform.i8042_detect() and instead
abort execution early if platform indicates that it can not possibly have
i8042 (x86_platform.legacy.i8042 equals X86_LEGACY_I8042_PLATFORM_ABSENT).

Reported-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/i8042-x86ia64io.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 2e6a938..6ecb214 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -991,7 +991,11 @@ static int __init i8042_pnp_init(void)
 #if defined(__ia64__)
 		return -ENODEV;
 #else
-		pr_info("PNP: No PS/2 controller found. Probing ports directly.\n");
+		pr_info("PNP: No PS/2 controller found.\n");
+		if (x86_platform.legacy.i8042 !=
+				X86_LEGACY_I8042_EXPECTED_PRESENT)
+			return -ENODEV;
+		pr_info("Probing ports directly.\n");
 		return 0;
 #endif
 	}
@@ -1078,8 +1082,8 @@ static int __init i8042_platform_init(void)
 
 #ifdef CONFIG_X86
 	u8 a20_on = 0xdf;
-	/* Just return if pre-detection shows no i8042 controller exist */
-	if (!x86_platform.i8042_detect())
+	/* Just return if platform does not have i8042 controller */
+	if (x86_platform.legacy.i8042 == X86_LEGACY_I8042_PLATFORM_ABSENT)
 		return -ENODEV;
 #endif
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH 3/4] x86/init: remove i8042_detect() form platform ops

[Dmitry Torokhov]

Now that i8042 uses flag in legacy platform data, i8042_detect() is
no longer used and can be removed.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/x86/include/asm/x86_init.h         | 2 --
 arch/x86/kernel/x86_init.c              | 2 --
 arch/x86/platform/ce4100/ce4100.c       | 6 ------
 arch/x86/platform/intel-mid/intel-mid.c | 7 -------
 4 files changed, 17 deletions(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index c4d09c7..85b2ae5 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -206,7 +206,6 @@ struct x86_legacy_features {
  * @set_wallclock:		set time back to HW clock
  * @is_untracked_pat_range	exclude from PAT logic
  * @nmi_init			enable NMI on cpus
- * @i8042_detect		pre-detect if i8042 controller exists
  * @save_sched_clock_state:	save state for sched_clock() on suspend
  * @restore_sched_clock_state:	restore state for sched_clock() on resume
  * @apic_post_init:		adjust apic if neeeded
@@ -228,7 +227,6 @@ struct x86_platform_ops {
 	bool (*is_untracked_pat_range)(u64 start, u64 end);
 	void (*nmi_init)(void);
 	unsigned char (*get_nmi_reason)(void);
-	int (*i8042_detect)(void);
 	void (*save_sched_clock_state)(void);
 	void (*restore_sched_clock_state)(void);
 	void (*apic_post_init)(void);
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index 76c5e52..ca85d20 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -89,7 +89,6 @@ struct x86_cpuinit_ops x86_cpuinit = {
 };
 
 static void default_nmi_init(void) { };
-static int default_i8042_detect(void) { return 1; };
 
 struct x86_platform_ops x86_platform = {
 	.calibrate_cpu			= native_calibrate_cpu,
@@ -100,7 +99,6 @@ struct x86_platform_ops x86_platform = {
 	.is_untracked_pat_range		= is_ISA_range,
 	.nmi_init			= default_nmi_init,
 	.get_nmi_reason			= default_get_nmi_reason,
-	.i8042_detect			= default_i8042_detect,
 	.save_sched_clock_state 	= tsc_save_sched_clock_state,
 	.restore_sched_clock_state 	= tsc_restore_sched_clock_state,
 };
diff --git a/arch/x86/platform/ce4100/ce4100.c b/arch/x86/platform/ce4100/ce4100.c
index b27bccd..a783caf 100644
--- a/arch/x86/platform/ce4100/ce4100.c
+++ b/arch/x86/platform/ce4100/ce4100.c
@@ -23,11 +23,6 @@
 #include <asm/io_apic.h>
 #include <asm/emergency-restart.h>
 
-static int ce4100_i8042_detect(void)
-{
-	return 0;
-}
-
 /*
  * The CE4100 platform has an internal 8051 Microcontroller which is
  * responsible for signaling to the external Power Management Unit the
@@ -145,7 +140,6 @@ static void sdv_pci_init(void)
 void __init x86_ce4100_early_setup(void)
 {
 	x86_init.oem.arch_setup = sdv_arch_setup;
-	x86_platform.i8042_detect = ce4100_i8042_detect;
 	x86_init.resources.probe_roms = x86_init_noop;
 	x86_init.mpparse.get_smp_config = x86_init_uint_noop;
 	x86_init.mpparse.find_smp_config = x86_init_noop;
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index ce119d2..cf8ff1c 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -156,12 +156,6 @@ out:
 	regulator_has_full_constraints();
 }
 
-/* MID systems don't have i8042 controller */
-static int intel_mid_i8042_detect(void)
-{
-	return 0;
-}
-
 /*
  * Moorestown does not have external NMI source nor port 0x61 to report
  * NMI status. The possible NMI sources are from pmu as a result of NMI
@@ -192,7 +186,6 @@ void __init x86_intel_mid_early_setup(void)
 	x86_cpuinit.setup_percpu_clockev = apbt_setup_secondary_clock;
 
 	x86_platform.calibrate_tsc = intel_mid_calibrate_tsc;
-	x86_platform.i8042_detect = intel_mid_i8042_detect;
 	x86_init.timers.wallclock_init = intel_mid_rtc_init;
 	x86_platform.get_nmi_reason = intel_mid_get_nmi_reason;
 
-- 
2.8.0.rc3.226.g39d4020

[PATCH 4/4] x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h

[Dmitry Torokhov]

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/x86/include/asm/x86_init.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 85b2ae5..7ba7e90 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -59,7 +59,7 @@ struct x86_init_irqs {
 
 /**
  * struct x86_init_oem - oem platform specific customizing functions
- * @arch_setup:			platform specific architecure setup
+ * @arch_setup:			platform specific architecture setup
  * @banner:			print a platform specific banner
  */
 struct x86_init_oem {
@@ -208,12 +208,12 @@ struct x86_legacy_features {
  * @nmi_init			enable NMI on cpus
  * @save_sched_clock_state:	save state for sched_clock() on suspend
  * @restore_sched_clock_state:	restore state for sched_clock() on resume
- * @apic_post_init:		adjust apic if neeeded
+ * @apic_post_init:		adjust apic if needed
  * @legacy:			legacy features
  * @set_legacy_features:	override legacy features. Use of this callback
  * 				is highly discouraged. You should only need
  * 				this if your hardware platform requires further
- * 				custom fine tuning far beyong what may be
+ * 				custom fine tuning far beyond what may be
  * 				possible in x86_early_init_platform_quirks() by
  * 				only using the current x86_hardware_subarch
  * 				semantics.
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042

[Marcos Paulo de Souza]

Hi Dmitry,

On Fri, Dec 09, 2016 at 12:57:37PM -0800, Dmitry Torokhov wrote:
> Hi,
> 
> Historically we did not trust PNP data regarding keyboard controllers on
> X86, but more and more boards get upset with us if they try to tell us that
> there is no keyboard controller and we still go and try to poke at where we
> think it might be. To work around this issue let's have a bit more faith in
> BIOS data, and if [lack] of PNP devices for mouse and keyboard matches whet
> firmware (basically ACPI FADT) tells us, let's abort i8042 probe.
> 
> We add a new flag (enum) to x86_platform.legacy structure so we can
> distinguish between cases where platform/subarch never has 8042 (such as
> MID platform) and cases where firmware says that it is not there, so that
> i8042 driver can either abort immediately or go and check for presence of
> PNP devices. We also remove x86_platform.i8042_detect() as it is no longer
> used (platforms can set value of x86_platform.legacy.i8042 as needed in
> quirks).
> 
> If you are OK with arch/x86 changes please apply together with the input
> part.

Looks very good, better than my initial idea of adding more code into i8042_detect.

Acked-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>

> 
> Thanks,
> Dmitry
> 
> Dmitry Torokhov (4):
>   x86/init: add i8042 state to the platform data
>   Input: i8042 - trust firmware a bit more when probing on X86
>   x86/init: remove i8042_detect() form platform ops
>   x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h
> 
>  arch/x86/include/asm/x86_init.h         | 26 +++++++++++++++++++++-----
>  arch/x86/kernel/acpi/boot.c             |  7 +++++++
>  arch/x86/kernel/platform-quirks.c       |  5 +++++
>  arch/x86/kernel/x86_init.c              |  2 --
>  arch/x86/platform/ce4100/ce4100.c       |  6 ------
>  arch/x86/platform/intel-mid/intel-mid.c |  7 -------
>  drivers/input/serio/i8042-x86ia64io.h   | 10 +++++++---
>  7 files changed, 40 insertions(+), 23 deletions(-)
> 
> -- 
> 2.8.0.rc3.226.g39d4020
> 

Re: [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042

[Takashi Iwai]

On Fri, 09 Dec 2016 21:57:37 +0100,
Dmitry Torokhov wrote:
> 
> Hi,
> 
> Historically we did not trust PNP data regarding keyboard controllers on
> X86, but more and more boards get upset with us if they try to tell us that
> there is no keyboard controller and we still go and try to poke at where we
> think it might be. To work around this issue let's have a bit more faith in
> BIOS data, and if [lack] of PNP devices for mouse and keyboard matches whet
> firmware (basically ACPI FADT) tells us, let's abort i8042 probe.
> 
> We add a new flag (enum) to x86_platform.legacy structure so we can
> distinguish between cases where platform/subarch never has 8042 (such as
> MID platform) and cases where firmware says that it is not there, so that
> i8042 driver can either abort immediately or go and check for presence of
> PNP devices. We also remove x86_platform.i8042_detect() as it is no longer
> used (platforms can set value of x86_platform.legacy.i8042 as needed in
> quirks).
> 
> If you are OK with arch/x86 changes please apply together with the input
> part.

I tested and worked fine.  For all patches:
  Tested-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi