[PATCH v3] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing

[PATCH v3] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing

[hongxing.zhu]

From: Richard Zhu <hongxing.zhu@nxp.com>

Commit 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators")
introduced a boot hang on i.MX6Q/DL variants by reordering the
initialization sequence, which exposed a critical PHY power control issue.

Root cause:
Before commit 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators"),
the sequence was:
  1. imx_pcie_assert_core_reset() - asserts TEST_PD, asserts REF_CLK_EN
  2. imx_pcie_clk_enable() - clears TEST_PD, asserts REF_CLK_EN
  3. Link training starts with TEST_PD properly cleared ✓

After commit 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators"),
the sequence became:
  1. imx_pcie_clk_enable() - clears TEST_PD, asserts REF_CLK_EN
  2. imx_pcie_assert_core_reset() - re-asserts TEST_PD, asserts REF_CLK_EN
  3. imx_pcie_deassert_core_reset() - does NOT clear TEST_PD
  4. Link training starts with TEST_PD still asserted ✗

The reordering caused TEST_PD to be cleared prematurely in clk_enable(),
then re-asserted by assert_core_reset(), and never cleared again before
link training, resulting in the boot hang.

The fix requires two interdependent changes that cannot be split:
1. Move TEST_PD control to imx6q_pcie_core_reset() where it belongs
   logically with reset operations
2. Remove TEST_PD manipulation from imx6q_pcie_enable_ref_clk() to
   prevent premature clearing

Applying only change #1 results in device detection failure because
TEST_PD gets cleared too early in clk_enable(), then re-asserted in
assert_core_reset(), then cleared again in deassert_core_reset(). This
premature clearing disrupts the proper PHY power-up sequence.

Both changes together ensure the correct sequence:
  1. REF_CLK_EN asserted in clk_enable() (TEST_PD untouched)
  2. TEST_PD asserted in assert_core_reset()
  3. TEST_PD cleared in deassert_core_reset()
  4. Link training starts with proper PHY state ✓

The previous delay in imx6q_pcie_enable_ref_clk() was a workaround for
async reset synchronization when ref clock and PHY power were coupled.
With proper sequencing, this delay is no longer needed.

i.MX6Q/DL PCIe PHY requires ~120us between TEST_PD de-assertion and link
training start. This timing is already satisfied by the PERST# toggling
sequence in imx_pcie_assert_perst(), which is called after
imx_pcie_deassert_core_reset(). The PERST# assertion time is much longer
than 120us, providing sufficient delay.

Additional changes:
- Update imx6qp_pcie_core_reset() to call imx6q_pcie_core_reset() for
  shared PHY power management, avoiding code duplication
- Add explicit imx_pcie_assert_core_reset() calls in error paths to
  ensure proper cleanup

Fixes: 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators")
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reported-by: Leonardo Costa <leoreis.costa@gmail.com>
Closes: https://lore.kernel.org/lkml/20260629143439.361560-1-leoreis.costa@gmail.com/
Tested-by: Leonardo Costa <leonardo.costa@toradex.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v3:
Update the commit descriptions to address the following items.
- Clarify the root cause of this regresssion.
- Describe why both changes are mandatory required to fix this
  regression.
- Justify the delay removal also.

Changes in v2:
Per Sashiko's review, invoke imx_pcie_assert_core_reset() explicitly
in error path of imx_pcie_host_init() and imx_pcie_host_exit().
---
 drivers/pci/controller/dwc/pci-imx6.c | 45 +++++++++++----------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index e25f938eefe23..45ab21c9769a4 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -681,21 +681,12 @@ static int imx_pcie_attach_pd(struct device *dev)
 
 static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
 {
-	if (enable) {
-		/* power up core phy and enable ref clock */
-		regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD);
-		/*
-		 * The async reset input need ref clock to sync internally,
-		 * when the ref clock comes after reset, internal synced
-		 * reset time is too short, cannot meet the requirement.
-		 * Add a ~10us delay here.
-		 */
-		usleep_range(10, 100);
-		regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN);
-	} else {
-		regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN);
-		regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD);
-	}
+	if (enable)
+		regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				IMX6Q_GPR1_PCIE_REF_CLK_EN);
+	else
+		regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				  IMX6Q_GPR1_PCIE_REF_CLK_EN);
 
 	return 0;
 }
@@ -824,23 +815,23 @@ static int imx6sx_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
 	return 0;
 }
 
