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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <156915e9-34c4-0b20-61b7-d6d5bfc584fa@quicinc.com>
Date:   Thu, 14 Jul 2022 21:13:11 +0530
From:   Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Felipe Balbi <balbi@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        <linux-arm-msm@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc3: qcom: Defer dwc3-qcom probe if dwc3 isn't
 probed properly


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@...cinc.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,