On Tue, Jan 30, 2024 at 12:15:55PM +0530, Manivannan Sadhasivam wrote: > On Mon, Jan 29, 2024 at 06:56:24PM +0530, Ajay Agarwal wrote: > > On Mon, Jan 29, 2024 at 01:42:54PM +0530, Manivannan Sadhasivam wrote: > > > On Mon, Jan 29, 2024 at 01:34:52PM +0530, Ajay Agarwal wrote: > > > > On Mon, Jan 29, 2024 at 12:40:25PM +0530, Manivannan Sadhasivam wrote: > > > > > On Mon, Jan 29, 2024 at 12:21:51PM +0530, Ajay Agarwal wrote: > > > > > > On Sat, Jan 20, 2024 at 08:04:34PM +0530, Manivannan Sadhasivam wrote: > > > > > > > On Fri, Jan 19, 2024 at 11:29:22PM +0530, Ajay Agarwal wrote: > > > > > > > > On Fri, Jan 19, 2024 at 01:22:19PM +0530, Manivannan Sadhasivam wrote: > > > > > > > > > On Fri, Jan 12, 2024 at 03:00:06PM +0530, Ajay Agarwal wrote: > > > > > > > > > > In dw_pcie_host_init() regardless of whether the link has been > > > > > > > > > > started or not, the code waits for the link to come up. Even in > > > > > > > > > > cases where start_link() is not defined the code ends up spinning > > > > > > > > > > in a loop for 1 second. Since in some systems dw_pcie_host_init() > > > > > > > > > > gets called during probe, this one second loop for each pcie > > > > > > > > > > interface instance ends up extending the boot time. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Which platform you are working on? Is that upstreamed? You should mention the > > > > > > > > > specific platform where you are observing the issue. > > > > > > > > > > > > > > > > > This is for the Pixel phone platform. The platform driver for the same > > > > > > > > is not upstreamed yet. It is in the process. > > > > > > > > > > > > > > > > > > > > > > Then you should submit this patch at the time of the driver submission. Right > > > > > > > now, you are trying to fix a problem which is not present in upstream. One can > > > > > > > argue that it is a problem for designware-plat driver, but honestly I do not > > > > > > > know how it works. > > > > > > > > > > > > > > - Mani > > > > > > > > > > > > > Yes Mani, this can be a problem for the designware-plat driver. To me, > > > > > > the problem of a second being wasted in the probe path seems pretty > > > > > > obvious. We will wait for the link to be up even though we are not > > > > > > starting the link training. Can this patch be accepted considering the > > > > > > problem in the dw-plat driver then? > > > > > > > > > > > > > > > > If that's the case with your driver, when are you starting the link training? > > > > > > > > > The link training starts later based on a userspace/debugfs trigger. > > > > > > > > > > Why does it happen as such? What's the problem in starting the link during > > > probe? Keep it in mind that if you rely on the userspace for starting the link > > > based on a platform (like Android), then if the same SoC or peripheral instance > > > get reused in other platform (non-android), the it won't be a seamless user > > > experience. > > > > > > If there are any other usecases, please state them. > > > > > > - Mani > > > > > This SoC is targeted for an android phone usecase and the endpoints > > being enumerated need to go through an appropriate and device specific > > power sequence which gets triggered only when the userspace is up. The > > PCIe probe cannot assume that the EPs have been powered up already and > > hence the link-up is not attempted. > > Still, I do not see the necessity to not call start_link() during probe. If you I am not adding any logic/condition around calling the start_link() itself. I am only avoiding the wait for the link to be up if the controller driver has not defined start_link(). > add PROBE_PREFER_ASYNCHRONOUS to your controller driver, this delay would become > negligible. The reason why I'm against not calling start_link() is due to below > reasons: > > 1. If the same SoC gets reused for other platforms, even other android phones > that powers up the endpoints during boot, then it creates a dependency with > userspace to always start the link even though the devices were available. > That's why we should never fix the behavior of the controller drivers based on a > single platform. I wonder how the behavior is changing with this patch. Do you have an example of a platform which does not have start_link() defined but would like to still wait for a second for the link to come up? For example, consider the intel-gw driver. The 1 sec wait time in its probe path is also a waste because it explicitly starts link training later in time. > > 2. This will create fragmentation among the DWC glue drivers w.r.t the behavior > and will become a maintenance nightmare (there are enough already). > > So, I'd suggest you to use the asynchronous probe mentioned above so that other > drivers may probe in parallel thus avoiding the delay that you are worried about. > > - Mani > > -- > மணிவண்ணன் சதாசிவம்
