Skip to content

Commit a9a1a48

Browse files
gclementLinus Walleij
authored andcommitted
pinctrl: armada-37xx: Fix gpio interrupt setup
Since commit dc749a0 ("gpiolib: allow gpio irqchip to map irqs dynamically"), the irqs for gpio are not statically allocated during in gpiochip_irqchip_add. This driver was based on this assumption for initializing the mask associated to each interrupt this led to a NULL pointer crash in the kernel: Unable to handle kernel NULL pointer dereference at virtual address 00000000 Mem abort info: Exception class = DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x00000068 CM = 0, WnR = 1 [0000000000000000] user address but active_mm is swapper Internal error: Oops: 96000044 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.13.0-06657-g3b9f8ed25dbe #576 Hardware name: Marvell Armada 3720 Development Board DB-88F3720-DDR3 (DT) task: ffff80001d908000 task.stack: ffff000008068000 PC is at armada_37xx_pinctrl_probe+0x5f8/0x670 LR is at armada_37xx_pinctrl_probe+0x5e8/0x670 pc : [<ffff000008e25cdc>] lr : [<ffff000008e25ccc>] pstate: 60000045 sp : ffff00000806bb80 x29: ffff00000806bb80 x28: 0000000000000024 x27: 000000000000000c x26: 0000000000000001 x25: ffff80001efee760 x24: 0000000000000000 x23: ffff80001db6f570 x22: ffff80001db6f438 x21: 0000000000000000 x20: ffff80001d9f4810 x19: ffff80001db6f418 x18: 0000000000000000 x17: 0000000000000001 x16: 0000000000000019 x15: ffffffffffffffff x14: 0140000000000000 x13: 0000000000000000 x12: 0000000000000030 x11: 0101010101010101 x10: 0000000000000040 x9 : ffff000009923580 x8 : ffff80001d400248 x7 : ffff80001d400270 x6 : 0000000000000000 x5 : ffff80001d400248 x4 : ffff80001d400270 x3 : 0000000000000000 x2 : 0000000000000001 x1 : 0000000000000001 x0 : 0000000000000000 Process swapper/0 (pid: 1, stack limit = 0xffff000008068000) Call trace: Exception stack(0xffff00000806ba40 to 0xffff00000806bb80) ba40: 0000000000000000 0000000000000001 0000000000000001 0000000000000000 ba60: ffff80001d400270 ffff80001d400248 0000000000000000 ffff80001d400270 ba80: ffff80001d400248 ffff000009923580 0000000000000040 0101010101010101 baa0: 0000000000000030 0000000000000000 0140000000000000 ffffffffffffffff bac0: 0000000000000019 0000000000000001 0000000000000000 ffff80001db6f418 bae0: ffff80001d9f4810 0000000000000000 ffff80001db6f438 ffff80001db6f570 bb00: 0000000000000000 ffff80001efee760 0000000000000001 000000000000000c bb20: 0000000000000024 ffff00000806bb80 ffff000008e25ccc ffff00000806bb80 bb40: ffff000008e25cdc 0000000060000045 ffff00000806bb60 ffff0000081189b8 bb60: ffffffffffffffff ffff00000811cf1c ffff00000806bb80 ffff000008e25cdc [<ffff000008e25cdc>] armada_37xx_pinctrl_probe+0x5f8/0x670 [<ffff00000859d8c8>] platform_drv_probe+0x58/0xb8 [<ffff00000859bb44>] driver_probe_device+0x22c/0x2d8 [<ffff00000859bcac>] __driver_attach+0xbc/0xc0 [<ffff000008599c84>] bus_for_each_dev+0x4c/0x98 [<ffff00000859b440>] driver_attach+0x20/0x28 [<ffff00000859af90>] bus_add_driver+0x1b8/0x228 [<ffff00000859c648>] driver_register+0x60/0xf8 [<ffff00000859df64>] __platform_driver_probe+0x74/0x130 [<ffff000008e256dc>] armada_37xx_pinctrl_driver_init+0x20/0x28 [<ffff000008083980>] do_one_initcall+0x38/0x128 [<ffff000008e00cf4>] kernel_init_freeable+0x188/0x22c [<ffff0000089b56e8>] kernel_init+0x10/0x100 [<ffff000008084bb0>] ret_from_fork+0x10/0x18 Code: f9403fa2 12001341 1100075a 9ac12041 (b9000001) ---[ end trace 8b0f4e05e1603208 ]--- This patch moves the initialization of the mask field in the irq_startup function. However some callbacks such as irq_set_type and irq_set_wake could be called before irq_startup. For those functions the mask is computed at each call which is not a issue as these functions are not located in a hot path but are used sporadically for configuration. Fixes: dc749a0 ("gpiolib: allow gpio irqchip to map irqs dynamically") Cc: <stable@vger.kernel.org> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
1 parent 4ce504c commit a9a1a48

