diff mbox

i8042 error at booting an Intel Cherry Trail-based device

Message ID s5hbmwuzjlg.wl-tiwai@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Takashi Iwai Dec. 2, 2016, 10:55 a.m. UTC
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

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

Comments

Marcos Paulo de Souza Dec. 6, 2016, 12:56 a.m. UTC | #1
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
--
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
Dmitry Torokhov Dec. 6, 2016, 6:07 a.m. UTC | #2
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.
Takashi Iwai Dec. 6, 2016, 10:36 a.m. UTC | #3
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
--
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
Dmitry Torokhov Dec. 6, 2016, 5:07 p.m. UTC | #4
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!
Takashi Iwai Dec. 6, 2016, 7:05 p.m. UTC | #5
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
--
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
diff mbox

Patch

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