| Message ID | 20231005201544.26983-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Input: atkbd - add skip_commands module parameter | expand |
Hi Hans, I very much support the inclusion of this patch, because there has been a similar keyboard issue on at least 3 (presumably 9) types of Lenovo laptops, which may also be avoided by simply skipping the GETID command. My patch and a list of the affected laptop types may be found at: https://github.com/yescallop/atkbd-nogetid In my last patch submission, I have included the issue details: https://lore.kernel.org/linux-input/20230530131340.39961-1-yesh25@mail2.sysu.edu.cn/ There were also two other patch submissions aimed at enabling `i8042.dumbkbd` on some HP laptops in order to avoid sending the GETID command, which isn't very desirable because it breaks the Caps Lock LED: https://lore.kernel.org/linux-input/2iAJTwqZV6lQs26cTb38RNYqxvsink6SRmrZ5h0cBUSuf9NT0tZTsf9fEAbbto2maavHJEOP8GA1evlKa6xjKOsaskDhtJWxjcnrgPigzVo=@gurevit.ch/ https://lore.kernel.org/linux-input/20210609073333.8425-1-egori@altlinux.org/ And another patch submisson aimed at fixing the issue generically, which, sadly, did not work on my laptop because the GETID command would trigger more errornous behaviours on it: https://lore.kernel.org/linux-input/20210201160336.16008-1-anton@cpp.in/ I hope that these materials will help people better understand the nature of the issue and the urgency to address it. Below are some comments on the patch: > +MODULE_PARM_DESC(skip_commands, "Bitfield where each bits skips a specific keyboard cmd (0 - 0x3f)"); "bits" -> "bit"? I think we may also need to document the new module parameter at Documentation/admin-guide/kernel-parameters.txt and clarify which bit skips which keyboard command. Lastly, would you think it is appropriate to include in this patch series the quirks for Lenovo laptops on which my patch was tested to work? If so, the quirk table entries would be: System vendor: "LENOVO" Product names: "82G2", "82NC", "82TK" Driver data : ATKBD_SKIP_GETID Above all, thank you for working out this nice patch. Regards, Shang On 2023/10/06 04:15, Hans de Goede wrote: > Hi all, > > While debugging a keyboard issue on some HP laptops adding i8042.dumbkbd > helped to avoid the issue. So one of the commands send by atkbd.c seemed > to be the culprit. > > This series a skip_commands option to help debug cases like this by adding > a bit-field which allows disabling a subset of the ps2_command() > calls the atkbd driver makes. > > It also replaces the existing atkbd_skip_deactivate flag > with the new parameter and adds a DMI quirk for the HP laptops > to avoid the keyboard issue there. > > Regards, > > Hans > > > Hans de Goede (3): > Input: atkbd - add skip_commands module parameter > Input: atkbd - drop atkbd_skip_deactivate flag > Input: atkbd - set skip_commands = ATKBD_SKIP_GETID for HP laptop > 15s-fq* laptops > > drivers/input/keyboard/atkbd.c | 88 ++++++++++++++++++++++++++-------- > 1 file changed, 69 insertions(+), 19 deletions(-) > > -- > 2.41.0 >
Hi Shang, On 10/17/23 15:53, Shang Ye wrote: > Hi Hans, > > I very much support the inclusion of this patch, because there has been > a similar keyboard issue on at least 3 (presumably 9) types of Lenovo > laptops, which may also be avoided by simply skipping the GETID command. > My patch and a list of the affected laptop types may be found at: > https://github.com/yescallop/atkbd-nogetid > > In my last patch submission, I have included the issue details: > https://lore.kernel.org/linux-input/20230530131340.39961-1-yesh25@mail2.sysu.edu.cn/ > > There were also two other patch submissions aimed at enabling > `i8042.dumbkbd` on some HP laptops in order to avoid sending the GETID > command, which isn't very desirable because it breaks the Caps Lock LED: > https://lore.kernel.org/linux-input/2iAJTwqZV6lQs26cTb38RNYqxvsink6SRmrZ5h0cBUSuf9NT0tZTsf9fEAbbto2maavHJEOP8GA1evlKa6xjKOsaskDhtJWxjcnrgPigzVo=@gurevit.ch/ > https://lore.kernel.org/linux-input/20210609073333.8425-1-egori@altlinux.org/ > > And another patch submisson aimed at fixing the issue generically, > which, sadly, did not work on my laptop because the GETID command would > trigger more errornous behaviours on it: > https://lore.kernel.org/linux-input/20210201160336.16008-1-anton@cpp.in/ Interesting, that might be the issue which is hitting the HP models which I wrote this series for too. > I hope that these materials will help people better understand the > nature of the issue and the urgency to address it. > > Below are some comments on the patch: > >> +MODULE_PARM_DESC(skip_commands, "Bitfield where each bits skips a specific keyboard cmd (0 - 0x3f)"); > > "bits" -> "bit"? Indeed, if we go with this patch-series this should be fixed. > I think we may also need to document the new module parameter at > Documentation/admin-guide/kernel-parameters.txt and clarify which bit > skips which keyboard command. > > Lastly, would you think it is appropriate to include in this patch > series the quirks for Lenovo laptops on which my patch was tested to > work? If so, the quirk table entries would be: > > System vendor: "LENOVO" > Product names: "82G2", "82NC", "82TK" > Driver data : ATKBD_SKIP_GETID Looking at your github and seeing how many models are affected, I'm thinking that we should maybe just skip the entire keyboard atkbd_probe() when atkbd->translated is set. The probe is really only necessary in the untranslated case to check if there is a mouse there or if there is one of the (quite old) special ps/2 keyboards there which have some special handling (search for "id == 0x" to find the special cases) these special cases are all only hit/valid when (atkbd->translated == 0) is true, so when atkbd->translated is true we can just skip the probe and use an assumed id of 0xab00 (already used when i8042.dumbkbd is set) and then immediately bail from atkbd_probe(). I think this generic solution is a better approach then any of the previous approaches since it is nice and KISS and does not rely on any DMI quirks. Regards, Hans > On 2023/10/06 04:15, Hans de Goede wrote: >> Hi all, >> >> While debugging a keyboard issue on some HP laptops adding i8042.dumbkbd >> helped to avoid the issue. So one of the commands send by atkbd.c seemed >> to be the culprit. >> >> This series a skip_commands option to help debug cases like this by adding >> a bit-field which allows disabling a subset of the ps2_command() >> calls the atkbd driver makes. >> >> It also replaces the existing atkbd_skip_deactivate flag >> with the new parameter and adds a DMI quirk for the HP laptops >> to avoid the keyboard issue there. >> >> Regards, >> >> Hans >> >> >> Hans de Goede (3): >> Input: atkbd - add skip_commands module parameter >> Input: atkbd - drop atkbd_skip_deactivate flag >> Input: atkbd - set skip_commands = ATKBD_SKIP_GETID for HP laptop >> 15s-fq* laptops >> >> drivers/input/keyboard/atkbd.c | 88 ++++++++++++++++++++++++++-------- >> 1 file changed, 69 insertions(+), 19 deletions(-) >> >> -- >> 2.41.0 >> >
Hi all, While debugging a keyboard issue on some HP laptops adding i8042.dumbkbd helped to avoid the issue. So one of the commands send by atkbd.c seemed to be the culprit. This series a skip_commands option to help debug cases like this by adding a bit-field which allows disabling a subset of the ps2_command() calls the atkbd driver makes. It also replaces the existing atkbd_skip_deactivate flag with the new parameter and adds a DMI quirk for the HP laptops to avoid the keyboard issue there. Regards, Hans Hans de Goede (3): Input: atkbd - add skip_commands module parameter Input: atkbd - drop atkbd_skip_deactivate flag Input: atkbd - set skip_commands = ATKBD_SKIP_GETID for HP laptop 15s-fq* laptops drivers/input/keyboard/atkbd.c | 88 ++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 19 deletions(-)