Re: [PATCH 0/5] usb: dwc3: qcom: Allow runtime PM

Konrad Dybcio
2023-03-27T10:33:24Z

On 27.03.2023 12:10, Manivannan Sadhasivam wrote:
> On Mon, Mar 27, 2023 at 11:24:58AM +0200, Konrad Dybcio wrote:
>>
>>
>> On 27.03.2023 11:17, Manivannan Sadhasivam wrote:
>>> On Mon, Mar 27, 2023 at 11:01:35AM +0200, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 25.03.2023 17:52, Manivannan Sadhasivam wrote:
>>>>> Hi,
>>>>>
>>>>> This series allows the dwc3-qcom driver to do runtime PM itself without
>>>>> userspace intervention. Still, userspace is required to enable runtime PM
>>>>> for dwc3 glue and xhci drivers as we cannot enable runtime PM for them.
>>>>> But this series avoids one more additional step.
>>>> What sort of 'userspace intervention' are we talking about?
>>>> echo mem > /sys/power/state?
>>>>
>>>
>>> I forgot to add that bit:
>>>
>>> echo auto > /sys/devices/platform/soc@0/a8f8800.usb/a800000.usb/xhci-hcd.1.auto/power/control
>>> echo auto > /sys/devices/platform/soc@0/a8f8800.usb/a800000.usb/power/control
>>>
>>> You need to set "auto" for the runtime control for both xhci and dwc drivers.
>>>
>>> Then if you don't connect a usb device, all 3 drivers (dwc3-qcom, dwc3, and
>>> xhci) will become runtime suspended after a delay of 5s (default delay).
>>>
>>> This can be confirmed by:
>>>
>>> cat /sys/devices/platform/soc@0/a8f8800.usb/power/runtime_status
>>>
>>> After connecting a usb device, they will all become "active".
>> Thanks! And if I'm following correctly, we can't enable runtime PM
>> for the DWC3 glue and XHCI drivers, as that would cause havoc on
>> other, non-qc platforms. Is that correct?
>>
>
> Kind of. Actually dwc3 glue is our qcom driver, other one is just dwc3 driver.
>
> The havoc would apply to xhci driver because, once it is suspended, one of its
> parent drivers has to resume it. And that requires runtime PM support for all
> the parent drivers which is currently not available.
Makes sense, thanks for the explanation!

>
> But for dwc3 driver, I'm not sure about the consequence though. Maybe I should
> send it as a separate patch later on and see what other platforms folks think
> of it.
May be worth a shot!

Konrad

>
> Thanks,
> Mani
>
>> Konrad
>>>
>>> Thanks,
>>> Mani
>>>
>>>> Konrad
>>>>>
>>>>> While enabling runtime PM, I noticed that the xhci driver suspends before
>>>>> catching the xhci interrupts during resume. This ended up deferring the
>>>>> device enumeration for some time. So I included a patch adding autosuspend
>>>>> delay of 200ms to the xhci driver. With this delay, usb enumeration happens
>>>>> properly.
>>>>>
>>>>> This series has been tested on SC8280XP-CRD and RB5 devices.
>>>>>
>>>>> Thanks,
>>>>> Mani
>>>>>
>>>>> Manivannan Sadhasivam (5):
>>>>> arm64: dts: qcom: sc8280xp: Add missing dwc3 quirks
>>>>> xhci: host: Use 200ms autosuspend delay for runtime suspend
>>>>> usb: dwc3: qcom: Fix null ptr access during runtime_suspend()
>>>>> usb: dwc3: qcom: Clear pending interrupt before enabling wake
>>>>> interrupt
>>>>> usb: dwc3: qcom: Allow runtime PM
>>>>>
>>>>> arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 14 ++++++++++++++
>>>>> drivers/usb/dwc3/dwc3-qcom.c | 13 +++++++++----
>>>>> drivers/usb/host/xhci-plat.c | 2 ++
>>>>> 3 files changed, 25 insertions(+), 4 deletions(-)
>>>>>
>>>
>