1 file changed

Lines changed: 22 additions & 19 deletions

File tree

drivers/pinctrl/mvebu/pinctrl-armada-37xx.c

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -550,9 +550,9 @@ static int armada_37xx_irq_set_wake(struct irq_data *d, unsigned int on)
550550
spin_lock_irqsave(&info->irq_lock, flags);
551551
val = readl(info->base + reg);
552552
if (on)
553-
val |= d->mask;
553+
val |= (BIT(d->hwirq % GPIO_PER_REG));
554554
else
555-
val &= ~d->mask;
555+
val &= ~(BIT(d->hwirq % GPIO_PER_REG));
556556
writel(val, info->base + reg);
557557
spin_unlock_irqrestore(&info->irq_lock, flags);
558558

@@ -571,10 +571,10 @@ static int armada_37xx_irq_set_type(struct irq_data *d, unsigned int type)
571571
val = readl(info->base + reg);
572572
switch (type) {
573573
case IRQ_TYPE_EDGE_RISING:
574-
val &= ~d->mask;
574+
val &= ~(BIT(d->hwirq % GPIO_PER_REG));
575575
break;
576576
case IRQ_TYPE_EDGE_FALLING:
577-
val |= d->mask;
577+
val |= (BIT(d->hwirq % GPIO_PER_REG));
578578
break;
579579
default:
580580
spin_unlock_irqrestore(&info->irq_lock, flags);
@@ -624,11 +624,27 @@ static void armada_37xx_irq_handler(struct irq_desc *desc)
624624
chained_irq_exit(chip, desc);
625625
}
626626

627+
static unsigned int armada_37xx_irq_startup(struct irq_data *d)
628+
{
629+
struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
630+
int irq = d->hwirq - chip->irq_base;
631+
/*
632+
* The mask field is a "precomputed bitmask for accessing the
633+
* chip registers" which was introduced for the generic
634+
* irqchip framework. As we don't use this framework, we can
635+
* reuse this field for our own usage.
636+
*/
637+
d->mask = BIT(irq % GPIO_PER_REG);
638+
639+
armada_37xx_irq_unmask(d);
640+
641+
return 0;
642+
}
643+
627644
static int armada_37xx_irqchip_register(struct platform_device *pdev,
628645
struct armada_37xx_pinctrl *info)
629646
{
630647
struct device_node *np = info->dev->of_node;
631-
int nrirqs = info->data->nr_pins;
632648
struct gpio_chip *gc = &info->gpio_chip;
633649
struct irq_chip *irqchip = &info->irq_chip;
634650
struct resource res;
@@ -666,8 +682,8 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev,
666682
irqchip->irq_unmask = armada_37xx_irq_unmask;
667683
irqchip->irq_set_wake = armada_37xx_irq_set_wake;
668684
irqchip->irq_set_type = armada_37xx_irq_set_type;
685+
irqchip->irq_startup = armada_37xx_irq_startup;
669686
irqchip->name = info->data->name;
670-
671687
ret = gpiochip_irqchip_add(gc, irqchip, 0,
672688
handle_edge_irq, IRQ_TYPE_NONE);
673689
if (ret) {
@@ -680,19 +696,6 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev,
680696
* controller. But we do not take advantage of this and use
681697
* the chained irq with all of them.
682698
*/
683-
for (i = 0; i < nrirqs; i++) {
684-
struct irq_data *d = irq_get_irq_data(gc->irq_base + i);
685-
686-
/*
687-
* The mask field is a "precomputed bitmask for
688-
* accessing the chip registers" which was introduced
689-
* for the generic irqchip framework. As we don't use
690-
* this framework, we can reuse this field for our own
691-
* usage.
692-
*/
693-
d->mask = BIT(i % GPIO_PER_REG);
694-
}
695-
696699
for (i = 0; i < nr_irq_parent; i++) {
697700
int irq = irq_of_parse_and_map(np, i);
698701

0 commit comments

Comments
 (0)