| Message ID | 20231206175818.2568-1-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Input: i8042: Quiet down probe failure messages | expand |
On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote: > The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the > following messages are emitted: > > i8042: PNP: No PS/2 controller found. > i8042: PNP: Probing ports directly. > i8042: Can't read CTR while initializing i8042 > i8042: probe of i8042 failed with error -5 > > The last two messages are ERR and WARN respectively. These messages > might be useful for one boot while diagnosing a problem for someone > but as there is no PS/2 controller in PNP or on the machine they're > needlessly noisy to emit every boot. > > Downgrade the CTR message to debug and change the error code for the > failure so that the base device code doesn't emit a warning. > > If someone has problems with i8042 and they need this information, > they can turn on dynamic debugging to get these messages. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- For the Framework 16, I think the following should be done. Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h under the i8042_dmi_quirk_table. This will prevent emitting the first two messages in the shared snippet. > 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 9fbb8d31575a..95dd585fdc1a 100644 > --- a/drivers/input/serio/i8042.c > +++ b/drivers/input/serio/i8042.c > @@ -1008,8 +1008,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 i8042_probe_defer ? -EPROBE_DEFER : -EIO; > + pr_debug("Can't read CTR while initializing\n"); I also think this error message should be pr_err in the situation that the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are likely looking for is avoiding emitting this message when the SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes. > + return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO; I do not think this change makes sense to me personally. It is indeed an I/O issue with the i8042 controller on the Framework motherboard, so the error should be -EIO when i8042_probe_defer is not set. > } > > } while (n < 2 || ctr[0] != ctr[1]); -- Thanks, Rahul Rameshbabu
On 12/6/2023 12:46, Rahul Rameshbabu wrote: > On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote: >> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the >> following messages are emitted: >> >> i8042: PNP: No PS/2 controller found. >> i8042: PNP: Probing ports directly. >> i8042: Can't read CTR while initializing i8042 >> i8042: probe of i8042 failed with error -5 >> >> The last two messages are ERR and WARN respectively. These messages >> might be useful for one boot while diagnosing a problem for someone >> but as there is no PS/2 controller in PNP or on the machine they're >> needlessly noisy to emit every boot. >> >> Downgrade the CTR message to debug and change the error code for the >> failure so that the base device code doesn't emit a warning. >> >> If someone has problems with i8042 and they need this information, >> they can turn on dynamic debugging to get these messages. >> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- > > For the Framework 16, I think the following should be done. > > Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller > probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h > under the i8042_dmi_quirk_table. This will prevent emitting the first > two messages in the shared snippet. I had tried this initially, and yes those first two messages are removed. But TBH I'm not too worried about those as they're INFO. Adding a quirk just switches them over to a new INFO message. But the ERR and WARN messages still come up. The 3 messages that show up are: i8042: PNP detection disabled i8042: Can't read CTR while initializing i8042 i8042: probe of i8042 failed with error -5 I'm more concerned that ERR and WARN messages show up making you think there is a problem to look into. > > >> 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 9fbb8d31575a..95dd585fdc1a 100644 >> --- a/drivers/input/serio/i8042.c >> +++ b/drivers/input/serio/i8042.c >> @@ -1008,8 +1008,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 i8042_probe_defer ? -EPROBE_DEFER : -EIO; >> + pr_debug("Can't read CTR while initializing\n"); > > I also think this error message should be pr_err in the situation that > the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are > likely looking for is avoiding emitting this message when the > SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes. SERIO_QUIRK_PROBE_DEFER isn't set on this machine. > >> + return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO; > > I do not think this change makes sense to me personally. It is indeed an > I/O issue with the i8042 controller on the Framework motherboard, so the > error should be -EIO when i8042_probe_defer is not set. With i8042.debug enabled I can see that the error is a wait read timeout. So I had thought -ENXIO ("No such device or address") made sense here. If you feel strongly that the errors and warnings should stay as is I I wonder if this should be looked at from i8042_pnp_init(). In the no PNP device declared case it still probes, why isn't PNP trusted? > >> } >> >> } while (n < 2 || ctr[0] != ctr[1]); > > -- > Thanks, > > Rahul Rameshbabu
On Wed, 06 Dec, 2023 13:22:25 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote: > On 12/6/2023 12:46, Rahul Rameshbabu wrote: >> On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote: >>> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the >>> following messages are emitted: >>> >>> i8042: PNP: No PS/2 controller found. >>> i8042: PNP: Probing ports directly. >>> i8042: Can't read CTR while initializing i8042 >>> i8042: probe of i8042 failed with error -5 >>> >>> The last two messages are ERR and WARN respectively. These messages >>> might be useful for one boot while diagnosing a problem for someone >>> but as there is no PS/2 controller in PNP or on the machine they're >>> needlessly noisy to emit every boot. >>> >>> Downgrade the CTR message to debug and change the error code for the >>> failure so that the base device code doesn't emit a warning. >>> >>> If someone has problems with i8042 and they need this information, >>> they can turn on dynamic debugging to get these messages. >>> >>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>> --- >> For the Framework 16, I think the following should be done. >> Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller >> probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h >> under the i8042_dmi_quirk_table. This will prevent emitting the first >> two messages in the shared snippet. > > I had tried this initially, and yes those first two messages are removed. But > TBH I'm not too worried about those as they're INFO. Adding a quirk just > switches them over to a new INFO message. > Right, I can imagine someone owning this device panic-ing about the logs in red/yellow in journalctl or dmesg output. That said, since we know that the device should not bother with PNP, I think we should add the quirk, none the less. > But the ERR and WARN messages still come up. The 3 messages that show up are: > > i8042: PNP detection disabled > i8042: Can't read CTR while initializing i8042 > i8042: probe of i8042 failed with error -5 > > I'm more concerned that ERR and WARN messages show up making you think there is > a problem to look into. > >> >>> 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 9fbb8d31575a..95dd585fdc1a 100644 >>> --- a/drivers/input/serio/i8042.c >>> +++ b/drivers/input/serio/i8042.c >>> @@ -1008,8 +1008,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 i8042_probe_defer ? -EPROBE_DEFER : -EIO; >>> + pr_debug("Can't read CTR while initializing\n"); >> I also think this error message should be pr_err in the situation that >> the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are >> likely looking for is avoiding emitting this message when the >> SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes. > > SERIO_QUIRK_PROBE_DEFER isn't set on this machine. > Yeah, I would like to add this quirk as well for the device potentially along with SERIO_QUIRK_NOPNP. I'll get into why I think this might be a good idea later in this email. >> >>> + return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO; >> I do not think this change makes sense to me personally. It is indeed an >> I/O issue with the i8042 controller on the Framework motherboard, so the >> error should be -EIO when i8042_probe_defer is not set. > > With i8042.debug enabled I can see that the error is a wait read timeout. So I > had thought -ENXIO ("No such device or address") made sense here. Right, I think that wait read timeout you are seeing is likely a symptom of something very similar to the experience on laptops like the ASUS ZenBook UX425UA, which inspired the introduction of the probe deferring in the i8042 driver. https://lore.kernel.org/lkml/20211112180022.10850-1-tiwai@suse.de/T/ In this case though, the ASUS ZenBook device did have a PS/2 device attached, while in the Framework device this shouldn't be the case. Will delve more into this nuance in my next section. > > If you feel strongly that the errors and warnings should stay as is I I wonder > if this should be looked at from i8042_pnp_init(). > > In the no PNP device declared case it still probes, why isn't PNP trusted? > My understanding is this is due to some PS/2 devices not supporting PNP so this manual probe path must still be taken even when we add the SERIO_QUIRK_NOPNP quirk. I think if we add the quirk for deferred probing for the device, we can then also have a patch that does not omit the error to the logs when this quirk is enabled. My question now becomes the following. If the Framework device does not want to expose its Intel 8042 controller to the host since its some unused hardware on the mainboard, why does it bother to advertise the 8042 over the ACPI table? Wouldn't it be better to have some UEFI update that fixes the ACPI table listing to avoid advertising the 8042? https://wiki.osdev.org/%228042%22_PS/2_Controller >> >>> } >>> } while (n < 2 || ctr[0] != ctr[1]); -- Thanks, Rahul Rameshbabu
On 12/6/2023 13:55, Rahul Rameshbabu wrote: > On Wed, 06 Dec, 2023 13:22:25 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote: >> On 12/6/2023 12:46, Rahul Rameshbabu wrote: >>> On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote: >>>> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the >>>> following messages are emitted: >>>> >>>> i8042: PNP: No PS/2 controller found. >>>> i8042: PNP: Probing ports directly. >>>> i8042: Can't read CTR while initializing i8042 >>>> i8042: probe of i8042 failed with error -5 >>>> >>>> The last two messages are ERR and WARN respectively. These messages >>>> might be useful for one boot while diagnosing a problem for someone >>>> but as there is no PS/2 controller in PNP or on the machine they're >>>> needlessly noisy to emit every boot. >>>> >>>> Downgrade the CTR message to debug and change the error code for the >>>> failure so that the base device code doesn't emit a warning. >>>> >>>> If someone has problems with i8042 and they need this information, >>>> they can turn on dynamic debugging to get these messages. >>>> >>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>>> --- >>> For the Framework 16, I think the following should be done. >>> Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller >>> probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h >>> under the i8042_dmi_quirk_table. This will prevent emitting the first >>> two messages in the shared snippet. >> >> I had tried this initially, and yes those first two messages are removed. But >> TBH I'm not too worried about those as they're INFO. Adding a quirk just >> switches them over to a new INFO message. >> > > Right, I can imagine someone owning this device panic-ing about the logs > in red/yellow in journalctl or dmesg output. That said, since we know > that the device should not bother with PNP, I think we should add the > quirk, none the less. > >> But the ERR and WARN messages still come up. The 3 messages that show up are: >> >> i8042: PNP detection disabled >> i8042: Can't read CTR while initializing i8042 >> i8042: probe of i8042 failed with error -5 >> >> I'm more concerned that ERR and WARN messages show up making you think there is >> a problem to look into. >> >>> >>>> 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 9fbb8d31575a..95dd585fdc1a 100644 >>>> --- a/drivers/input/serio/i8042.c >>>> +++ b/drivers/input/serio/i8042.c >>>> @@ -1008,8 +1008,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 i8042_probe_defer ? -EPROBE_DEFER : -EIO; >>>> + pr_debug("Can't read CTR while initializing\n"); >>> I also think this error message should be pr_err in the situation that >>> the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are >>> likely looking for is avoiding emitting this message when the >>> SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes. >> >> SERIO_QUIRK_PROBE_DEFER isn't set on this machine. >> > > Yeah, I would like to add this quirk as well for the device potentially > along with SERIO_QUIRK_NOPNP. I'll get into why I think this might be a > good idea later in this email. OK if we end up with a quirk for this system in the end I'll add both. > >>> >>>> + return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO; >>> I do not think this change makes sense to me personally. It is indeed an >>> I/O issue with the i8042 controller on the Framework motherboard, so the >>> error should be -EIO when i8042_probe_defer is not set. >> >> With i8042.debug enabled I can see that the error is a wait read timeout. So I >> had thought -ENXIO ("No such device or address") made sense here. > > Right, I think that wait read timeout you are seeing is likely a symptom > of something very similar to the experience on laptops like the ASUS > ZenBook UX425UA, which inspired the introduction of the probe deferring > in the i8042 driver. > > https://lore.kernel.org/lkml/20211112180022.10850-1-tiwai@suse.de/T/ > > In this case though, the ASUS ZenBook device did have a PS/2 device > attached, while in the Framework device this shouldn't be the case. Will > delve more into this nuance in my next section. OK let me experiment with this and get back on what happens. > >> >> If you feel strongly that the errors and warnings should stay as is I I wonder >> if this should be looked at from i8042_pnp_init(). >> >> In the no PNP device declared case it still probes, why isn't PNP trusted? >> > > My understanding is this is due to some PS/2 devices not supporting PNP > so this manual probe path must still be taken even when we add the > SERIO_QUIRK_NOPNP quirk. I think if we add the quirk for deferred > probing for the device, we can then also have a patch that does not omit > the error to the logs when this quirk is enabled. > > My question now becomes the following. If the Framework device does not > want to expose its Intel 8042 controller to the host since its some > unused hardware on the mainboard, why does it bother to advertise the > 8042 over the ACPI table? Wouldn't it be better to have some UEFI update > that fixes the ACPI table listing to avoid advertising the 8042? > > https://wiki.osdev.org/%228042%22_PS/2_Controller You hit on the head my confusion. The laptop doesn't advertise any of the PNP IDs listed in pnp_kbd_devids. I double checked in /sys/bus/acpi/devices. It seems that they're ignored anyway and will probe whether they are there or not. So how could the firmware actually advertise this so the kernel would trust it? > >>> >>>> } >>>> } while (n < 2 || ctr[0] != ctr[1]); > > -- > Thanks, > > Rahul Rameshbabu
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index 9fbb8d31575a..95dd585fdc1a 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -1008,8 +1008,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 i8042_probe_defer ? -EPROBE_DEFER : -EIO; + pr_debug("Can't read CTR while initializing\n"); + return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO; } } while (n < 2 || ctr[0] != ctr[1]);
The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the following messages are emitted: i8042: PNP: No PS/2 controller found. i8042: PNP: Probing ports directly. i8042: Can't read CTR while initializing i8042 i8042: probe of i8042 failed with error -5 The last two messages are ERR and WARN respectively. These messages might be useful for one boot while diagnosing a problem for someone but as there is no PS/2 controller in PNP or on the machine they're needlessly noisy to emit every boot. Downgrade the CTR message to debug and change the error code for the failure so that the base device code doesn't emit a warning. If someone has problems with i8042 and they need this information, they can turn on dynamic debugging to get these messages. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/input/serio/i8042.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)