Re: [PATCH v3 3/3] PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are gated

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: dw-rockchip: leaked IRQ depth permanently masks INTx on reset failure
--

commit d458997529aaf8410a72de39760a744213ab9092
Author: Shawn Lin <shawn.lin@rock-chips.com>
Subject: PCI: dw-rockchip: Mask the INTx IRQ while the controller clocks are gated

This commit masks the INTx IRQ during root port resets to prevent synchronous
external aborts when downstream devices keep their INTx line asserted. The IRQ
is left masked on error paths since the clocks remain gated.

> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 561c296c770eb..aee5ca8a6ca84 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -924,6 +924,16 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
>  	u32 val;
>  	int ret;
>  
> +	/*
> +	 * Devices may keep their INTx line asserted across the reset. Mask
> +	 * the INTx IRQ so that the chained handler does not touch the
> +	 * unclocked APB bus, which would raise a synchronous external abort.
> +	 * The IRQ is re-enabled once the clocks are restored, and is
> +	 * deliberately left masked on the error paths where the controller
> +	 * remains unclocked.
> +	 */
> +	disable_irq(rockchip->intx_irq);

[Severity: High]
Since disable_irq() increments the IRQ descriptor depth unconditionally,
does leaving the IRQ masked on error paths cause a permanent loss of INTx
interrupts if the reset is retried?

If rockchip_pcie_rc_reset_root_port() is called again after a failure,
disable_irq() will be called a second time, incrementing the depth counter
to 2. If that retry succeeds:

[ ... ]

> @@ -974,6 +984,7 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
>  
>  	/* Ignore errors, the link may come up later */
>  	dw_pcie_wait_for_link(pci);
> +	enable_irq(rockchip->intx_irq);

the depth counter will only be decremented to 1, and the INTx interrupts for
all downstream devices will remain permanently masked. Is there a way to
ensure the enable/disable depth remains balanced when recovering from reset
failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/1790044622-164744-1-git-send-email-shawn.lin@rock-chips.com?part=3