pinctrl: add IRQF_EARLY_RESUME flags with gpio irq for elan touchpad.
Subsystems (4)
PATCH 1/1 pinctrl: add IRQF_EARLY_RESUME flags with gpio irq for elan touchpad.
From: Caicai <[email protected]>
I had tested two Notebook machines, the Intel i5(or amd ryzen) with elan touchpad, and there's a probability that the touchpad won't work after going to the S3/S4 to wake up, that it would appear no more than 15 times. I found that there's no interrupt to check for /proc/interrupt. It was found that the gpio interrupt of i2c was also not on top. By adding the gpio interrupt flags with IRQF_EARLY_RESUME, now the touchpad tested 200 + times works well.
--- drivers/pinctrl/intel/pinctrl-intel.c | 2 +- drivers/pinctrl/pinctrl-amd.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/pinctrl/intel/pinctrl-intel.c +1 -1
@@ -1189,7 +1189,7 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)* controllers share the same interrupt line. */ ret =devm_request_irq (pctrl->dev, irq, intel_gpio_irq, -IRQF_SHARED |IRQF_NO_THREAD , +IRQF_SHARED |IRQF_NO_THREAD |IRQF_EARLY_RESUME ,dev_name (pctrl->dev), pctrl);if (ret) {dev_err (pctrl->dev,"failed to request interrupt\n" );
drivers/pinctrl/pinctrl-amd.c +1 -1
@@ -937,7 +937,7 @@ static int amd_gpio_probe(struct platform_device *pdev) } ret =devm_request_irq (&pdev->dev, irq_base, amd_gpio_irq_handler, -IRQF_SHARED ,KBUILD_MODNAME , gpio_dev); +IRQF_SHARED |IRQF_EARLY_RESUME |IRQF_NO_THREAD ,KBUILD_MODNAME , gpio_dev);if (ret) goto out2;
Signature · 1 line
2.20.1
Quoted 11 lines · On Fri, Apr 24, 2020 at 12:16 PM <[email protected]> wrote:
From: Caicai <[email protected]>
I had tested two Notebook machines, the Intel i5(or amd ryzen) with elan touchpad, and there's a probability that the touchpad won't work after going to the S3/S4 to wake up, that it would appear no more than 15 times. I found that there's no interrupt to check for
/proc/interrupt. It was found that the gpio interrupt of i2c was also not on top. By adding the gpio interrupt flags withIRQF_EARLY_RESUME, now the touchpad tested 200 + times works well.
Thanks for the contribution!
You need
a) to split this to patch per driver (I can't take or speak for AMD code, I'm not a maintainer and don't have hardware)
b) explain what this flag actually means and does
On top of that we need to test this widely to avoid regressions.
Signature · 2 lines
With Best Regards, Andy Shevchenko
+Cc: Hans.
I can't speak for AMD, but I think it may be useful for Intel pin control. However, I didn't check what may be the side effects of this change and neither contributor answered to my comments...
So, just heads up.
Quoted 48 lines · On Fri, Apr 24, 2020 at 05:12:01PM +0800, [email protected] wrote:
From: Caicai <[email protected]>
I had tested two Notebook machines, the Intel i5(or amd ryzen) with elan touchpad, and there's a probability that the touchpad won't work after going to the S3/S4 to wake up, that it would appear no more than 15 times. I found that there's no interrupt to check for
/proc/interrupt. It was found that the gpio interrupt of i2c was also not on top. By adding the gpio interrupt flags withIRQF_EARLY_RESUME, now the touchpad tested 200 + times works well.Signed-off-by Caicai <[email protected]>--- drivers/pinctrl/intel/pinctrl-intel.c | 2 +- drivers/pinctrl/pinctrl-amd.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)drivers/pinctrl/intel/pinctrl-intel.c +1
-1@@ -1189,7 +1189,7 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)* controllers share the same interrupt line. */ ret =devm_request_irq (pctrl->dev, irq, intel_gpio_irq, -IRQF_SHARED |IRQF_NO_THREAD , +IRQF_SHARED |IRQF_NO_THREAD |IRQF_EARLY_RESUME ,dev_name (pctrl->dev), pctrl);if (ret) {dev_err (pctrl->dev,"failed to request interrupt\n" );drivers/pinctrl/pinctrl-amd.c +1
-1@@ -937,7 +937,7 @@ static int amd_gpio_probe(struct platform_device *pdev) } ret =devm_request_irq (&pdev->dev, irq_base, amd_gpio_irq_handler, -IRQF_SHARED ,KBUILD_MODNAME , gpio_dev); +IRQF_SHARED |IRQF_EARLY_RESUME |IRQF_NO_THREAD ,KBUILD_MODNAME , gpio_dev);if (ret) goto out2;Signature · 1 line
2.20.1
Signature · 2 lines
With Best Regards, Andy Shevchenko
Hi,
Quoted 6 lines · Andy Shevchenko
+Cc: Hans.
I can't speak for AMD, but I think it may be useful for Intel pin control. However, I didn't check what may be the side effects of this change and neither contributor answered to my comments...
AFAICT setting IRQF_EARLY_RESUME causes it to not be resumed during normal IRQ resuming, but earlier during the syscore_resume() call.
There are 2 problems with this:
1. When using S3 suspend syscore_resume() runs before any device suspend handlers, so any state restoring which the GPIO controller's resume handler is doing has not been done yet. While esp. after S3 suspend the restoring may be important (s2idle suspend should not touch the GPIO registers contents).
2. When using S2idle suspend syscore_resume() *never* runs, and IRQs marked with IRQF_EARLY_RESUME are skipped during the normal IRQ resume phase, so these IRQs will never be resumed.
Also IRQF_EARLY_RESUME is used almost no where:
[hans@x1 linux]$ ack -l IRQF_EARLY_RESUME driversdrivers/xen/events/events_base.cdrivers/rtc/rtc-sc27xx.cdrivers/mfd/twl4030-irq.c
Which is probably why we have not yet hit the s2idle issue with it.
So this all in all seems like a bad idea, and we need to better root cause the issue with these Elan touchpads and fix the actual issue.
Regards,
Hans
Quoted 53 lines
So, just heads up.
Quoted 48 lines · On Fri, Apr 24, 2020 at 05:12:01PM +0800, [email protected] wrote:
From: Caicai <[email protected]>
I had tested two Notebook machines, the Intel i5(or amd ryzen) with elan touchpad, and there's a probability that the touchpad won't work after going to the S3/S4 to wake up, that it would appear no more than 15 times. I found that there's no interrupt to check for
/proc/interrupt. It was found that the gpio interrupt of i2c was also not on top. By adding the gpio interrupt flags withIRQF_EARLY_RESUME, now the touchpad tested 200 + times works well.Signed-off-by Caicai <[email protected]>--- drivers/pinctrl/intel/pinctrl-intel.c | 2 +- drivers/pinctrl/pinctrl-amd.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)drivers/pinctrl/intel/pinctrl-intel.c +1
-1@@ -1189,7 +1189,7 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)* controllers share the same interrupt line. */ ret =devm_request_irq (pctrl->dev, irq, intel_gpio_irq, -IRQF_SHARED |IRQF_NO_THREAD , +IRQF_SHARED |IRQF_NO_THREAD |IRQF_EARLY_RESUME ,dev_name (pctrl->dev), pctrl);if (ret) {dev_err (pctrl->dev,"failed to request interrupt\n" );drivers/pinctrl/pinctrl-amd.c +1
-1@@ -937,7 +937,7 @@ static int amd_gpio_probe(struct platform_device *pdev) } ret =devm_request_irq (&pdev->dev, irq_base, amd_gpio_irq_handler, -IRQF_SHARED ,KBUILD_MODNAME , gpio_dev); +IRQF_SHARED |IRQF_EARLY_RESUME |IRQF_NO_THREAD ,KBUILD_MODNAME , gpio_dev);if (ret) goto out2;Signature · 1 line
2.20.1
Quoted 19 lines · Hans de Goede
Quoted 6 lines · Andy Shevchenko
+Cc: Hans.
I can't speak for AMD, but I think it may be useful for Intel pin control. However, I didn't check what may be the side effects of this change and neither contributor answered to my comments...
AFAICT setting
IRQF_EARLY_RESUMEcauses it to not be resumed during normal IRQ resuming, but earlier during thesyscore_resume()call.There are 2 problems with this:
1. When using S3 suspend
syscore_resume()runs before any device suspend handlers, so any state restoring which the GPIO controller's resume handler is doing has not been done yet. While esp. after S3 suspend the restoring may be important (s2idle suspend should not touch the GPIO registers contents).
Quoted 5 lines
2. When using S2idle suspend
syscore_resume()*never* runs, and IRQs marked withIRQF_EARLY_RESUMEare skipped during the normal IRQ resume phase, so these IRQs will never be resumed.Also
IRQF_EARLY_RESUMEis used almost no where:
Yes, I also noticed this.
Quoted 9 lines
[hans@x1 linux]$ ack -l
IRQF_EARLY_RESUMEdriversdrivers/xen/events/events_base.cdrivers/rtc/rtc-sc27xx.cdrivers/mfd/twl4030-irq.cWhich is probably why we have not yet hit the s2idle issue with it.
So this all in all seems like a bad idea, and we need to better root cause the issue with these Elan touchpads and fix the actual issue.
Thanks! That is useful to know.
Quoted 13 lines
Quoted 13 lines
So, just heads up.
Quoted 10 lines · On Fri, Apr 24, 2020 at 05:12:01PM +0800, [email protected] wrote:
From: Caicai <[email protected]>
I had tested two Notebook machines, the Intel i5(or amd ryzen) with elan touchpad, and there's a probability that the touchpad won't work after going to the S3/S4 to wake up, that it would appear no more than 15 times. I found that there's no interrupt to check for
/proc/interrupt. It was found that the gpio interrupt of i2c was also not on top. By adding the gpio interrupt flags withIRQF_EARLY_RESUME, now the touchpad tested 200 + times works well.
Signature · 2 lines
With Best Regards, Andy Shevchenko