[PATCH v4 0/3] Add root port reset to support link recovery

[PATCH v4 0/3] Add root port reset to support link recovery

[Richard Zhu]

Based on the following patch-set[1] issued by Mani.
Add support for resetting the Root Port for i.MX PCIe to enable link recovery.

[1] [v7,0/4] PCI: Add support for resetting the Root Ports in a platform specific way

PCIe links can go down due to various unexpected circumstances. This patch series
adds root port reset support for link recovery on i.MX PCIe controllers when the
optional "intr" interrupt is present.

When a link down event is detected, the root port reset uninitializes and
reinitializes the PCIe controller, then restarts the PCIe link.

On i.MX95 platforms, link events and PME share the same interrupt line.
Link event interrupts cannot use only an IRQ thread handler because the PME
driver uses request_irq() to bind the PME interrupt directly with only the
IRQF_SHARED flag set.

To address this, we register one handler with IRQF_SHARED for link event
interrupts and manipulate the enable bits of link events to ensure the same
interrupt source is triggered only once at a time.

Additionally, this series adds 'intr', 'aer', and 'pme' interrupt entries to
the i.MX6Q PCIe binding to support PCIe event-based interrupts for general
controller events, Advanced Error Reporting, and Power Management Events
respectively.

Changes in v4:
- Set these new added three interrupts as optional interrupt.

Changes in v3:
- Don't add a new if:block; Drop the maxItems constraint of the interrupts
  property for i.MX95 PCIe.
- Add constraints for the interrupts property for other variants.
- Regarding the ABI break: add descriptions explaining why these new
  interrupts are mandatory and required by i.MX95 PCIe.

Changes in v2:
- Constrain the new added three interrupt entries to be valid only for the
  i.MX95 variant using conditional schemas

[PATCH v4 1/3] dt-bindings: PCI: imx6q-pcie: Add intr, aer and pme
[PATCH v4 2/3] arm64: dts: imx95: Add dma, intr, aer and pme
[PATCH v4 3/3] PCI: imx6: Add root port reset to support link

Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml |   6 +++++
arch/arm64/boot/dts/freescale/imx95.dtsi                  |  16 +++++++++---
drivers/pci/controller/dwc/pci-imx6.c                     | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 141 insertions(+), 4 deletions(-)

[PATCH v4 1/3] dt-bindings: PCI: imx6q-pcie: Add intr, aer and pme interrupts

[Richard Zhu]

Add optional interrupt entries to the i.MX6Q PCIe binding to support
event-based interrupt handling:

- intr: General controller events and link state changes
- aer: Advanced Error Reporting events
- pme: Power Management Events

These interrupts enable proper handling of AER, PME, and other
controller-specific events when supported by the hardware.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 9d1349855b42..cf709132ff1e 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -58,12 +58,18 @@ properties:
     items:
       - description: builtin MSI controller.
       - description: builtin DMA controller.
+      - description: PCIe event interrupt.
+      - description: builtin AER SPI standalone interrupt line.
+      - description: builtin PME SPI standalone interrupt line.
 
   interrupt-names:
     minItems: 1
     items:
       - const: msi
       - const: dma
+      - const: intr
+      - const: aer
+      - const: pme
 
   reset-gpio:
     description: Should specify the GPIO for controlling the PCI bus device

base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
prerequisite-patch-id: b47e314ffa7c4d406797ecd88b943068a814fb9e
prerequisite-patch-id: af0a125aaa64bd3f6fcf55825d967ad6c4339716
prerequisite-patch-id: 6f434b1e2c82ec8da9b502c4287171babc53dcaf
prerequisite-patch-id: 9c6e0e10a650652f14a346a9f0ff824d53c530d1
prerequisite-patch-id: 0bddec7b62daafe2877ca92766d8b35e8c41dfe1
prerequisite-patch-id: 277480331fcb80aeee5848afa1eee4532c4a1646
prerequisite-patch-id: 9ce151e05ca8ff8e66e3b2ef37114c89dae9dbea
-- 
2.37.1

[PATCH v4 2/3] arm64: dts: imx95: Add dma, intr, aer and pme interrupts for PCIe

[Richard Zhu]

The current PCIe device tree configuration only defines the MSI
interrupt, which is sufficient for basic PCIe operation but limits
advanced functionality.

