> -----Original Message----- > From: Tim Harvey [mailto:tharvey@xxxxxxxxxxxxx] > Sent: Tuesday, September 23, 2014 8:29 PM > To: Lucas Stach; Zhu Richard-R65037 > Cc: linux-pci-owner@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; Guo Shawn- > R65073; Fabio Estevam > Subject: Re: [PATCH v2 2/5] PCI: imx6: wait the clocks to stabilize after > ref_en > > On Tue, Sep 23, 2014 at 2:56 AM, Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote: > > Am Dienstag, den 23.09.2014, 12:11 +0800 schrieb Richard Zhu: > >> - a while delay is mandatory required after pcie_ref_clk_en is set. > >> Otherwise, the system would be hang on imx6qdl ard boards, because > >> that imx6qdl boards don't have the reset_gpio. > >> - the clocks should be stable already after the "clk_prepare_enable" > >> is return. So I think it's ok to move the usleep delay after the > >> pcie_ref_en is set. > >> > > > > You are describing a lot of the conditions around the issue, but not > > the issue itself, which makes it hard to follow your commit message. > > After looking at the code I think the problem is this (and should be > > described > > accordingly): > > > > For boards without a reset gpio we skip the delay between enabling the > > pcie_ref_clk and touching the RC registers for configuration. > > Apparently this hangs when the clocks are not yet settled in the DW > > PCIe core. So we need to make sure that there is always an appropriate > > delay between those two actions. [Richard] Thanks. > > > > I have not found this constraint anywhere in the i.MX6 Reference > > Manual, nor in the DW PCIe documents I have access to, which makes me > > a bit feel a bit unhappy about this. Richard, do you have better info > > on why this delay is needed and how long it needs to be? Or is this just > empirical? [Richard] I used to fix one less than 0.3% percentage randomly link down issue during the warm-reset loop stress tests. I used get some info from design team that " 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." "at least 4us delay is required after ref clock stable and before ssp_en is assert" I would add about 10us delay just between clocks are stable and ssp_en is assert later. > > > > In general I'm ok with this patch, but still want a confirmation from > > Tim that this doesn't break anything. > > I agree with Lucas' comments and also agree that this can use some testing. > Based on my previous findings PCI link is very fragile. It will take me a few > days to get a proper test setup in a thermal chamber with a host of boards but > I will report back when I have findings. > > Tim > Best Regards Richard Zhu > > > >> Signed-off-by: Richard Zhu <r65037@xxxxxxxxxxxxx> > >> --- > >> drivers/pci/host/pci-imx6.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/pci/host/pci-imx6.c > >> b/drivers/pci/host/pci-imx6.c index 233fe8a..bc4222b 100644 > >> --- a/drivers/pci/host/pci-imx6.c > >> +++ b/drivers/pci/host/pci-imx6.c > >> @@ -275,15 +275,15 @@ 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); > >> 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); > > > > -- > > Pengutronix e.K. | Lucas Stach | > > Industrial Linux Solutions | http://www.pengutronix.de/ | > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-pci" > > in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo > > info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
