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

Krishna Kurapati PSSNV
2022-07-14T15:43:31Z

On 7/14/2022 8:58 PM, Greg Kroah-Hartman wrote:
> 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
Hi Greg,

I tested only on SC7180 and SC7280 devices. It was working on SC7280,
(the first run of of_platform_populate made sure dwc3 probe is done).
On SC7180, during the first run of dwc3-qcom probe, we observed dwc3
probe getting deferred after of_platform_populate.
As you suggested, I can try and check on other devices using this driver
and confirm if only SC7180 has this issue. 🙂

Thanks,
Krishna,