On Tue, Aug 01, 2023 at 03:05:46PM +0800, Kevin Xie wrote: > On 2023/8/1 7:12, Bjorn Helgaas wrote: > ... > > The delay required by sec 6.6.1 is a minimum of 100ms following exit > > from reset or, for fast links, 100ms after link training completes. > > > > The comment at the call of advk_pcie_wait_for_link() [2] says it is > > the delay required by sec 6.6.1, but that doesn't seem right to me. > > > > For one thing, I don't think 6.6.1 says anything about "link up" being > > the end of a delay. So if we want to do the delay required by 6.6.1, > > "wait_for_link()" doesn't seem like quite the right name. > > > > For another, all the *_wait_for_link() functions can return success > > after 0ms, 90ms, 180ms, etc. They're unlikely to return after 0ms, > > but 90ms is quite possible. If we avoided the 0ms return and > > LINK_WAIT_USLEEP_MIN were 100ms instead of 90ms, that should be enough > > for slow links, where we need 100ms following "exit from reset." > > > > But it's still not enough for fast links where we need 100ms "after > > link training completes" because we don't know when training > > completed. If training completed 89ms into *_wait_for_link(), we only > > delay 1ms after that. > > That's the point, we will add a extra 100ms after PERST# de-assert > in the patch-v3 according to Base Spec r6.0 - 6.6.1: > msleep(100); > gpiod_set_value_cansleep(pcie->reset_gpio, 0); > > + /* As the requirement in PCIe base spec r6.0, system must wait a > + * minimum of 100 ms following exit from a Conventional Reset > + * before sending a Configuration Request to the device.*/ > + msleep(100); > + > if (starfive_pcie_host_wait_for_link(pcie)) > return -EIO; For fast links (links that support > 5.0 GT/s), the 100ms starts *after* link training completes. The above looks OK if starfive only supports slow links, but then I'm not sure why we would need starfive_pcie_host_wait_for_link(). Bjorn
