From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933655AbdGKPoB (ORCPT ); Tue, 11 Jul 2017 11:44:01 -0400 Received: from muru.com ([72.249.23.125]:54124 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933384AbdGKPoA (ORCPT ); Tue, 11 Jul 2017 11:44:00 -0400 Date: Tue, 11 Jul 2017 08:43:55 -0700 From: Tony Lindgren To: Thomas Gleixner Cc: Linus Torvalds , Sebastian Reichel , LKML , Andrew Morton , Ingo Molnar , "H. Peter Anvin" , Pavel Machek , Linus Walleij , Grygorii Strashko Subject: Re: [GIT pull] irq updates for 4.13 Message-ID: <20170711154355.GX3730@atomide.com> References: <20170710133505.eo6w73kq2327n34p@earth> <20170711135131.GW3730@atomide.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.2 (2017-04-18) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Thomas Gleixner [170711 08:07]: > On Tue, 11 Jul 2017, Thomas Gleixner wrote: > > On Tue, 11 Jul 2017, Tony Lindgren wrote: > > > * Thomas Gleixner [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 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 [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 Signed-off-by: Tony Lindgren --- 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