Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
@ 2026-07-22  7:10 hongxing.zhu
  2026-07-27 21:40 ` Bjorn Helgaas
  2026-07-27 21:42 ` Bjorn Helgaas
  0 siblings, 2 replies; 5+ messages in thread
From: hongxing.zhu @ 2026-07-22  7:10 UTC (permalink / raw)
  To: frank.li, l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas,
	s.hauer, kernel, festevam
  Cc: linux-pci, linux-arm-kernel, imx, linux-kernel, Richard Zhu,
	Leonardo Costa, Leonardo Costa, Frank Li

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.

The i.MX6Q/DL PCIe PHY requires approximately 120us between TEST_PD
de-assertion and link training start. Add usleep_range(200, 500) in
imx6q_pcie_core_reset() after clearing TEST_PD to satisfy this
requirement.

Additional changes:
Add explicit imx_pcie_assert_core_reset() calls in error paths and
host_exit() to ensure no power leak.

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 v4:
Refer to Sashiko' reivew of v3 patch.
- Adjust imx_pcie_assert_core_reset() in imx_pcie_host_exit().
- Add the delay explicitly after TEST_PD is cleared, since the PERST# GPIO
might be optional.

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 | 41 ++++++++++++++-------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index e25f938eefe23..c94b5e991aecd 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;
 }
@@ -826,6 +817,12 @@ static int imx6sx_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)
+		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);
 	regmap_update_bits(imx_pcie->iomuxc_gpr, IOMUXC_GPR1, IMX6Q_GPR1_PCIE_SW_RST,
 			   assert ? IMX6Q_GPR1_PCIE_SW_RST : 0);
 	if (!assert)
@@ -836,11 +833,15 @@ 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)
 {
-	if (!assert)
-		return 0;
+	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);
 
-	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);
+	if (!assert)
+		usleep_range(200, 500);
 
 	return 0;
 }
@@ -1451,6 +1452,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);
@@ -1470,6 +1472,7 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct imx_pcie *imx_pcie = to_imx_pcie(pci);
 
