From mboxrd@z Thu Jan 1 00:00:00 1970 From: Baruch Siach Subject: Re: [PATCH] gpio: pl061: move irqdomain initialization Date: Wed, 27 Nov 2013 15:41:59 +0200 Message-ID: <20131127134159.GF32436@tarshish> References: <1385557881-3667-1-git-send-email-linus.walleij@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from guitar.tcltek.co.il ([192.115.133.116]:33594 "EHLO mx.tkos.co.il" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753502Ab3K0NmD (ORCPT ); Wed, 27 Nov 2013 08:42:03 -0500 Content-Disposition: inline In-Reply-To: <1385557881-3667-1-git-send-email-linus.walleij@linaro.org> Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Linus Walleij Cc: linux-gpio@vger.kernel.org, Russell King , Alexandre Courbot , Rob Herring , Haojian Zhuang Hi Linus, On Wed, Nov 27, 2013 at 02:11:21PM +0100, Linus Walleij wrote: > The PL061 driver had the irqdomain initialization in an unfortunate > place: when used with device tree (and thus passing the base IRQ > 0) the driver would work, as this registers an irqdomain and waits > for mappings to be done dynamically as the devices request their > IRQs, whereas when booting using platform data the irqdomain core > would attempt to allocate IRQ descriptors dynamically (which works > fine) but also to associate the irq_domain_associate_many() on all > IRQs, which in turn will call the mapping function which at this > point will try to set the type of the IRQ and then tries to acquire > a non-initialized spinlock yielding a backtrace like this: > > CPU: 0 PID: 1 Comm: swapper Not tainted 3.13.0-rc1+ #652 > Backtrace: > [] (dump_backtrace) from [] (show_stack+0x18/0x1c) > r6:c798ace0 r5:00000000 r4:c78257e0 r3:00200140 > [] (show_stack) from [] (dump_stack+0x20/0x28) > [] (dump_stack) from [] (__lock_acquire+0x1c0/0x1b80) > [] (__lock_acquire) from [] (lock_acquire+0x6c/0x80) > r10:00000000 r9:c0455234 r8:00000060 r7:c047d798 r6:600000d3 r5:00000000 > r4:c782c000 > [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x60/0x74) > r6:c01a1100 r5:800000d3 r4:c798acd0 > [] (_raw_spin_lock_irqsave) from [] (pl061_irq_type+0x28/0x) > r6:00000000 r5:00000000 r4:c798acd0 > [] (pl061_irq_type) from [] (__irq_set_trigger+0x70/0x104) > r6:00000000 r5:c01a10d8 r4:c046da1c r3:c01a10d8 > [] (__irq_set_trigger) from [] (irq_set_irq_type+0x40/0x60) > r10:c043240c r8:00000060 r7:00000000 r6:c046da1c r5:00000060 r4:00000000 > [] (irq_set_irq_type) from [] (pl061_irq_map+0x40/0x54) > r6:c79693c0 r5:c798acd0 r4:00000060 > [] (pl061_irq_map) from [] (irq_domain_associate+0xc0/0x190) > r5:00000060 r4:c046da1c > [] (irq_domain_associate) from [] (irq_domain_associate_man) > r8:00000008 r7:00000000 r6:c79693c0 r5:00000060 r4:00000000 > [] (irq_domain_associate_many) from [] (irq_domain_add_simp) > r8:c046578c r7:c035b72c r6:c79693c0 r5:00000060 r4:00000008 r3:00000008 > [] (irq_domain_add_simple) from [] (pl061_probe+0xc4/0x22c) > r6:00000060 r5:c0464380 r4:c798acd0 > [] (pl061_probe) from [] (amba_probe+0x74/0xe0) > r10:c043240c r9:c0455234 r8:00000000 r7:c047d7f8 r6:c047d744 r5:00000000 > r4:c0464380 > > This moves the irqdomain initialization to a point where the spinlock > and GPIO chip are both fully propulated, so the callbacks can be used > without crashes. > > I had some problem reproducing the crash, as the devm_kzalloc():ed > zeroed memory would seemingly mask the spinlock as something OK, > but by poisoning the lock like this: > > u32 *dum; > dum = (u32 *) &chip->lock; > *dum = 0xaaaaaaaaU; > > I could reproduce, fix and test the patch. > > Reported-by: Russell King > Cc: Rob Herring > Cc: Haojian Zhuang > Cc: Baruch Siach > Signed-off-by: Linus Walleij Isn't this a matter for -stable (v3.10+)? baruch > --- > drivers/gpio/gpio-pl061.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c > index f22f7f3e2e53..b4d42112d02d 100644 > --- a/drivers/gpio/gpio-pl061.c > +++ b/drivers/gpio/gpio-pl061.c > @@ -286,11 +286,6 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id) > if (!chip->base) > return -ENOMEM; > > - chip->domain = irq_domain_add_simple(adev->dev.of_node, PL061_GPIO_NR, > - irq_base, &pl061_domain_ops, chip); > - if (!chip->domain) > - return -ENODEV; > - > spin_lock_init(&chip->lock); > > chip->gc.request = pl061_gpio_request; > @@ -320,6 +315,11 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id) > irq_set_chained_handler(irq, pl061_irq_handler); > irq_set_handler_data(irq, chip); > > + chip->domain = irq_domain_add_simple(adev->dev.of_node, PL061_GPIO_NR, > + irq_base, &pl061_domain_ops, chip); > + if (!chip->domain) > + return -ENODEV; > + > for (i = 0; i < PL061_GPIO_NR; i++) { > if (pdata) { > if (pdata->directions & (1 << i)) -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -