Patchwork [3.11,161/272] gpio: pl061: move irqdomain initialization

login
register
mail settings
Submitter Luis Henriques
Date Dec. 6, 2013, 12:57 p.m.
Message ID <1386334761-25517-162-git-send-email-luis.henriques@canonical.com>
Download mbox | patch
Permalink /patch/297978/
State New
Headers show

Comments

Luis Henriques - Dec. 6, 2013, 12:57 p.m.
3.11.10.1 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Linus Walleij <linus.walleij@linaro.org>

commit 2ba3154d9cb13697b97723cce75633b48adfe826 upstream.

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:
[<c0016f0c>] (dump_backtrace) from [<c00172ac>] (show_stack+0x18/0x1c)
 r6:c798ace0 r5:00000000 r4:c78257e0 r3:00200140
[<c0017294>] (show_stack) from [<c0329ea0>] (dump_stack+0x20/0x28)
[<c0329e80>] (dump_stack) from [<c004fa80>] (__lock_acquire+0x1c0/0x1b80)
[<c004f8c0>] (__lock_acquire) from [<c0051970>] (lock_acquire+0x6c/0x80)
 r10:00000000 r9:c0455234 r8:00000060 r7:c047d798 r6:600000d3 r5:00000000
 r4:c782c000
[<c0051904>] (lock_acquire) from [<c032e484>] (_raw_spin_lock_irqsave+0x60/0x74)
 r6:c01a1100 r5:800000d3 r4:c798acd0
[<c032e424>] (_raw_spin_lock_irqsave) from [<c01a1100>] (pl061_irq_type+0x28/0x)
 r6:00000000 r5:00000000 r4:c798acd0
[<c01a10d8>] (pl061_irq_type) from [<c0059ef4>] (__irq_set_trigger+0x70/0x104)
 r6:00000000 r5:c01a10d8 r4:c046da1c r3:c01a10d8
[<c0059e84>] (__irq_set_trigger) from [<c005b348>] (irq_set_irq_type+0x40/0x60)
 r10:c043240c r8:00000060 r7:00000000 r6:c046da1c r5:00000060 r4:00000000
[<c005b308>] (irq_set_irq_type) from [<c01a1208>] (pl061_irq_map+0x40/0x54)
 r6:c79693c0 r5:c798acd0 r4:00000060
[<c01a11c8>] (pl061_irq_map) from [<c005d27c>] (irq_domain_associate+0xc0/0x190)
 r5:00000060 r4:c046da1c
[<c005d1bc>] (irq_domain_associate) from [<c005d604>] (irq_domain_associate_man)
 r8:00000008 r7:00000000 r6:c79693c0 r5:00000060 r4:00000000
[<c005d5d0>] (irq_domain_associate_many) from [<c005d864>] (irq_domain_add_simp)
 r8:c046578c r7:c035b72c r6:c79693c0 r5:00000060 r4:00000008 r3:00000008
[<c005d814>] (irq_domain_add_simple) from [<c01a1380>] (pl061_probe+0xc4/0x22c)
 r6:00000060 r5:c0464380 r4:c798acd0
[<c01a12bc>] (pl061_probe) from [<c01c0450>] (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 <linux@arm.linux.org.uk>
Cc: Rob Herring <robherring2@gmail.com>
Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Baruch Siach <baruch@tkos.co.il>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 drivers/gpio/gpio-pl061.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Patch

diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 6a4bd0d..4fbe12d 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))