Re: [PATCH v2 10/15] PCI: rcar-gen4: Take over the iMSI-RX interrupt
From: Koichiro Den
Date: Wed Sep 30 2026 - 02:38:53 EST
On Tue, Sep 29, 2026 at 07:43:28PM +0200, Geert Uytterhoeven wrote:
> Hi Den-san, Marek,
>
> On Mon, 28 Sept 2026 at 18:53, Koichiro Den <den@xxxxxxxxxxxxx> wrote:
> > On R-Car Gen4, intreq_pcim_sub ("msi") carries more than the integrated
> > MSI receiver: the controller's reset requests and the Root Port's PME
> > and bandwidth notifications are signalled on the same line, and the
> > following patches need to handle them. With the DesignWare core owning
> > the line through its chained handler, the driver would have to hook into
> > that handler when iMSI-RX is used and request the line itself otherwise.
> >
> > Instead, request the interrupt in the driver in all configurations and
> > set pp->msi_irq[0] to -ENODEV so the core does not install its chained
> > handler, as spear13xx, keembay and dra7xx do. The handler demultiplexes
> > the MSIs through dw_handle_msi_irq() when the APP block reports
> > msi_ctrl_int. With an external MSI controller or pci=nomsi the iMSI-RX
> > is not set up, so keep msi_ctrl_int masked rather than enabled, and the
> > handler has nothing to do there yet. Request the interrupt before
> > enumeration, as endpoint drivers may use MSIs from their probe, with
> > IRQF_NO_THREAD so the MSIs are demultiplexed in hard IRQ context like
> > the chained handler did. The interrupt is required by the binding.
> >
> > Release the interrupt in .deinit, before asserting the controller reset
> > and disabling its clocks. Disable it around a Root Port reset because
> > the handler accesses the MSI status registers through DBI.
> >
> > The DT routes downstream INTx to the same line, but the driver has never
> > supported INTx (no INTx domain, INTx enables never set), so requesting
> > the line exclusively takes nothing away.
> >
> > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> FTR, this interacts badly with "[PATCH v2] PCI: rcar-gen4: Add missing
> PM ops"[1] during resume from s2idle:
Hi Geert, thanks for the heads-up.
I'll fix this in v3. I'm planning to address this together with the "Known gap"
noted below the commit message, by moving IRQ setup/teardown out of
.init()/.deinit(). On hindsight, I should have done so in v2. That should also
avoid freeing and re-requesting IRQs during suspend/resume, without needing
IRQF_NO_SUSPEND.
By the way, AFAIK Marek has posted v4 of the PM ops patch:
https://lore.kernel.org/r/20260922175525.288106-1-marek.vasut+renesas@xxxxxxxxxxx/
I had only (re-)tested v3 on the EP side (not RC side):
https://lore.kernel.org/r/ain27ypszl767h3fygurivurojxctyjy2yprada6lgjef5c623@23nn7tsrecb6/
but while looking at your report and reproducing, I noticed that
rcar_gen4_pcie_host_ops lacks .pme_turn_off implementation,
so I wonder if it should also set 'pp->use_atu_msg = true'.
I mean, something like this:
------8<-------8<------
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 1f8821e3a424..b8237962de18 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -671,6 +671,8 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
return -ENODEV;
pp->num_vectors = MAX_MSI_IRQS;
+ /* Reserve an iATU window for the generic PME_Turn_Off implementation. */
+ pp->use_atu_msg = true;
pp->ops = &rcar_gen4_pcie_host_ops;
return dw_pcie_host_init(pp);
------8<-------8<------
Marek, I'd appreciate your thoughts on this too.
Perhaps this small change could be included in v5, if you agree.
Best regards,
Koichiro
>
> Unbalanced enable for IRQ 57
> WARNING: kernel/irq/manage.c:775 at __enable_irq+0x38/0x64, CPU#3:
> s2idle/686
> Modules linked in:
> CPU: 3 UID: 0 PID: 686 Comm: s2idle Not tainted
> 7.3.0-rc5-rcar3-09216-ga295839bfe55 #696 PREEMPT
> Hardware name: Renesas Gray Hawk Single board based on r8a779h0 (DT)
> pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : __enable_irq+0x38/0x64
> lr : __enable_irq+0x38/0x64
> sp : ffffffc08904bab0
> x29: ffffffc08904bab0 x28: ffffff8446a4a880 x27: ffffffc084e64828
> x26: ffffffc084e64838 x25: ffffffc08069e630 x24: ffffff8443ad7cb0
> x23: 0000000000000000 x22: 0000000000000000 x21: 0000000000000070
> x20: 0000000000000039 x19: ffffff8443ad7c00 x18: 00000000e630bb27
> x17: 0000000000000000 x16: 0000000000000000 x15: 0720072007200720
> x14: 0720072007200720 x13: 0720072007200720 x12: 0000000000000566
> x11: 0000000000000000 x10: ffffffc08418a038 x9 : ffffffc08158a090
> x8 : ffffffc08904b798 x7 : ffffffc08904b7a0 x6 : 3ffffffffff7ffff
> x5 : fffffffffff7ffff x4 : 0000000000000000 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffff8446a4a880
> Call trace:
> __enable_irq+0x38/0x64 (P)
> resume_irqs+0xf0/0x130
> resume_device_irqs+0x10/0x18
> dpm_resume_noirq+0xe8/0x190
> suspend_devices_and_enter+0x524/0x594
> pm_suspend+0x22c/0x270
> state_store+0xa8/0xe8
> kobj_attr_store+0x14/0x24
> sysfs_kf_write+0x4c/0x64
> kernfs_fop_write_iter+0x13c/0x184
> vfs_write+0x148/0x1b4
> ksys_write+0x78/0xe0
> __arm64_sys_write+0x14/0x1c
> invoke_syscall+0xa0/0x100
> el0_svc_common.constprop.0+0xb0/0xcc
> do_el0_svc+0x18/0x20
> el0_svc+0x3c/0x114
> el0t_64_sync_handler+0x58/0x134
> el0t_64_sync+0x158/0x15c
> irq event stamp: 0
> hardirqs last enabled at (0): [<0000000000000000>] 0x0
> hardirqs last disabled at (0): [<ffffffc080092a2c>]
> copy_process+0x9e0/0x17b8
> softirqs last enabled at (0): [<ffffffc080092a34>]
> copy_process+0x9e8/0x17b8
> softirqs last disabled at (0): [<0000000000000000>] 0x0
> ---[ end trace 0000000000000000 ]---
>
> [1] https://lore.kernel.org/20260907153711.653861-1-marek.vasut+renesas@xxxxxxxxxxx/
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds