linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042
@ 2016-12-09 20:57 Dmitry Torokhov
  2016-12-09 20:57 ` [PATCH 1/4] x86/init: add i8042 state to the platform data Dmitry Torokhov
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2016-12-09 20:57 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar
  Cc: H. Peter Anvin, x86, linux-kernel, linux-input, Takashi Iwai,
	Marcos Paulo de Souza

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] x86/init: add i8042 state to the platform data
  2016-12-09 20:57 [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Dmitry Torokhov
@ 2016-12-09 20:57 ` Dmitry Torokhov
  2016-12-19 10:39   ` [tip:x86/urgent] x86/init: Add " tip-bot for Dmitry Torokhov
  2016-12-09 20:57 ` [PATCH 2/4] Input: i8042 - trust firmware a bit more when probing on X86 Dmitry Torokhov
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2016-12-09 20:57 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar
  Cc: H. Peter Anvin, x86, linux-kernel, linux-input, Takashi Iwai,
	Marcos Paulo de Souza

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] Input: i8042 - trust firmware a bit more when probing on X86
  2016-12-09 20:57 [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Dmitry Torokhov
  2016-12-09 20:57 ` [PATCH 1/4] x86/init: add i8042 state to the platform data Dmitry Torokhov
@ 2016-12-09 20:57 ` Dmitry Torokhov
  2016-12-19 10:40   ` [tip:x86/urgent] Input: i8042 - Trust " tip-bot for Dmitry Torokhov
  2016-12-09 20:57 ` [PATCH 3/4] x86/init: remove i8042_detect() form platform ops Dmitry Torokhov
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2016-12-09 20:57 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar
  Cc: H. Peter Anvin, x86, linux-kernel, linux-input, Takashi Iwai,
	Marcos Paulo de Souza

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] x86/init: remove i8042_detect() form platform ops
  2016-12-09 20:57 [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Dmitry Torokhov
  2016-12-09 20:57 ` [PATCH 1/4] x86/init: add i8042 state to the platform data Dmitry Torokhov
  2016-12-09 20:57 ` [PATCH 2/4] Input: i8042 - trust firmware a bit more when probing on X86 Dmitry Torokhov
@ 2016-12-09 20:57 ` Dmitry Torokhov
  2016-12-19 10:41   ` [tip:x86/urgent] x86/init: Remove i8042_detect() from " tip-bot for Dmitry Torokhov
  2016-12-09 20:57 ` [PATCH 4/4] x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h Dmitry Torokhov
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2016-12-09 20:57 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar
  Cc: H. Peter Anvin, x86, linux-kernel, linux-input, Takashi Iwai,
	Marcos Paulo de Souza

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h
  2016-12-09 20:57 [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2016-12-09 20:57 ` [PATCH 3/4] x86/init: remove i8042_detect() form platform ops Dmitry Torokhov
@ 2016-12-09 20:57 ` Dmitry Torokhov
  2016-12-19 10:41   ` [tip:x86/urgent] x86/init: Fix a couple of comment typos tip-bot for Dmitry Torokhov
  2016-12-09 23:54 ` [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Marcos Paulo de Souza
  2016-12-12 14:30 ` Takashi Iwai
  5 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2016-12-09 20:57 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar
  Cc: H. Peter Anvin, x86, linux-kernel, linux-input, Takashi Iwai,
	Marcos Paulo de Souza

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042
  2016-12-09 20:57 [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2016-12-09 20:57 ` [PATCH 4/4] x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h Dmitry Torokhov
@ 2016-12-09 23:54 ` Marcos Paulo de Souza
  2016-12-12 14:30 ` Takashi Iwai
  5 siblings, 0 replies; 11+ messages in thread
From: Marcos Paulo de Souza @ 2016-12-09 23:54 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-input, Takashi Iwai

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
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042
  2016-12-09 20:57 [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2016-12-09 23:54 ` [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Marcos Paulo de Souza
@ 2016-12-12 14:30 ` Takashi Iwai
  5 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2016-12-12 14:30 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-input, Takashi Iwai, Marcos Paulo de Souza

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tip:x86/urgent] x86/init: Add i8042 state to the platform data
  2016-12-09 20:57 ` [PATCH 1/4] x86/init: add i8042 state to the platform data Dmitry Torokhov
@ 2016-12-19 10:39   ` tip-bot for Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Dmitry Torokhov @ 2016-12-19 10:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, mingo, linux-kernel, hpa, marcos.souza.org, tiwai, dmitry.torokhov

Commit-ID:  93ffa9a479ffb65d045e74e141346e7f107fcde1
Gitweb:     http://git.kernel.org/tip/93ffa9a479ffb65d045e74e141346e7f107fcde1
Author:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
AuthorDate: Fri, 9 Dec 2016 12:57:38 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 19 Dec 2016 11:34:15 +0100

x86/init: Add i8042 state to the platform data

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>
Tested-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Cc: linux-input@vger.kernel.org
Link: http://lkml.kernel.org/r/1481317061-31486-2-git-send-email-dmitry.torokhov@gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 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 6f65b0e..64422f8 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -930,6 +930,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;
 	}
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [tip:x86/urgent] Input: i8042 - Trust firmware a bit more when probing on X86
  2016-12-09 20:57 ` [PATCH 2/4] Input: i8042 - trust firmware a bit more when probing on X86 Dmitry Torokhov
@ 2016-12-19 10:40   ` tip-bot for Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Dmitry Torokhov @ 2016-12-19 10:40 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: dmitry.torokhov, marcos.souza.org, mingo, linux-kernel, tglx, hpa, tiwai

Commit-ID:  d79e141c1c6ea7cb70c169971d522b88c8d5b419
Gitweb:     http://git.kernel.org/tip/d79e141c1c6ea7cb70c169971d522b88c8d5b419
Author:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
AuthorDate: Fri, 9 Dec 2016 12:57:39 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 19 Dec 2016 11:34:15 +0100

Input: i8042 - Trust firmware a bit more when probing on X86

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-and-tested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Cc: linux-input@vger.kernel.org
Link: http://lkml.kernel.org/r/1481317061-31486-3-git-send-email-dmitry.torokhov@gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 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 73a4e68..77551f5 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -983,7 +983,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
 	}
@@ -1070,8 +1074,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
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [tip:x86/urgent] x86/init: Remove i8042_detect() from platform ops
  2016-12-09 20:57 ` [PATCH 3/4] x86/init: remove i8042_detect() form platform ops Dmitry Torokhov
@ 2016-12-19 10:41   ` tip-bot for Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Dmitry Torokhov @ 2016-12-19 10:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, mingo, hpa, linux-kernel, tiwai, marcos.souza.org, dmitry.torokhov

Commit-ID:  32786fdc9506aeba98278c1844d4bfb766863832
Gitweb:     http://git.kernel.org/tip/32786fdc9506aeba98278c1844d4bfb766863832
Author:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
AuthorDate: Fri, 9 Dec 2016 12:57:40 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 19 Dec 2016 11:34:15 +0100

x86/init: Remove i8042_detect() from platform ops

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>
Tested-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Cc: linux-input@vger.kernel.org
Link: http://lkml.kernel.org/r/1481317061-31486-4-git-send-email-dmitry.torokhov@gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 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 0bd9f12..11a93f0 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 __ro_after_init = {
 	.calibrate_cpu			= native_calibrate_cpu,
@@ -100,7 +99,6 @@ struct x86_platform_ops x86_platform __ro_after_init = {
 	.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 821cb41..ce4b067 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 7850128..12a2725 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -161,12 +161,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
@@ -197,7 +191,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;
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [tip:x86/urgent] x86/init: Fix a couple of comment typos
  2016-12-09 20:57 ` [PATCH 4/4] x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h Dmitry Torokhov
@ 2016-12-19 10:41   ` tip-bot for Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Dmitry Torokhov @ 2016-12-19 10:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: marcos.souza.org, dmitry.torokhov, tglx, linux-kernel, mingo, hpa

Commit-ID:  22d3c0d63b1108af0b4ef1cfdad1f6ef0710da30
Gitweb:     http://git.kernel.org/tip/22d3c0d63b1108af0b4ef1cfdad1f6ef0710da30
Author:     Dmitry Torokhov <dmitry.torokhov@gmail.com>
AuthorDate: Fri, 9 Dec 2016 12:57:41 -0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 19 Dec 2016 11:34:16 +0100

x86/init: Fix a couple of comment typos

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
Cc: linux-input@vger.kernel.org
Link: http://lkml.kernel.org/r/1481317061-31486-5-git-send-email-dmitry.torokhov@gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 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.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-12-19 10:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-09 20:57 [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Dmitry Torokhov
2016-12-09 20:57 ` [PATCH 1/4] x86/init: add i8042 state to the platform data Dmitry Torokhov
2016-12-19 10:39   ` [tip:x86/urgent] x86/init: Add " tip-bot for Dmitry Torokhov
2016-12-09 20:57 ` [PATCH 2/4] Input: i8042 - trust firmware a bit more when probing on X86 Dmitry Torokhov
2016-12-19 10:40   ` [tip:x86/urgent] Input: i8042 - Trust " tip-bot for Dmitry Torokhov
2016-12-09 20:57 ` [PATCH 3/4] x86/init: remove i8042_detect() form platform ops Dmitry Torokhov
2016-12-19 10:41   ` [tip:x86/urgent] x86/init: Remove i8042_detect() from " tip-bot for Dmitry Torokhov
2016-12-09 20:57 ` [PATCH 4/4] x86/init: fix a couple typos in arch/x86/include/asm/x86_init.h Dmitry Torokhov
2016-12-19 10:41   ` [tip:x86/urgent] x86/init: Fix a couple of comment typos tip-bot for Dmitry Torokhov
2016-12-09 23:54 ` [PATCH 0/4] x86: Trust firmware a bit more about presence of 8042 Marcos Paulo de Souza
2016-12-12 14:30 ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).