On Tue, Jul 04, 2023 at 09:36:43PM +0530, Verma, Achal wrote: > On 7/3/2023 9:51 PM, Bjorn Helgaas wrote: > > In subject, "Fix" doesn't convey much information. Does it increase? > > Decrease? How much time are we talking about? PERST# deassert is at > > one end of the delay; what event is at the other end? > > How about "Increase delay to 100ms for PERST# deassert from moment > power-rails achieve operating limits" Maybe something like "Delay 100ms T_PVPERL from power stable to PERST# inactive" to match the language in the spec? > > Is this delay for the benefit of the Root Port or for the attached > > Endpoint? If the latter, my guess is that some Endpoints might > > tolerate the current shorter delay, while others might require > > more, and it doesn't sound like "TI's K3 SoC" would be relevant > > here. > > Its for the endpoints, TI's EVB doesn't exhibit any issues with > 100us delay but some customer reported the issue with shorter delay. I wouldn't bother mentioning "some custom platform implemented using TI's K3 SOCs" then, because the problem is that the driver didn't observe T_PVPERL, so the problem will happen with some endpoints but not others. > > Numbers like 100ms that come from the PCIe specs should have #defines > > for them. If we don't have one already, can you add one, please? > > Sure, will do it in next revision but should this go in some generic PCI > header file or just pci-j721e.c I think it should be in drivers/pci/pci.h so all the controller drivers can use the same thing. Obviously none of them *currently* use it, although there are a bunch of "msleep(100)" and a few comments that mention T_PVPERL. Bjorn