Re: [EXTERNAL] Re: [PATCH] PCI: j721e: Fix delay before PERST# deassert

[Bjorn Helgaas]

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