Re: [PATCH v4] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
Bjorn Helgaas <[email protected]>
| Newsgroups | dev.linux.lists.imx,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <20260727214004.GA1263823@bhelgaas> |
On Wed, Jul 22, 2026 at 03:10:30PM +0800, [email protected] wrote: > From: Richard Zhu <[email protected]> > > 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 <[email protected]> > Reported-by: Leonardo Costa <[email protected]> > Closes: https://lore.kernel.org/lkml/[email protected]/ > Tested-by: Leonardo Costa <[email protected]> > Reviewed-by: Frank Li <[email protected]> > --- > 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.