+	imx_pcie_assert_core_reset(imx_pcie);
 	if (imx_pcie->phy) {
 		if (phy_power_off(imx_pcie->phy))
 			dev_err(pci->dev, "unable to power off PHY\n");
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
  2026-07-22  7:10 [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing hongxing.zhu
@ 2026-07-27 21:40 ` Bjorn Helgaas
  2026-07-28  2:25   ` Hongxing Zhu (OSS)
  2026-07-27 21:42 ` Bjorn Helgaas
  1 sibling, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2026-07-27 21:40 UTC (permalink / raw)
  To: hongxing.zhu
  Cc: frank.li, l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas,
	s.hauer, kernel, festevam, linux-pci, linux-arm-kernel, imx,
	linux-kernel, Richard Zhu, Leonardo Costa, Leonardo Costa

On Wed, Jul 22, 2026 at 03:10:30PM +0800, hongxing.zhu@oss.nxp.com wrote:
> 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.
> 
> The i.MX6Q/DL PCIe PHY requires approximately 120us between TEST_PD
> de-assertion and link training start. Add usleep_range(200, 500) in
> imx6q_pcie_core_reset() after clearing TEST_PD to satisfy this
> requirement.
> 
> Additional changes:
> Add explicit imx_pcie_assert_core_reset() calls in error paths and
> host_exit() to ensure no power leak.
> 
> 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 v4:
> Refer to Sashiko' reivew of v3 patch.
> - Adjust imx_pcie_assert_core_reset() in imx_pcie_host_exit().
> - Add the delay explicitly after TEST_PD is cleared, since the PERST# GPIO
> might be optional.
> 
> 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 | 41 ++++++++++++++-------------
>  1 file changed, 22 insertions(+), 19 deletions(-)

I'd propose the following commit log to focus on the effects of
TEST_PD (setting TEST_PD powers down the PHY, clearing it powers up
the PHY) and to match the regmap_set/clear language in the patch
itself:

610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators")
introduced a boot hang on i.MX6Q/DL variants by reordering
imx_pcie_host_init() to call imx6q_pcie_enable_ref_clk() (which powered up
the PHY) before imx6q_pcie_core_reset() (which powered it back down).

Before 610fa91d9863, the sequence was:

  1. imx_pcie_assert_core_reset() - power down PHY (set TEST_PD), set
     REF_CLK_EN
  2. imx_pcie_clk_enable() - power up PHY (clear TEST_PD), set REF_CLK_EN
  3. Link training starts with PHY powered up (TEST_PD cleared)
  4. Link training succeeds

After 610fa91d9863, the sequence became:

  1. imx_pcie_clk_enable() - power up PHY (clear TEST_PD), set REF_CLK_EN
  2. imx_pcie_assert_core_reset() - power down PHY (set TEST_PD), set
     REF_CLK_EN
  3. imx_pcie_deassert_core_reset() - does nothing
  4. Link training starts with PHY powered down (TEST_PD set)
  5. Link training fails and boot hangs

To fix this:

  - Remove TEST_PD PHY power control from imx6q_pcie_enable_ref_clk()

  - Remove REF_CLK_EN control from imx6q_pcie_core_reset()

  - Add TEST_PD PHY power control to imx6qp_pcie_core_reset(), which
    previously relied on imx6q_pcie_enable_ref_clk() to power up the PHY by
    clearing TEST_PD

  - Clear TEST_PD to power on PHY in imx_pcie_deassert_core_reset()

These changes together ensure the correct sequence:

  1. REF_CLK_EN set in clk_enable() (TEST_PD untouched)
  2. TEST_PD set in assert_core_reset() (PHY power off)
  3. TEST_PD cleared in deassert_core_reset() (PHY power on)
  4. Link training starts with proper PHY state

The i.MX6Q/DL PCIe PHY requires approximately 120us between TEST_PD
de-assertion and link training start. Add usleep_range(200, 500) in
imx6q_pcie_core_reset() after clearing TEST_PD to satisfy this
requirement.

Add explicit imx_pcie_assert_core_reset() calls in error paths and
host_exit() to ensure no power leak.



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
  2026-07-22  7:10 [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing hongxing.zhu
  2026-07-27 21:40 ` Bjorn Helgaas
@ 2026-07-27 21:42 ` Bjorn Helgaas
  2026-07-28  2:25   ` Hongxing Zhu (OSS)
  1 sibling, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2026-07-27 21:42 UTC (permalink / raw)
  To: hongxing.zhu
  Cc: frank.li, l.stach, lpieralisi, kwilczynski, mani, robh, bhelgaas,
	s.hauer, kernel, festevam, linux-pci, linux-arm-kernel, imx,
	linux-kernel, Richard Zhu, Leonardo Costa, Leonardo Costa

On Wed, Jul 22, 2026 at 03:10:30PM +0800, hongxing.zhu@oss.nxp.com wrote:
> 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.

I intend to apply this patch for v7.2, but I have a separate question
about why the link training failure causes a boot *hang*, not just a
boot where we couldn't enumerate any PCIe devices.  It seems like the
link training should time out, and we could emit a message (if
desired) and continue without the PCIe controller.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
  2026-07-27 21:40 ` Bjorn Helgaas
@ 2026-07-28  2:25   ` Hongxing Zhu (OSS)
  0 siblings, 0 replies; 5+ messages in thread
From: Hongxing Zhu (OSS) @ 2026-07-28  2:25 UTC (permalink / raw)
  To: Bjorn Helgaas, Hongxing Zhu (OSS)
  Cc: Frank Li, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, robh@kernel.org,
	bhelgaas@google.com, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	imx@lists.linux.dev, linux-kernel@vger.kernel.org, Hongxing Zhu,
	Leonardo Costa, Leonardo Costa


> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, July 28, 2026 5:40 AM
> To: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; robh@kernel.org;
> bhelgaas@google.com; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org;
> Hongxing Zhu <hongxing.zhu@nxp.com>; Leonardo Costa
> <leoreis.costa@gmail.com>; Leonardo Costa <leonardo.costa@toradex.com>
> Subject: Re: [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper
> PHY power sequencing
> 
> [You don't often get email from helgaas@kernel.org. Learn why this is important
> at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Wed, Jul 22, 2026 at 03:10:30PM +0800, hongxing.zhu@oss.nxp.com wrote:
> > 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.
> >
> > The i.MX6Q/DL PCIe PHY requires approximately 120us between TEST_PD
> > de-assertion and link training start. Add usleep_range(200, 500) in
> > imx6q_pcie_core_reset() after clearing TEST_PD to satisfy this
> > requirement.
> >
> > Additional changes:
> > Add explicit imx_pcie_assert_core_reset() calls in error paths and
> > host_exit() to ensure no power leak.
> >
> > 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@gma
> > il.com/
> > Tested-by: Leonardo Costa <leonardo.costa@toradex.com>
> > Reviewed-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Changes in v4:
> > Refer to Sashiko' reivew of v3 patch.
> > - Adjust imx_pcie_assert_core_reset() in imx_pcie_host_exit().
> > - Add the delay explicitly after TEST_PD is cleared, since the PERST#
> > GPIO might be optional.
> >
> > 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 | 41
> > ++++++++++++++-------------
> >  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> I'd propose the following commit log to focus on the effects of TEST_PD (setting
> TEST_PD powers down the PHY, clearing it powers up the PHY) and to match the
> regmap_set/clear language in the patch
> itself:
> 
> 610fa91d9863 ("PCI: imx6: Assert PERST# before enabling regulators") introduced
> a boot hang on i.MX6Q/DL variants by reordering
> imx_pcie_host_init() to call imx6q_pcie_enable_ref_clk() (which powered up the
> PHY) before imx6q_pcie_core_reset() (which powered it back down).
> 
> Before 610fa91d9863, the sequence was:
> 
>   1. imx_pcie_assert_core_reset() - power down PHY (set TEST_PD), set
>      REF_CLK_EN
>   2. imx_pcie_clk_enable() - power up PHY (clear TEST_PD), set REF_CLK_EN
>   3. Link training starts with PHY powered up (TEST_PD cleared)
>   4. Link training succeeds
> 
> After 610fa91d9863, the sequence became:
> 
>   1. imx_pcie_clk_enable() - power up PHY (clear TEST_PD), set REF_CLK_EN
>   2. imx_pcie_assert_core_reset() - power down PHY (set TEST_PD), set
>      REF_CLK_EN
>   3. imx_pcie_deassert_core_reset() - does nothing
>   4. Link training starts with PHY powered down (TEST_PD set)
>   5. Link training fails and boot hangs
> 
> To fix this:
> 
>   - Remove TEST_PD PHY power control from imx6q_pcie_enable_ref_clk()
> 
>   - Remove REF_CLK_EN control from imx6q_pcie_core_reset()
> 
>   - Add TEST_PD PHY power control to imx6qp_pcie_core_reset(), which
>     previously relied on imx6q_pcie_enable_ref_clk() to power up the PHY by
>     clearing TEST_PD
> 
>   - Clear TEST_PD to power on PHY in imx_pcie_deassert_core_reset()
> 
> These changes together ensure the correct sequence:
> 
>   1. REF_CLK_EN set in clk_enable() (TEST_PD untouched)
>   2. TEST_PD set in assert_core_reset() (PHY power off)
>   3. TEST_PD cleared in deassert_core_reset() (PHY power on)
>   4. Link training starts with proper PHY state
> 
> The i.MX6Q/DL PCIe PHY requires approximately 120us between TEST_PD de-
> assertion and link training start. Add usleep_range(200, 500) in
> imx6q_pcie_core_reset() after clearing TEST_PD to satisfy this requirement.
> 
> Add explicit imx_pcie_assert_core_reset() calls in error paths and
> host_exit() to ensure no power leak.
Hi Bjorn:
Thanks for your kindly help.
Your new proposed commit message is totally fine for me.

Best Regards
Richard Zhu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
  2026-07-27 21:42 ` Bjorn Helgaas
@ 2026-07-28  2:25   ` Hongxing Zhu (OSS)
  0 siblings, 0 replies; 5+ messages in thread
From: Hongxing Zhu (OSS) @ 2026-07-28  2:25 UTC (permalink / raw)
  To: Bjorn Helgaas, Hongxing Zhu (OSS)
  Cc: Frank Li, l.stach@pengutronix.de, lpieralisi@kernel.org,
	kwilczynski@kernel.org, mani@kernel.org, robh@kernel.org,
	bhelgaas@google.com, s.hauer@pengutronix.de,
	kernel@pengutronix.de, festevam@gmail.com,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	imx@lists.linux.dev, linux-kernel@vger.kernel.org, Hongxing Zhu,
	Leonardo Costa, Leonardo Costa

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, July 28, 2026 5:42 AM
> To: Hongxing Zhu (OSS) <hongxing.zhu@oss.nxp.com>
> Cc: Frank Li <frank.li@nxp.com>; l.stach@pengutronix.de; lpieralisi@kernel.org;
> kwilczynski@kernel.org; mani@kernel.org; robh@kernel.org;
> bhelgaas@google.com; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; imx@lists.linux.dev; linux-kernel@vger.kernel.org;
> Hongxing Zhu <hongxing.zhu@nxp.com>; Leonardo Costa
> <leoreis.costa@gmail.com>; Leonardo Costa <leonardo.costa@toradex.com>
> Subject: Re: [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper
> PHY power sequencing
> 
> On Wed, Jul 22, 2026 at 03:10:30PM +0800, hongxing.zhu@oss.nxp.com wrote:
> > 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.
> 
> I intend to apply this patch for v7.2, but I have a separate question about why the
> link training failure causes a boot *hang*, not just a boot where we couldn't
> enumerate any PCIe devices.  It seems like the link training should time out, and
> we could emit a message (if
> desired) and continue without the PCIe controller.[] 
Before link training, the imx_setup_phy_mpll() function performs PHY
read/write operations for i.MX6Q/DL PCIe. These operations cause a boot hang
when the PHY is not powered up properly, which occurs before we even reach the
link training timeout mechanism.

Best Regards
Richard Zhu


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-28  2:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22  7:10 [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing hongxing.zhu
2026-07-27 21:40 ` Bjorn Helgaas
2026-07-28  2:25   ` Hongxing Zhu (OSS)
2026-07-27 21:42 ` Bjorn Helgaas
2026-07-28  2:25   ` Hongxing Zhu (OSS)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox