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@xxxxxxxxxxxxxxxxx/ > > 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@xxxxxxxxxxxx/ > > 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@xxxxxx/ 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 >> >
