| Message ID | 653c8e893d4de1ab4dcade2ca2da9f2cfe369c2c.1470735102.git.baolin.wang@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Peter, On 10 August 2016 at 14:18, Peter Chen <hzpeterchen@gmail.com> wrote: > 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@linuxfoundation.org> 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? Because now there is no slave attached, we will not power on usb controller and resume xhci. > > 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@linaro.org> >> >> --- >> >> 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@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > > Best Regards, > Peter Chen -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); + } + udev->do_remote_wakeup = w; }
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. Then we should check if it resumes successfully in choose_wakeup() function, if it failed we should clear the runtime errors by pm_runtime_set_suspended() function to avoid runtime resume failure. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- drivers/usb/core/driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html