Add the following interrupt lines to pcie0 and pcie1 nodes:
- dma: DMA interrupt for PCIe DMA operations
- intr: General controller events and link state changes
- aer: Advanced Error Reporting interrupt
- pme: Power Management Event interrupt

This enables enhanced PCIe features and capabilities that were
previously unavailable due to missing interrupt definitions.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx95.dtsi | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
index adcc0e1d3696..f40388958717 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -1948,8 +1948,12 @@ pcie0: pcie@4c300000 {
 			bus-range = <0x00 0xff>;
 			num-lanes = <1>;
 			num-viewport = <8>;
-			interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi";
+			interrupts = <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi", "dma", "intr", "aer", "pme";
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0x7>;
 			interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
@@ -2023,8 +2027,12 @@ pcie1: pcie@4c380000 {
 			bus-range = <0x00 0xff>;
 			num-lanes = <1>;
 			num-viewport = <8>;
-			interrupts = <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "msi";
+			interrupts = <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 317 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 316 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi", "dma", "intr", "aer", "pme";
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0x7>;
 			interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.37.1

[PATCH v4 3/3] PCI: imx6: Add root port reset to support link recovery

[Richard Zhu]

The PCIe link can go down due to various unexpected circumstances. Add
root port reset support to enable link recovery for the i.MX PCIe
controller when the optional "intr" interrupt is present.

Reset root port to uninitialize, initialize the PCIe controller, and
restart the PCIe link at end when a link down event happens.

On i.MX95 platforms, link events and PME share the same interrupt line.
The link event interrupt cannot use a threaded-only IRQ handler because
the PME driver uses request_irq() with only the IRQF_SHARED flag set,
which requires a primary handler.

To handle this shared interrupt scenario, register a primary interrupt
handler with IRQF_SHARED for link events and manipulate the link event
enable bits to ensure the shared interrupt source triggers only one
handler at a time.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 123 ++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 1034ac5c5f5c..79c92c77b85b 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -34,6 +34,7 @@
 #include <linux/pm_runtime.h>
 
 #include "../../pci.h"
+#include "../pci-host-common.h"
 #include "pcie-designware.h"
 
 #define IMX8MQ_GPR_PCIE_REF_USE_PAD		BIT(9)
@@ -78,6 +79,10 @@
 #define IMX95_SID_MASK				GENMASK(5, 0)
 #define IMX95_MAX_LUT				32
 
+#define IMX95_LINK_INT_CTRL_STS			0x1040
+#define IMX95_LINK_DOWN_INT_STS			BIT(11)
+#define IMX95_LINK_DOWN_INT_EN			BIT(10)
+
 #define IMX95_PCIE_RST_CTRL			0x3010
 #define IMX95_PCIE_COLD_RST			BIT(0)
 
@@ -125,6 +130,8 @@ enum imx_pcie_variants {
 #define IMX_PCIE_MAX_INSTANCES	2
 
 struct imx_pcie;
+static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
+				    struct pci_dev *pdev);
 
 struct imx_pcie_drvdata {
 	enum imx_pcie_variants variant;
@@ -158,6 +165,7 @@ struct imx_pcie {
 	bool			supports_clkreq;
 	bool			enable_ext_refclk;
 	struct regmap		*iomuxc_gpr;
+	u32			lnk_intr;
 	u16			msi_ctrl;
 	u32			controller_id;
 	struct reset_control	*pciephy_reset;
@@ -1301,6 +1309,13 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 
 	imx_setup_phy_mpll(imx_pcie);
 
+	/*
+	 * Callback invoked by PCI core when link down is detected and
+	 * recovery is needed.
+	 */
+	if (pp->bridge)
+		pp->bridge->reset_root_port = imx_pcie_reset_root_port;
+
 	return 0;
 
 err_phy_off:
@@ -1568,6 +1583,9 @@ static int imx_pcie_suspend_noirq(struct device *dev)
 	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
 		return 0;
 
+	if (imx_pcie->lnk_intr)
+		regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+				  IMX95_LINK_DOWN_INT_EN);
 	imx_pcie_msi_save_restore(imx_pcie, true);
 	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
 		imx_pcie_lut_save(imx_pcie);
@@ -1618,6 +1636,9 @@ static int imx_pcie_resume_noirq(struct device *dev)
 	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
 		imx_pcie_lut_restore(imx_pcie);
 	imx_pcie_msi_save_restore(imx_pcie, false);
+	if (imx_pcie->lnk_intr)
+		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+				IMX95_LINK_DOWN_INT_EN);
 
 	return 0;
 }
@@ -1627,6 +1648,84 @@ static const struct dev_pm_ops imx_pcie_pm_ops = {
 				  imx_pcie_resume_noirq)
 };
 
