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