lkml.org 
[lkml]   [2017]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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
    \
     
     \ /
      Last update: 2017-07-11 17:45    [W:4.163 / U:0.136 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site