[V1] PCI: imx6: Wait the clocks to stabilize after ref_en

Commit Message

Richard Zhu Oct. 24, 2014, 2:30 a.m. UTC

From: Richard Zhu <[e-mail]>

For boards without a reset GPIO we skip the delay between enabling the
pcie_ref_clk and touching the RC registers for configuration.
This hangs the system if there isn't a proper delay to ensure the clocks
are settled in the DW PCIe core.

Also iMX6Q always needs an additional 10us delay to make sure the reset
is propagated through the core, as we don't have an explicitly
controlled reset input on this SoC.

Signed-off-by: Richard Zhu <[e-mail]>
Tested-by: Tim Harvey <[e-mail]>
Tested-by: Fabio Estevam <[e-mail]>
Tested-by: Fabio Estevam <[e-mail]>
---
 drivers/pci/host/pci-imx6.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Fabio Estevam Oct. 24, 2014, 7:01 p.m. UTC | #1

Hi Richard,

On Fri, Oct 24, 2014 at 12:30 AM, Richard Zhu <[e-mail]> wrote:

> -       /* allow the clocks to stabilize */
> -       usleep_range(200, 500);
> -
>         /* power up core phy and enable ref clock */
>         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>                         IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
> +       /*
> +        * 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 one ~10us delay here.
> +        */
> +       udelay(10);

I am curious about this delay here: have you seen boot issues without
it? If so, which hardware was that?
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fabio Estevam Oct. 25, 2014, 10:14 a.m. UTC | #2

On Fri, Oct 24, 2014 at 10:31 PM, [e-mail]
<[e-mail]> wrote:
> Hi Fabio:
> This bug is found during the stress warm-reset stress test.
> There are about three times randomly link down issue during the consecutive
> 10000 times system warm reset tests on one customer’s golden board.
> Regarding to imx internal pcie design situation, synposys gave the
> suggestion, and it works.

Ok, great. Thanks for providing the details. I am happy with this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233fe8a..eac96fb 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -275,15 +275,22 @@  static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 		goto err_pcie;
 	}
 
-	/* allow the clocks to stabilize */
-	usleep_range(200, 500);
-
 	/* power up core phy and enable ref clock */
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
+	/*
+	 * 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 one ~10us delay here.
+	 */
+	udelay(10);
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
 
+	/* allow the clocks to stabilize */
+	usleep_range(200, 500);
+
 	/* Some boards don't have PCIe reset GPIO. */
 	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
 		gpio_set_value(imx6_pcie->reset_gpio, 0);