i8042 error at booting an Intel Cherry Trail-based device

i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

Hi Dmitry,

I've been testing a small machine with Intel Cherry Trail chipset, and
noticed that the kernel spews errors always like:

 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

Especially the second one ("Can't read CTR...") is annoying since it's
in KERN_ERR level and thus appears even booted with quiet boot
option.  Actually this is the only error message appearing at boot, so
I'd love to get rid of it.

What is the preferred way to reduce this?  For example, is a patch
like below OK to simply change the log level and the error code?


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] i8042: Reduce the log level of i8042 CTR read error

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 is annoying, since it's the only error message with KERN_ERR
level appearing during the boot.

This patch changes the kernel log level to KERN_INFO for that message,
and replaces the error code to -ENODEV so that this probe failure
won't be complained like the above.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/input/serio/i8042.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index b4e1ac5c9ea8..2c2683e357e9 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -980,8 +980,8 @@ static int i8042_controller_init(void)
 			udelay(50);
 
 		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
-			pr_err("Can't read CTR while initializing i8042\n");
-			return -EIO;
+			pr_info("Can't read CTR while initializing i8042\n");
+			return -ENODEV;
 		}
 
 	} while (n < 2 || ctr[0] != ctr[1]);
-- 
2.10.2

Re: i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

On Mon, 28 Nov 2016 14:56:36 +0100,
Takashi Iwai wrote:
> 
> Hi Dmitry,
> 
> I've been testing a small machine with Intel Cherry Trail chipset, and
> noticed that the kernel spews errors always like:
> 
>  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
> 
> Especially the second one ("Can't read CTR...") is annoying since it's
> in KERN_ERR level and thus appears even booted with quiet boot
> option.  Actually this is the only error message appearing at boot, so
> I'd love to get rid of it.
> 
> What is the preferred way to reduce this?  For example, is a patch
> like below OK to simply change the log level and the error code?

Any suggestion?

Adding Jiri to Cc, as he had a similar issue in the past.


thanks,

Takashi

> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] i8042: Reduce the log level of i8042 CTR read error
> 
> 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 is annoying, since it's the only error message with KERN_ERR
> level appearing during the boot.
> 
> This patch changes the kernel log level to KERN_INFO for that message,
> and replaces the error code to -ENODEV so that this probe failure
> won't be complained like the above.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/input/serio/i8042.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index b4e1ac5c9ea8..2c2683e357e9 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -980,8 +980,8 @@ static int i8042_controller_init(void)
>  			udelay(50);
>  
>  		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
> -			pr_err("Can't read CTR while initializing i8042\n");
> -			return -EIO;
> +			pr_info("Can't read CTR while initializing i8042\n");
> +			return -ENODEV;
>  		}
>  
>  	} while (n < 2 || ctr[0] != ctr[1]);
> -- 
> 2.10.2
> 

Re: i8042 error at booting an Intel Cherry Trail-based device

[Dmitry Torokhov]

Hi Takashi,

On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> Hi Dmitry,
> 
> I've been testing a small machine with Intel Cherry Trail chipset, and
> noticed that the kernel spews errors always like:
> 
>  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
> 
> Especially the second one ("Can't read CTR...") is annoying since it's
> in KERN_ERR level and thus appears even booted with quiet boot
> option.  Actually this is the only error message appearing at boot, so
> I'd love to get rid of it.
> 
> What is the preferred way to reduce this?  For example, is a patch
> like below OK to simply change the log level and the error code?

No, because if controller is actually present this is a hard failure and
we should be reporting it, not suppressing it.

The issue is that we did not believe PNP data and in this case we should
have. Unfortunately in old days there was a lot of crap in PNP/ACPI
tables, but it could be better now. We can try, in addition to PNP
matching, checking 8042 flag in "Fixed ACPI Description Table Boot
Architecture Flags" in FADT and if it also shows there is no 8042 then
bail.

Thanks.

> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] i8042: Reduce the log level of i8042 CTR read error
> 
> 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 is annoying, since it's the only error message with KERN_ERR
> level appearing during the boot.
> 
> This patch changes the kernel log level to KERN_INFO for that message,
> and replaces the error code to -ENODEV so that this probe failure
> won't be complained like the above.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/input/serio/i8042.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index b4e1ac5c9ea8..2c2683e357e9 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -980,8 +980,8 @@ static int i8042_controller_init(void)
>  			udelay(50);
>  
>  		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
> -			pr_err("Can't read CTR while initializing i8042\n");
> -			return -EIO;
> +			pr_info("Can't read CTR while initializing i8042\n");
> +			return -ENODEV;
>  		}
>  
>  	} while (n < 2 || ctr[0] != ctr[1]);
> -- 
> 2.10.2
> 

