pinctrl: intel: save HOSTSW_OWN register over suspend/resume

Message ID 20171114104136.25595-1-chiu@endlessm.com
State New
Headers show
Series
  • pinctrl: intel: save HOSTSW_OWN register over suspend/resume
Related show

Commit Message

Chris Chiu Nov. 14, 2017, 10:41 a.m.
The touchpad in the Asus laptop model X540NA is unresponsive
after suspend/resume. The following error appears during resume:

i2c_hid i2c-ELAN1200:00: failed to reset device.

The problem here is that i2c_hid does not notice the interrupt
being generated at this point. Because the GPIO which connects
to touchpad interrupt is in ACPI mode after resume and no longer
work as IRQ.

Fix this by saving HOSTSW_OWN register during suspend and restore
them at resume.

Signed-off-by: Chris Chiu <chiu@endlessm.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Chris Chiu Nov. 15, 2017, 8:08 a.m. | #1
Hi Mika,

    Yes, that’s the most weird part.

Chris

從我的 iPhone 傳送

> Mika Westerberg <mika.westerberg@linux.intel.com> 於 2017年11月15日 下午4:04 寫道:
> 
>> On Tue, Nov 14, 2017 at 06:41:36PM +0800, Chris Chiu wrote:
>> The touchpad in the Asus laptop model X540NA is unresponsive
>> after suspend/resume. The following error appears during resume:
>> 
>> i2c_hid i2c-ELAN1200:00: failed to reset device.
>> 
>> The problem here is that i2c_hid does not notice the interrupt
>> being generated at this point. Because the GPIO which connects
>> to touchpad interrupt is in ACPI mode after resume and no longer
>> work as IRQ.
> 
> Hmm, are you saying it is initially not in ACPI mode but after resume it
> magically is?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Nov. 15, 2017, 10:13 a.m. | #2
On Wed, Nov 15, 2017 at 04:08:32PM +0800, Chris Chiu wrote:
>     Yes, that’s the most weird part.

Sounds pretty much like a BIOS issue. Do you know if there is a newer
BIOS and have you tried that already?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Chiu Nov. 15, 2017, 10:19 a.m. | #3
Hi Mika,
    I've confirmed with Asus and they said it's the latest BIOS for
shipment and verified OK on Windows. So their BIOS team will not do
anything for this.

Chris

On Wed, Nov 15, 2017 at 6:13 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, Nov 15, 2017 at 04:08:32PM +0800, Chris Chiu wrote:
>>     Yes, that’s the most weird part.
>
> Sounds pretty much like a BIOS issue. Do you know if there is a newer
> BIOS and have you tried that already?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Nov. 16, 2017, 12:44 p.m. | #4
On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
> Hi Mika,
>     I've confirmed with Asus and they said it's the latest BIOS for
> shipment and verified OK on Windows. So their BIOS team will not do
> anything for this.

I'll ask around if our Windows people know anything about this. My gut
feeling is that the Windows driver does not touch HOSTSW_OWN either.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Chiu Nov. 16, 2017, 1:27 p.m. | #5
On Thu, Nov 16, 2017 at 8:44 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
>> Hi Mika,
>>     I've confirmed with Asus and they said it's the latest BIOS for
>> shipment and verified OK on Windows. So their BIOS team will not do
>> anything for this.
>
> I'll ask around if our Windows people know anything about this. My gut
> feeling is that the Windows driver does not touch HOSTSW_OWN either.

Thanks. Please let me know if you need any information. I still keep
the machine.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Nov. 17, 2017, 6:49 a.m. | #6
On Thu, Nov 16, 2017 at 09:27:51PM +0800, Chris Chiu wrote:
> On Thu, Nov 16, 2017 at 8:44 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
> >> Hi Mika,
> >>     I've confirmed with Asus and they said it's the latest BIOS for
> >> shipment and verified OK on Windows. So their BIOS team will not do
> >> anything for this.
> >
> > I'll ask around if our Windows people know anything about this. My gut
> > feeling is that the Windows driver does not touch HOSTSW_OWN either.
> 
> Thanks. Please let me know if you need any information. I still keep
> the machine.

Got confirmation from Windows people. So Windows pretty much saves and
restores the same registers than we do (padcfg + ie).

Have you tried whether s2idle works instead of S3 suspend? You can try
it like

  # echo freeze > /sys/power/state

