Re: [PATCH 09/11] PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] subsystem: mixing goto and scoped_guard in rcar_gen4_pcie_rp_irq_init()
--

commit 6f1a91ac675be05b5a2f016c1ae299cc233169a3
Author: Koichiro Den <den@valinux.co.jp>
Subject: PCI: rcar-gen4: Route Root Port AER to a virtual Root Port IRQ

This commit routes the Root Port's AER notifications to a virtual Root Port
IRQ by falling back to INTx. It creates a single-entry virtual IRQ domain
for the Root Port and ensures the IRQ mask state is tracked in software for
recovery.

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 238755852045e..c24ac82a1ec65 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
> +static int rcar_gen4_pcie_rp_irq_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> +	struct device *dev = dw->dev;
> +	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");
> +
> +	/* Keep the notifications masked until a port service requests the IRQ. */
> +	scoped_guard(raw_spinlock_irqsave, &rcar->app_lock) {
> +		rcar->rp_irq_enabled = false;
> +		rcar_gen4_pcie_app_irq_sync_locked(rcar);
> +	}
> +	writel(RP_AER_MASK, rcar->base + PCIEINTSTS0CLR);
> +
[ ... ]
> +	if (irq) {
> +		/* Dispatch the child IRQ in hard IRQ context, including on PREEMPT_RT. */
> +		ret = devm_request_irq(dev, irq,
> +				       rcar_gen4_pcie_aer_irq_handler,
> +				       IRQF_NO_THREAD, "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;
> +	}
> +
> +	/*
> +	 * The Root Port MSI capabilities are hidden (no_rp_msi), so the port
> +	 * services fall back to INTx; route it to the virtual IRQ.
> +	 */
> +	pp->bridge->map_irq = rcar_gen4_pcie_map_irq;
> +
> +	return 0;
> +
> +err_dispose_mapping:
> +	irq_dispose_mapping(rcar->rp_irq);
> +	rcar->rp_irq = 0;
> +err_remove_domain:

[Severity: Low]
Does rcar_gen4_pcie_rp_irq_init() mix scoped_guard with goto-based error
handling labels?

The subsystem guidelines in include/linux/cleanup.h state that mixing
goto-based cleanup and scope-based cleanup helpers in the same function
creates confusing ownership semantics, and is explicitly prohibited.

For a given routine, it's expected to either convert all resources to
scope-based cleanup, or convert none of them.

> +	irq_domain_remove(rcar->rp_irq_domain);
> +	rcar->rp_irq_domain = NULL;
> +err_free_fwnode:
> +	irq_domain_free_fwnode(rcar->rp_fwnode);
> +	rcar->rp_fwnode = NULL;
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918032038.2216471-1-den@valinux.co.jp?part=9