On Wed, Aug 10, 2016 at 10:33:31AM +0800, Baolin Wang wrote:
> Hi Greg,
>
> On 9 August 2016 at 18:26, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Tue, Aug 09, 2016 at 05:33:33PM +0800, Baolin Wang wrote:
> >> When the usb device has entered suspend state by runtime suspend method, and
> >> the sustem also try to enter suspend state by issuing usb_dev_suspend(), it
> >> will issue pm_runtime_resume() function to deal with wrong wakeup setting in
> >> choose_wakeup() function.
> >>
> >> But if usb device resumes failed due to xhci has been into suspend state and
> >> hardware is not accessible, which will set runtime errors. Thus when there is
> >> slave attached, usb device will resume failed by runtime resume method due to
> >> previous runtime errors.
> >
> > I really can't parse the first sentance in this paragraph, what exactly
> > makes xhci so "unique" here?
>
> Sorry for confusing, I try to explain it clearly. Considering strict
> power management for mobile device, we should also power off the usb
> controller if there are no slaves attached even though it is usb host
> function.
>
> For example: No slave attached----> usb interface runtime suspend
> ----> usb device runtime suspend -----> xhci suspend -----> power off
> usb controller. After that if the system wants to enter suspend state,
> then it also will issue usb_dev_suspend(), then the
> pm_runtime_resume() function (issued in choose_wakeup() function) will
> return -ESHUTDOWN due to xhci has been suspend and hardware is not
> accessible.
Why the controller does not be resumed when the root hub has issued
runtime resume?
Peter
>
> After system entering resume state, if there is slave attached ---->
> power on usb controller -----> xhci resume -----> usb device runtime
> resume ----> usb interface runtime resume. Usb device will resume
> failed if runtime errors is set (-ESHUTDOWN), thus we should clear the
> runtime errors in choose_wakeup() function to avoid this situation.
>
> >
> >> Then we should check if it resumes successfully in choose_wakeup() function,
> >
> > what is "it"?
>
> It present pm_runtime_resume() issued in choose_wakeup() function.
>
> >
> >> if it failed we should clear the runtime errors by pm_runtime_set_suspended()
> >> function to avoid runtime resume failure.
> >
> > Again, what is "it"?
>
> It present pm_runtime_resume() issued in choose_wakeup() function.
>
> >
> >>
> >> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
> >> ---
> >> drivers/usb/core/driver.c | 9 +++++++--
> >> 1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> >> index dadd1e8d..a1a0f5f 100644
> >> --- a/drivers/usb/core/driver.c
> >> +++ b/drivers/usb/core/driver.c
> >> @@ -1412,6 +1412,7 @@ static int usb_resume_both(struct usb_device *udev, pm_message_t msg)
> >> static void choose_wakeup(struct usb_device *udev, pm_message_t msg)
> >> {
> >> int w;
> >> + int ret;
> >>
> >> /* Remote wakeup is needed only when we actually go to sleep.
> >> * For things like FREEZE and QUIESCE, if the device is already
> >> @@ -1431,8 +1432,12 @@ static void choose_wakeup(struct usb_device *udev, pm_message_t msg)
> >> /* If the device is autosuspended with the wrong wakeup setting,
> >> * autoresume now so the setting can be changed.
> >> */
> >> - if (udev->state == USB_STATE_SUSPENDED && w != udev->do_remote_wakeup)
> >> - pm_runtime_resume(&udev->dev);
> >> + if (udev->state == USB_STATE_SUSPENDED && w != udev->do_remote_wakeup) {
> >> + ret = pm_runtime_resume(&udev->dev);
> >> + if (ret == -ESHUTDOWN)
> >> + pm_runtime_set_suspended(&udev->dev);
> >
> > why is 'ret' needed:
> > if (pm_runtime_resume(&udev->dev) == -ESHUTDOWN)
>
> OK. I can modify it in next version if you agree this patch.
>
> >
> >
> > Why would resume fail?
>
> Like I explained above. Thanks.
>
> --
> Baolin.wang
> Best Regards
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
