Am 01.03.22 um 08:54 schrieb Dmitry Torokhov: > Hi, > > On Mon, Feb 28, 2022 at 07:50:55PM +0100, Werner Sembach wrote: >> Am 28.02.22 um 14:00 schrieb Hans de Goede: >>> Hi all, >>> >>> On 2/28/22 12:48, Werner Sembach wrote: >>>> A lot of modern Clevo barebones have touchpad and/or keyboard issues after >>>> suspend, fixable with reset + nomux + nopnp + noloop. Luckily, none of them >>>> have an external PS/2 port so this can safely be set for all of them. >>>> >>>> I'm not entirely sure if every device listed really needs all four quirks, >>>> but after testing and production use. No negative effects could be >>>> observed when setting all four. >>>> >>>> The list is quite massive as neither the TUXEDO nor the Clevo dmi strings >>>> have been very consistent historically. I tried to keep the list as short >>>> as possible without risking on missing an affected device. >>>> >>>> This is revision 2 where the Clevo N150CU barebone is removed again, as it >>>> might have problems with the fix and needs further investigations. Also >>>> the SchenkerTechnologiesGmbH System-/Board-Vendor string variations are >>>> added. >>>> >>>> Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx> >>>> Cc: stable@xxxxxxxxxxxxxxx >>> Looking at the patch I think it would be better to split this into >>> 2 patches": >>> >>> 1. Merge all the existing separate tables into 1 table and use the dmi_system_id.driver_data >>> field to store which combination of the 4 quirks apply to which models. >>> >>> This will already help reducing the tables since some of the models are >>> already listed in 2 or more tables. So you would get something like this: >>> >>> #define SERIO_QUIRK_RESET BIT(0) >>> #define SERIO_QUIRK_NOMUX BIT(1) >>> #define SERIO_QUIRK_NOPNP BIT(2) >>> #define SERIO_QUIRK_NOLOOP BIT(3) >>> #define SERIO_QUIRK_NOSELFTEST BIT(4) >>> // etc. >>> >>> static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = { >>> { >>> /* Entroware Proteus */ >>> .matches = { >>> DMI_MATCH(DMI_SYS_VENDOR, "Entroware"), >>> DMI_MATCH(DMI_PRODUCT_NAME, "Proteus"), >>> DMI_MATCH(DMI_PRODUCT_VERSION, "EL07R4"), >>> }, >>> .driver_data = (void *)(SERIO_QUIRK_RESET | SERIO_QUIRK_NOMUX) >>> }, >>> {} >>> }; >>> >>> I picked the Entroware EL07R4 as example here because it needs both the reset and nomux quirks. >>> >>> And then when checking the quirks do: >>> >>> #ifdef CONFIG_X86 >>> const struct dmi_system_id *dmi_id; >>> long quirks = 0; >>> >>> dmi_id = dmi_first_match(i8042_dmi_quirk_table); >>> if (dmi_id) >>> quirks = (long)dmi_id->driver_data; >>> >>> if (i8042_reset == I8042_RESET_DEFAULT) { >>> if (quirks & SERIO_QUIRK_RESET) >>> i8042_reset = I8042_RESET_ALWAYS; >>> if (quirks & SERIO_QUIRK_NOSELFTEST) >>> i8042_reset = I8042_RESET_NEVER; >>> } >>> >>> //etc. >>> >>> >>> This way you can reduce all the tables to just 1 table. Please >>> also sort the table alphabetically, first by vendor, then sub-sort >>> by model. This way you can find more entries to merge and it >>> is a good idea to have big tables like this sorted in some way >>> regardless. >>> >>> >>> And then once this big refactoring patch is done (sorry), you >>> can add a second patch on top: >>> >>> 2. Add the models you want to quirk to the new merged tabled >>> and now you only need to add 1 table entry per model, rather >>> then 4, making the patch much smaller. >>> >>> >>> This is a refactoring which IMHO we should likely already >>> have done a while ago, but now with your patch it really is >>> time we do this. >>> >>> I hope the above makes sense, if not don't hesitate to ask >>> questions. Also note this is how *I* would do this, but >>> I'm not the input subsys-maintainer, ultimately this is >>> Dmitry's call and he may actually dislike with I'm proposing! >> Yes, it does make sense. I could follow you and I too think it's a good idea. I will hopefully find time to work on this >> refactoring in the next days. > Yes, I think this is a great idea as we have many instances where > the same entries are present in several tables. No problem. I hope mail mails get through now ^^. > >>> I don't expect that Dmitry will dislike this, but you never know. >>> >>> Also unfortunately Dmitry lately has only a limited amount of >>> time to spend on input subsys maintenance so in my experience >>> it may be a while before you get a reply from Dmitry. >> Ok, thanks for the info. As I wrote in the other mail, I was worried (or paranoid xD) that I got flagged as spam or >> something. > It did indeed, I am not sure why. This does not invalidate what Hans > said - lately I was not able to spend as much time on input as I wanted. > > Regarding this patch - it looks like board names are pretty unique in > many cases, so I wonder if we could not save some memory by omitting the > vendor info (especially because some, like "Notebook", are very generic > anyways) and go simply by the board. Think in this case it would be helpful to have a DMI_MATCH_NOT at hand, just in case there is a collision in the future. Kind regards, Werner Sembach > > Thanks. >