All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Daniel Palmer <daniel@0x0f.com>,
	Linus Walleij <linus.walleij@linaro.org>
Cc: DTML <devicetree@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Romain Perier <romain.perier@gmail.com>
Subject: Re: [PATCH 2/3] irqchip: SigmaStar SSD20xD gpi
Date: Tue, 21 Sep 2021 09:27:32 +0100	[thread overview]
Message-ID: <87lf3quydn.wl-maz@kernel.org> (raw)
In-Reply-To: <CAFr9PXmA07Up_wfJzzgZeYwE5ZrwnLqjBvLG3CERGHOLeay0Cg@mail.gmail.com>

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Daniel Palmer <daniel@0x0f.com>,
	Linus Walleij <linus.walleij@linaro.org>
Cc: DTML <devicetree@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Romain Perier <romain.perier@gmail.com>
Subject: Re: [PATCH 2/3] irqchip: SigmaStar SSD20xD gpi
Date: Tue, 21 Sep 2021 09:27:32 +0100	[thread overview]
Message-ID: <87lf3quydn.wl-maz@kernel.org> (raw)
In-Reply-To: <CAFr9PXmA07Up_wfJzzgZeYwE5ZrwnLqjBvLG3CERGHOLeay0Cg@mail.gmail.com>

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.

  reply	other threads:[~2021-09-21  8:29 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-14 10:04 [PATCH 0/3] SigmaStar SSD20XD GPIO interrupt controller Daniel Palmer
2021-09-14 10:04 ` Daniel Palmer
2021-09-14 10:04 ` [PATCH 1/3] dt-bindings: interrupt-controller: Add SigmaStar SSD20xD gpi Daniel Palmer
2021-09-14 10:04   ` Daniel Palmer
2021-09-21 20:36   ` Rob Herring
2021-09-21 20:36     ` Rob Herring
2021-09-14 10:04 ` [PATCH 2/3] irqchip: " Daniel Palmer
2021-09-14 10:04   ` Daniel Palmer
2021-09-20  9:45   ` Marc Zyngier
2021-09-20  9:45     ` Marc Zyngier
2021-09-20 10:05     ` Daniel Palmer
2021-09-20 10:05       ` Daniel Palmer
2021-09-20 11:24       ` Marc Zyngier
2021-09-20 11:24         ` Marc Zyngier
2021-09-21  4:16         ` Daniel Palmer
2021-09-21  4:16           ` Daniel Palmer
2021-09-21  8:27           ` Marc Zyngier [this message]
2021-09-21  8:27             ` Marc Zyngier
2021-09-21 18:23             ` Linus Walleij
2021-09-21 18:23               ` Linus Walleij
2021-09-30 12:39               ` Daniel Palmer
2021-09-30 12:39                 ` Daniel Palmer
2021-09-30 13:07                 ` Marc Zyngier
2021-09-30 13:07                   ` Marc Zyngier
2021-09-30 13:10                   ` Daniel Palmer
2021-09-30 13:10                     ` Daniel Palmer
2021-09-30 13:06               ` Marc Zyngier
2021-09-30 13:06                 ` Marc Zyngier
2021-09-30 13:36                 ` Daniel Palmer
2021-09-30 13:36                   ` Daniel Palmer
2021-09-30 13:53                   ` Marc Zyngier
2021-09-30 13:53                     ` Marc Zyngier
2021-09-30 13:59                     ` Daniel Palmer
2021-09-30 13:59                       ` Daniel Palmer
2021-09-30 14:11                       ` Marc Zyngier
2021-09-30 14:11                         ` Marc Zyngier
2021-09-30 16:10                         ` Linus Walleij
2021-09-30 16:10                           ` Linus Walleij
2021-09-30 16:13                   ` Linus Walleij
2021-09-30 16:13                     ` Linus Walleij
2021-10-01 12:33                     ` Daniel Palmer
2021-10-01 12:33                       ` Daniel Palmer
2021-10-02  3:08                     ` Daniel Palmer
2021-10-02  3:08                       ` Daniel Palmer
2021-09-21  6:11         ` Daniel Palmer
2021-09-21  6:11           ` Daniel Palmer
2021-09-14 10:04 ` [PATCH 3/3] ARM: dts: mstar: Add gpi interrupt controller to i2m Daniel Palmer
2021-09-14 10:04   ` Daniel Palmer
2021-09-14 15:59 ` [PATCH 0/3] SigmaStar SSD20XD GPIO interrupt controller Andrew Lunn
2021-09-14 15:59   ` Andrew Lunn
2021-09-15  9:06   ` Daniel Palmer
2021-09-15  9:06     ` Daniel Palmer
2021-09-15 20:34     ` Andrew Lunn
2021-09-15 20:34       ` Andrew Lunn
2021-09-20  8:36       ` Daniel Palmer
2021-09-20  8:36         ` Daniel Palmer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lf3quydn.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=daniel@0x0f.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=romain.perier@gmail.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.