-- 
Dmitry

Re: i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

On Thu, 01 Dec 2016 03:29:23 +0100,
Dmitry Torokhov wrote:
> 
> Hi Takashi,
> 
> On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> > Hi Dmitry,
> > 
> > I've been testing a small machine with Intel Cherry Trail chipset, and
> > noticed that the kernel spews errors always like:
> > 
> >  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
> > 
> > Especially the second one ("Can't read CTR...") is annoying since it's
> > in KERN_ERR level and thus appears even booted with quiet boot
> > option.  Actually this is the only error message appearing at boot, so
> > I'd love to get rid of it.
> > 
> > What is the preferred way to reduce this?  For example, is a patch
> > like below OK to simply change the log level and the error code?
> 
> No, because if controller is actually present this is a hard failure and
> we should be reporting it, not suppressing it.
> 
> The issue is that we did not believe PNP data and in this case we should
> have. Unfortunately in old days there was a lot of crap in PNP/ACPI
> tables, but it could be better now. We can try, in addition to PNP
> matching, checking 8042 flag in "Fixed ACPI Description Table Boot
> Architecture Flags" in FADT and if it also shows there is no 8042 then
> bail.

That sounds promising.  Indeed FACL.dsl shows like:

[000h 0000   4]                    Signature : "FACP"    [Fixed ACPI Description Table (FADT)]
[004h 0004   4]                 Table Length : 0000010C
....
               Legacy Devices Supported (V2) : 0
            8042 Present on ports 60/64 (V2) : 0

If a test patch gets ready, let me know, I'll give it a try.


Thanks!

Takashi

Re: i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

On Thu, 01 Dec 2016 08:19:46 +0100,
Takashi Iwai wrote:
> 
> On Thu, 01 Dec 2016 03:29:23 +0100,
> Dmitry Torokhov wrote:
> > 
> > Hi Takashi,
> > 
> > On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> > > Hi Dmitry,
> > > 
> > > I've been testing a small machine with Intel Cherry Trail chipset, and
> > > noticed that the kernel spews errors always like:
> > > 
> > >  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
> > > 
> > > Especially the second one ("Can't read CTR...") is annoying since it's
> > > in KERN_ERR level and thus appears even booted with quiet boot
> > > option.  Actually this is the only error message appearing at boot, so
> > > I'd love to get rid of it.
> > > 
> > > What is the preferred way to reduce this?  For example, is a patch
> > > like below OK to simply change the log level and the error code?
> > 
> > No, because if controller is actually present this is a hard failure and
> > we should be reporting it, not suppressing it.
> > 
> > The issue is that we did not believe PNP data and in this case we should
> > have. Unfortunately in old days there was a lot of crap in PNP/ACPI
> > tables, but it could be better now. We can try, in addition to PNP
> > matching, checking 8042 flag in "Fixed ACPI Description Table Boot
> > Architecture Flags" in FADT and if it also shows there is no 8042 then
> > bail.
> 
> That sounds promising.  Indeed FACL.dsl shows like:
> 
> [000h 0000   4]                    Signature : "FACP"    [Fixed ACPI Description Table (FADT)]
> [004h 0004   4]                 Table Length : 0000010C
> ....
>                Legacy Devices Supported (V2) : 0
>             8042 Present on ports 60/64 (V2) : 0
> 
> If a test patch gets ready, let me know, I'll give it a try.

FYI, a hack like below seems working.


Takashi

---
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index 073246c7d163..ed6ab702e4b7 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -9,6 +9,7 @@
 
 #ifdef CONFIG_X86
 #include <asm/x86_init.h>
