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.
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 <[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 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.