Re: [PATCH] pinctrl: mcp23s08: Allocate irq_chip dynamic

From: Jan Kundrát
Date: Tue Mar 05 2019 - 10:27:38 EST

On pondÄlà 21. ledna 2019 14:19:49 CET, Linus Walleij wrote:
On Fri, Jan 11, 2019 at 4:16 PM Lars Poeschel <poeschel@xxxxxxxxxxx> wrote:

Keeping the irq_chip definition static shares it with multiple instances
of the mcp23s08 gpiochip in the system. This is bad and now we get this
warning from gpiolib core:

"detected irqchip that is shared with multiple gpiochips: please fix the
driver."

Hence, move the irq_chip definition from being driver static into the
struct mcp23s08. So a unique irq_chip is used for each gpiochip
instance.

Signed-off-by: Lars Poeschel <poeschel@xxxxxxxxxxx>

Patch applied.

Tested-by: Jan KundrÃt <jan.kundrat@xxxxxxxxx>
Fixes: 171948ea33e14 (gpiolib: check if irqchip already has the irq hook replacements)

This commit should probably go to 5.0-stable and 4.20-stable as well because commit 171948ea33e14 first appeared in 4.20. I have been using 4.19 previsouly, which works, but 5.0 oopses on boot for me with two MCP23S17 chips (sorry, I just don't know how to get more than 10 lines of stack trace in this oops):

[ 1.011926] gpio gpiochip3: (mcp23s17.1): detected irqchip that is shared with multiple gpiochips: please fix the driver.
[ 1.022936] GPIO line 486 (I2C XOR ready) hogged as input
[ 1.028550] Unable to handle kernel NULL pointer dereference at virtual address 00000054
[ 1.036675] pgd = (ptrval)
[ 1.039388] [00000054] *pgd=00000000
[ 1.042980] Internal error: Oops: 5 [#1] SMP ARM
[ 1.047613] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.0.0 #1
[ 1.053460] Hardware name: Marvell Armada 380/385 (Device Tree)
[ 1.059401] PC is at __dev_printk+0x10/0x84
[ 1.063595] LR is at _dev_info+0x44/0x68
[ 1.067528] pc : [<c0435b50>] lr : [<c0436764>] psr: 20000013
[ 1.073811] sp : ef4afbb4 ip : ef4afbbc fp : eed0a040
[ 1.079048] r10: 00000000 r9 : c0b32834 r8 : 00000000
[ 1.084286] r7 : c016f8cc r6 : 00000000 r5 : ef9f7ba0 r4 : c0b07bc8
[ 1.090829] r3 : ef4afbd8 r2 : ef4afbbc r1 : 00000008 r0 : c08cf6fc
[ 1.097374] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 1.104528] Control: 10c5387d Table: 0000404a DAC: 00000051
[ 1.110289] Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
[ 1.116311] Stack: (0xef4afbb4 to 0xef4b0000)
[ 1.120679] fba0: c0436764 ef4afbd8 c08f7134
[ 1.128881] fbc0: ef4afbb8 36415da1 c0bbc194 eed0a08c c03bce58 c08f7134 ef714c40 00000000
[ 1.137082] fbe0: eed0a08c 00000002 eecf9840 eed0a064 c0b07bc8 eed0a400 eed0a08c 00000002
[ 1.145283] fc00: eecf9840 c03b9484 00000000 00000001 00000000 00000000 00004c4c 36415da1
[ 1.153485] fc20: 00000000 00000003 eed0a400 00000002 00000000 c09019b8 ef4afc5c c03b97ac
[ 1.161686] fc40: 00000001 ffffffff 00000010 00000001 0000010c c0b07bc8 00000002 00000006
[ 1.169887] fc60: ffffffff 36415da1 00000000 eed0a400 00000000 c0b32768 00000000 00000000
[ 1.178088] fc80: c0b32778 00000000 00000000 c04d0218 c0bb8590 eed0a400 c0bb859c c04391e8
[ 1.186290] fca0: eed0a400 c0b32778 ef4afd1c c0439848 00000001 00000000 00000000 c043965c
[ 1.194491] fcc0: eed0a400 c0b32778 c0b32768 00000000 c0b07bc8 ef4afd1c c0439848 00000001
[ 1.202692] fce0: 00000000 00000000 00000000 c0437ca4 00000000 ef54cc6c ef5c8938 36415da1
[ 1.210894] fd00: eed0a400 eed0a400 c0b07bc8 eed0a434 eee79c00 c04394c4 c093b958 eed0a400
[ 1.219095] fd20: 00000001 36415da1 eed0a408 eed0a400 c0b43fd4 eee79c00 eed0a400 c0437e60
[ 1.227296] fd40: eed0a408 c0b07bc8 00000000 c0436244 00000006 c04d56f8 00000000 eed0a400
[ 1.235497] fd60: 000001be 36415da1 eed0a400 eee79c00 00000000 ef575410 00000001 ef9f7bec
[ 1.243699] fd80: c0b07bc8 c04d2620 eee79c00 ef9f7ba0 eed0a400 00000000 00000001 c04d2e4c
[ 1.251900] fda0: 00000000 c05cff48 c0901258 c0913c9c c0913ca8 c0913dd4 00989680 36415da1
[ 1.260102] fdc0: eee79f48 eee79c00 ef575400 eee79f48 ef575410 00000002 c0b07bc8 f0997680
[ 1.268303] fde0: c08d00d8 c04d618c 00000000 c02ab604 c08f7a40 c09141b8 eee79c00 c0b07bc8
[ 1.276504] fe00: 00000003 36415da1 c08fed94 ef575410 00000000 c0b449bc 00000000 00000000
[ 1.284706] fe20: c0b449bc 00000000 c0a5185c c043a8c4 c0bb8590 ef575410 c0bb859c 00000000
[ 1.292907] fe40: 00000000 c04391e8 ef575410 c0b449bc ef575444 c0439768 00000000 c0a5183c
[ 1.301108] fe60: c0a004a8 c043965c ef9f7144 c0439768 00000000 ef575410 c0b449bc ef575444
[ 1.309310] fe80: c0439768 00000000 c0a5183c c0a004a8 c0a5185c c0439844 ef579a34 c0b07bc8
[ 1.317511] fea0: c0b449bc c043771c 00000000 ef484d58 ef579a34 36415da1 c0b3b638 c0b449bc
[ 1.325712] fec0: ef737c80 c0b3b638 00000000 c0438114 c0914250 c0439fa8 c0b449bc c0b449bc
[ 1.333914] fee0: c0b07bc8 c0a2f38c ffffe000 c0439f78 c0b62260 c0b07bc8 c0a2f38c c0a0101c
[ 1.342115] ff00: efffcdb7 c013dd34 c0941a58 c0921800 00000000 00000006 00000006 c08d5294
[ 1.350316] ff20: 00000000 c0b07bc8 c08e2294 c08d5308 c0b64e00 efffcda4 efffcdb2 36415da1
[ 1.358518] ff40: c0a51840 c0b62260 c0a60a80 36415da1 c0b62260 c0a60c44 00000007 c0b64e00
[ 1.366719] ff60: c0b64e00 c0a0139c 00000006 00000006 00000000 c0a004a8 000000b8 00000000
[ 1.374920] ff80: 00000000 00000000 c071f84c 00000000 00000000 00000000 00000000 00000000
[ 1.383121] ffa0: 00000000 c071f854 00000000 c01010e8 00000000 00000000 00000000 00000000
[ 1.391322] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 1.399523] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 1.407729] [<c0435b50>] (__dev_printk) from [<ef4afbb8>] (0xef4afbb8)
[ 1.414276] Code: e3510000 0a00001a e52de004 e1a0c002 (e591304c) [ 1.420405] ---[ end trace 1243f20a0e53cc41 ]---
[ 1.425040] Kernel panic - not syncing: Fatal exception
[ 1.430282] CPU1: stopping
[ 1.433000] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D 5.0.0 #1
[ 1.440242] Hardware name: Marvell Armada 380/385 (Device Tree)
[ 1.446188] [<c01103ac>] (unwind_backtrace) from [<c010c1e4>] (show_stack+0x10/0x14)
[ 1.453958] [<c010c1e4>] (show_stack) from [<c0708af0>] (dump_stack+0x88/0x9c)
[ 1.461205] [<c0708af0>] (dump_stack) from [<c010f0b8>] (handle_IPI+0x348/0x380)
[ 1.468626] [<c010f0b8>] (handle_IPI) from [<c03af568>] (gic_handle_irq+0x8c/0x90)
[ 1.476220] [<c03af568>] (gic_handle_irq) from [<c0101a0c>] (__irq_svc+0x6c/0x90)
[ 1.483723] Exception stack(0xef4cdf60 to 0xef4cdfa8)
[ 1.488789] df60: 00000000 00002418 ef9e4de0 c0118940 ffffe000 c0b07bf0 c0b07c30 00000002
[ 1.496991] df80: 00000000 c0b07bc8 c0a69a00 c0b61f0c 00000000 ef4cdfb0 c0108c94 c0108c98
[ 1.505191] dfa0: 60000013 ffffffff
[ 1.508693] [<c0101a0c>] (__irq_svc) from [<c0108c98>] (arch_cpu_idle+0x38/0x3c)
[ 1.516113] [<c0108c98>] (arch_cpu_idle) from [<c014ccd4>] (do_idle+0x1c4/0x1fc)
[ 1.523533] [<c014ccd4>] (do_idle) from [<c014cfa8>] (cpu_startup_entry+0x18/0x20)
[ 1.531124] [<c014cfa8>] (cpu_startup_entry) from [<0010252c>] (0x10252c)
[ 1.537933] Rebooting in 10 seconds..

If I cherry-pick the following two commits to my 5.0, my board boots once again:

16f4372fd7a5 pinctrl: mcp23s08: use struct_size() in devm_kzalloc()
19ab5ca9b77d pinctrl: mcp23s08: Allocate irq_chip dynamic

While I am not getting a complete stack trace, it seems likely that the culprit is indeed 171948ea33e14. It does not add a mere warning, it also changes behavior so that a chip's irq_enable and irq_disable are not properly initialized. There's no error code in this function, so the rest of the code cannot reasonably catch this new behavior. My recommendation would be to actually get rid of that early return, but if the point is to *really* get people to notice, well, I noticed :).

As on "why am I hitting this while nobody else did", my board has a shared IRQ line which is active during bootup, perhaps that might have something to do with this -- I don't know.

Thanks for this fixup, Lars.

Cheers,
Jan