Re: [PATCH v2 10/15] PCI: rcar-gen4: Take over the iMSI-RX interrupt

[Marek Vasut <marek.vasut@mailbox.org>]

On 9/30/26 8:38 AM, Koichiro Den wrote:
> 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@valinux.co.jp> 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@valinux.co.jp>
>>
>> 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@mailbox.org/
> 
> 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.
This does make sense, yes, thank you for spotting this. I will be 
sending a V5 of the PM ops patch now.

[...]