Re: [PATCH 2/3] irqchip: SigmaStar SSD20xD gpi

[Marc Zyngier <maz@kernel.org>]

On Tue, 21 Sep 2021 05:16:35 +0100,
Daniel Palmer <daniel@0x0f.com> wrote:

+ Linus.

> So if I set irq_chip_ack_parent as the ack callback I get another explosion:
> 
> # gpiomon -r 0 44
> [   22.370689] 8<--- cut here ---
> [   22.373802] Unable to handle kernel NULL pointer dereference at
> virtual address 00000018
> [   22.381945] pgd = (ptrval)
> [   22.384685] [00000018] *pgd=235cb835, *pte=00000000, *ppte=00000000
> [   22.391038] Internal error: Oops: 17 [#1] SMP ARM
> [   22.395776] Modules linked in:
> [   22.398860] CPU: 1 PID: 193 Comm: gpiomon Not tainted 5.15.0-rc2+ #2566
> [   22.405515] Hardware name: MStar/Sigmastar Armv7 (Device Tree)
> [   22.411376] PC is at irq_chip_ack_parent+0x8/0x10
> [   22.416120] LR is at __irq_do_set_handler+0x3c/0x11c
> [   22.421119] pc : [<c017f498>]    lr : [<c018029c>]    psr: a0040093
> [   22.427419] sp : c3505d68  ip : ffffe000  fp : 00000000
> [   22.432673] r10: c0d592d4  r9 : 00000001  r8 : 00000000
> [   22.437927] r7 : c3502618  r6 : 00000000  r5 : c017b9cc  r4 : c3502600
> [   22.444489] r3 : 00000000  r2 : c10bb294  r1 : c10bb294  r0 : c26a3440
> [   22.451053] Flags: NzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
> Segment user
> [   22.458317] Control: 10c5387d  Table: 235b006a  DAC: 00000055
> ---snip---
> [   22.725196] [<c017f498>] (irq_chip_ack_parent) from [<c018029c>]
> (__irq_do_set_handler+0x3c/0x11c)
> [   22.734219] [<c018029c>] (__irq_do_set_handler) from [<c01803b4>]
> (__irq_set_handler+0x38/0x50)
> [   22.742976] [<c01803b4>] (__irq_set_handler) from [<c0181880>]
> (irq_domain_set_info+0x34/0x48)
> [   22.751649] [<c0181880>] (irq_domain_set_info) from [<c046f838>]
> (gpiochip_hierarchy_irq_domain_alloc+0x104/0x228)
> [   22.762069] [<c046f838>] (gpiochip_hierarchy_irq_domain_alloc) from
> [<c0182c38>] (__irq_domain_alloc_irqs+0xd8/0x318)
> [   22.772748] [<c0182c38>] (__irq_domain_alloc_irqs) from
> [<c01832e8>] (irq_create_fwspec_mapping+0x22c/0x298)
> [   22.782641] [<c01832e8>] (irq_create_fwspec_mapping) from
> [<c0470124>] (gpiochip_to_irq+0x60/0x84)
> [   22.791664] [<c0470124>] (gpiochip_to_irq) from [<c046ef18>]
> (gpiod_to_irq+0x48/0x60)
> [   22.799552] [<c046ef18>] (gpiod_to_irq) from [<c0477a48>]
> (gpio_ioctl+0x1b4/0x420)
> [   22.807178] [<c0477a48>] (gpio_ioctl) from [<c0262e4c>] (vfs_ioctl+0x20/0x38)
> [   22.814371] [<c0262e4c>] (vfs_ioctl) from [<c0263708>] (sys_ioctl+0xb0/0x818)
> [   22.821564] [<c0263708>] (sys_ioctl) from [<c0100060>]
> (ret_fast_syscall+0x0/0x1c)
> [   22.829190] Exception stack(0xc3505fa8 to 0xc3505ff0)
> [   22.834273] 5fa0:                   ???????? ???????? ????????
> ???????? ???????? ????????
> [   22.842488] 5fc0: ???????? ???????? ???????? ???????? ????????
> ???????? ???????? ????????
> [   22.850701] 5fe0: ???????? ???????? ???????? ????????
> [   22.855790] Code: e593301c e12fff13 e5900018 e5903010 (e5933018)
> [   22.861919] ---[ end trace 10524aa06eced7e3 ]---

This seems to be caused by your GPIO driver installing a flow handler
(via irq_domain_set_info()), which is a bit odd. I would expect that
only the root irqchip in the hierarchy would do that.

At the point where this is called, the hierarchy isn't fully populated
(the irq_domain_alloc_irqs_parent() call comes after that), and
irq_chip_ack_parent() explodes as above.

Linus: is there a reason why the gpiolib insist on setting its own
handler while building the hierarchy? I guess this could be worked
around by swapping the calls to irq_domain_set_info and
irq_domain_alloc_irqs_parent, but having two levels of the hierarchy
competing for the flow handler looks a bit odd.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.