Thanks Tim.
Best regards
Richard
> 在 2014年10月2日，上午2:00，"Tim Harvey" <tharvey@xxxxxxxxxxxxx> 写道：
>
>> On Tue, Sep 23, 2014 at 5:28 AM, Tim Harvey <tharvey@xxxxxxxxxxxxx> wrote:
>>> 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.
>>>
>>> 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?
>>>
>>> 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
>>
>>>
>>>> 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);
>
> I tested this across temperature over 300+ boots each on several IMX6
> based boards with switches and did not encounter any link failures.
>
> Tested-by: Tim Harvey <tharvey@xxxxxxxxxxxxx>
?韬{.n?????%??檩??w?{.n???{炳???骅w* jg????????G??⒏ ⒎?:+v???????? ????＂??????
