Re: [PATCH v5] PCI: dwc: Wait for link up only if link is started

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Wed, Feb 14, 2024 at 04:02:28PM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 06, 2024 at 10:40:43PM +0530, Manivannan Sadhasivam wrote:
> > ...
> 
> > ... And for your usecase, allowing the controller driver to start
> > the link post boot just because a device on your Pixel phone comes
> > up later is not a good argument. You _should_not_ define the
> > behavior of a controller driver based on one platform, it is really
> > a bad design.
> 
> I haven't followed the entire discussion, and I don't know much about
> the specifics of Ajay's situation, but from the controller driver's
> point of view, shouldn't a device coming up later look like a normal
> hot-add?
> 

Yes, but most of the form factors that these drivers work with do not support
native hotplug. So users have to rescan the bus through sysfs.

> I think most drivers are designed with the assumption that Endpoints
> are present and powered up at the time of host controller probe, which
> seems a little stronger than necessary.
> 

Most of the drivers work with endpoints that are fixed in the board design (like
M.2), so the endpoints would be up when the controller probes.

> I think the host controller probe should initialize the Root Port such
> that its LTSSM enters the Detect state, and that much should be
> basically straight-line code with no waiting.  If no Endpoint is
> attached, i.e., "the slot is empty", it would be nice if the probe
> could then complete immediately without waiting at all.
> 

Atleast on Qcom platforms, the LTSSM would be in "Detect" state even if no
endpoints are found during probe. Then once an endpoint comes up later, link
training happens and user can rescan the bus through sysfs.

But, I don't know the real need of 1s loop to wait for the link. It predates my
work on DWC drivers. Maybe Lorenzo could shed some light. I could not find the
reference in both DWC and PCIe specs (maybe my grep was bad).

> If the link comes up later, could we handle it as a hot-add?  This
> might be an actual hot-add, or it might be that an Endpoint was
> present at boot but link training didn't complete until later.
> 
> I admit it doesn't look trivial to actually implement this.  We would
> need to be able to detect link-up events, e.g., via hotplug or other
> link management interrupts.  Lacking that hardware functionality, we
> might need driver-specific code to wait for the link to come up
> (possibly drivers could skip the wait if they can detect the "slot
> empty" case).
> 
> Also, the hotplug functionality (pciehp or acpiphp) is currently
> initialized later and there's probably a race with enabling and
> detecting hot-add events in the "slot occupied" case.
> 

As I mentioned above, hotplug is not possible in all the cases. There is a
series floating to add GPIO based hotplug, but still that requires board
designers to route a dedicated GPIO to the endpoint.

To conclude, we do need to check for the existence of the endpoints during
probe. But whether the driver should wait for 1s for the link to come up,
should be clarified by Lorenzo.

- Mani

-- 
மணிவண்ணன் சதாசிவம்