Bug 203995 - Touchpad generates IRQ storm after S3, revert commit c538b9436751 "pinctrl: intel: Only restore pins that are used by the driver" can solve the issue
Summary: Touchpad generates IRQ storm after S3, revert commit c538b9436751 "pinctrl: i...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_other
URL:
Keywords:
: 198473 201175 204367 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-06-26 10:46 UTC by Kai-Heng Feng
Modified: 2020-10-23 22:57 UTC (History)
8 users (show)

See Also:
Kernel Version: 5.2-rc6
Tree: Mainline
Regression: No


Attachments
dmesg (83.31 KB, text/plain)
2019-06-26 10:48 UTC, Kai-Heng Feng
Details
dmesg, c538b9436751 reverted (82.71 KB, text/plain)
2019-06-26 10:50 UTC, Kai-Heng Feng
Details
/proc/interrupts (12.52 KB, text/plain)
2019-06-26 10:50 UTC, Kai-Heng Feng
Details
pinctrl debugfs contents (58.78 KB, text/plain)
2019-06-26 10:56 UTC, Kai-Heng Feng
Details
Save and restore all pins that are in GPIO mode (1.36 KB, patch)
2019-07-02 09:24 UTC, Mika Westerberg
Details | Diff
tables.dat (1.68 MB, text/plain)
2019-07-02 10:21 UTC, Kai-Heng Feng
Details
dmidecode (13.06 KB, text/plain)
2019-07-03 07:27 UTC, Kai-Heng Feng
Details
Prints more debug info, Mika's patch applied (125.53 KB, text/plain)
2019-07-10 06:14 UTC, Kai-Heng Feng
Details
diff to print all pin values (1.79 KB, patch)
2019-07-16 05:01 UTC, Kai-Heng Feng
Details | Diff
dmesg which prints all pin values (153.23 KB, text/plain)
2019-07-16 05:02 UTC, Kai-Heng Feng
Details

Description Kai-Heng Feng 2019-06-26 10:46:31 UTC
The device is "intel-gpio  258  ALP001E:00" in /proc/interrupts.

Resume message on mainline kernel:
[  189.134507] cannonlake-pinctrl INT3450:00: restored mask 0/0 0x000000
[  189.134520] cannonlake-pinctrl INT3450:00: restored mask 0/1 0x000000
[  189.134563] cannonlake-pinctrl INT3450:00: restored mask 1/0 0x000000
[  189.134578] cannonlake-pinctrl INT3450:00: restored mask 1/1 0x000000
[  189.134592] cannonlake-pinctrl INT3450:00: restored mask 1/2 0x000000
[  189.134606] cannonlake-pinctrl INT3450:00: restored mask 1/3 0x000000
[  189.134620] cannonlake-pinctrl INT3450:00: restored mask 1/4 0x000000
[  189.134635] cannonlake-pinctrl INT3450:00: restored mask 1/5 0x000000
[  189.134707] cannonlake-pinctrl INT3450:00: restored mask 2/0 0x000000
[  189.134722] cannonlake-pinctrl INT3450:00: restored mask 2/1 0x000000
[  189.134737] cannonlake-pinctrl INT3450:00: restored mask 2/2 0x000000
[  189.134752] cannonlake-pinctrl INT3450:00: restored mask 2/3 0x000000
[  189.134767] cannonlake-pinctrl INT3450:00: restored mask 2/4 0x000000
[  189.134842] cannonlake-pinctrl INT3450:00: restored mask 3/0 0x000000
[  189.134857] cannonlake-pinctrl INT3450:00: restored mask 3/1 0x000000
[  189.134871] cannonlake-pinctrl INT3450:00: restored mask 3/2 0x000000
[  189.134886] cannonlake-pinctrl INT3450:00: restored mask 3/3 0x000000

Resume message when commit c538b9436751 reverted:
[   31.646422] cannonlake-pinctrl INT3450:00: restored pin 205 padcfg0 0x80800102
[   31.647818] cannonlake-pinctrl INT3450:00: restored pin 220 padcfg0 0x84000201
[   31.649457] cannonlake-pinctrl INT3450:00: restored pin 238 padcfg0 0x84000200
[   31.654689] cannonlake-pinctrl INT3450:00: restored mask 0/0 0x000000
[   31.654703] cannonlake-pinctrl INT3450:00: restored mask 0/1 0x000000
[   31.654746] cannonlake-pinctrl INT3450:00: restored mask 1/0 0x000000
[   31.654760] cannonlake-pinctrl INT3450:00: restored mask 1/1 0x000000
[   31.654774] cannonlake-pinctrl INT3450:00: restored mask 1/2 0x000000
[   31.654789] cannonlake-pinctrl INT3450:00: restored mask 1/3 0x000000
[   31.654803] cannonlake-pinctrl INT3450:00: restored mask 1/4 0x000000
[   31.654817] cannonlake-pinctrl INT3450:00: restored mask 1/5 0x000000
[   31.654882] cannonlake-pinctrl INT3450:00: restored mask 2/0 0x000000
[   31.654899] cannonlake-pinctrl INT3450:00: restored mask 2/1 0x000000
[   31.654912] cannonlake-pinctrl INT3450:00: restored mask 2/2 0x000000
[   31.654927] cannonlake-pinctrl INT3450:00: restored mask 2/3 0x000000
[   31.654942] cannonlake-pinctrl INT3450:00: restored mask 2/4 0x000000
[   31.655017] cannonlake-pinctrl INT3450:00: restored mask 3/0 0x000000
[   31.655032] cannonlake-pinctrl INT3450:00: restored mask 3/1 0x000000
[   31.655046] cannonlake-pinctrl INT3450:00: restored mask 3/2 0x000000
[   31.655061] cannonlake-pinctrl INT3450:00: restored mask 3/3 0x000000
Comment 1 Kai-Heng Feng 2019-06-26 10:48:55 UTC
Created attachment 283441 [details]
dmesg
Comment 2 Kai-Heng Feng 2019-06-26 10:50:08 UTC
Created attachment 283443 [details]
dmesg, c538b9436751 reverted
Comment 3 Kai-Heng Feng 2019-06-26 10:50:32 UTC
Created attachment 283445 [details]
/proc/interrupts
Comment 4 Kai-Heng Feng 2019-06-26 10:56:11 UTC
Created attachment 283447 [details]
pinctrl debugfs contents
Comment 5 Mika Westerberg 2019-06-28 07:12:28 UTC
Does it happen with s2idle? I'm thinking that could this be a BIOS issue because it leaves some pins in wrong state upon S3 exit. 

Is this a production system or some prototype?
Comment 6 Kai-Heng Feng 2019-07-02 07:18:35 UTC
(In reply to Mika Westerberg from comment #5)
> Does it happen with s2idle? I'm thinking that could this be a BIOS issue
> because it leaves some pins in wrong state upon S3 exit. 
It doesn't happen with s2idle. The system is default to use S3 though.

> 
> Is this a production system or some prototype?
It's a production system.

Do you understand why it's 258 in /proc/interrupts:
intel-gpio  258  ALP001E:00
But when c538b9436751 is reverted, the pins saved are 205, 220 and 235, not 258, the issue is gone?
Comment 7 Mika Westerberg 2019-07-02 09:24:18 UTC
Actually it used to save all pins but only restore those that are changed from the saved value. In this case they are 205, 220 and 235. I suspect some of those may be used to control the touchpad interrupt/power somehow which could explain why it fails. I'm also suspecting this is more like BIOS issue but one thing to try is to restore all pins that were in GPIO mode and see if that helps. I'll attach a patch that tries to do it so you can try it out.
Comment 8 Mika Westerberg 2019-07-02 09:24:57 UTC
Created attachment 283515 [details]
Save and restore all pins that are in GPIO mode
Comment 9 Kai-Heng Feng 2019-07-02 09:55:56 UTC
Your patch works for the system.

Thanks!
Comment 10 Andy Shevchenko 2019-07-02 10:16:08 UTC
By the way, can you attach output of `acpidump -o tables.dat` on the affected system?
Comment 11 Kai-Heng Feng 2019-07-02 10:21:32 UTC
Created attachment 283517 [details]
tables.dat
Comment 12 Andy Shevchenko 2019-07-02 19:14:15 UTC
Thanks, can you also attach an output of `dmidecode`?

I guess we might need a DMI based quirk to narrow the affected platforms, while, it it's possible, to submit a bug against BIOS thru vendor channels.
Comment 13 Kai-Heng Feng 2019-07-03 07:27:53 UTC
Created attachment 283525 [details]
dmidecode
Comment 14 Kai-Heng Feng 2019-07-03 07:28:49 UTC
This is the system in question:
https://www8.hp.com/us/en/laptops/product-details/18865648
Comment 15 Kai-Heng Feng 2019-07-03 07:29:24 UTC
So will patch in comment #8 be upstreamed?
Comment 16 Andy Shevchenko 2019-07-09 17:50:54 UTC
We are looking for the best approach how to do this.

By the way, can you dump /sys/kernel/debug/gpio and also extend the existing prints in pinctrl-intel.c to see _previous_ values of the registers in working case?
Comment 17 Kai-Heng Feng 2019-07-10 05:37:04 UTC
# cat /sys/kernel/debug/gpio
gpiochip0: GPIOs 148-511, parent: platform/INT3450:00, INT3450:00:
Comment 18 Kai-Heng Feng 2019-07-10 06:14:34 UTC
Created attachment 283599 [details]
Prints more debug info, Mika's patch applied

$ git diff
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 0c25103f9afe..ea42e7aed767 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1480,12 +1480,16 @@ int intel_pinctrl_suspend_noirq(struct device *dev)
 
                val = readl(intel_get_padcfg(pctrl, desc->number, PADCFG0));
                pads[i].padcfg0 = val & ~PADCFG0_GPIORXSTATE;
+               dev_dbg(dev, "saved pin %u padcfg0 %#08x\n", desc->number, pads[i].padcfg0);
                val = readl(intel_get_padcfg(pctrl, desc->number, PADCFG1));
                pads[i].padcfg1 = val;
+               dev_dbg(dev, "saved pin %u padcfg1 %#08x\n", desc->number, pads[i].padcfg1);
 
                padcfg = intel_get_padcfg(pctrl, desc->number, PADCFG2);
-               if (padcfg)
+               if (padcfg) {
                        pads[i].padcfg2 = readl(padcfg);
+                       dev_dbg(dev, "saved pin %u padcfg2 %#08x\n", desc->number, pads[i].padcfg2);
+               }
        }
 
        communities = pctrl->context.communities;
