Interrupt storm from pinctrl-amd on Acer AN515-42

Message ID 4085fc648ff5086bd6e6237d74d2a11e945a617b.camel@gmail.com
State New
Headers show
Series
  • Interrupt storm from pinctrl-amd on Acer AN515-42
Related show

Commit Message

Leonard Crestez Dec. 27, 2018, 11:02 p.m.
Hello,

My Acer Nitro 5 AN515-42 laptop with a Ryzen 2700U hangs on boot with 
recent kernel on an interrupt storm from pinctrl-amd.

Older kernels work but this seems to be because this module was disabled 
by default. I tried to copy over old driver from 4.9 but it still 
experiences same issue.

Digging a little deeper it seems the touchpad interrupt is active on 
boot and since it's configured as "level" and no touchpad driver is 
available yet there does not seem to be any way to clear it.

I don't know how this should be handled, booting with an active enabled but 
unclearable interrupt seems like a platform bug to me. There is even an 
option to set touchpad to "basic" which does some sort of ps2 emulation 
but the IRQ issue still happens!

One workaround is to explicitly disable the interrupt from the handler 
if no mapping is found; this will keep it disabled until 
amd_gpio_irq_set_type is called later.


When in "i2c mode" the touchpad has an ACPI hid "ELAN0504", there are
many similar compatibe hids in elan_i2c driver and if I add this one it
probes successfully and handles irqs but fails to report input (i2c 
read data is invalid).

Same laptop experiences some severe p-state throttling issues so there
are many things wrong here. Let me know if you want more version info
or ACPI dumps.

--
Regards,
Leonard

Comments

Linus Walleij Dec. 28, 2018, 12:48 p.m. | #1
On Fri, Dec 28, 2018 at 12:02 AM Leonard Crestez <cdleonard@gmail.com> wrote:

> Digging a little deeper it seems the touchpad interrupt is active on
> boot and since it's configured as "level" and no touchpad driver is
> available yet there does not seem to be any way to clear it.

I think these are called "spurious interrupts".

> I don't know how this should be handled, booting with an active enabled but
> unclearable interrupt seems like a platform bug to me. There is even an
> option to set touchpad to "basic" which does some sort of ps2 emulation
> but the IRQ issue still happens!
>
> One workaround is to explicitly disable the interrupt from the handler
> if no mapping is found; this will keep it disabled until
> amd_gpio_irq_set_type is called later.

I don't know how x86 and ACPI systems usually deal with this stuff
so I'm kind of lost. On the embedded systems that I develop on,
I would just disable all interrupts on probe() (usually writing 0x0 in
some interrupt enable register) and then they will get enabled
once consumers need them.

But I have come to understand that maybe ACPI systems are
not so happy about drivers doing things like that?

Yours,
Linus Walleij
Thomas Gleixner Jan. 8, 2019, 3:01 p.m. | #2
On Fri, 28 Dec 2018, Linus Walleij wrote:

> On Fri, Dec 28, 2018 at 12:02 AM Leonard Crestez <cdleonard@gmail.com> wrote:
> 
> > Digging a little deeper it seems the touchpad interrupt is active on
> > boot and since it's configured as "level" and no touchpad driver is
> > available yet there does not seem to be any way to clear it.
> 
> I think these are called "spurious interrupts".
> 
> > I don't know how this should be handled, booting with an active enabled but
> > unclearable interrupt seems like a platform bug to me. There is even an
> > option to set touchpad to "basic" which does some sort of ps2 emulation
> > but the IRQ issue still happens!
> >
> > One workaround is to explicitly disable the interrupt from the handler
> > if no mapping is found; this will keep it disabled until
> > amd_gpio_irq_set_type is called later.
> 
> I don't know how x86 and ACPI systems usually deal with this stuff
> so I'm kind of lost. On the embedded systems that I develop on,
> I would just disable all interrupts on probe() (usually writing 0x0 in
> some interrupt enable register) and then they will get enabled
> once consumers need them.

That's the right thing to do.

> But I have come to understand that maybe ACPI systems are
> not so happy about drivers doing things like that?

Each driver has to invoke a request_irq() variant, which enables the
interrupt line. So there should be no problem when disabling all interrupts
on probe.

Thanks,

	tglx

Patch

--- drivers/pinctrl/pinctrl-amd.c
+++ drivers/pinctrl/pinctrl-amd.c
@@ -567,22 +567,27 @@  static irqreturn_t amd_gpio_irq_handler(int irq, void *dev_id)
                        regval = readl(regs + i);
                        if (!(regval & PIN_IRQ_PENDING) ||
                            !(regval & BIT(INTERRUPT_MASK_OFF)))
                                continue;
                        irq = irq_find_mapping(gc->irq.domain, irqnr + i);
-                       generic_handle_irq(irq);
+                       if (irq) {
+                               generic_handle_irq(irq);
+                               ret = IRQ_HANDLED;
+                       }
 
                        /* Clear interrupt.
                         * We must read the pin register again, in case the
                         * value was changed while executing
                         * generic_handle_irq() above.
                         */
                        raw_spin_lock_irqsave(&gpio_dev->lock, flags);
                        regval = readl(regs + i);
+                       /* Disable if pending but unmapped */
+                       if (!irq && (regval & PIN_IRQ_PENDING))
+                               regval &= ~BIT(INTERRUPT_ENABLE_OFF);
                        writel(regval, regs + i);
                        raw_spin_unlock_irqrestore(&gpio_dev->lock, flags);
-                       ret = IRQ_HANDLED;
                }
        }
 
        /* Signal EOI to the GPIO unit */