Created attachment 283441 [details]
dmesg

Created attachment 283443 [details]
dmesg, c538b9436751 reverted

Created attachment 283445 [details]
/proc/interrupts

Created attachment 283447 [details]
pinctrl debugfs contents

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?

(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?

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.

Created attachment 283515 [details]
Save and restore all pins that are in GPIO mode

Your patch works for the system.

Thanks!

By the way, can you attach output of `acpidump -o tables.dat` on the affected system?

Created attachment 283517 [details]
tables.dat

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.

Created attachment 283525 [details]
dmidecode

This is the system in question:
https://www8.hp.com/us/en/laptops/product-details/18865648

So will patch in comment #8 be upstreamed?

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?

# cat /sys/kernel/debug/gpio
gpiochip0: GPIOs 148-511, parent: platform/INT3450:00, INT3450:00:

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;

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).

For time being I'm considering to put your patch with narrowed down the platforms based on DMI table.

Created attachment 283727 [details]
diff to print all pin values

Created attachment 283729 [details]
dmesg which prints all pin values

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

*** Bug 198473 has been marked as a duplicate of this bug. ***

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.

Seems like there's different approach to the same problem:
https://github.com/endlessm/linux/pull/494

(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.

Hi Andy, I can confirm that that patch that Kai-Heng found fixes my touchpad issues with restoring from suspend.

(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.

(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.

*** Bug 201175 has been marked as a duplicate of this bug. ***

*** Bug 204367 has been marked as a duplicate of this bug. ***

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.