Re: [PATCH v2 10/15] PCI: rcar-gen4: Take over the iMSI-RX interrupt
From: Marek Vasut
Date: Sat Oct 03 2026 - 21:18:55 EST
On 9/30/26 8:38 AM, Koichiro Den wrote:
On Tue, Sep 29, 2026 at 07:43:28PM +0200, Geert Uytterhoeven wrote:This does make sense, yes, thank you for spotting this. I will be sending a V5 of the PM ops patch now.
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.
[...]