Re: [PATCH v5 13/17] PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ

[Koichiro Den]

On Wed, Oct 21, 2026 at 02:19:46AM +0200, Marek Vasut wrote:
> On 10/20/26 5:10 AM, Koichiro Den wrote:
> > On R-Car Gen4 SoCs, the APP block exposes the Root Port interrupt
> > signals in PCIEINTSTS0 and routes them to platform IRQs: AER to
> > intreq_pcim_lperr, and PME and bandwidth notifications to
> > intreq_pcim_sub. On an R-Car S4 with GIC ITS, a link retrain set
> > LnkSta.LBMS and latched PCIEINTSTS0[23] (cfg_bw_mgt_msi), but the Root
> > Port's MSI never fired while endpoint MSIs worked fine. With iMSI-RX the
> > Root Port MSI capabilities are already hidden, but its INTx maps to
> > intreq_pcim_sub. Note that even before this driver took over the IRQ
> > with rcar_gen4_pcie_msi_irq_handler, the port service was unable to
> > request it, as the DWC core used that IRQ for its chained MSI handler.
> > In either case, iMSI-RX or an external MSI controller, Root Port
> > interrupts are not delivered to the port services.
> > 
> > Hide the Root Port MSI/MSI-X capabilities so the port services use
> > INTx regardless of the MSI controller. Do this in the R-Car host setup,
> > which also runs on reset and resume. Use a virtual IRQ to deliver
> > notifications from separate platform IRQs to the standard handlers.
> > Route AER from the optional "aer" interrupt. DTs without it remain
> > supported, but Root Port AER stays unavailable.
> > 
> > The APP block may still receive MSI-form notifications because hiding
> > the Root Port's MSI capabilities does not clear MSICAP0.MSIE. Handle
> > these alongside the INTx-form level signals. Clear the MSI-form latch
> > before the AER handler clears Root Error Status, so a late APP clear
> > does not erase a new notification.
> > 
> > Check for reset requests before dispatch: on S4, DBI accesses from
> > the port services can hang the host immediately after link down.
> > 
> > The next patch also feeds PME and bandwidth notifications from "msi"
> > into this IRQ, so both parents may dispatch concurrently.
> > handle_level_irq() calls irq_ack() before checking whether it can run
> > the handlers. Keep the APP latches set while the IRQ is in progress or
> > disabled, so the level parents can retry through the reset checks.
> > This avoids losing notifications without a raw lock around port
> > services that take regular spinlocks.
> > 
> > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > ---
> > Changes since v4:
> >    - Hide the Root Port MSI/MSI-X capabilities in host_setup() instead of
> >      adding a no_rp_msi flag to the DWC core. (Shawn)
> >      https://lore.kernel.org/r/bi3rvyqpdu4iq6pfyrqvx7eplyim6gmg2ijal67mpateobmtlq@gec7zewfm76r/
> >      I did not carry Marek's Reviewed-by tag over due to the change.
> 
> Bjorn/Manivannan, can you please have a look at this patch and the MSI
> handling, to be sure this is OK ?
> 
> (no further comments from me below this point)
> 
> Thank you !

Marek, thanks for the review.

Bjorn/Mani, just a gentle ping. Could one of you take a look? To summarize:

- v4 had 'no_rp_msi' flag to the DWC core. Shawn suggested keeping that in the
  glue driver instead, which I found reasonable:
  https://lore.kernel.org/r/20261015033103.3076869-14-den@xxxxxxxxxxxxx/
- v5 hides them in rcar_gen4_pcie_host_setup(). (this patch)

If you'd rather I send v6 first, rebased on the pp->msi_irq_domain rename Shawn
and Geert pointed out, please let me know. Otherwise I'll wait for your comments
before respinning.

Best regards,
Koichiro Den

