| Message ID | 4085fc648ff5086bd6e6237d74d2a11e945a617b.camel@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Interrupt storm from pinctrl-amd on Acer AN515-42 | expand |
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
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
--- 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 */