Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86
@ 2020-01-23 21:02 Hans de Goede
  2020-01-24 15:19 ` Thomas Gleixner
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2020-01-23 21:02 UTC (permalink / raw)
  To: Mika Westerberg, Andy Shevchenko, Linus Walleij, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin
  Cc: Hans de Goede, linux-gpio, linux-acpi, x86, linux-kernel

Modern x86 laptops are starting to use GPIO pins as interrupts more
and more, e.g. touchpads and touchscreens have almost all moved away
from PS/2 and USB to using I2C with a GPIO pin as interrupt.
Modern x86 laptops also have almost all moved to using s2idle instead
of using the system S3 ACPI power state to suspend.

The Intel and AMD pinctrl drivers do not define irq_retrigger handlers
for the irqchips they register, this is causing edge triggered interrupts
which happen while suspended using s2idle to get lost.

One specific example of this is the lid switch on some devices, lid
switches used to be handled by the embedded-controller, but now the
lid open/closed sensor is sometimes directly connected to a GPIO pin.
On most devices the ACPI code for this looks like this:

Method (_E00, ...) {
	Notify (LID0, 0x80) // Status Change
}

Where _E00 is an ACPI event handler for changes on both edges of the GPIO
connected to the lid sensor, this event handler is then combined with an
_LID method which directly reads the pin. When the device is resumed by
opening the lid, the GPIO interrupt will wake the system, but because the
pinctrl irqchip doesn't have an irq_retrigger handler, the Notify will not
happen. This is not a problem in the case the _LID method directly reads
the GPIO, because the drivers/acpi/button.c code will call _LID on resume
anyways.

But some devices have an event handler for the GPIO connected to the
lid sensor which looks like this:

Method (_E00, ...) {
	if (LID_GPIO == One)
		LIDS = One
	else
		LIDS = Zero
	Notify (LID0, 0x80) // Status Change
}

And the _LID method returns the cached LIDS value, since on open we
do not re-run the edge-interrupt handler when we re-enable IRQS on resume
(because of the missing irq_retrigger handler), _LID now will keep
reporting closed, as LIDS was never changed to reflect the open status,
this causes userspace to re-resume the laptop again shortly after opening
the lid.

The Intel GPIO controllers do not allow implementing irq_retrigger without
emulating it in software, at which point we are better of just using the
generic HARDIRQS_SW_RESEND mechanism rather then re-implementing software
emulation for this separately in aprox. 14 different pinctrl drivers.

This commit selects HARDIRQS_SW_RESEND solving the problem of
edge-triggered GPIO interrupts not being re-triggered on resume when they
were triggered during suspend (s2idle) and/or when they were the cause of
the wakeup.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
I'm sending this out as a RFC since I'm not %100 sure this is the best
solution and it seems like a somewhat big change to make.

Also maybe we should add a Cc: stable@vger.kernel.org ??? This seems like
somewhat a big change for that but it does solve some real issues...
---
Changes in v2:
-v2 is really a resend because I forgot to add the pinctrl people to the Cc
-While at it also fix some typos in the commit message
---
 arch/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c1cbfc7b3ae8..8f8128047b49 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -118,6 +118,7 @@ config X86
 	select GENERIC_IRQ_PROBE
 	select GENERIC_IRQ_RESERVATION_MODE
 	select GENERIC_IRQ_SHOW
+	select HARDIRQS_SW_RESEND
 	select GENERIC_PENDING_IRQ		if SMP
 	select GENERIC_SMP_IDLE_THREAD
 	select GENERIC_STRNCPY_FROM_USER
-- 
2.24.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86
  2020-01-23 21:02 [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86 Hans de Goede
@ 2020-01-24 15:19 ` Thomas Gleixner
  2020-03-11 18:24   ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2020-01-24 15:19 UTC (permalink / raw)
  To: Hans de Goede, Mika Westerberg, Andy Shevchenko, Linus Walleij,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin
  Cc: Hans de Goede, linux-gpio, linux-acpi, x86, linux-kernel

Hans,

Hans de Goede <hdegoede@redhat.com> writes:
>
> The Intel GPIO controllers do not allow implementing irq_retrigger without
> emulating it in software, at which point we are better of just using the
> generic HARDIRQS_SW_RESEND mechanism rather then re-implementing software
> emulation for this separately in aprox. 14 different pinctrl drivers.

Indeed.

> I'm sending this out as a RFC since I'm not %100 sure this is the best
> solution and it seems like a somewhat big change to make.