+#include <linux/acpi.h>
 #endif
 
 /*
@@ -1055,6 +1056,13 @@ static int __init i8042_pnp_init(void)
 #if defined(__ia64__)
 		return -ENODEV;
 #else
+#ifdef CONFIG_ACPI
+		if (acpi_gbl_FADT.header.revision >= 3 &&
+		    !(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) {
+			pr_info("PNP: No PS/2 controller found and disabled in ACPI\n");
+			return -ENODEV;
+		}
+#endif
 		pr_info("PNP: No PS/2 controller found. Probing ports directly.\n");
 		return 0;
 #endif

Re: i8042 error at booting an Intel Cherry Trail-based device

[Marcos Paulo de Souza]

Hi Takashi,

On Fri, Dec 02, 2016 at 11:55:07AM +0100, Takashi Iwai wrote:
> On Thu, 01 Dec 2016 08:19:46 +0100,
> Takashi Iwai wrote:
> > 
> > On Thu, 01 Dec 2016 03:29:23 +0100,
> > Dmitry Torokhov wrote:
> > > 
> > > Hi Takashi,
> > > 
> > > On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> > > > Hi Dmitry,
> > > > 
> > > > I've been testing a small machine with Intel Cherry Trail chipset, and
> > > > noticed that the kernel spews errors always like:
> > > > 
> > > >  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
> > > > 
> > > > Especially the second one ("Can't read CTR...") is annoying since it's
> > > > in KERN_ERR level and thus appears even booted with quiet boot
> > > > option.  Actually this is the only error message appearing at boot, so
> > > > I'd love to get rid of it.
> > > > 
> > > > What is the preferred way to reduce this?  For example, is a patch
> > > > like below OK to simply change the log level and the error code?
> > > 
> > > No, because if controller is actually present this is a hard failure and
> > > we should be reporting it, not suppressing it.
> > > 
> > > The issue is that we did not believe PNP data and in this case we should
> > > have. Unfortunately in old days there was a lot of crap in PNP/ACPI
> > > tables, but it could be better now. We can try, in addition to PNP
> > > matching, checking 8042 flag in "Fixed ACPI Description Table Boot
> > > Architecture Flags" in FADT and if it also shows there is no 8042 then
> > > bail.
> > 
> > That sounds promising.  Indeed FACL.dsl shows like:
> > 
> > [000h 0000   4]                    Signature : "FACP"    [Fixed ACPI Description Table (FADT)]
> > [004h 0004   4]                 Table Length : 0000010C
> > ....
> >                Legacy Devices Supported (V2) : 0
> >             8042 Present on ports 60/64 (V2) : 0
> > 
> > If a test patch gets ready, let me know, I'll give it a try.
> 
> FYI, a hack like below seems working.
> 
> 
> Takashi
> 
> ---
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index 073246c7d163..ed6ab702e4b7 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -9,6 +9,7 @@
>  
>  #ifdef CONFIG_X86
>  #include <asm/x86_init.h>
> +#include <linux/acpi.h>
>  #endif
>  
>  /*
> @@ -1055,6 +1056,13 @@ static int __init i8042_pnp_init(void)
>  #if defined(__ia64__)
>  		return -ENODEV;
>  #else
> +#ifdef CONFIG_ACPI
> +		if (acpi_gbl_FADT.header.revision >= 3 &&
> +		    !(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) {
> +			pr_info("PNP: No PS/2 controller found and disabled in ACPI\n");
> +			return -ENODEV;
> +		}
> +#endif
>  		pr_info("PNP: No PS/2 controller found. Probing ports directly.\n");
>  		return 0;
>  #endif

I'm not an expert in any subsystem but, maybe this "hack" could be added
to default_i8042_detect in arch/x86/kernel/x86_init.c? Currently it is
enabled by default, but different Intel platform like ce4100 and
intel-mid disables it explicit.

I mentioned "hack" because following osdev.org[1] using ACPI is the
correct way to detect if i8042 exists. Pardon me if this not applies in
this situation, or if I missed something.

[1]
http://wiki.osdev.org/%228042%22_PS/2_Controller#Step_2:_Determine_if_the_PS.2F2_Controller_Exists

Thanks,

> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: i8042 error at booting an Intel Cherry Trail-based device

[Dmitry Torokhov]

On December 5, 2016 4:56:05 PM PST, Marcos Paulo de Souza <marcos.souza.org@gmail.com> wrote:
>Hi Takashi,
>
>On Fri, Dec 02, 2016 at 11:55:07AM +0100, Takashi Iwai wrote:
>> On Thu, 01 Dec 2016 08:19:46 +0100,
>> Takashi Iwai wrote:
>> > 
>> > On Thu, 01 Dec 2016 03:29:23 +0100,
>> > Dmitry Torokhov wrote:
>> > > 
>> > > Hi Takashi,
>> > > 
>> > > On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
>> > > > Hi Dmitry,
>> > > > 
>> > > > I've been testing a small machine with Intel Cherry Trail
>chipset, and
>> > > > noticed that the kernel spews errors always like:
>> > > > 
>> > > >  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
>> > > > 
>> > > > Especially the second one ("Can't read CTR...") is annoying
>since it's
>> > > > in KERN_ERR level and thus appears even booted with quiet boot
>> > > > option.  Actually this is the only error message appearing at
>boot, so
>> > > > I'd love to get rid of it.
>> > > > 
>> > > > What is the preferred way to reduce this?  For example, is a
>patch
>> > > > like below OK to simply change the log level and the error
>code?
>> > > 
>> > > No, because if controller is actually present this is a hard
>failure and
>> > > we should be reporting it, not suppressing it.
>> > > 
>> > > The issue is that we did not believe PNP data and in this case we
>should
>> > > have. Unfortunately in old days there was a lot of crap in
>PNP/ACPI
>> > > tables, but it could be better now. We can try, in addition to
>PNP
>> > > matching, checking 8042 flag in "Fixed ACPI Description Table
>Boot
>> > > Architecture Flags" in FADT and if it also shows there is no 8042
>then
>> > > bail.
>> > 
>> > That sounds promising.  Indeed FACL.dsl shows like:
>> > 
>> > [000h 0000   4]                    Signature : "FACP"    [Fixed
>ACPI Description Table (FADT)]
>> > [004h 0004   4]                 Table Length : 0000010C
>> > ....
>> >                Legacy Devices Supported (V2) : 0
>> >             8042 Present on ports 60/64 (V2) : 0
>> > 
>> > If a test patch gets ready, let me know, I'll give it a try.
>> 
>> FYI, a hack like below seems working.
>> 
>> 
>> Takashi
>> 
>> ---
>> diff --git a/drivers/input/serio/i8042-x86ia64io.h
>b/drivers/input/serio/i8042-x86ia64io.h
>> index 073246c7d163..ed6ab702e4b7 100644
>> --- a/drivers/input/serio/i8042-x86ia64io.h
>> +++ b/drivers/input/serio/i8042-x86ia64io.h
>> @@ -9,6 +9,7 @@
>>  
>>  #ifdef CONFIG_X86
>>  #include <asm/x86_init.h>
>> +#include <linux/acpi.h>
>>  #endif
>>  
>>  /*
>> @@ -1055,6 +1056,13 @@ static int __init i8042_pnp_init(void)
>>  #if defined(__ia64__)
>>  		return -ENODEV;
>>  #else
>> +#ifdef CONFIG_ACPI
>> +		if (acpi_gbl_FADT.header.revision >= 3 &&
>> +		    !(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) {
>> +			pr_info("PNP: No PS/2 controller found and disabled in ACPI\n");
>> +			return -ENODEV;
>> +		}
>> +#endif
>>  		pr_info("PNP: No PS/2 controller found. Probing ports
>directly.\n");
>>  		return 0;
>>  #endif
>
>I'm not an expert in any subsystem but, maybe this "hack" could be
>added
>to default_i8042_detect in arch/x86/kernel/x86_init.c? Currently it is
>enabled by default, but different Intel platform like ce4100 and
>intel-mid disables it explicit.
>
>I mentioned "hack" because following osdev.org[1] using ACPI is the
>correct way to detect if i8042 exists. Pardon me if this not applies in
>this situation, or if I missed something.

That is the proper way of detecting i8042 if you trust firmware; historically we do not, and so we want to make sure that PNP data agrees with fadt data.


Thanks.

-- 
Dmitry

Re: i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

On Tue, 06 Dec 2016 07:07:54 +0100,
Dmitry Torokhov wrote:
> 
> On December 5, 2016 4:56:05 PM PST, Marcos Paulo de Souza <marcos.souza.org@gmail.com> wrote:
> >Hi Takashi,
> >
> >On Fri, Dec 02, 2016 at 11:55:07AM +0100, Takashi Iwai wrote:
> >> On Thu, 01 Dec 2016 08:19:46 +0100,
> >> Takashi Iwai wrote:
> >> > 
> >> > On Thu, 01 Dec 2016 03:29:23 +0100,
> >> > Dmitry Torokhov wrote:
> >> > > 
> >> > > Hi Takashi,
> >> > > 
> >> > > On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> >> > > > Hi Dmitry,
> >> > > > 
> >> > > > I've been testing a small machine with Intel Cherry Trail
> >chipset, and
> >> > > > noticed that the kernel spews errors always like:
> >> > > > 
> >> > > >  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
> >> > > > 
> >> > > > Especially the second one ("Can't read CTR...") is annoying
> >since it's
> >> > > > in KERN_ERR level and thus appears even booted with quiet boot
> >> > > > option.  Actually this is the only error message appearing at
> >boot, so
> >> > > > I'd love to get rid of it.
> >> > > > 
> >> > > > What is the preferred way to reduce this?  For example, is a
> >patch
> >> > > > like below OK to simply change the log level and the error
> >code?
> >> > > 
> >> > > No, because if controller is actually present this is a hard
> >failure and
> >> > > we should be reporting it, not suppressing it.
> >> > > 
> >> > > The issue is that we did not believe PNP data and in this case we
> >should
> >> > > have. Unfortunately in old days there was a lot of crap in
> >PNP/ACPI
> >> > > tables, but it could be better now. We can try, in addition to
> >PNP
> >> > > matching, checking 8042 flag in "Fixed ACPI Description Table
> >Boot
> >> > > Architecture Flags" in FADT and if it also shows there is no 8042
> >then
> >> > > bail.
> >> > 
> >> > That sounds promising.  Indeed FACL.dsl shows like:
> >> > 
> >> > [000h 0000   4]                    Signature : "FACP"    [Fixed
> >ACPI Description Table (FADT)]
> >> > [004h 0004   4]                 Table Length : 0000010C
> >> > ....
> >> >                Legacy Devices Supported (V2) : 0
> >> >             8042 Present on ports 60/64 (V2) : 0
> >> > 
> >> > If a test patch gets ready, let me know, I'll give it a try.
> >> 
> >> FYI, a hack like below seems working.
> >> 
> >> 
> >> Takashi
> >> 
> >> ---
> >> diff --git a/drivers/input/serio/i8042-x86ia64io.h
> >b/drivers/input/serio/i8042-x86ia64io.h
> >> index 073246c7d163..ed6ab702e4b7 100644
> >> --- a/drivers/input/serio/i8042-x86ia64io.h
> >> +++ b/drivers/input/serio/i8042-x86ia64io.h
> >> @@ -9,6 +9,7 @@
> >>  
> >>  #ifdef CONFIG_X86
> >>  #include <asm/x86_init.h>
> >> +#include <linux/acpi.h>
> >>  #endif
> >>  
> >>  /*
> >> @@ -1055,6 +1056,13 @@ static int __init i8042_pnp_init(void)
> >>  #if defined(__ia64__)
> >>  		return -ENODEV;
> >>  #else
> >> +#ifdef CONFIG_ACPI
> >> +		if (acpi_gbl_FADT.header.revision >= 3 &&
> >> +		    !(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) {
> >> +			pr_info("PNP: No PS/2 controller found and disabled in ACPI\n");
> >> +			return -ENODEV;
> >> +		}
> >> +#endif
> >>  		pr_info("PNP: No PS/2 controller found. Probing ports
> >directly.\n");
> >>  		return 0;
> >>  #endif
> >
> >I'm not an expert in any subsystem but, maybe this "hack" could be
> >added
> >to default_i8042_detect in arch/x86/kernel/x86_init.c? Currently it is
> >enabled by default, but different Intel platform like ce4100 and
> >intel-mid disables it explicit.
> >
> >I mentioned "hack" because following osdev.org[1] using ACPI is the
> >correct way to detect if i8042 exists. Pardon me if this not applies in
> >this situation, or if I missed something.
> 
> That is the proper way of detecting i8042 if you trust firmware; historically we do not, and so we want to make sure that PNP data agrees with fadt data.

So it depends on how well you trust the firmware.  If we assume ACPI
providing always correctly, it can be put in default_i8042_detect, and
it'd be a better place indeed.  OTOH, if we don't trust ACPI,
especially on older machines, and let at first probing ACPI PnP no
matter whether FADT bit is set, we'd need to put the check after PnP
probe like my patch.

My patch assumes that the BIOS is new and good enough if FADT revision
is 3 or greater.  The only concern is whether this is really good
enough.  I just hope so.

In anyway, Dmitry, if you're happy with it, I'll cook up the proper
patch for the merge.  Let me know.


thanks,

Takashi

Re: i8042 error at booting an Intel Cherry Trail-based device

[Dmitry Torokhov]

Hi Takashi,

On Tue, Dec 06, 2016 at 11:36:09AM +0100, Takashi Iwai wrote:
> On Tue, 06 Dec 2016 07:07:54 +0100,
> Dmitry Torokhov wrote:
> > 
> > On December 5, 2016 4:56:05 PM PST, Marcos Paulo de Souza <marcos.souza.org@gmail.com> wrote:
> > >Hi Takashi,
> > >
> > >On Fri, Dec 02, 2016 at 11:55:07AM +0100, Takashi Iwai wrote:
> > >> On Thu, 01 Dec 2016 08:19:46 +0100,
> > >> Takashi Iwai wrote:
> > >> > 
> > >> > On Thu, 01 Dec 2016 03:29:23 +0100,
> > >> > Dmitry Torokhov wrote:
> > >> > > 
> > >> > > Hi Takashi,
> > >> > > 
> > >> > > On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> > >> > > > Hi Dmitry,
> > >> > > > 
> > >> > > > I've been testing a small machine with Intel Cherry Trail
> > >chipset, and
> > >> > > > noticed that the kernel spews errors always like:
> > >> > > > 
> > >> > > >  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
> > >> > > > 
> > >> > > > Especially the second one ("Can't read CTR...") is annoying
> > >since it's
> > >> > > > in KERN_ERR level and thus appears even booted with quiet boot
> > >> > > > option.  Actually this is the only error message appearing at
> > >boot, so
> > >> > > > I'd love to get rid of it.
> > >> > > > 
> > >> > > > What is the preferred way to reduce this?  For example, is a
> > >patch
> > >> > > > like below OK to simply change the log level and the error
> > >code?
> > >> > > 
> > >> > > No, because if controller is actually present this is a hard
> > >failure and
> > >> > > we should be reporting it, not suppressing it.
> > >> > > 
> > >> > > The issue is that we did not believe PNP data and in this case we
> > >should
> > >> > > have. Unfortunately in old days there was a lot of crap in
> > >PNP/ACPI
> > >> > > tables, but it could be better now. We can try, in addition to
> > >PNP
> > >> > > matching, checking 8042 flag in "Fixed ACPI Description Table
> > >Boot
> > >> > > Architecture Flags" in FADT and if it also shows there is no 8042
> > >then
> > >> > > bail.
> > >> > 
> > >> > That sounds promising.  Indeed FACL.dsl shows like:
> > >> > 
> > >> > [000h 0000   4]                    Signature : "FACP"    [Fixed
> > >ACPI Description Table (FADT)]
> > >> > [004h 0004   4]                 Table Length : 0000010C
> > >> > ....
> > >> >                Legacy Devices Supported (V2) : 0
> > >> >             8042 Present on ports 60/64 (V2) : 0
> > >> > 
> > >> > If a test patch gets ready, let me know, I'll give it a try.
> > >> 
> > >> FYI, a hack like below seems working.
> > >> 
> > >> 
> > >> Takashi
> > >> 
> > >> ---
> > >> diff --git a/drivers/input/serio/i8042-x86ia64io.h
> > >b/drivers/input/serio/i8042-x86ia64io.h
> > >> index 073246c7d163..ed6ab702e4b7 100644
> > >> --- a/drivers/input/serio/i8042-x86ia64io.h
> > >> +++ b/drivers/input/serio/i8042-x86ia64io.h
> > >> @@ -9,6 +9,7 @@
> > >>  
> > >>  #ifdef CONFIG_X86
> > >>  #include <asm/x86_init.h>
> > >> +#include <linux/acpi.h>
> > >>  #endif
> > >>  
> > >>  /*
> > >> @@ -1055,6 +1056,13 @@ static int __init i8042_pnp_init(void)
> > >>  #if defined(__ia64__)
> > >>  		return -ENODEV;
> > >>  #else
> > >> +#ifdef CONFIG_ACPI
> > >> +		if (acpi_gbl_FADT.header.revision >= 3 &&
> > >> +		    !(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) {
> > >> +			pr_info("PNP: No PS/2 controller found and disabled in ACPI\n");
> > >> +			return -ENODEV;
> > >> +		}
> > >> +#endif
> > >>  		pr_info("PNP: No PS/2 controller found. Probing ports
> > >directly.\n");
> > >>  		return 0;
> > >>  #endif
> > >
> > >I'm not an expert in any subsystem but, maybe this "hack" could be
> > >added
> > >to default_i8042_detect in arch/x86/kernel/x86_init.c? Currently it is
> > >enabled by default, but different Intel platform like ce4100 and
> > >intel-mid disables it explicit.
> > >
> > >I mentioned "hack" because following osdev.org[1] using ACPI is the
> > >correct way to detect if i8042 exists. Pardon me if this not applies in
> > >this situation, or if I missed something.
> > 
> > That is the proper way of detecting i8042 if you trust firmware; historically we do not, and so we want to make sure that PNP data agrees with fadt data.
> 
> So it depends on how well you trust the firmware.  If we assume ACPI
> providing always correctly, it can be put in default_i8042_detect, and
> it'd be a better place indeed.  OTOH, if we don't trust ACPI,
> especially on older machines, and let at first probing ACPI PnP no
> matter whether FADT bit is set, we'd need to put the check after PnP
> probe like my patch.
> 
> My patch assumes that the BIOS is new and good enough if FADT revision
> is 3 or greater.  The only concern is whether this is really good
> enough.  I just hope so.

FWIW FADT revision 3 is defined in ACPI 2.0 as far as I know. So not too
new.

> 
> In anyway, Dmitry, if you're happy with it, I'll cook up the proper
> patch for the merge.  Let me know.

I am happy with the idea, but as far as implementation goes I think we
need to add this flag to x86_platform.legacy structure, initialize
x86_platform_legacy.i8042_present = 1 in
x86_early_init_platform_quirks(), and adjust as needed in
acpi_parse_fadt().

Then we can use it in i8042 instead of checking FADT by hand.

Thanks!

-- 
Dmitry

Re: i8042 error at booting an Intel Cherry Trail-based device

[Takashi Iwai]

On Tue, 06 Dec 2016 18:07:05 +0100,
Dmitry Torokhov wrote:
> 
> Hi Takashi,
> 
> On Tue, Dec 06, 2016 at 11:36:09AM +0100, Takashi Iwai wrote:
> > On Tue, 06 Dec 2016 07:07:54 +0100,
> > Dmitry Torokhov wrote:
> > > 
> > > On December 5, 2016 4:56:05 PM PST, Marcos Paulo de Souza <marcos.souza.org@gmail.com> wrote:
> > > >Hi Takashi,
> > > >
> > > >On Fri, Dec 02, 2016 at 11:55:07AM +0100, Takashi Iwai wrote:
> > > >> On Thu, 01 Dec 2016 08:19:46 +0100,
> > > >> Takashi Iwai wrote:
> > > >> > 
> > > >> > On Thu, 01 Dec 2016 03:29:23 +0100,
> > > >> > Dmitry Torokhov wrote:
> > > >> > > 
> > > >> > > Hi Takashi,
> > > >> > > 
> > > >> > > On Mon, Nov 28, 2016 at 02:56:36PM +0100, Takashi Iwai wrote:
> > > >> > > > Hi Dmitry,
> > > >> > > > 
> > > >> > > > I've been testing a small machine with Intel Cherry Trail
> > > >chipset, and
> > > >> > > > noticed that the kernel spews errors always like:
> > > >> > > > 
> > > >> > > >  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
> > > >> > > > 
> > > >> > > > Especially the second one ("Can't read CTR...") is annoying
> > > >since it's
> > > >> > > > in KERN_ERR level and thus appears even booted with quiet boot
> > > >> > > > option.  Actually this is the only error message appearing at
> > > >boot, so
> > > >> > > > I'd love to get rid of it.
> > > >> > > > 
> > > >> > > > What is the preferred way to reduce this?  For example, is a
> > > >patch
> > > >> > > > like below OK to simply change the log level and the error
> > > >code?
> > > >> > > 
> > > >> > > No, because if controller is actually present this is a hard
> > > >failure and
> > > >> > > we should be reporting it, not suppressing it.
> > > >> > > 
> > > >> > > The issue is that we did not believe PNP data and in this case we
> > > >should
> > > >> > > have. Unfortunately in old days there was a lot of crap in
> > > >PNP/ACPI
> > > >> > > tables, but it could be better now. We can try, in addition to
> > > >PNP
> > > >> > > matching, checking 8042 flag in "Fixed ACPI Description Table
> > > >Boot
> > > >> > > Architecture Flags" in FADT and if it also shows there is no 8042
> > > >then
> > > >> > > bail.
> > > >> > 
> > > >> > That sounds promising.  Indeed FACL.dsl shows like:
> > > >> > 
> > > >> > [000h 0000   4]                    Signature : "FACP"    [Fixed
> > > >ACPI Description Table (FADT)]
> > > >> > [004h 0004   4]                 Table Length : 0000010C
> > > >> > ....
> > > >> >                Legacy Devices Supported (V2) : 0
> > > >> >             8042 Present on ports 60/64 (V2) : 0
> > > >> > 
> > > >> > If a test patch gets ready, let me know, I'll give it a try.
> > > >> 
> > > >> FYI, a hack like below seems working.
> > > >> 
> > > >> 
> > > >> Takashi
> > > >> 
> > > >> ---
> > > >> diff --git a/drivers/input/serio/i8042-x86ia64io.h
> > > >b/drivers/input/serio/i8042-x86ia64io.h
> > > >> index 073246c7d163..ed6ab702e4b7 100644
> > > >> --- a/drivers/input/serio/i8042-x86ia64io.h
> > > >> +++ b/drivers/input/serio/i8042-x86ia64io.h
> > > >> @@ -9,6 +9,7 @@
> > > >>  
> > > >>  #ifdef CONFIG_X86
> > > >>  #include <asm/x86_init.h>
> > > >> +#include <linux/acpi.h>
> > > >>  #endif
> > > >>  
> > > >>  /*
> > > >> @@ -1055,6 +1056,13 @@ static int __init i8042_pnp_init(void)
> > > >>  #if defined(__ia64__)
> > > >>  		return -ENODEV;
> > > >>  #else
> > > >> +#ifdef CONFIG_ACPI
> > > >> +		if (acpi_gbl_FADT.header.revision >= 3 &&
> > > >> +		    !(acpi_gbl_FADT.boot_flags & ACPI_FADT_8042)) {
> > > >> +			pr_info("PNP: No PS/2 controller found and disabled in ACPI\n");
> > > >> +			return -ENODEV;
> > > >> +		}
> > > >> +#endif
> > > >>  		pr_info("PNP: No PS/2 controller found. Probing ports
> > > >directly.\n");
> > > >>  		return 0;
> > > >>  #endif
> > > >
> > > >I'm not an expert in any subsystem but, maybe this "hack" could be
> > > >added
> > > >to default_i8042_detect in arch/x86/kernel/x86_init.c? Currently it is
> > > >enabled by default, but different Intel platform like ce4100 and
> > > >intel-mid disables it explicit.
> > > >
> > > >I mentioned "hack" because following osdev.org[1] using ACPI is the
> > > >correct way to detect if i8042 exists. Pardon me if this not applies in
> > > >this situation, or if I missed something.
> > > 
> > > That is the proper way of detecting i8042 if you trust firmware; historically we do not, and so we want to make sure that PNP data agrees with fadt data.
> > 
> > So it depends on how well you trust the firmware.  If we assume ACPI
> > providing always correctly, it can be put in default_i8042_detect, and
> > it'd be a better place indeed.  OTOH, if we don't trust ACPI,
> > especially on older machines, and let at first probing ACPI PnP no
> > matter whether FADT bit is set, we'd need to put the check after PnP
> > probe like my patch.
> > 
> > My patch assumes that the BIOS is new and good enough if FADT revision
> > is 3 or greater.  The only concern is whether this is really good
> > enough.  I just hope so.
> 
> FWIW FADT revision 3 is defined in ACPI 2.0 as far as I know. So not too
> new.
> > 
> > In anyway, Dmitry, if you're happy with it, I'll cook up the proper
> > patch for the merge.  Let me know.
> 
> I am happy with the idea, but as far as implementation goes I think we
> need to add this flag to x86_platform.legacy structure, initialize
> x86_platform_legacy.i8042_present = 1 in
> x86_early_init_platform_quirks(), and adjust as needed in
> acpi_parse_fadt().
> 
> Then we can use it in i8042 instead of checking FADT by hand.

That sounds good.  I hope we'll get it soon ;)


Thanks!

Takashi