If that works, I'm guessing that this system uses s2idle and that's also
what Windows uses and could explain why it works in Windows.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Chiu Nov. 17, 2017, 8:11 a.m. | #7
On Fri, Nov 17, 2017 at 2:49 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Thu, Nov 16, 2017 at 09:27:51PM +0800, Chris Chiu wrote:
>> On Thu, Nov 16, 2017 at 8:44 PM, Mika Westerberg
>> <mika.westerberg@linux.intel.com> wrote:
>> > On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
>> >> Hi Mika,
>> >>     I've confirmed with Asus and they said it's the latest BIOS for
>> >> shipment and verified OK on Windows. So their BIOS team will not do
>> >> anything for this.
>> >
>> > I'll ask around if our Windows people know anything about this. My gut
>> > feeling is that the Windows driver does not touch HOSTSW_OWN either.
>>
>> Thanks. Please let me know if you need any information. I still keep
>> the machine.
>
> Got confirmation from Windows people. So Windows pretty much saves and
> restores the same registers than we do (padcfg + ie).
>
> Have you tried whether s2idle works instead of S3 suspend? You can try
> it like
>
>   # echo freeze > /sys/power/state
>
> If that works, I'm guessing that this system uses s2idle and that's also
> what Windows uses and could explain why it works in Windows.

Unfortunately, I cant wake it up from neither power button nor any key
event after  "# echo freeze > /sys/power/state"....
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Nov. 21, 2017, 10:52 a.m. | #8
On Fri, Nov 17, 2017 at 04:11:31PM +0800, Chris Chiu wrote:
> On Fri, Nov 17, 2017 at 2:49 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Thu, Nov 16, 2017 at 09:27:51PM +0800, Chris Chiu wrote:
> >> On Thu, Nov 16, 2017 at 8:44 PM, Mika Westerberg
> >> <mika.westerberg@linux.intel.com> wrote:
> >> > On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
> >> >> Hi Mika,
> >> >>     I've confirmed with Asus and they said it's the latest BIOS for
> >> >> shipment and verified OK on Windows. So their BIOS team will not do
> >> >> anything for this.
> >> >
> >> > I'll ask around if our Windows people know anything about this. My gut
> >> > feeling is that the Windows driver does not touch HOSTSW_OWN either.
> >>
> >> Thanks. Please let me know if you need any information. I still keep
> >> the machine.
> >
> > Got confirmation from Windows people. So Windows pretty much saves and
> > restores the same registers than we do (padcfg + ie).
> >
> > Have you tried whether s2idle works instead of S3 suspend? You can try
> > it like
> >
> >   # echo freeze > /sys/power/state
> >
> > If that works, I'm guessing that this system uses s2idle and that's also
> > what Windows uses and could explain why it works in Windows.
> 
> Unfortunately, I cant wake it up from neither power button nor any key
> event after  "# echo freeze > /sys/power/state"....

Yeah, it is S3 platform based on the acpidump you shared. Although
freeze should still work.

Have you dumped the pin config registers through debugfs before and
after suspend? Are there any other differences except the HOSTSW_OWN
thing?
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chris Chiu Nov. 21, 2017, 11:54 a.m. | #9
On Tue, Nov 21, 2017 at 6:52 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Fri, Nov 17, 2017 at 04:11:31PM +0800, Chris Chiu wrote:
>> On Fri, Nov 17, 2017 at 2:49 PM, Mika Westerberg
>> <mika.westerberg@linux.intel.com> wrote:
>> > On Thu, Nov 16, 2017 at 09:27:51PM +0800, Chris Chiu wrote:
>> >> On Thu, Nov 16, 2017 at 8:44 PM, Mika Westerberg
>> >> <mika.westerberg@linux.intel.com> wrote:
>> >> > On Wed, Nov 15, 2017 at 06:19:56PM +0800, Chris Chiu wrote:
>> >> >> Hi Mika,
>> >> >>     I've confirmed with Asus and they said it's the latest BIOS for
>> >> >> shipment and verified OK on Windows. So their BIOS team will not do
>> >> >> anything for this.
>> >> >
>> >> > I'll ask around if our Windows people know anything about this. My gut
>> >> > feeling is that the Windows driver does not touch HOSTSW_OWN either.
>> >>
>> >> Thanks. Please let me know if you need any information. I still keep
>> >> the machine.
>> >
>> > Got confirmation from Windows people. So Windows pretty much saves and
>> > restores the same registers than we do (padcfg + ie).
>> >
>> > Have you tried whether s2idle works instead of S3 suspend? You can try
>> > it like
>> >
>> >   # echo freeze > /sys/power/state
>> >
>> > If that works, I'm guessing that this system uses s2idle and that's also
>> > what Windows uses and could explain why it works in Windows.
>>
>> Unfortunately, I cant wake it up from neither power button nor any key
>> event after  "# echo freeze > /sys/power/state"....
>
> Yeah, it is S3 platform based on the acpidump you shared. Although
> freeze should still work.
>
> Have you dumped the pin config registers through debugfs before and
> after suspend? Are there any other differences except the HOSTSW_OWN
> thing?

