Re: [PATCH 4/4] usb: dwc3: core: Support the dwc3 host suspend/resume

[Felipe Balbi <balbi@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2795 bytes --]


Hi,

Baolin Wang <baolin.wang@linaro.org> writes:

[...]

>>>>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
>>>>> index 2e960ed..2ec3eff 100644
>>>>> --- a/drivers/usb/dwc3/host.c
>>>>> +++ b/drivers/usb/dwc3/host.c
>>>>> @@ -17,8 +17,11 @@
>>>>>
>>>>>  #include <linux/platform_device.h>
>>>>>  #include <linux/usb/xhci_pdriver.h>
>>>>> +#include <linux/usb.h>
>>>>> +#include <linux/usb/hcd.h>
>>>>>
>>>>>  #include "core.h"
>>>>> +#include "../host/xhci.h"
>>>>>
>>>>>  int dwc3_host_init(struct dwc3 *dwc)
>>>>>  {
>>>>> @@ -91,6 +94,8 @@ int dwc3_host_init(struct dwc3 *dwc)
>>>>>       memset(&pdata, 0, sizeof(pdata));
>>>>>
>>>>>       pdata.usb3_lpm_capable = dwc->usb3_lpm_capable;
>>>>> +     /* dwc3 controller need an extraordinary delay when suspending xhci. */
>>>>> +     pdata.usb3_slow_suspend = 1;
>>>>>
>>>>>       ret = platform_device_add_data(xhci, &pdata, sizeof(pdata));
>>>>>       if (ret) {
>>>>> @@ -128,3 +133,30 @@ void dwc3_host_exit(struct dwc3 *dwc)
>>>>>                         dev_name(&dwc->xhci->dev));
>>>>>       platform_device_unregister(dwc->xhci);
>>>>>  }
>>>>> +
>>>>> +int dwc3_host_suspend(struct dwc3 *dwc)
>>>>> +{
>>>>> +     struct usb_hcd *hcd = dev_get_drvdata(&dwc->xhci->dev);
>>>>> +     struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>>>> +     int ret, cnt = 20;
>>>>> +
>>>>> +try_again:
>>>>> +      /* We should wait for xhci bus has been into suspend mode firstly. */
>>>>> +     ret = xhci_suspend(xhci, device_may_wakeup(&dwc->xhci->dev));
>>>>> +     if (ret && --cnt > 0) {
>>>>> +             dev_warn(dwc->dev, "xhci suspend failed %d, try again...\n",
>>>>> +                      ret);
>>>>> +             msleep(200);
>>>>> +             goto try_again;
>>>>> +     }
>>>>> +
>>>>> +     return ret;
>>>>> +}
>>>>> +
>>>>> +int dwc3_host_resume(struct dwc3 *dwc)
>>>>> +{
>>>>> +     struct usb_hcd *hcd = dev_get_drvdata(&dwc->xhci->dev);
>>>>> +     struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>>>>> +
>>>>> +     return xhci_resume(xhci, 0);
>>>>> +}
>>>>
>>>> This is wrong. XHCI is a child of dwc3, when dwc3 suspends,
>>>> xhci_supend() has already been called. Why isn't it called for you?
>>>
>>> xhci-plat.c did not set the runtime PM callbacks, how can we issued
>>> xhci_supend() by runtime PM from dwc3?
>>
>> wouldn't it be nicer to just initialize PM runtime callbacks from
>> xhci-plat? It just needs to be added and verified, right?
>
> Yes, you are right. Alan and Peter also suggested me to implement the
> runtime PM callbacks for xhci-plat. I am also testing this and I will
> send out the patch if it works well. So please ignore this patch.
> Thanks for your comments.

cool, thanks a lot :-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]