Re: [PATCH] usb: core: Add runtime resume checking

[Baolin Wang]

On 10 August 2016 at 17:07, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
> On Wed, Aug 10, 2016 at 02:43:50PM +0800, Baolin Wang wrote:
>> Hi Peter,
>>
>> On 10 August 2016 at 14:18, Peter Chen <hzpeterchen@xxxxxxxxx> 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@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?
>>
>> Because now there is no slave attached, we will not power on usb
>> controller and resume xhci.
>>
>
> I don't know why you set policy like this. Even the controller is
> resumed, it will still be suspended by system suspend. What's more,
> if you disallow it, how can you change your wakeup setting?
> Eg, at runtime suspend, we enable wakeup by default. But for system
> sleep, we disable wakeup by default.
>
> At runtime, if there is no device on the port, even we resumes the
> controller and roohub, it still will be suspended soon.

For saving power, if there is no slave attached, why we need to power
on usb controller and resume xhci? Especially for mobile device.

Moreover the usb device runtime suspend/resume is separate with xhci
suspend/resume, when xhci entered suspend state, only slave attaching
can resume xhci.

-- 
Baolin.wang
Best Regards