Re: [PATCH] usb: dwc3: qcom: Defer dwc3-qcom probe if dwc3 isn't probed properly

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Jul 14, 2022 at 08:39:31PM +0530, Krishna Kurapati PSSNV wrote:
> 
> On 7/14/2022 8:32 PM, Greg Kroah-Hartman wrote:
> > On Thu, Jul 14, 2022 at 08:25:16PM +0530, Krishna Kurapati wrote:
> > > On SC7180 devices, it is observed that dwc3 probing is deferred
> > > because device_links_check_suppliers() finds that '88e3000.phy'
> > > isn't ready yet.
> > > 
> > > As a part of its probe call, dwc3-qcom driver checks if dwc3 core
> > > is wakeup capable or not. If the dwc3 core is wakeup capable, driver
> > > configures dwc-qcom's power domain to be always ON. Also it configures
> > > dp/dm interrupts accordingly to support wakeup from system suspend.
> > > 
> > > More info regarding the same can be found at:
> > > commit d9be8d5c5b03 ("usb: dwc3: qcom: Keep power domain on to retain controller status"
> > > commit 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
> > > 
> > > In the event, dwc3 probe gets deferred and is processed after dwc3-qcom
> > > probe, driver ends up reading the wakeup capability of dwc3 core as false
> > > leading to instability in suspend/resume path.
> > > 
> > > To avoid this scenario, ensure dwc3_probe is successful by checking
> > > if appropriate driver is assigned to it or not after the of_platform_populate
> > > call. If it isn't then defer dwc3-qcom probe as well.
> > > 
> > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> > > ---
> > >   drivers/usb/dwc3/dwc3-qcom.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > index 7703655..096d1414 100644
> > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > @@ -722,6 +722,9 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
> > >   		dev_err(dev, "failed to get dwc3 platform device\n");
> > >   	}
> > > +	if (!qcom->dwc3->dev.driver)
> > > +		return -EPROBE_DEFER;
> > > +
> > Why not limit this check to a device type like your changelog mentions?
> > 
> > thanks,
> > 
> > greg k-h
> Hi Greg,
> 
> I wanted to make it common to any device using this driver. Although on
> devices
> ike SC7280,this issue is not seen but the code would hold good. Do you see
> any
> concerns if we do it this way ?

How many different types of hardware did you test this change on and
what was the results?

thanks,

greg k-h