On Tue, May 12, 2020 at 12:09:06PM -0500, Bjorn Helgaas wrote: > I think that code would be more readable as something like: > > if (dev->link_active_reporting) > wait_for_link_active(dev); > msleep(delay); > > Obviously we still want the printks and error/timeout checking, but > fundamentally we want to wait for the link to be active (if possible), > then wait the 100ms. That structure isn't as clear as it could be > when the delay is buried inside pcie_wait_for_link_delay(). > > It's not completely trivial to restructure it like that because of > pcie_wait_for_link() and its callers. But I think it's worth pushing > on it a little bit to see if it could be done reasonably. OK, I see what I can do to make it clearer. > This does expose the fact that per spec, a hot-plug capable port that > supports only 5 GT/s must support link_active_reporting, but sec 6.6.1 > says we *don't* need to wait for link training to complete before > waiting 100ms. So maybe we end up waiting slightly more than > necessary. Probably not worth worrying about? I would say so. Better wait a bit too much than the opposite IMHO.
