[PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"

[PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"

[Werner Sembach]

This is a continuation of
https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/

That fix did fix the keyboard not responding at all sometimes after resume,
but at the price of it being laggy for some time after boot. Additionally
setting atkbd.reset removes that lag.

This patch comes in 2 parts: The first one adds a quirk to atkbd to set
atkbd.reset and the second one then applies that and the i8042 quirks to
the affected devices.

[PATCH v2 1/2] Input: atkbd - Make it possible to apply atkbd.reset via kernel quirk

[Werner Sembach]

atkbd.reset was only a command line parameter. Some devices might have a
known bug that can be worked around by just permanently applying this
quirk.

This patch adds the ability to do this on the kernel level for known buggy
devices.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: stable@vger.kernel.org
---
 drivers/input/keyboard/atkbd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 246958795f60..ef65c46c4efe 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -1731,6 +1731,12 @@ static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
 	return 1;
 }
 
+static int __init atkbd_reset_fixup(const struct dmi_system_id *id)
+{
+	atkbd_reset = true;
+	return 1;
+}
+
 /*
  * NOTE: do not add any more "force release" quirks to this table.  The
  * task of adjusting list of keys that should be "released" automatically
-- 
2.34.1

Re: [PATCH v2 1/2] Input: atkbd - Make it possible to apply atkbd.reset via kernel quirk

[Greg KH]

On Mon, Feb 27, 2023 at 07:59:06PM +0100, Werner Sembach wrote:
> atkbd.reset was only a command line parameter. Some devices might have a
> known bug that can be worked around by just permanently applying this
> quirk.
> 
> This patch adds the ability to do this on the kernel level for known buggy
> devices.
> 
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/input/keyboard/atkbd.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> index 246958795f60..ef65c46c4efe 100644
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -1731,6 +1731,12 @@ static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
>  	return 1;
>  }
>  
> +static int __init atkbd_reset_fixup(const struct dmi_system_id *id)
> +{
> +	atkbd_reset = true;
> +	return 1;

Why is this returning 1?  Who calls this?

And this should be a per-device attribute, not a global one, right?

thanks,

greg k-h

Re: [PATCH v2 1/2] Input: atkbd - Make it possible to apply atkbd.reset via kernel quirk

[Werner Sembach]

Am 28.02.23 um 08:34 schrieb Greg KH:
> On Mon, Feb 27, 2023 at 07:59:06PM +0100, Werner Sembach wrote:
>> atkbd.reset was only a command line parameter. Some devices might have a
>> known bug that can be worked around by just permanently applying this
>> quirk.
>>
>> This patch adds the ability to do this on the kernel level for known buggy
>> devices.
>>
>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>> Cc: stable@vger.kernel.org
>> ---
>>   drivers/input/keyboard/atkbd.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
>> index 246958795f60..ef65c46c4efe 100644
>> --- a/drivers/input/keyboard/atkbd.c
>> +++ b/drivers/input/keyboard/atkbd.c
>> @@ -1731,6 +1731,12 @@ static int __init atkbd_deactivate_fixup(const struct dmi_system_id *id)
>>   	return 1;
>>   }
>>   
>> +static int __init atkbd_reset_fixup(const struct dmi_system_id *id)
>> +{
>> +	atkbd_reset = true;
>> +	return 1;
> Why is this returning 1?  Who calls this?

This function is following the format of the other fixup functions directly 
above it.

It is there to be called as a callback via the atkbd_dmi_quirk_table on a per 
device basis. See 2nd patch of this patchset.

Greetings,

Werner

>
> And this should be a per-device attribute, not a global one, right?
>
> thanks,
>
> greg k-h

[PATCH v2 2/2] Input: i8042 - add 2 more TUXEDO devices to i8042 and atkbd quirk tables

[Werner Sembach]

A lot of modern Clevo barebones have touchpad and/or keyboard issues after
suspend fixable with nomux + reset + noloop + nopnp. 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.

Setting SERIO_QUIRK_NOMUX or SERIO_QUIRK_RESET_ALWAYS makes the keyboard
very laggy for ~5 seconds after boot and sometimes also after resume. To
fix this atkbd_reset_fixup is also applied for this device via a quirk,
which fixes it again.

Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
Cc: stable@vger.kernel.org
---
 drivers/input/keyboard/atkbd.c        | 24 ++++++++++++++++++++++++
 drivers/input/serio/i8042-acpipnpio.h | 26 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index ef65c46c4efe..e89918347357 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -1886,6 +1886,30 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 		},
 		.callback = atkbd_deactivate_fixup,
 	},
+	/*
+	 * Some Clevo devices need this reset, otherwise they keyboard may
+	 * be laggy after boot and/or resume for ~5 seconds.
+	 */
+	{
+		/*
+		 * Clevo device, DMI_BOARD_VENDOR and DMI_SYSTEM_VENDOR
+		 * differ between resellers.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "N150CU"),
+		},
+		.callback = atkbd_reset_fixup,
+	},
+	{
+		/*
+		 * Clevo device, DMI_BOARD_VENDOR and DMI_SYSTEM_VENDOR
+		 * differ between resellers.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NHxxRZQ"),
+		},
+		.callback = atkbd_reset_fixup,
+	},
 	{ }
 };
 
diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index efc61736099b..6ed1044b4ba5 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -1116,6 +1116,19 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
 					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
 	},
+	{
+		/*
+		 * Setting SERIO_QUIRK_NOMUX or SERIO_QUIRK_RESET_ALWAYS makes the
+		 * keyboard very laggy for ~5 seconds after boot and sometimes also
+		 * after resume. To fix this atkbd_reset_fixup is also applied for
+		 * this device via a quirk, which fixes it again.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "N150CU"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
+					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
+	},
 	{
 		.matches = {
 			DMI_MATCH(DMI_BOARD_NAME, "NH5xAx"),
@@ -1123,6 +1136,19 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
 					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
 	},
+	{
+		/*
+		 * Setting SERIO_QUIRK_NOMUX or SERIO_QUIRK_RESET_ALWAYS makes the
+		 * keyboard very laggy for ~5 seconds after boot and sometimes also
+		 * after resume. To fix this atkbd_reset_fixup is also applied for
+		 * this device via a quirk, which fixes it again.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NHxxRZQ"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
+					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
+	},
 	{
 		.matches = {
 			DMI_MATCH(DMI_BOARD_NAME, "NL5xRU"),
-- 
2.34.1

Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"

[Hans de Goede]

Hi Werner,

On 2/27/23 19:59, Werner Sembach wrote:
> This is a continuation of
> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
> 
> That fix did fix the keyboard not responding at all sometimes after resume,
> but at the price of it being laggy for some time after boot. Additionally
> setting atkbd.reset removes that lag.
> 
> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
> atkbd.reset and the second one then applies that and the i8042 quirks to
> the affected devices.

Can you please rework this series so that the quirk based setting of
the "atkbd.reset" equivalent on the kernel commandline becomes another
SERIO_QUIRK_* flag and avoid the duplication of the DMI ids?

Regards,

Hans

Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"

[Werner Sembach]

Am 28.02.23 um 09:41 schrieb Hans de Goede:
> Hi Werner,
>
> On 2/27/23 19:59, Werner Sembach wrote:
>> This is a continuation of
>> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
>>
>> That fix did fix the keyboard not responding at all sometimes after resume,
>> but at the price of it being laggy for some time after boot. Additionally
>> setting atkbd.reset removes that lag.
>>
>> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
>> atkbd.reset and the second one then applies that and the i8042 quirks to
>> the affected devices.
> Can you please rework this series so that the quirk based setting of
> the "atkbd.reset" equivalent on the kernel commandline becomes another
> SERIO_QUIRK_* flag and avoid the duplication of the DMI ids?

I'm not sure how to cleanly do this, since atkbd is an own module?

Kind Regards,

Werner

>
> Regards,
>
> Hans
>
>
>

Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"

[Werner Sembach]

Am 27.02.23 um 19:59 schrieb Werner Sembach:
> This is a continuation of
> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
>
> That fix did fix the keyboard not responding at all sometimes after resume,
> but at the price of it being laggy for some time after boot. Additionally
> setting atkbd.reset removes that lag.
>
> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
> atkbd.reset and the second one then applies that and the i8042 quirks to
> the affected devices.
>
>
Somehow, for my testing last week these patches seemed work, but now i still see 
occasional laggy keyboard after boot. So sadly the atkbd_reset quirk didn't fix 
the issue after all.

Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"

[Hans de Goede]

Hi,

On 3/1/23 17:57, Werner Sembach wrote:
> 
> Am 27.02.23 um 19:59 schrieb Werner Sembach:
>> This is a continuation of
>> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
>>
>> That fix did fix the keyboard not responding at all sometimes after resume,
>> but at the price of it being laggy for some time after boot. Additionally
>> setting atkbd.reset removes that lag.
>>
>> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
>> atkbd.reset and the second one then applies that and the i8042 quirks to
>> the affected devices.
>>
>>
> Somehow, for my testing last week these patches seemed work, but now i still see occasional laggy keyboard after boot. So sadly the atkbd_reset quirk didn't fix the issue after all.

Ok, well I guess that also resolves the discussion about trying to avoid
the duplicate DMI table entries.

FWIW I did not realize that the other quirk was in another module and
I don't have a good answer how to solve this in a way that avoids
adding the DMI matches twice.

Regards,

Hans