+static irqreturn_t imx_pcie_lnk_irq_isr(int irq, void *priv)
+{
+	struct imx_pcie *imx_pcie = priv;
+	struct dw_pcie *pci = imx_pcie->pci;
+	struct device *dev = pci->dev;
+	u32 val;
+
+	regmap_read(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS, &val);
+	if (val & IMX95_LINK_DOWN_INT_STS) {
+		dev_dbg(dev, "PCIe link down detected, initiating recovery\n");
+		regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+				  IMX95_LINK_DOWN_INT_EN);
+		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+				IMX95_LINK_DOWN_INT_STS);
+
+		return IRQ_WAKE_THREAD;
+	} else {
+		return IRQ_NONE;
+	}
+}
+
+static irqreturn_t imx_pcie_lnk_irq_thread(int irq, void *priv)
+{
+	struct imx_pcie *imx_pcie = priv;
+	struct dw_pcie *pci = imx_pcie->pci;
+	struct dw_pcie_rp *pp = &pci->pp;
+	struct pci_dev *port;
+
+	for_each_pci_bridge(port, pp->bridge->bus)
+		if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
+			pci_host_handle_link_down(port);
+
+	regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
+			IMX95_LINK_DOWN_INT_EN);
+
+	return IRQ_HANDLED;
+}
+
+static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
+				    struct pci_dev *pdev)
+{
+	struct pci_bus *bus = bridge->bus;
+	struct dw_pcie_rp *pp = bus->sysdata;
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
+	int ret;
+
+	imx_pcie_msi_save_restore(imx_pcie, true);
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
+		imx_pcie_lut_save(imx_pcie);
+	imx_pcie_stop_link(pci);
+	imx_pcie_host_exit(pp);
+
+	ret = imx_pcie_host_init(pp);
+	if (ret) {
+		dev_err(pci->dev, "Failed to re-init PCIe\n");
+		return ret;
+	}
+	ret = dw_pcie_setup_rc(pp);
+	if (ret)
+		goto err_host_deinit;
+
+	imx_pcie_start_link(pci);
+	dw_pcie_wait_for_link(pci);
+
+	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
+		imx_pcie_lut_restore(imx_pcie);
+	imx_pcie_msi_save_restore(imx_pcie, false);
+
+	dev_dbg(pci->dev, "Root port reset completed\n");
+	return 0;
+
+err_host_deinit:
+	imx_pcie_host_exit(pp);
+
+	return ret;
+}
+
 static int imx_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1829,9 +1928,33 @@ static int imx_pcie_probe(struct platform_device *pdev)
 			val |= PCI_MSI_FLAGS_ENABLE;
 			dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
 		}
+
+		/* Get link event irq if it is present */
+		imx_pcie->lnk_intr = platform_get_irq_byname(pdev, "intr");
+		if (imx_pcie->lnk_intr > 0) {
+			ret = devm_request_threaded_irq(dev, imx_pcie->lnk_intr,
+							imx_pcie_lnk_irq_isr,
+							imx_pcie_lnk_irq_thread,
+							IRQF_SHARED,
+							"lnk", imx_pcie);
+			if (ret) {
+				dev_err_probe(dev, ret,
+					      "unable to request LNK IRQ\n");
+				goto err_host_deinit;
+			}
+
+			regmap_set_bits(imx_pcie->iomuxc_gpr,
+					IMX95_LINK_INT_CTRL_STS,
+					IMX95_LINK_DOWN_INT_EN);
+		}
 	}
 
 	return 0;
+
+err_host_deinit:
+	dw_pcie_host_deinit(&pci->pp);
+
+	return ret;
 }
 
 static void imx_pcie_shutdown(struct platform_device *pdev)
-- 
2.37.1

RE: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link recovery

[Bough Chen]

