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

[Bjorn Helgaas <helgaas@kernel.org>]

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.