Debounce filter setting should be independent from IRQ type setting
because according to the ACPI specs, there are separate arguments for
specifying debounce timeout and IRQ type in GpioIo() and GpioInt().
Together with commit 06abe8291bc31839950f7d0362d9979edc88a666
("pinctrl: amd: fix incorrect way to disable debounce filter") and
Andy's patch "gpiolib: acpi: Take into account debounce settings" [1],
this will fix broken touchpads for laptops whose BIOS set the
debounce timeout to a relatively large value. For example, the BIOS
of Lenovo AMD gaming laptops including Legion-5 15ARH05 (R7000),
Legion-5P (R7000P) and IdeaPad Gaming 3 15ARH05, set the debounce
timeout to 124.8ms. This led to the kernel receiving only ~7 HID
reports per second from the Synaptics touchpad
(MSFT0001:00 06CB:7F28).
Existing touchpads like [2][3] are not troubled by this bug because
the debounce timeout has been set to 0 by the BIOS before enabling
the debounce filter in setting IRQ type.
[1] https://lore.kernel.org/linux-gpio/[email protected]/
[2] https://github.com/Syniurge/i2c-amd-mp2/issues/11#issuecomment-721331582
[3] https://forum.manjaro.org/t/random-short-touchpad-freezes/30832/28
Cc: Hans de Goede <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Benjamin Tissoires <[email protected]>
Cc: [email protected]
Reviewed-by: Andy Shevchenko <[email protected]>
BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1887190
Link: https://lore.kernel.org/linux-gpio/CAHp75VcwiGREBUJ0A06EEw-SyabqYsp%2Bdqs2DpSrhaY-2GVdAA%40mail.gmail.com/
Signed-off-by: Coiby Xu <[email protected]>
---
Changelog v4:
- Note in the commit message that this patch depends on other two
patches to fix the broken touchpad [Hans de Goede]
- Add in the commit message that one more touchpad could be fixed.
Changelog v3:
- Explain why the driver mistakenly took the slightly deviated value
of debounce timeout in the commit message (patch 2/4) [Andy Shevchenko]
- Explain why other touchpads are affected by the issue of setting debounce
filter in IRQ type setting in the commit message (patch 4/4)
Changelog v2:
- Message-Id to Link and grammar fixes for commit messages [Andy Shevchenko]
---
drivers/pinctrl/pinctrl-amd.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index 4aea3e05e8c6..899c16c17b6d 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -429,7 +429,6 @@ static int amd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
pin_reg &= ~BIT(LEVEL_TRIG_OFF);
pin_reg &= ~(ACTIVE_LEVEL_MASK << ACTIVE_LEVEL_OFF);
pin_reg |= ACTIVE_HIGH << ACTIVE_LEVEL_OFF;
- pin_reg |= DB_TYPE_REMOVE_GLITCH << DB_CNTRL_OFF;
irq_set_handler_locked(d, handle_edge_irq);
break;
@@ -437,7 +436,6 @@ static int amd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
pin_reg &= ~BIT(LEVEL_TRIG_OFF);
pin_reg &= ~(ACTIVE_LEVEL_MASK << ACTIVE_LEVEL_OFF);
pin_reg |= ACTIVE_LOW << ACTIVE_LEVEL_OFF;
- pin_reg |= DB_TYPE_REMOVE_GLITCH << DB_CNTRL_OFF;
irq_set_handler_locked(d, handle_edge_irq);
break;
@@ -445,7 +443,6 @@ static int amd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
pin_reg &= ~BIT(LEVEL_TRIG_OFF);
pin_reg &= ~(ACTIVE_LEVEL_MASK << ACTIVE_LEVEL_OFF);
pin_reg |= BOTH_EADGE << ACTIVE_LEVEL_OFF;
- pin_reg |= DB_TYPE_REMOVE_GLITCH << DB_CNTRL_OFF;
irq_set_handler_locked(d, handle_edge_irq);
break;
@@ -453,8 +450,6 @@ static int amd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
pin_reg |= LEVEL_TRIGGER << LEVEL_TRIG_OFF;
pin_reg &= ~(ACTIVE_LEVEL_MASK << ACTIVE_LEVEL_OFF);
pin_reg |= ACTIVE_HIGH << ACTIVE_LEVEL_OFF;
- pin_reg &= ~(DB_CNTRl_MASK << DB_CNTRL_OFF);
- pin_reg |= DB_TYPE_PRESERVE_LOW_GLITCH << DB_CNTRL_OFF;
irq_set_handler_locked(d, handle_level_irq);
break;
@@ -462,8 +457,6 @@ static int amd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
pin_reg |= LEVEL_TRIGGER << LEVEL_TRIG_OFF;
pin_reg &= ~(ACTIVE_LEVEL_MASK << ACTIVE_LEVEL_OFF);
pin_reg |= ACTIVE_LOW << ACTIVE_LEVEL_OFF;
- pin_reg &= ~(DB_CNTRl_MASK << DB_CNTRL_OFF);
- pin_reg |= DB_TYPE_PRESERVE_HIGH_GLITCH << DB_CNTRL_OFF;
irq_set_handler_locked(d, handle_level_irq);
break;
--
2.29.2
On Wed, Nov 25, 2020 at 3:03 PM Coiby Xu <[email protected]> wrote:
>
> Debounce filter setting should be independent from IRQ type setting
> because according to the ACPI specs, there are separate arguments for
> specifying debounce timeout and IRQ type in GpioIo() and GpioInt().
>
> Together with commit 06abe8291bc31839950f7d0362d9979edc88a666
> ("pinctrl: amd: fix incorrect way to disable debounce filter") and
> Andy's patch "gpiolib: acpi: Take into account debounce settings" [1],
> this will fix broken touchpads for laptops whose BIOS set the
> debounce timeout to a relatively large value. For example, the BIOS
> of Lenovo AMD gaming laptops including Legion-5 15ARH05 (R7000),
> Legion-5P (R7000P) and IdeaPad Gaming 3 15ARH05, set the debounce
> timeout to 124.8ms. This led to the kernel receiving only ~7 HID
> reports per second from the Synaptics touchpad
> (MSFT0001:00 06CB:7F28).
>
> Existing touchpads like [2][3] are not troubled by this bug because
> the debounce timeout has been set to 0 by the BIOS before enabling
> the debounce filter in setting IRQ type.
>
> [1] https://lore.kernel.org/linux-gpio/[email protected]/
JFYI: this is nowadays
8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
(No need to recend, just an information that can be applied maybe by Linus)
> [2] https://github.com/Syniurge/i2c-amd-mp2/issues/11#issuecomment-721331582
> [3] https://forum.manjaro.org/t/random-short-touchpad-freezes/30832/28
--
With Best Regards,
Andy Shevchenko
On Wed, Nov 25, 2020 at 2:03 PM Coiby Xu <[email protected]> wrote:
> Debounce filter setting should be independent from IRQ type setting
> because according to the ACPI specs, there are separate arguments for
> specifying debounce timeout and IRQ type in GpioIo() and GpioInt().
>
> Together with commit 06abe8291bc31839950f7d0362d9979edc88a666
> ("pinctrl: amd: fix incorrect way to disable debounce filter") and
> Andy's patch "gpiolib: acpi: Take into account debounce settings" [1],
> this will fix broken touchpads for laptops whose BIOS set the
> debounce timeout to a relatively large value. For example, the BIOS
> of Lenovo AMD gaming laptops including Legion-5 15ARH05 (R7000),
> Legion-5P (R7000P) and IdeaPad Gaming 3 15ARH05, set the debounce
> timeout to 124.8ms. This led to the kernel receiving only ~7 HID
> reports per second from the Synaptics touchpad
> (MSFT0001:00 06CB:7F28).
>
> Existing touchpads like [2][3] are not troubled by this bug because
> the debounce timeout has been set to 0 by the BIOS before enabling
> the debounce filter in setting IRQ type.
>
> [1] https://lore.kernel.org/linux-gpio/[email protected]/
> [2] https://github.com/Syniurge/i2c-amd-mp2/issues/11#issuecomment-721331582
> [3] https://forum.manjaro.org/t/random-short-touchpad-freezes/30832/28
>
> Cc: Hans de Goede <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Cc: Benjamin Tissoires <[email protected]>
> Cc: [email protected]
> Reviewed-by: Andy Shevchenko <[email protected]>
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1887190
> Link: https://lore.kernel.org/linux-gpio/CAHp75VcwiGREBUJ0A06EEw-SyabqYsp%2Bdqs2DpSrhaY-2GVdAA%40mail.gmail.com/
> Signed-off-by: Coiby Xu <[email protected]>
> ---
> Changelog v4:
> - Note in the commit message that this patch depends on other two
> patches to fix the broken touchpad [Hans de Goede]
> - Add in the commit message that one more touchpad could be fixed.
Patch applied for fixes adding a reference to Andy's commit.
Thanks for sorting this out!
Yours,
Linus Walleij
On Fri, Dec 04, 2020 at 10:03:43AM +0100, Linus Walleij wrote:
>On Wed, Nov 25, 2020 at 2:03 PM Coiby Xu <[email protected]> wrote:
>
>> Debounce filter setting should be independent from IRQ type setting
>> because according to the ACPI specs, there are separate arguments for
>> specifying debounce timeout and IRQ type in GpioIo() and GpioInt().
>>
>> Together with commit 06abe8291bc31839950f7d0362d9979edc88a666
>> ("pinctrl: amd: fix incorrect way to disable debounce filter") and
>> Andy's patch "gpiolib: acpi: Take into account debounce settings" [1],
>> this will fix broken touchpads for laptops whose BIOS set the
>> debounce timeout to a relatively large value. For example, the BIOS
>> of Lenovo AMD gaming laptops including Legion-5 15ARH05 (R7000),
>> Legion-5P (R7000P) and IdeaPad Gaming 3 15ARH05, set the debounce
>> timeout to 124.8ms. This led to the kernel receiving only ~7 HID
>> reports per second from the Synaptics touchpad
>> (MSFT0001:00 06CB:7F28).
>>
>> Existing touchpads like [2][3] are not troubled by this bug because
>> the debounce timeout has been set to 0 by the BIOS before enabling
>> the debounce filter in setting IRQ type.
>>
>> [1] https://lore.kernel.org/linux-gpio/[email protected]/
>> [2] https://github.com/Syniurge/i2c-amd-mp2/issues/11#issuecomment-721331582
>> [3] https://forum.manjaro.org/t/random-short-touchpad-freezes/30832/28
>>
>> Cc: Hans de Goede <[email protected]>
>> Cc: Andy Shevchenko <[email protected]>
>> Cc: Benjamin Tissoires <[email protected]>
>> Cc: [email protected]
>> Reviewed-by: Andy Shevchenko <[email protected]>
>> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1887190
>> Link: https://lore.kernel.org/linux-gpio/CAHp75VcwiGREBUJ0A06EEw-SyabqYsp%2Bdqs2DpSrhaY-2GVdAA%40mail.gmail.com/
>> Signed-off-by: Coiby Xu <[email protected]>
>> ---
>> Changelog v4:
>> - Note in the commit message that this patch depends on other two
>> patches to fix the broken touchpad [Hans de Goede]
>> - Add in the commit message that one more touchpad could be fixed.
>
>Patch applied for fixes adding a reference to Andy's commit.
>Thanks for sorting this out!
>
Thank you for applying the patch!
>Yours,
>Linus Walleij
--
Best regards,
Coiby
On Wed, Nov 25, 2020 at 03:24:20PM +0200, Andy Shevchenko wrote:
>On Wed, Nov 25, 2020 at 3:03 PM Coiby Xu <[email protected]> wrote:
>>
>> Debounce filter setting should be independent from IRQ type setting
>> because according to the ACPI specs, there are separate arguments for
>> specifying debounce timeout and IRQ type in GpioIo() and GpioInt().
>>
>> Together with commit 06abe8291bc31839950f7d0362d9979edc88a666
>> ("pinctrl: amd: fix incorrect way to disable debounce filter") and
>> Andy's patch "gpiolib: acpi: Take into account debounce settings" [1],
>> this will fix broken touchpads for laptops whose BIOS set the
>> debounce timeout to a relatively large value. For example, the BIOS
>> of Lenovo AMD gaming laptops including Legion-5 15ARH05 (R7000),
>> Legion-5P (R7000P) and IdeaPad Gaming 3 15ARH05, set the debounce
>> timeout to 124.8ms. This led to the kernel receiving only ~7 HID
>> reports per second from the Synaptics touchpad
>> (MSFT0001:00 06CB:7F28).
>>
>> Existing touchpads like [2][3] are not troubled by this bug because
>> the debounce timeout has been set to 0 by the BIOS before enabling
>> the debounce filter in setting IRQ type.
>>
>> [1] https://lore.kernel.org/linux-gpio/[email protected]/
>
>JFYI: this is nowadays
>8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
>
Thank you for the info! Next time I will also check the linux-next
tree:)
>(No need to recend, just an information that can be applied maybe by Linus)
>
>> [2] https://github.com/Syniurge/i2c-amd-mp2/issues/11#issuecomment-721331582
>> [3] https://forum.manjaro.org/t/random-short-touchpad-freezes/30832/28
>
>--
>With Best Regards,
>Andy Shevchenko
--
Best regards,
Coiby