| Message ID | 1414118200-2018-2-git-send-email-richard.zhu@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Am Freitag, den 24.10.2014, 10:36 +0800 schrieb Richard Zhu: > From: Richard Zhu <r65037@freescale.com> > > 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 <richard.zhu@freescale.com> > Tested-by: Tim Harvey <tharvey@gateworks.com> > Tested-by: Fabio Estevam <fabio.estevam@freescale.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Acked-by: Lucas Stach <l.stach@pengutronix.de> Bjorn, please pick this up as a fix for 3.18. > --- > drivers/pci/host/pci-imx6.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > 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);
Hi Richard, On Fri, Oct 24, 2014 at 12:36 AM, Richard Zhu <richard.zhu@freescale.com> wrote: > From: Richard Zhu <r65037@freescale.com> > > 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 <richard.zhu@freescale.com> > Tested-by: Tim Harvey <tharvey@gateworks.com> > Tested-by: Fabio Estevam <fabio.estevam@freescale.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> Please always add Bjorn on Cc. > --- > drivers/pci/host/pci-imx6.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > 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); The patch that myself and Tim has given a Tested-by tag did not contain this udelay: http://www.spinics.net/lists/linux-pci/msg34748.html In your original patch it only moved the the usleep range and that's the version that Tim has run intensive boot tests over temperature, so I would prefer that you re-send a patch like the original one. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Freitag, den 24.10.2014, 09:57 -0200 schrieb Fabio Estevam: > Hi Richard, > > On Fri, Oct 24, 2014 at 12:36 AM, Richard Zhu <richard.zhu@freescale.com> wrote: > > From: Richard Zhu <r65037@freescale.com> > > > > 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 <richard.zhu@freescale.com> > > Tested-by: Tim Harvey <tharvey@gateworks.com> > > Tested-by: Fabio Estevam <fabio.estevam@freescale.com> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > > Please always add Bjorn on Cc. > > > --- > > drivers/pci/host/pci-imx6.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > 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); > > The patch that myself and Tim has given a Tested-by tag did not > contain this udelay: > http://www.spinics.net/lists/linux-pci/msg34748.html > > In your original patch it only moved the the usleep range and that's > the version that Tim has run intensive boot tests over temperature, so > I would prefer that you re-send a patch like the original one. Given that the moving of the usleep didn't cause any ill effects I'm pretty confident that the additional 10us will not break thing. As this short sleep is certainly needed on some platforms it will get added eventually anyways, so I'm fine with having it in this patch. Regards, Lucas
On Fri, Oct 24, 2014 at 10:45 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > Given that the moving of the usleep didn't cause any ill effects I'm > pretty confident that the additional 10us will not break thing. > > As this short sleep is certainly needed on some platforms it will get > added eventually anyways, so I'm fine with having it in this patch. Ok, but my point is that a Tested-by tag should not be added if a new patch is resent with modifications, and only the previous version was actually tested. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Richard, On Fri, Oct 24, 2014 at 12:36 AM, Richard Zhu <richard.zhu@freescale.com> wrote: > From: Richard Zhu <r65037@freescale.com> > > 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 <richard.zhu@freescale.com> > Tested-by: Tim Harvey <tharvey@gateworks.com> > Tested-by: Fabio Estevam <fabio.estevam@freescale.com> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> The previous Tested-by tags that myself and Tim gave were for the previous version of the patch without the delay. I tested this new version of the patch now and you have my new tag :-) Tested-by: Fabio Estevam <fabio.estevam@freescale.com> I would suggest you to resed this patch (you can add Lucas's Acked-by) and also make sure to copy Bjorn so that it can be applied to 3.18 as a fix. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);