Yup, I checked the value of the corresponded pin. It shows following before
suspend
pin 18 (GPIO_18) GPIO 0x40800102 0x00024075

Then after resume
pin 18 (GPIO_18) GPIO 0x40800102 0x00024075 [ACPI]

What else register do you suggest me to compare? The PADCFG2 is invalid
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Nov. 21, 2017, 12:04 p.m. | #10
On Tue, Nov 21, 2017 at 07:54:26PM +0800, Chris Chiu wrote:
> Yup, I checked the value of the corresponded pin. It shows following before
> suspend
> pin 18 (GPIO_18) GPIO 0x40800102 0x00024075
> 
> Then after resume
> pin 18 (GPIO_18) GPIO 0x40800102 0x00024075 [ACPI]

OK, so ownership is changed to ACPI.

> What else register do you suggest me to compare? The PADCFG2 is invalid

It's fine APL does not have PADCFG2.

Hmm, I don't understand how this can work in Windows either. The Windows
people told me that they don't save and restore anything else than
padcfg registers + ie. If the ownership is changed to ACPI it means you
don't get interrupts anymore (only GPEs) and that applies to Windows as
well.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 6dc1096d3d34..eaafa0e534f3 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -83,6 +83,7 @@  struct intel_pad_context {
 
 struct intel_community_context {
 	u32 *intmask;
+	u32 *modemask;
 };
 
 struct intel_pinctrl_context {
@@ -1158,6 +1159,7 @@  static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
 	for (i = 0; i < pctrl->ncommunities; i++) {
 		struct intel_community *community = &pctrl->communities[i];
 		u32 *intmask;
+		u32 *modemask;
 
 		intmask = devm_kcalloc(pctrl->dev, community->ngpps,
 				       sizeof(*intmask), GFP_KERNEL);
@@ -1165,6 +1167,13 @@  static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
 			return -ENOMEM;
 
 		communities[i].intmask = intmask;
+
+		modemask = devm_kcalloc(pctrl->dev, community->ngpps,
+				       sizeof(*modemask), GFP_KERNEL);
+		if (!modemask)
+			return -ENOMEM;
+
+		communities[i].modemask = modemask;
 	}
 
 	pctrl->context.pads = pads;
@@ -1329,6 +1338,10 @@  int intel_pinctrl_suspend(struct device *dev)
 		base = community->regs + community->ie_offset;
 		for (gpp = 0; gpp < community->ngpps; gpp++)
 			communities[i].intmask[gpp] = readl(base + gpp * 4);
+
+		base = community->regs + community->hostown_offset;
+		for (gpp = 0; gpp < community->ngpps; gpp++)
+			communities[i].modemask[gpp] = readl(base + gpp * 4);
 	}
 
 	return 0;
@@ -1411,7 +1424,14 @@  int intel_pinctrl_resume(struct device *dev)
 		base = community->regs + community->ie_offset;
 		for (gpp = 0; gpp < community->ngpps; gpp++) {
 			writel(communities[i].intmask[gpp], base + gpp * 4);
-			dev_dbg(dev, "restored mask %d/%u %#08x\n", i, gpp,
+			dev_dbg(dev, "restored intmask %d/%u %#08x\n", i, gpp,
+				readl(base + gpp * 4));
+		}
+
+		base = community->regs + community->hostown_offset;
+		for (gpp = 0; gpp < community->ngpps; gpp++) {
+			writel(communities[i].modemask[gpp], base + gpp * 4);
+			dev_dbg(dev, "restored modemask %d/%u %#08x\n", i, gpp,
 				readl(base + gpp * 4));
 		}
 	}