> 
> >   drivers/pci/controller/dwc/pcie-rcar-gen4.c | 213 +++++++++++++++++++-
> >   1 file changed, 210 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > index 50d4b74e63b0..b0440f2c5329 100644
> > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > @@ -14,10 +14,12 @@
> >   #include <linux/io.h>
> >   #include <linux/iopoll.h>
> >   #include <linux/irqchip/arm-gic-v3.h>
> > +#include <linux/irqdomain.h>
> >   #include <linux/module.h>
> >   #include <linux/of.h>
> >   #include <linux/of_address.h>
> >   #include <linux/of_irq.h>
> > +#include <linux/of_pci.h>
> >   #include <linux/pci.h>
> >   #include <linux/phy/phy.h>
> >   #include <linux/platform_device.h>
> > @@ -59,6 +61,11 @@
> >   #define SMLH_LINK_UP		BIT(7)
> >   #define RDLH_LINK_UP		BIT(6)
> > +/* Root Port notifications routed to intreq_pcim_lperr ("aer") */
> > +#define AER_RC_ERR_MSI		BIT(29)
> > +#define AER_RC_ERR_INT		BIT(27)
> > +#define RP_AER_MASK		(AER_RC_ERR_MSI | AER_RC_ERR_INT)
> > +
> >   /* PCIe DMA Interrupt Status Enable */
> >   #define PCIEDMAINTSTSEN		0x0314
> >   #define PCIEDMAINTSTSEN_INIT	GENMASK(15, 0)
> > @@ -68,6 +75,7 @@
> >   /* PCIe Interrupt Status 0 Clear */
> >   #define PCIEINTSTS0CLR		0x0340
> > +#define RP_LATCHED_MASK		AER_RC_ERR_MSI
> >   /* Port Logic Registers 89 */
> >   #define PRTLGC89		0x0b70
> > @@ -133,6 +141,12 @@ struct rcar_gen4_pcie {
> >   	struct work_struct link_down_work;
> >   	/* Allow Root Port event handling after enumeration and until teardown. */
> >   	bool rp_events_armed;
> > +	/* Virtual Root Port INTx shared by the port services */
> > +	struct irq_domain *rp_irq_domain;
> > +	unsigned int rp_irq;
> > +	bool rp_irq_enabled;
> > +	/* intreq_pcim_lperr ("aer"): Root Port AER notifications, optional */
> > +	int aer_irq;
> >   	/*
> >   	 * The controller needs reinitializing; the APP interrupt sources
> >   	 * stay masked until a reset succeeds.
> > @@ -565,7 +579,7 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)
> >   	lockdep_assert_held(&rcar->app_lock);
> >   	val = readl(rcar->base + PCIEINTSTS0EN);
> > -	val &= ~MSI_CTRL_INT;
> > +	val &= ~(MSI_CTRL_INT | RP_AER_MASK);
> >   	/*
> >   	 * Note that rcar->rp_events_armed is not checked here because device
> >   	 * removal may still need iMSI-RX interrupts after Root Port events have
> > @@ -573,6 +587,8 @@ static void rcar_gen4_pcie_app_irq_sync_locked(struct rcar_gen4_pcie *rcar)
> >   	 */
> >   	if (!rcar->reinit_pending && pp->use_imsi_rx && pci_msi_enabled())
> >   		val |= MSI_CTRL_INT;
> > +	if (rp_events && rcar->rp_irq_enabled && rcar->aer_irq > 0)
> > +		val |= RP_AER_MASK;
> >   	writel(val, rcar->base + PCIEINTSTS0EN);
> >   	val = readl(rcar->base + PCIERSTSTSEN);
> > @@ -675,6 +691,15 @@ static bool rcar_gen4_pcie_handle_link_down(struct rcar_gen4_pcie *rcar)
> >   	return rcar->reinit_pending;
> >   }
> > +static void rcar_gen4_pcie_rp_irq_dispatch(struct rcar_gen4_pcie *rcar)
> > +{
> > +	/* Recheck reset requests after the parent's status read. */
> > +	if (rcar_gen4_pcie_handle_link_down(rcar))
> > +		return;
> > +
> > +	generic_handle_domain_irq(rcar->rp_irq_domain, 0);
> > +}
> > +
> >   static void rcar_gen4_pcie_link_down_work(struct work_struct *work)
> >   {
> >   	struct rcar_gen4_pcie *rcar =
> > @@ -769,6 +794,162 @@ static void rcar_gen4_pcie_rp_events_disarm(struct rcar_gen4_pcie *rcar)
> >   	cancel_work_sync(&rcar->link_down_work);
> >   }
> > +static void rcar_gen4_pcie_rp_irq_mask(struct irq_data *d)
> > +{
> > +	struct rcar_gen4_pcie *rcar = irq_data_get_irq_chip_data(d);
> > +
> > +	guard(raw_spinlock_irqsave)(&rcar->app_lock);
> > +	rcar->rp_irq_enabled = false;
> > +	rcar_gen4_pcie_app_irq_sync_locked(rcar);
> > +}
> > +
> > +static void rcar_gen4_pcie_rp_irq_unmask(struct irq_data *d)
> > +{
> > +	struct rcar_gen4_pcie *rcar = irq_data_get_irq_chip_data(d);
> > +
> > +	guard(raw_spinlock_irqsave)(&rcar->app_lock);
> > +	rcar->rp_irq_enabled = true;
> > +	rcar_gen4_pcie_app_irq_sync_locked(rcar);
> > +}
> > +
> > +static void rcar_gen4_pcie_rp_irq_ack(struct irq_data *d)
> > +{
> > +	struct rcar_gen4_pcie *rcar = irq_data_get_irq_chip_data(d);
> > +
> > +	/*
> > +	 * handle_level_irq() may skip the handlers after calling irq_ack().
> > +	 * Keep notifications latched if the IRQ is disabled or in progress,
> > +	 * so they raise a parent IRQ again after unmasking. The descriptor
> > +	 * lock serializes this check with IRQD_IRQ_INPROGRESS.
> > +	 */
> > +	if (irqd_irq_disabled(d) || irqd_irq_inprogress(d))
> > +		return;
> > +
> > +	/* Clear APP latches before the service clears Root Error Status. */
> > +	writel(RP_LATCHED_MASK, rcar->base + PCIEINTSTS0CLR);
> > +}
> > +
> > +static const struct irq_chip rcar_gen4_pcie_rp_irq_chip = {
> > +	.name = "rcar-gen4-rp",
> > +	.irq_mask = rcar_gen4_pcie_rp_irq_mask,
> > +	.irq_unmask = rcar_gen4_pcie_rp_irq_unmask,
> > +	.irq_ack = rcar_gen4_pcie_rp_irq_ack,
> > +};
> > +
> > +static int rcar_gen4_pcie_rp_irq_map(struct irq_domain *domain,
> > +				     unsigned int irq, irq_hw_number_t hwirq)
> > +{
> > +	irq_set_chip_and_handler(irq, &rcar_gen4_pcie_rp_irq_chip,
> > +				 handle_level_irq);
> > +	irq_set_chip_data(irq, domain->host_data);
> > +	irq_set_status_flags(irq, IRQ_LEVEL);
> > +	irq_set_noprobe(irq);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct irq_domain_ops rcar_gen4_pcie_rp_irq_domain_ops = {
> > +	.map = rcar_gen4_pcie_rp_irq_map,
> > +};
> > +
> > +static irqreturn_t rcar_gen4_pcie_aer_irq_handler(int irq, void *data)
> > +{
> > +	struct rcar_gen4_pcie *rcar = data;
> > +	u32 status;
> > +
> > +	/* The line also fires on link down; reset requests come first. */
> > +	if (rcar_gen4_pcie_handle_link_down(rcar))
> > +		return IRQ_HANDLED;
> > +
> > +	status = readl(rcar->base + PCIEINTSTS0) &
> > +		 readl(rcar->base + PCIEINTSTS0EN);
> > +	if (!(status & RP_AER_MASK))
> > +		return IRQ_NONE;
> > +
> > +	rcar_gen4_pcie_rp_irq_dispatch(rcar);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int rcar_gen4_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> > +{
> > +	struct pci_host_bridge *bridge = pci_find_host_bridge(dev->bus);
> > +	struct rcar_gen4_pcie *rcar = dev_get_drvdata(bridge->dev.parent);
> > +
> > +	/* Downstream devices use standard OF INTx mapping */
> > +	if (!pci_is_root_bus(dev->bus))
> > +		return of_irq_parse_and_map_pci(dev, slot, pin);
> > +
> > +	/* Root Port uses the virtual IRQ (always INTA) */
> > +	return rcar->rp_irq;
> > +}
> > +
> > +static int rcar_gen4_pcie_rp_irq_init(struct rcar_gen4_pcie *rcar)
> > +{
> > +	struct device *dev = rcar->dw.dev;
> > +	struct fwnode_handle *fwnode;
> > +	int irq, ret;
> > +
> > +	irq = platform_get_irq_byname_optional(rcar->pdev, "aer");
> > +	if (irq == -ENXIO)
> > +		irq = 0;	/* old DT: Root Port AER stays unavailable */
> > +	else if (irq < 0)
> > +		return dev_err_probe(dev, irq, "Failed to get AER IRQ\n");
> > +
> > +	fwnode = irq_domain_alloc_named_fwnode(dev_name(dev));
> > +	if (!fwnode)
> > +		return -ENOMEM;
> > +
> > +	rcar->rp_irq_domain = irq_domain_create_linear(fwnode, 1,
> > +						       &rcar_gen4_pcie_rp_irq_domain_ops,
> > +						       rcar);
> > +	if (!rcar->rp_irq_domain) {
> > +		ret = -ENOMEM;
> > +		goto err_free_fwnode;
> > +	}
> > +
> > +	rcar->rp_irq = irq_create_mapping(rcar->rp_irq_domain, 0);
> > +	if (!rcar->rp_irq) {
> > +		ret = -ENOMEM;
> > +		goto err_remove_domain;
> > +	}
> > +
> > +	if (irq) {
> > +		/*
> > +		 * Dispatch the child IRQ in hard IRQ context, including on
> > +		 * PREEMPT_RT. Enable it together with the "msi" parent IRQ.
> > +		 */
> > +		ret = devm_request_irq(dev, irq, rcar_gen4_pcie_aer_irq_handler,
> > +				       IRQF_NO_THREAD | IRQF_NO_AUTOEN,
> > +				       "rcar-gen4-pcie-aer", rcar);
> > +		if (ret) {
> > +			dev_err_probe(dev, ret, "Failed to request AER IRQ\n");
> > +			goto err_dispose_mapping;
> > +		}
> > +		rcar->aer_irq = irq;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_dispose_mapping:
> > +	irq_dispose_mapping(rcar->rp_irq);
> > +err_remove_domain:
> > +	irq_domain_remove(rcar->rp_irq_domain);
> > +err_free_fwnode:
> > +	irq_domain_free_fwnode(fwnode);
> > +
> > +	return ret;
> > +}
> > +
> > +static void rcar_gen4_pcie_rp_irq_deinit(struct rcar_gen4_pcie *rcar)
> > +{
> > +	struct fwnode_handle *fwnode = rcar->rp_irq_domain->fwnode;
> > +
> > +	irq_dispose_mapping(rcar->rp_irq);
> > +	irq_domain_remove(rcar->rp_irq_domain);
> > +	irq_domain_free_fwnode(fwnode);
> > +}
> > +
> >   static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
> >   					struct pci_dev *dev)
> >   {
> > @@ -854,6 +1035,13 @@ static int rcar_gen4_pcie_host_setup(struct dw_pcie_rp *pp)
> >   	if (ret)
> >   		return ret;
> > +	/*
> > +	 * Root Port MSIs are not delivered. Hide the MSI/MSI-X capabilities
> > +	 * so port services use the virtual INTx IRQ.
> > +	 */
> > +	dw_pcie_remove_capability(dw, PCI_CAP_ID_MSI);
> > +	dw_pcie_remove_capability(dw, PCI_CAP_ID_MSIX);
> > +
> >   	msleep(PCIE_T_PVPERL_MS);	/* pe_rst requires 100msec delay */
> >   	rcar_gen4_pcie_host_perst_assert(pp, false);
> > @@ -869,6 +1057,8 @@ static void rcar_gen4_pcie_quiesce_irqs(struct rcar_gen4_pcie *rcar)
> >   	/* The MSI status lives in DBI; keep the handler away during the reset. */
> >   	disable_irq(rcar->msi_irq);
> > +	if (rcar->aer_irq > 0)
> > +		disable_irq(rcar->aer_irq);
> >   }
> >   static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,
> > @@ -879,6 +1069,8 @@ static void rcar_gen4_pcie_resume_irqs(struct rcar_gen4_pcie *rcar,
> >   		rcar_gen4_pcie_app_irq_sync_locked(rcar);
> >   	}
> > +	if (rcar->aer_irq > 0)
> > +		enable_irq(rcar->aer_irq);
> >   	enable_irq(rcar->msi_irq);
> >   }
> > @@ -945,6 +1137,8 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
> >   	if (pp->bridge) {
> >   		pp->bridge->enable_device = rcar_gen4_pcie_enable_device;
> >   		pp->bridge->reset_root_port = rcar_gen4_pcie_reset_root_port;
> > +		/* Route Root Port INTx to the virtual IRQ. */
> > +		pp->bridge->map_irq = rcar_gen4_pcie_map_irq;
> >   	}
> >   	rcar_gen4_pcie_host_perst_assert(pp, true);
> > @@ -1003,10 +1197,14 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> >   	raw_spin_lock_init(&rcar->app_lock);
> >   	INIT_WORK(&rcar->link_down_work, rcar_gen4_pcie_link_down_work);
> > -	ret = rcar_gen4_pcie_msi_irq_init(rcar);
> > +	ret = rcar_gen4_pcie_rp_irq_init(rcar);
> >   	if (ret)
> >   		return ret;
> > +	ret = rcar_gen4_pcie_msi_irq_init(rcar);
> > +	if (ret)
> > +		goto err_rp_irq_deinit;
> > +
> >   	pp->num_vectors = MAX_MSI_IRQS;
> >   	/* Reserve an iATU window for the generic PME_Turn_Off implementation. */
> >   	pp->use_atu_msg = true;
> > @@ -1018,7 +1216,15 @@ static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> >   	pp->msi_irq[0] = -ENODEV;
> >   	pp->ops = &rcar_gen4_pcie_host_ops;
> > -	return dw_pcie_host_init(pp);
> > +	ret = dw_pcie_host_init(pp);
> > +	if (ret)
> > +		goto err_rp_irq_deinit;
> > +
> > +	return 0;
> > +
> > +err_rp_irq_deinit:
> > +	rcar_gen4_pcie_rp_irq_deinit(rcar);
> > +	return ret;
> >   }
> >   static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> > @@ -1026,6 +1232,7 @@ static void rcar_gen4_remove_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
> >   	/* Cancel recovery work before removing the PCI bus */
> >   	rcar_gen4_pcie_rp_events_disarm(rcar);
> >   	dw_pcie_host_deinit(&rcar->dw.pp);
> > +	rcar_gen4_pcie_rp_irq_deinit(rcar);
> >   }
> >   /* Endpoint mode */
> 
> 
> -- 
> Best regards,
> Marek Vasut