> -----Original Message-----
> From: Richard Zhu <hongxing.zhu@nxp.com>
> Sent: 2026年5月13日 10:51
> To: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> bhelgaas@google.com; Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de;
> lpieralisi@kernel.org; kwilczynski@kernel.org; mani@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org;
> Hongxing Zhu <hongxing.zhu@nxp.com>
> Subject: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link recovery
> 
> The PCIe link can go down due to various unexpected circumstances. Add root
> port reset support to enable link recovery for the i.MX PCIe controller when the
> optional "intr" interrupt is present.
> 
> Reset root port to uninitialize, initialize the PCIe controller, and restart the PCIe
> link at end when a link down event happens.
> 
> On i.MX95 platforms, link events and PME share the same interrupt line.
> The link event interrupt cannot use a threaded-only IRQ handler because the
> PME driver uses request_irq() with only the IRQF_SHARED flag set, which
> requires a primary handler.
> 
> To handle this shared interrupt scenario, register a primary interrupt handler
> with IRQF_SHARED for link events and manipulate the link event enable bits to
> ensure the shared interrupt source triggers only one handler at a time.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 123 ++++++++++++++++++++++++++
>  1 file changed, 123 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 1034ac5c5f5c..79c92c77b85b 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -34,6 +34,7 @@
>  #include <linux/pm_runtime.h>
> 
>  #include "../../pci.h"
> +#include "../pci-host-common.h"
>  #include "pcie-designware.h"
> 
>  #define IMX8MQ_GPR_PCIE_REF_USE_PAD		BIT(9)
> @@ -78,6 +79,10 @@
>  #define IMX95_SID_MASK				GENMASK(5, 0)
>  #define IMX95_MAX_LUT				32
> 
> +#define IMX95_LINK_INT_CTRL_STS			0x1040
> +#define IMX95_LINK_DOWN_INT_STS			BIT(11)
> +#define IMX95_LINK_DOWN_INT_EN			BIT(10)
> +
>  #define IMX95_PCIE_RST_CTRL			0x3010
>  #define IMX95_PCIE_COLD_RST			BIT(0)
> 
> @@ -125,6 +130,8 @@ enum imx_pcie_variants {
>  #define IMX_PCIE_MAX_INSTANCES	2
> 
>  struct imx_pcie;
> +static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
> +				    struct pci_dev *pdev);
> 
>  struct imx_pcie_drvdata {
>  	enum imx_pcie_variants variant;
> @@ -158,6 +165,7 @@ struct imx_pcie {
>  	bool			supports_clkreq;
>  	bool			enable_ext_refclk;
>  	struct regmap		*iomuxc_gpr;
> +	u32			lnk_intr;
>  	u16			msi_ctrl;
>  	u32			controller_id;
>  	struct reset_control	*pciephy_reset;
> @@ -1301,6 +1309,13 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> *pp)
> 
>  	imx_setup_phy_mpll(imx_pcie);
> 
> +	/*
> +	 * Callback invoked by PCI core when link down is detected and
> +	 * recovery is needed.
> +	 */
> +	if (pp->bridge)
> +		pp->bridge->reset_root_port = imx_pcie_reset_root_port;
> +
>  	return 0;
> 
>  err_phy_off:
> @@ -1568,6 +1583,9 @@ static int imx_pcie_suspend_noirq(struct device
> *dev)
>  	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
>  		return 0;
> 
> +	if (imx_pcie->lnk_intr)
> +		regmap_clear_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> +				  IMX95_LINK_DOWN_INT_EN);
>  	imx_pcie_msi_save_restore(imx_pcie, true);
>  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
>  		imx_pcie_lut_save(imx_pcie);
> @@ -1618,6 +1636,9 @@ static int imx_pcie_resume_noirq(struct device *dev)
>  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
>  		imx_pcie_lut_restore(imx_pcie);
>  	imx_pcie_msi_save_restore(imx_pcie, false);
> +	if (imx_pcie->lnk_intr)
> +		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
> +				IMX95_LINK_DOWN_INT_EN);
> 
>  	return 0;
>  }
> @@ -1627,6 +1648,84 @@ static const struct dev_pm_ops imx_pcie_pm_ops =
> {
>  				  imx_pcie_resume_noirq)
>  };
> 
> +static irqreturn_t imx_pcie_lnk_irq_isr(int irq, void *priv) {
> +	struct imx_pcie *imx_pcie = priv;
> +	struct dw_pcie *pci = imx_pcie->pci;
> +	struct device *dev = pci->dev;
> +	u32 val;
> +
> +	regmap_read(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS, &val);
> +	if (val & IMX95_LINK_DOWN_INT_STS) {
> +		dev_dbg(dev, "PCIe link down detected, initiating recovery\n");
> +		regmap_clear_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> +				  IMX95_LINK_DOWN_INT_EN);
> +		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
> +				IMX95_LINK_DOWN_INT_STS);

