* [PATCH v5] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
@ 2026-07-28 3:51 hongxing.zhu
2026-07-28 16:37 ` Bjorn Helgaas
0 siblings, 1 reply; 2+ messages in thread
From: hongxing.zhu @ 2026-07-28 3:51 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
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.
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 v5:
Update the commit log suggested by Bjorn.
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] 2+ messages in thread
* Re: [PATCH v5] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing
2026-07-28 3:51 [PATCH v5] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing hongxing.zhu
@ 2026-07-28 16:37 ` Bjorn Helgaas
0 siblings, 0 replies; 2+ messages in thread
From: Bjorn Helgaas @ 2026-07-28 16:37 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 Tue, Jul 28, 2026 at 11:51:59AM +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
> 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.
>
> 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>
Applied to pci/for-linus for v7.2, thanks very much!
> ---
> Changes in v5:
> Update the commit log suggested by Bjorn.
>
> 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 [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-28 16:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28 3:51 [PATCH v5] PCI: imx6: Fix i.MX6Q/DL boot hang caused by improper PHY power sequencing hongxing.zhu
2026-07-28 16:37 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox