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

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: <87a8ga42i6.fsf@linux.intel.com>
Date:	Thu, 18 Aug 2016 16:42:41 +0300
From:	Felipe Balbi <balbi@...nel.org>
To:	Baolin Wang <baolin.wang@...aro.org>
Cc:	Greg KH <gregkh@...uxfoundation.org>, mathias.nyman@...el.com,
	USB <linux-usb@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH 4/4] usb: dwc3: core: Support the dwc3 host suspend/resume


Hi,

Baolin Wang <baolin.wang@...aro.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

Download attachment "signature.asc" of type "application/pgp-signature" (801 bytes)