It's not that bad. The only affected interrupt chips on x86 should be
secondary interrupt chips like the GPIO controller.

ioapic/msi/... have irq_retrigger() functionality, so it won't do the
software resend.

I just need to stare at the legacy PIC and the virt stuff.

> Also maybe we should add a Cc: stable@vger.kernel.org ??? This seems like
> somewhat a big change for that but it does solve some real issues...

Yes. Let me stare at the couple of weird irqchips which might get
surprised. I'll teach them not to do that :)

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86
  2020-01-24 15:19 ` Thomas Gleixner
@ 2020-03-11 18:24   ` Hans de Goede
  2020-03-11 21:31     ` Thomas Gleixner
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2020-03-11 18:24 UTC (permalink / raw)
  To: Thomas Gleixner, Mika Westerberg, Andy Shevchenko, Linus Walleij,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin
  Cc: linux-gpio, linux-acpi, x86, linux-kernel

Hi Thomas,

On 1/24/20 4:19 PM, Thomas Gleixner wrote:
> Hans,
> 
> Hans de Goede <hdegoede@redhat.com> writes:
>>
>> The Intel GPIO controllers do not allow implementing irq_retrigger without
>> emulating it in software, at which point we are better of just using the
>> generic HARDIRQS_SW_RESEND mechanism rather then re-implementing software
>> emulation for this separately in aprox. 14 different pinctrl drivers.
> 
> Indeed.
> 
>> I'm sending this out as a RFC since I'm not %100 sure this is the best
>> solution and it seems like a somewhat big change to make.
> 
> It's not that bad. The only affected interrupt chips on x86 should be
> secondary interrupt chips like the GPIO controller.
> 
> ioapic/msi/... have irq_retrigger() functionality, so it won't do the
> software resend.
> 
> I just need to stare at the legacy PIC and the virt stuff.
> 
>> Also maybe we should add a Cc: stable@vger.kernel.org ??? This seems like
>> somewhat a big change for that but it does solve some real issues...
> 
> Yes. Let me stare at the couple of weird irqchips which might get
> surprised. I'll teach them not to do that :)

I know that you are very busy, still I'm wondering is there any progress
on this ?

Regards,

Hans


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86
  2020-03-11 18:24   ` Hans de Goede
@ 2020-03-11 21:31     ` Thomas Gleixner
  2020-03-11 21:49       ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2020-03-11 21:31 UTC (permalink / raw)
  To: Hans de Goede, Mika Westerberg, Andy Shevchenko, Linus Walleij,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin
  Cc: linux-gpio, linux-acpi, x86, linux-kernel

Hans de Goede <hdegoede@redhat.com> writes:
>> I just need to stare at the legacy PIC and the virt stuff.
>> 
>>> Also maybe we should add a Cc: stable@vger.kernel.org ??? This seems like
>>> somewhat a big change for that but it does solve some real issues...
>> 
>> Yes. Let me stare at the couple of weird irqchips which might get
>> surprised. I'll teach them not to do that :)
>
> I know that you are very busy, still I'm wondering is there any progress
> on this ?

Bah. That fell through the cracks, but actually I looked at this due to
the PCI-E AER wreckage. So yes, this is fine, but we want:

 https://lkml.kernel.org/r/20200306130623.590923677@linutronix.de
 https://lkml.kernel.org/r/20200306130623.684591280@linutronix.de

if we want to backport this to stable.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86
  2020-03-11 21:31     ` Thomas Gleixner
@ 2020-03-11 21:49       ` Hans de Goede
  2020-03-11 22:09         ` Thomas Gleixner
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2020-03-11 21:49 UTC (permalink / raw)
  To: Thomas Gleixner, Mika Westerberg, Andy Shevchenko, Linus Walleij,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin
  Cc: linux-gpio, linux-acpi, x86, linux-kernel

Hi,

On 3/11/20 10:31 PM, Thomas Gleixner wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
>>> I just need to stare at the legacy PIC and the virt stuff.
>>>
>>>> Also maybe we should add a Cc: stable@vger.kernel.org ??? This seems like
>>>> somewhat a big change for that but it does solve some real issues...
>>>
>>> Yes. Let me stare at the couple of weird irqchips which might get
>>> surprised. I'll teach them not to do that :)
>>
>> I know that you are very busy, still I'm wondering is there any progress
>> on this ?
> 
> Bah. That fell through the cracks, but actually I looked at this due to
> the PCI-E AER wreckage. So yes, this is fine, but we want:
> 
>   https://lkml.kernel.org/r/20200306130623.590923677@linutronix.de
>   https://lkml.kernel.org/r/20200306130623.684591280@linutronix.de
> 
> if we want to backport this to stable.

