mbox series

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

Message ID 20230227185907.569154-1-wse@tuxedocomputers.com
Headers show
Series Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix" | expand

Message

Werner Sembach Feb. 27, 2023, 6:59 p.m. UTC
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.

Comments

Greg Kroah-Hartman Feb. 28, 2023, 7:34 a.m. UTC | #1
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
Hans de Goede Feb. 28, 2023, 8:41 a.m. UTC | #2
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
Werner Sembach Feb. 28, 2023, 10:58 a.m. UTC | #3
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
Werner Sembach Feb. 28, 2023, 11:07 a.m. UTC | #4
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
>
>
>
Werner Sembach March 1, 2023, 4:57 p.m. UTC | #5
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.
Hans de Goede March 1, 2023, 5:52 p.m. UTC | #6
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