Re: [PATCH] PCI: apple: Reset the port for 100ms on probe

On Thursday 18 November 2021 12:57:46 Marc Zyngier wrote:

On Thu, 18 Nov 2021 10:31:56 +0000,
Pali Rohár [off-list ref] wrote:

quoted

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!

quoted

quoted

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...

quoted

And of course after the releasing PERST# that 100ms post-PERST# delay is
required.

That we agree on.

quoted

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/ (local)

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel