On Thursday 18 November 2021 12:57:46 Marc Zyngier wrote: > On Thu, 18 Nov 2021 10:31:56 +0000, > Pali Rohár <pali@xxxxxxxxxx> wrote: > > For power-on it is probably overkill, but I think that delay between > > flipping PERST# should be there. IIRC Compex WLE1216 wifi card needs to > > be at least 10-11ms in reset. Last year, during testing of this card I > > saw that if PERST#-based reset was shorter then card was completely > > undetected. > > The only delay we really need is Tperst-clk. Random bugs on random > devices don't apply here, as the system is completely closed (there is > no slot to add anything). Once we have TB running one of these days, > we will see whether this still holds. Ok! > > > In practice, I can completely remove the initial Tpvperl delay (we > > > have been powered-on for a long time already, and the clock is stable > > > when we come back from setting it up), and cut the second one by half > > > without observing any ill effect (though I feel safer keeping it to > > > its nominal value). > > > > My opinion is that this patch does not power on/off card in PCIe slot. > > And because card is powered-on for a long time (as you wrote), it means > > that Tpvperl delay does not apply here. That is why I think that > > different delay (How long should be PCIe card in Warm Reset state) > > should be used _between_ flipping PERST# signal. > > My reading of the spec is that the only thing we need while #PERST is > asserted is Tperst-clk. The value you keep arguing about doesn't seem > to exist as such in the spec, because it appears to be endpoint > specific. Well, I was not able to find it in the spec too, that is why I do not know... > > And of course after the releasing PERST# that 100ms post-PERST# delay is > > required. > > That we agree on. > > > I have an idea to move PERST# handling (with all delays) from controller > > drivers to pci core functions. Because basically every driver > > re-implements these delays in its probe function. I wrote this idea with > > some details in email. If you have a time, could you look at it? I > > summarized here also details about delays (like Tpvperl, Tperstclk, ..): > > https://lore.kernel.org/linux-pci/20211022183808.jdeo7vntnagqkg7g@pali/ > > That's a laudable goal. What isn't clear to me is whether you intend > to move the whole state machine into core code, or just have a set of > helpers that the driver calls into. IMO, the former is what we really > need, while the latter only rids us of the simple stuff. Now I'm just collecting comments and feedbacks for this idea. I think that state machine in core code is what we need.