So far I have seen a few, but not a lot of devices which need this, so
I'm not 100% sure what to do here.

Do you consider this change safe / suitable for stable if those 2 patches
are backported and applied first?

Regards,

Hans


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86
  2020-03-11 21:49       ` Hans de Goede
@ 2020-03-11 22:09         ` Thomas Gleixner
  2020-03-12 12:06           ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2020-03-11 22:09 UTC (permalink / raw)
  To: Hans de Goede, Mika Westerberg, Andy Shevchenko, Linus Walleij,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin
  Cc: linux-gpio, linux-acpi, x86, linux-kernel

Hans de Goede <hdegoede@redhat.com> writes:
> On 3/11/20 10:31 PM, Thomas Gleixner wrote:
>> Hans de Goede <hdegoede@redhat.com> writes:
>>>> I just need to stare at the legacy PIC and the virt stuff.
>>>>
>>>>> Also maybe we should add a Cc: stable@vger.kernel.org ??? This seems like
>>>>> somewhat a big change for that but it does solve some real issues...
>>>>
>>>> Yes. Let me stare at the couple of weird irqchips which might get
>>>> surprised. I'll teach them not to do that :)
>>>
>>> I know that you are very busy, still I'm wondering is there any progress
>>> on this ?
>> 
>> Bah. That fell through the cracks, but actually I looked at this due to
>> the PCI-E AER wreckage. So yes, this is fine, but we want:
>> 
>>   https://lkml.kernel.org/r/20200306130623.590923677@linutronix.de
>>   https://lkml.kernel.org/r/20200306130623.684591280@linutronix.de
>> 
>> if we want to backport this to stable.
>
> So far I have seen a few, but not a lot of devices which need this, so
> I'm not 100% sure what to do here.
>
> Do you consider this change safe / suitable for stable if those 2 patches
> are backported and applied first?

I think so. The two patches are on my list for backports anyway, but I
wanted to give them some time to simmer.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86
  2020-03-11 22:09         ` Thomas Gleixner
@ 2020-03-12 12:06           ` Hans de Goede
  0 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2020-03-12 12:06 UTC (permalink / raw)
  To: Thomas Gleixner, Mika Westerberg, Andy Shevchenko, Linus Walleij,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin
  Cc: linux-gpio, linux-acpi, x86, linux-kernel

Hi,

On 3/11/20 11:09 PM, Thomas Gleixner wrote:
> Hans de Goede <hdegoede@redhat.com> writes:
>> On 3/11/20 10:31 PM, Thomas Gleixner wrote:
>>> Hans de Goede <hdegoede@redhat.com> writes:
>>>>> I just need to stare at the legacy PIC and the virt stuff.
>>>>>
>>>>>> Also maybe we should add a Cc: stable@vger.kernel.org ??? This seems like
>>>>>> somewhat a big change for that but it does solve some real issues...
>>>>>
>>>>> Yes. Let me stare at the couple of weird irqchips which might get
>>>>> surprised. I'll teach them not to do that :)
>>>>
>>>> I know that you are very busy, still I'm wondering is there any progress
>>>> on this ?
>>>
>>> Bah. That fell through the cracks, but actually I looked at this due to
>>> the PCI-E AER wreckage. So yes, this is fine, but we want:
>>>
>>>    https://lkml.kernel.org/r/20200306130623.590923677@linutronix.de
>>>    https://lkml.kernel.org/r/20200306130623.684591280@linutronix.de
>>>
>>> if we want to backport this to stable.
>>
>> So far I have seen a few, but not a lot of devices which need this, so
>> I'm not 100% sure what to do here.
>>
>> Do you consider this change safe / suitable for stable if those 2 patches
>> are backported and applied first?
> 
> I think so. The two patches are on my list for backports anyway, but I
> wanted to give them some time to simmer.

OK, I'll submit this patch for stable then once your backports have landed.

Regards,

Hans


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-23 21:02 [RFC v2] x86: Select HARDIRQS_SW_RESEND on x86 Hans de Goede
2020-01-24 15:19 ` Thomas Gleixner
2020-03-11 18:24   ` Hans de Goede
2020-03-11 21:31     ` Thomas Gleixner
2020-03-11 21:49       ` Hans de Goede
2020-03-11 22:09         ` Thomas Gleixner
2020-03-12 12:06           ` Hans de Goede

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git