Messages in this thread Patch in this message | | | Date | Tue, 11 Jul 2017 08:43:55 -0700 | From | Tony Lindgren <> | Subject | Re: [GIT pull] irq updates for 4.13 |
| |
* Thomas Gleixner <tglx@linutronix.de> [170711 08:07]: > On Tue, 11 Jul 2017, Thomas Gleixner wrote: > > On Tue, 11 Jul 2017, Tony Lindgren wrote: > > > * Thomas Gleixner <tglx@linutronix.de> [170711 02:48]: > > > And "external abort on non-linefetch" means something is not clocked > > > in this case. The following alone makes things boot for me again, but I don't > > > quite follow what has now changed with the ordering.. Thomas, any ideas? > > > > Ah. Now that makes sense. > > > > Unpatched the ordering is: > > > > chip_bus_lock(desc); > > irq_request_resources(desc); > > > > Now the offending change reordered the calls. OMAP gpio has: > > > > omap_gpio_irq_bus_lock() > > pm_runtime_get_sync(bank->chip.parent); > > > > So that at least explains the error. So that omap gpio irq chip (ab)uses > > the bus_lock() callback to do runtime power management. Sigh, I did not > > expect that. Let me have a deeper look if that's OMAP only or whether this > > happens in other places as well.
OK that explains.
> So OMAP-GPIO is the only driver which abuses bus_lock/unlock() in that way > and gets surprised.
OK. Grygorii, care to take a look if there's a better way to deal with runtime PM for gpio-omap?
Meanwhile, below is my fix again with a proper description so we can have working -rc1.
Regards,
Tony
8< ------ From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony@atomide.com> Date: Tue, 11 Jul 2017 06:40:50 -0700 Subject: [PATCH] gpio: omap: Fix external abort on non-linefetch
Commit 46e48e257360 ("genirq: Move irq resource handling out of spinlocked region") caused external abort on non-linefetch for n900 and droid 4. Turns out this was caused by runtime PM use in gpio-omap in chip_bus_lock:
* Thomas Gleixner <tglx@linutronix.de> [170711 08:07]: > > Unpatched the ordering is: > > > > chip_bus_lock(desc); > > irq_request_resources(desc); > > > > Now the offending change reordered the calls. OMAP gpio has: > > > > omap_gpio_irq_bus_lock() > > pm_runtime_get_sync(bank->chip.parent); > > > > So that at least explains the error. So that omap gpio irq chip (ab)uses > > the bus_lock() callback to do runtime power management. Sigh, I did not > > expect that.
Let's fix it with pm_runtime_get_sync() for now, then further improvments can be done when we have a better way to deal with it.
Reported-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/gpio/gpio-omap.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -917,13 +917,24 @@ static int omap_gpio_get_direction(struct gpio_chip *chip, unsigned offset) struct gpio_bank *bank; unsigned long flags; void __iomem *reg; - int dir; + int error, dir; bank = gpiochip_get_data(chip); reg = bank->base + bank->regs->direction; + error = pm_runtime_get_sync(bank->chip.parent); + if (error < 0) { + dev_err(bank->chip.parent, + "Could not enable gpio bank %p: %d\n", + bank, error); + pm_runtime_put_noidle(bank->chip.parent); + + return error; + } raw_spin_lock_irqsave(&bank->lock, flags); dir = !!(readl_relaxed(reg) & BIT(offset)); raw_spin_unlock_irqrestore(&bank->lock, flags); + pm_runtime_put_sync(bank->chip.parent); + return dir; } -- 2.13.2
| |