Hi Richard

Better to add comment here to point out that write the IMX95_LINK_DOWN_INT_STS means clear this bit, or mention this bit is W1C.

Regards
Haibo Chen

RE: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link recovery

[Hongxing Zhu]

> -----Original Message-----
> From: Bough Chen <haibo.chen@nxp.com>
> Sent: Wednesday, May 13, 2026 11:32 AM
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; bhelgaas@google.com; Frank Li
> <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com
> Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org;
> Hongxing Zhu <hongxing.zhu@nxp.com>
> Subject: RE: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link
> recovery
> 
> > -----Original Message-----
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> > Sent: 2026年5月13日 10:51
> > To: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > bhelgaas@google.com; Frank Li <frank.li@nxp.com>;
> > l.stach@pengutronix.de; lpieralisi@kernel.org; kwilczynski@kernel.org;
> > mani@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com
> > Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > devicetree@vger.kernel.org; imx@lists.linux.dev;
> > linux-kernel@vger.kernel.org; Hongxing Zhu <hongxing.zhu@nxp.com>
> > Subject: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link
> > recovery
> >
> > The PCIe link can go down due to various unexpected circumstances. Add
> > root port reset support to enable link recovery for the i.MX PCIe
> > controller when the optional "intr" interrupt is present.
> >
> > Reset root port to uninitialize, initialize the PCIe controller, and
> > restart the PCIe link at end when a link down event happens.
> >
> > On i.MX95 platforms, link events and PME share the same interrupt line.
> > The link event interrupt cannot use a threaded-only IRQ handler
> > because the PME driver uses request_irq() with only the IRQF_SHARED
> > flag set, which requires a primary handler.
> >
> > To handle this shared interrupt scenario, register a primary interrupt
> > handler with IRQF_SHARED for link events and manipulate the link event
> > enable bits to ensure the shared interrupt source triggers only one handler at a
> time.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 123
> > ++++++++++++++++++++++++++
> >  1 file changed, 123 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 1034ac5c5f5c..79c92c77b85b 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -34,6 +34,7 @@
> >  #include <linux/pm_runtime.h>
> >
> >  #include "../../pci.h"
> > +#include "../pci-host-common.h"
> >  #include "pcie-designware.h"
> >
> >  #define IMX8MQ_GPR_PCIE_REF_USE_PAD		BIT(9)
> > @@ -78,6 +79,10 @@
> >  #define IMX95_SID_MASK				GENMASK(5, 0)
> >  #define IMX95_MAX_LUT				32
> >
> > +#define IMX95_LINK_INT_CTRL_STS			0x1040
> > +#define IMX95_LINK_DOWN_INT_STS			BIT(11)
> > +#define IMX95_LINK_DOWN_INT_EN			BIT(10)
> > +
> >  #define IMX95_PCIE_RST_CTRL			0x3010
> >  #define IMX95_PCIE_COLD_RST			BIT(0)
> >
> > @@ -125,6 +130,8 @@ enum imx_pcie_variants {
> >  #define IMX_PCIE_MAX_INSTANCES	2
> >
> >  struct imx_pcie;
> > +static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
> > +				    struct pci_dev *pdev);
> >
> >  struct imx_pcie_drvdata {
> >  	enum imx_pcie_variants variant;
> > @@ -158,6 +165,7 @@ struct imx_pcie {
> >  	bool			supports_clkreq;
> >  	bool			enable_ext_refclk;
> >  	struct regmap		*iomuxc_gpr;
> > +	u32			lnk_intr;
> >  	u16			msi_ctrl;
> >  	u32			controller_id;
> >  	struct reset_control	*pciephy_reset;
> > @@ -1301,6 +1309,13 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> > *pp)
> >
> >  	imx_setup_phy_mpll(imx_pcie);
> >
> > +	/*
> > +	 * Callback invoked by PCI core when link down is detected and
> > +	 * recovery is needed.
> > +	 */
> > +	if (pp->bridge)
> > +		pp->bridge->reset_root_port = imx_pcie_reset_root_port;
> > +
> >  	return 0;
> >
> >  err_phy_off:
> > @@ -1568,6 +1583,9 @@ static int imx_pcie_suspend_noirq(struct device
> > *dev)
> >  	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
> >  		return 0;
> >
> > +	if (imx_pcie->lnk_intr)
> > +		regmap_clear_bits(imx_pcie->iomuxc_gpr,
> > IMX95_LINK_INT_CTRL_STS,
> > +				  IMX95_LINK_DOWN_INT_EN);
> >  	imx_pcie_msi_save_restore(imx_pcie, true);
> >  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> >  		imx_pcie_lut_save(imx_pcie);
> > @@ -1618,6 +1636,9 @@ static int imx_pcie_resume_noirq(struct device *dev)
> >  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> >  		imx_pcie_lut_restore(imx_pcie);
> >  	imx_pcie_msi_save_restore(imx_pcie, false);
> > +	if (imx_pcie->lnk_intr)
> > +		regmap_set_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > +				IMX95_LINK_DOWN_INT_EN);
> >
> >  	return 0;
> >  }
> > @@ -1627,6 +1648,84 @@ static const struct dev_pm_ops imx_pcie_pm_ops
> > = {
> >  				  imx_pcie_resume_noirq)
> >  };
> >
> > +static irqreturn_t imx_pcie_lnk_irq_isr(int irq, void *priv) {
> > +	struct imx_pcie *imx_pcie = priv;
> > +	struct dw_pcie *pci = imx_pcie->pci;
> > +	struct device *dev = pci->dev;
> > +	u32 val;
> > +
> > +	regmap_read(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS, &val);
> > +	if (val & IMX95_LINK_DOWN_INT_STS) {
> > +		dev_dbg(dev, "PCIe link down detected, initiating recovery\n");
> > +		regmap_clear_bits(imx_pcie->iomuxc_gpr,
> > IMX95_LINK_INT_CTRL_STS,
> > +				  IMX95_LINK_DOWN_INT_EN);
> > +		regmap_set_bits(imx_pcie->iomuxc_gpr,
> IMX95_LINK_INT_CTRL_STS,
> > +				IMX95_LINK_DOWN_INT_STS);
> 
> Hi Richard
> 
> Better to add comment here to point out that write the
> IMX95_LINK_DOWN_INT_STS means clear this bit, or mention this bit is W1C.
Okay, would add the comment later. Thanks.

Best Regards
Richard Zhu
> 
> Regards
> Haibo Chen

Re: [PATCH v4 1/3] dt-bindings: PCI: imx6q-pcie: Add intr, aer and pme interrupts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Strictly ordered `items` array prevents omitting intermediate optional interrupts.
- [Medium] Global addition of optional interrupts loosens schema validation for older hardware generations.
--

commit c716cc823581eb06c45ae0ee7c1441e2e4b80ec3
Author: Richard Zhu <hongxing.zhu@nxp.com>

dt-bindings: PCI: imx6q-pcie: Add intr, aer and pme interrupts

This commit adds optional interrupt entries (intr, aer, pme) to the i.MX6Q
PCIe binding to support event-based interrupt handling.

> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index 9d1349855b422..cf709132ff1e2 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> @@ -58,12 +58,18 @@ properties:
>      items:
>        - description: builtin MSI controller.
>        - description: builtin DMA controller.
> +      - description: PCIe event interrupt.
> +      - description: builtin AER SPI standalone interrupt line.
> +      - description: builtin PME SPI standalone interrupt line.

By adding these optional interrupts at the top level, the maximum number
of interrupts implicitly increases from 2 to 5 for all compatible strings
in this binding.

Will this allow older hardware generations that physically only support
2 interrupts to silently pass validation if they incorrectly specify up to 5?

Should the existing allOf conditional blocks explicitly restrict maxItems
for the interrupts property on variants lacking these new capabilities?

>  
>    interrupt-names:
>      minItems: 1
>      items:
>        - const: msi
>        - const: dma
> +      - const: intr
> +      - const: aer
> +      - const: pme

If a hardware variant supports the msi and intr interrupts but lacks the
optional dma interrupt, won't this strict array validation fail?

Since dt-schema enforces exact positional matching for items arrays, a device
tree specifying interrupt-names = "msi", "intr" would have the "intr" entry
evaluated against the second constraint (- const: dma).

Does this need an enum/anyOf structure with minItems/maxItems, or specific
allOf conditionals per SoC to correctly handle omitting intermediate entries?

>  
>    reset-gpio:
>      description: Should specify the GPIO for controlling the PCI bus device

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513025101.1498104-1-hongxing.zhu@nxp.com?part=1

Re: [PATCH v4 3/3] PCI: imx6: Add root port reset to support link recovery

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] Optional IRQ negative error code implicitly cast to `u32` causes probe failure on older platforms.
- [High] Iterating over `bus->devices` without acquiring `pci_bus_sem` creates a Use-After-Free race condition.
- [High] Read-Modify-Write regmap operations on W1C status bits silently clear unhandled interrupts.
- [High] Disabling hardware clocks while a shared interrupt remains active enables a synchronous external abort vector.
- [High] Using a standard mmio regmap from a primary hardirq handler causes sleep-in-atomic deadlocks on PREEMPT_RT.
- [Medium] System shutdown callback inadvertently triggers a link recovery thread, leaving hardware out of reset.
- [Medium] Non-atomic LUT save/restore operations in the recovery thread lack mutex protection.
- [Medium] Failure to mask the hardware interrupt on driver unbind causes an unhandled interrupt storm.
--

commit c32252b83882f7c8756d22c0a5aa880135207cb1
Author: Richard Zhu <hongxing.zhu@nxp.com>

PCI: imx6: Add root port reset to support link recovery

The PCIe link can go down due to various unexpected circumstances. Add
root port reset support to enable link recovery for the i.MX PCIe
controller when the optional "intr" interrupt is present.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index e35044cc52185..38e0a16655092 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
[ ... ]
> @@ -1573,6 +1588,9 @@ static int imx_pcie_suspend_noirq(struct device *dev)
>  	if (!(imx_pcie->drvdata->flags & IMX_PCIE_FLAG_SUPPORTS_SUSPEND))
>  		return 0;
>  
> +	if (imx_pcie->lnk_intr)
> +		regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
> +				  IMX95_LINK_DOWN_INT_EN);