-static int imx6qp_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
+static int imx6q_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
 {
-	regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_SW_RST,
-			   assert ? IMX6Q_GPR1_PCIE_SW_RST : 0);
-	if (!assert)
-		usleep_range(200, 500);
+	if (assert)
+		regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				IMX6Q_GPR1_PCIE_TEST_PD);
+	else
+		regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1,
+				  IMX6Q_GPR1_PCIE_TEST_PD);
 
 	return 0;
 }
 
-static int imx6q_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
+static int imx6qp_pcie_core_reset(struct imx_pcie *imx_pcie, bool assert)
 {
-	if (!assert)
-		return 0;
-
-	regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD);
-	regmap_set_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_REF_CLK_EN);
+	imx6q_pcie_core_reset(imx_pcie, assert);
+	regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_SW_RST,
+			   assert ? IMX6Q_GPR1_PCIE_SW_RST : 0);
 
 	return 0;
 }
@@ -1451,6 +1442,7 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 	return 0;
 
 err_phy_off:
+	imx_pcie_assert_core_reset(imx_pcie);
 	phy_power_off(imx_pcie->phy);
 err_phy_exit:
 	phy_exit(imx_pcie->phy);
@@ -1475,6 +1467,7 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
 			dev_err(pci->dev, "unable to power off PHY\n");
 		phy_exit(imx_pcie->phy);
 	}
+	imx_pcie_assert_core_reset(imx_pcie);
 	imx_pcie_clk_disable(imx_pcie);
 
 	if (!pci->pp.skip_pwrctrl_off)
-- 
2.34.1

Re: [PATCH v3] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The removal of explicit PHY power-up delays causes a boot regression on boards without a PERST# GPIO or explicit Root Port nodes in the device tree.
- [Medium] Incorrect teardown ordering in `imx_pcie_host_exit()` violates LIFO symmetry by asserting the core reset after the PHY is powered off and exited.
--

--- Patch [1]: [PATCH v3] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing ---
commit 00ce7804e098bc0e8573a15a850f6250a9cb1245
Author: Richard Zhu <hongxing.zhu@nxp.com>
PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing

This commit fixes a boot hang on i.MX6Q/DL variants by correcting the PHY power
sequencing and TEST_PD control logic. It removes an explicit delay during clock
enable, assuming the PERST# toggling sequence provides sufficient stabilization
time. It also adds explicit core reset assertions in the error paths.

Link: https://lore.kernel.org/lkml/20260629143439.361560-1-leoreis.costa@gmail.com/

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index e25f938eefe23..45ab21c9769a4 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -681,21 +681,12 @@ static int imx_pcie_attach_pd(struct device *dev)
>  
>  static int imx6q_pcie_enable_ref_clk(struct imx_pcie *imx_pcie, bool enable)
>  {
> -	if (enable) {
> -		/* power up core phy and enable ref clock */
> -		regmap_clear_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_TEST_PD);
> -		/*
> -		 * The async reset input need ref clock to sync internally,
> -		 * when the ref clock comes after reset, internal synced
> -		 * reset time is too short, cannot meet the requirement.
> -		 * Add a ~10us delay here.
> -		 */
> -		usleep_range(10, 100);

[Severity: High]
The commit message states that the ~120us delay needed between TEST_PD
de-assertion and link training is satisfied by the PERST# toggling sequence.
Does this break boards that don't define a reset GPIO (like
imx6qdl-sabrelite.dtsi)?

If the device tree lacks a PERST# GPIO or explicit Root Port nodes,
imx_pcie_assert_perst() returns immediately:

static void imx_pcie_assert_perst(struct imx_pcie *imx_pcie, bool assert)
{
        [ ... ]
        if (!bridge || list_empty(&bridge->ports))
                return;
        [ ... ]
}

Because of this early return, there will be no delay before link training
begins in imx_pcie_host_init(). Could this lead to a boot hang on these
boards due to the missing PHY stabilization delay?

[ ... ]

> @@ -1475,6 +1467,7 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
>  		if (phy_power_off(imx_pcie->phy))
>  			dev_err(pci->dev, "unable to power off PHY\n");
>  		phy_exit(imx_pcie->phy);
>  	}
> +	imx_pcie_assert_core_reset(imx_pcie);

[Severity: Medium]
Is the teardown ordering correct here?

During initialization, the core reset is asserted before powering on the PHY.
The error path err_phy_off in imx_pcie_host_init() correctly mirrors this
by asserting the core reset before powering off the PHY.

However, this sequence in imx_pcie_host_exit() asserts the core reset
after the PHY has already been powered off and exited. Should
imx_pcie_assert_core_reset() be called before phy_power_off() to maintain
proper LIFO teardown symmetry?

>  	imx_pcie_clk_disable(imx_pcie);
>  
>  	if (!pci->pp.skip_pwrctrl_off)

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