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:
> [<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 <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Haojian Zhuang <[email protected]>
> Cc: Baruch Siach <[email protected]>
> Signed-off-by: Linus Walleij <[email protected]>
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------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