Comment 19 Andy Shevchenko 2019-07-15 09:22:36 UTC
Kai, what I meant is to see the value in the register immediately before restoring. I would like to get the pin(s) which became configured differently.

So, I really would like to understand a bit more about that pin(s). If you know any contact in BIOS to shed a light who and why is using that pin(s).
Comment 20 Andy Shevchenko 2019-07-15 09:23:40 UTC
For time being I'm considering to put your patch with narrowed down the platforms based on DMI table.
Comment 21 Kai-Heng Feng 2019-07-16 05:01:55 UTC
Created attachment 283727 [details]
diff to print all pin values
Comment 22 Kai-Heng Feng 2019-07-16 05:02:48 UTC
Created attachment 283729 [details]
dmesg which prints all pin values
Comment 23 Will Roberts 2019-07-26 08:53:06 UTC
Hi Andy, I just wanted to write to say that Mika's patch fixes a problem with a touchpad restoring from S3 described here:

https://bugzilla.redhat.com/show_bug.cgi?id=1543769#c185

My hardware (acpidump, dmidecode, etc.) is described in this bug:

https://bugzilla.kernel.org/show_bug.cgi?id=198473
Comment 24 Andy Shevchenko 2019-07-26 10:10:20 UTC
*** Bug 198473 has been marked as a duplicate of this bug. ***
Comment 25 Hans de Goede 2019-07-27 19:18:33 UTC
I must say I'm not a fan of the idea to fix this with a DMI quirk, that is very likely to lead to a whack-a-mole game.

What is the problem with Mika's patch? That seems like a good solution to me. If the pin is marked as gpio and is not marked as reserved by the firmware restoring the setting seems like the right thing to do to me.
Comment 26 Kai-Heng Feng 2019-08-17 07:24:38 UTC
Seems like there's different approach to the same problem:
https://github.com/endlessm/linux/pull/494
Comment 27 Andy Shevchenko 2019-08-17 08:45:18 UTC
(In reply to Hans de Goede from comment #25)
> What is the problem with Mika's patch? That seems like a good solution to
> me. If the pin is marked as gpio and is not marked as reserved by the
> firmware restoring the setting seems like the right thing to do to me.
The problem of not understanding the root cause

(In reply to Kai-Heng Feng from comment #26)
> Seems like there's different approach to the same problem:
> https://github.com/endlessm/linux/pull/494

Yeah, can you confirm it fixes the issue you have?
I'm in favour of that one because it explains the root cause.
Comment 28 Will Roberts 2019-08-20 22:10:01 UTC
Hi Andy, I can confirm that that patch that Kai-Heng found fixes my touchpad issues with restoring from suspend.
Comment 29 Andy Shevchenko 2019-08-21 12:52:39 UTC
(In reply to Will Roberts from comment #28)
> Hi Andy, I can confirm that that patch that Kai-Heng found fixes my touchpad
> issues with restoring from suspend.

Thank you!
It it in the queue for v5.4 and then can be back ported for stable kernels.
Comment 30 Hans de Goede 2019-08-21 13:37:39 UTC
(In reply to Andy Shevchenko from comment #29)
> (In reply to Will Roberts from comment #28)
> > Hi Andy, I can confirm that that patch that Kai-Heng found fixes my
> touchpad
> > issues with restoring from suspend.
> 
> Thank you!
> It it in the queue for v5.4 and then can be back ported for stable kernels.

IMHO it would be better to get this into 5.3 as a fix, given that this has been a real problem for multiple users for quite a while now.
Comment 31 Andy Shevchenko 2019-09-10 10:40:49 UTC
*** Bug 201175 has been marked as a duplicate of this bug. ***
Comment 32 Chen Yu 2019-09-30 01:44:59 UTC
*** Bug 204367 has been marked as a duplicate of this bug. ***
Comment 33 fbeachler 2020-10-23 22:57:01 UTC
https://bugzilla.kernel.org/buglist.cgi?bug_status=__open__&content=elantech&list_id=1074017&order=Importance&query_format=specific shows a list of apparently repeated problems with ELAN mouse/touchpads which may indicate a class of problems with this driver that have never been properly resolved.

Note You need to log in before you can comment on or make changes to this bug.