On Thu, Feb 22, 2024 at 10:00:09AM +0530, Ajay Agarwal wrote: > On Thu, Feb 15, 2024 at 07:39:08PM +0530, Manivannan Sadhasivam wrote: > > 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. > > > Lorenzo himself applied my patch [1] which had caused the regression on > QCOM platforms. So I am assuming that he is okay with removing this 1s > delay. > > [1] https://lore.kernel.org/all/168509076553.135117.7288121992217982937.b4-ty@xxxxxxxxxx/ > Hi Mani, I am wondering if you have any thoughts on the above comment? > > - Mani > > > > -- > > மணிவண்ணன் சதாசிவம்