Does using regmap_clear_bits() and regmap_set_bits() here unintentionally
clear pending Write-1-to-Clear (W1C) status bits?

Since these functions perform a read-modify-write operation, if a W1C status
bit like IMX95_LINK_DOWN_INT_STS is active during the read phase, writing it
back will clear the status bit. This could potentially result in losing
pending link-down or PME events before their handlers can process them.

[ ... ]
> @@ -1632,6 +1653,84 @@ static const struct dev_pm_ops imx_pcie_pm_ops = {
>  				  imx_pcie_resume_noirq)
>  };
>  
> +static irqreturn_t imx_pcie_lnk_irq_isr(int irq, void *priv)
> +{
> +	struct imx_pcie *imx_pcie = priv;
> +	struct dw_pcie *pci = imx_pcie->pci;
> +	struct device *dev = pci->dev;
> +	u32 val;
> +
> +	regmap_read(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS, &val);

Could this regmap_read() call cause a sleep-in-atomic deadlock on PREEMPT_RT?

Since this handler is registered with IRQF_SHARED, it runs in primary hardirq
context. The iomuxc_gpr regmap is initialized via devm_regmap_init_mmio()
without enabling the use_raw_spinlock flag, meaning it relies on a standard
spinlock. On PREEMPT_RT kernels, this maps to a sleepable rt_mutex, which
cannot be acquired safely from hardirq context.

> +	if (val & IMX95_LINK_DOWN_INT_STS) {
> +		dev_dbg(dev, "PCIe link down detected, initiating recovery\n");
> +		regmap_clear_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
> +				  IMX95_LINK_DOWN_INT_EN);
> +		regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
> +				IMX95_LINK_DOWN_INT_STS);
> +
> +		return IRQ_WAKE_THREAD;
> +	} else {
> +		return IRQ_NONE;
> +	}
> +}
> +
> +static irqreturn_t imx_pcie_lnk_irq_thread(int irq, void *priv)
> +{
> +	struct imx_pcie *imx_pcie = priv;
> +	struct dw_pcie *pci = imx_pcie->pci;
> +	struct dw_pcie_rp *pp = &pci->pp;
> +	struct pci_dev *port;
> +
> +	for_each_pci_bridge(port, pp->bridge->bus)

Is it safe to iterate over the bus devices list here without acquiring the
pci_bus_sem read lock?

Since this threaded handler executes asynchronously, concurrent PCI subsystem
modifications like hotplug events or driver unbinding could mutate the list.
Traversing bus->devices without down_read(&pci_bus_sem) might expose the
system to list corruption or use-after-free dereferences.

> +		if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
> +			pci_host_handle_link_down(port);
> +
> +	regmap_set_bits(imx_pcie->iomuxc_gpr, IMX95_LINK_INT_CTRL_STS,
> +			IMX95_LINK_DOWN_INT_EN);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int imx_pcie_reset_root_port(struct pci_host_bridge *bridge,
> +				    struct pci_dev *pdev)
> +{
> +	struct pci_bus *bus = bridge->bus;
> +	struct dw_pcie_rp *pp = bus->sysdata;
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
> +	int ret;
> +
> +	imx_pcie_msi_save_restore(imx_pcie, true);
> +	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_LUT))
> +		imx_pcie_lut_save(imx_pcie);

Should these LUT save and restore operations be protected by the
imx_pcie->lock mutex?

Other parts of the driver appear to protect concurrent LUT accesses using
this mutex. Since the recovery thread runs asynchronously, racing with
concurrent endpoint configuration routines could lead to interleaved I/O
operations and corrupt the LUT index.

> +	imx_pcie_stop_link(pci);
> +	imx_pcie_host_exit(pp);

Does fully disabling the clocks here risk a synchronous external abort?

The imx_pcie_host_exit() function completely disables the PCIe controller
clocks. If another device sharing this interrupt line triggers an interrupt
while the clocks are off, the kernel will invoke all registered primary
handlers. If the PME handler attempts to read PCIe Root Port configuration
registers, accessing unclocked memory-mapped registers could trigger a
synchronous external abort.

[ ... ]
> @@ -1834,9 +1933,33 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  			val |= PCI_MSI_FLAGS_ENABLE;
>  			dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
>  		}
> +
> +		/* Get link event irq if it is present */
> +		imx_pcie->lnk_intr = platform_get_irq_byname(pdev, "intr");
> +		if (imx_pcie->lnk_intr > 0) {

Could this check fail to handle missing optional interrupts on older
platforms?

The platform_get_irq_byname() function returns a negative error code like
-ENXIO when the interrupt is missing. Because lnk_intr is declared as a u32,
the negative error code is implicitly cast to a large positive integer.
The condition > 0 will evaluate to true, and the driver will attempt to
request an invalid massive IRQ number, unconditionally failing the probe.

> +			ret = devm_request_threaded_irq(dev, imx_pcie->lnk_intr,
> +							imx_pcie_lnk_irq_isr,
> +							imx_pcie_lnk_irq_thread,
> +							IRQF_SHARED,
> +							"lnk", imx_pcie);
> +			if (ret) {
> +				dev_err_probe(dev, ret,
> +					      "unable to request LNK IRQ\n");
> +				goto err_host_deinit;
> +			}
> +
> +			regmap_set_bits(imx_pcie->iomuxc_gpr,
> +					IMX95_LINK_INT_CTRL_STS,
> +					IMX95_LINK_DOWN_INT_EN);

Is there a mechanism to disable the hardware interrupt on driver unbind?

The interrupt is enabled in the IOMUXC GPR register during probe. If the
driver is unbound and its IRQ handler is freed without a removal callback
clearing the IMX95_LINK_DOWN_INT_EN bit, the hardware could assert the
shared interrupt continuously if the link goes down. Since no handler
would be registered to acknowledge it, this might lead to an unhandled
interrupt storm.

> +		}
>  	}
>  
>  	return 0;
> +
> +err_host_deinit:
> +	dw_pcie_host_deinit(&pci->pp);
> +
> +	return ret;
>  }
>  
>  static void imx_pcie_shutdown(struct platform_device *pdev)

Will asserting the core reset during shutdown unintentionally trigger the
link recovery thread?

During system reboot, imx_pcie_shutdown() calls imx_pcie_assert_core_reset()
to quiesce the hardware, which physically drops the PCIe link. Because the
interrupt enable bit is not cleared during shutdown, this intentional link
drop will trigger the hardware interrupt.

The resulting interrupt will queue imx_pcie_lnk_irq_thread(), which calls
imx_pcie_reset_root_port(). This will explicitly deassert the core reset,
unintentionally bringing the hardware back out of reset and defeating the
purpose of the shutdown routine.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513025101.1498104-1-hongxing.zhu@nxp.com?part=3