Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi, On 9 December 2016 at 01:52, Felipe Balbi wrote: > > Hi, > > Baolin Wang writes: >>> Baolin Wang writes: >> On 28 November 2016 at 14:43, Baolin Wang wrote: >>> For some mobile devices with strict power management, we also want to >>> suspend >>> the host when the slave is detached for power saving. Thus we add the >>> host >>> suspend/resume functions to support this requirement. >>> >>> Signed-off-by: Baolin Wang >>> --- >>> Changes since v3: >>> - No updates. >>> >>> Changes since v2: >>> - Remove pm_children_suspended() and other unused macros. >>> >>> Changes since v1: >>>- Add pm_runtime.h head file to avoid kbuild error. >>> --- >>> drivers/usb/dwc3/Kconfig |7 +++ >>> drivers/usb/dwc3/core.c | 26 +- >>> drivers/usb/dwc3/core.h | 15 +++ >>> drivers/usb/dwc3/host.c | 37 + >>> 4 files changed, 84 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >>> index a45b4f1..47bb2f3 100644 >>> --- a/drivers/usb/dwc3/Kconfig >>> +++ b/drivers/usb/dwc3/Kconfig >>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >>> >>> endchoice >>> >>> +config USB_DWC3_HOST_SUSPEND >>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y > > why do you need another Kconfig for this? Just enable it already :-p I assume some platforms may do not need this feature. But okay, I can remove this kconfig and enable it. >>> >>> thanks :-) >>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 9a4a5e4..7ad4bc3 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device >>> *pdev) >>> pm_runtime_use_autosuspend(dev); >>> pm_runtime_set_autosuspend_delay(dev, >>> DWC3_DEFAULT_AUTOSUSPEND_DELAY); >>> pm_runtime_enable(dev); >>> + pm_suspend_ignore_children(dev, true); > > why do you need this? Since the dwc3 device is the parent deive of xhci device, if we want to suspend dwc3 device, we need to suspend child device (xhci device) manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In pm_runtime_put_sync(), it will check if the children (xhci device) of dwc3 device have been in suspend state, if not we will suspend dwc3 device failed. We get/put the child device manually in parent device's runtime callback, we need to ignore the child device's runtime state, or it will failed due to the dependency. >>> >>> I see. Good explanation :-) >>> >>> There's one thing though, if you want to runtime suspend the gadget and >>> dwc3 is working on peripheral mode, host side (XHCI) should already be >>> runtime suspended because there's nothing attached to it. Why isn't it >>> runtime suspended? >> >> Since we have get the runtime count for xHCI device in >> xhci_plat_probe(), in case it will suspend automatically if some >> controllers do not want xHCI enters runtime suspend automatically. So >> the parent device (dwc3 device) need to get/put the xHCI device's >> runtime count to resume/suspend xHCI. > > IMHO, that's a bug in xhci-plat, not something dwc3 should work around. Maybe you misunderstood my meaning and I should make it clear. If dwc3 is just working on peripheral mode, then no need to initialize the host (xhci) things, which means it is invalid to runtime suspend/resume xHCI device and we can just runtime suspend/resume the gadget. If dwc3 is working on OTG mode, which will create and add the USB hcd for host side. Then if we want to suspend dwc3, we need to suspend xHCI device manually though now dwc3 acts as peripheral. The USB device (bus) of host side will runtime suspend automatically if there are no slave attached (by usb_runtime_suspend()--->usb_suspend_both()--->usb_suspend_interface()--->usb_suspend_device()--->generic_suspend()--->hcd_bus_suspend()--->xhci_bus_suspend()), but we should not suspend xHCI device (issuing xhci_suspend()) now, since some controllers did not implement the runtime PM to control xHCI device's runtime state, which will cause controllers can not resume xHCI device (issuing xhci_resume()) if the xHCI device suspend automatically by its child device. Thus we should get the runtime count for xHCI device in xhci_plat_probe() in case xHCI device will also suspend automatically by its child device. According to that, for xHCI device's parent: dwc3 device, we should put the xHCI device's runtime count to suspend xHCI device manually. Hope I make it clear. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in
Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi, Baolin Wang writes: >> Baolin Wang writes: > On 28 November 2016 at 14:43, Baolin Wang wrote: >> For some mobile devices with strict power management, we also want to >> suspend >> the host when the slave is detached for power saving. Thus we add the >> host >> suspend/resume functions to support this requirement. >> >> Signed-off-by: Baolin Wang >> --- >> Changes since v3: >> - No updates. >> >> Changes since v2: >> - Remove pm_children_suspended() and other unused macros. >> >> Changes since v1: >>- Add pm_runtime.h head file to avoid kbuild error. >> --- >> drivers/usb/dwc3/Kconfig |7 +++ >> drivers/usb/dwc3/core.c | 26 +- >> drivers/usb/dwc3/core.h | 15 +++ >> drivers/usb/dwc3/host.c | 37 + >> 4 files changed, 84 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig >> index a45b4f1..47bb2f3 100644 >> --- a/drivers/usb/dwc3/Kconfig >> +++ b/drivers/usb/dwc3/Kconfig >> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE >> >> endchoice >> >> +config USB_DWC3_HOST_SUSPEND >> + bool "Choose if the DWC3 host (xhci) can be suspend/resume" >> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y why do you need another Kconfig for this? Just enable it already :-p >>> >>> I assume some platforms may do not need this feature. But okay, I can >>> remove this kconfig and enable it. >> >> thanks :-) >> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >> index 9a4a5e4..7ad4bc3 100644 >> --- a/drivers/usb/dwc3/core.c >> +++ b/drivers/usb/dwc3/core.c >> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) >> pm_runtime_use_autosuspend(dev); >> pm_runtime_set_autosuspend_delay(dev, >> DWC3_DEFAULT_AUTOSUSPEND_DELAY); >> pm_runtime_enable(dev); >> + pm_suspend_ignore_children(dev, true); why do you need this? >>> >>> Since the dwc3 device is the parent deive of xhci device, if we want >>> to suspend dwc3 device, we need to suspend child device (xhci device) >>> manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In >>> pm_runtime_put_sync(), it will check if the children (xhci device) of >>> dwc3 device have been in suspend state, if not we will suspend dwc3 >>> device failed. >>> >>> We get/put the child device manually in parent device's runtime >>> callback, we need to ignore the child device's runtime state, or it >>> will failed due to the dependency. >> >> I see. Good explanation :-) >> >> There's one thing though, if you want to runtime suspend the gadget and >> dwc3 is working on peripheral mode, host side (XHCI) should already be >> runtime suspended because there's nothing attached to it. Why isn't it >> runtime suspended? > > Since we have get the runtime count for xHCI device in > xhci_plat_probe(), in case it will suspend automatically if some > controllers do not want xHCI enters runtime suspend automatically. So > the parent device (dwc3 device) need to get/put the xHCI device's > runtime count to resume/suspend xHCI. IMHO, that's a bug in xhci-plat, not something dwc3 should work around. -- balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi, On 8 December 2016 at 19:02, Felipe Balbi wrote: > > Hi, > > Baolin Wang writes: On 28 November 2016 at 14:43, Baolin Wang wrote: > For some mobile devices with strict power management, we also want to > suspend > the host when the slave is detached for power saving. Thus we add the host > suspend/resume functions to support this requirement. > > Signed-off-by: Baolin Wang > --- > Changes since v3: > - No updates. > > Changes since v2: > - Remove pm_children_suspended() and other unused macros. > > Changes since v1: >- Add pm_runtime.h head file to avoid kbuild error. > --- > drivers/usb/dwc3/Kconfig |7 +++ > drivers/usb/dwc3/core.c | 26 +- > drivers/usb/dwc3/core.h | 15 +++ > drivers/usb/dwc3/host.c | 37 + > 4 files changed, 84 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig > index a45b4f1..47bb2f3 100644 > --- a/drivers/usb/dwc3/Kconfig > +++ b/drivers/usb/dwc3/Kconfig > @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE > > endchoice > > +config USB_DWC3_HOST_SUSPEND > + bool "Choose if the DWC3 host (xhci) can be suspend/resume" > + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y >>> >>> why do you need another Kconfig for this? Just enable it already :-p >> >> I assume some platforms may do not need this feature. But okay, I can >> remove this kconfig and enable it. > > thanks :-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 9a4a5e4..7ad4bc3 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) > pm_runtime_use_autosuspend(dev); > pm_runtime_set_autosuspend_delay(dev, > DWC3_DEFAULT_AUTOSUSPEND_DELAY); > pm_runtime_enable(dev); > + pm_suspend_ignore_children(dev, true); >>> >>> why do you need this? >> >> Since the dwc3 device is the parent deive of xhci device, if we want >> to suspend dwc3 device, we need to suspend child device (xhci device) >> manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In >> pm_runtime_put_sync(), it will check if the children (xhci device) of >> dwc3 device have been in suspend state, if not we will suspend dwc3 >> device failed. >> >> We get/put the child device manually in parent device's runtime >> callback, we need to ignore the child device's runtime state, or it >> will failed due to the dependency. > > I see. Good explanation :-) > > There's one thing though, if you want to runtime suspend the gadget and > dwc3 is working on peripheral mode, host side (XHCI) should already be > runtime suspended because there's nothing attached to it. Why isn't it > runtime suspended? Since we have get the runtime count for xHCI device in xhci_plat_probe(), in case it will suspend automatically if some controllers do not want xHCI enters runtime suspend automatically. So the parent device (dwc3 device) need to get/put the xHCI device's runtime count to resume/suspend xHCI. -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi, Baolin Wang writes: >>> On 28 November 2016 at 14:43, Baolin Wang wrote: For some mobile devices with strict power management, we also want to suspend the host when the slave is detached for power saving. Thus we add the host suspend/resume functions to support this requirement. Signed-off-by: Baolin Wang --- Changes since v3: - No updates. Changes since v2: - Remove pm_children_suspended() and other unused macros. Changes since v1: - Add pm_runtime.h head file to avoid kbuild error. --- drivers/usb/dwc3/Kconfig |7 +++ drivers/usb/dwc3/core.c | 26 +- drivers/usb/dwc3/core.h | 15 +++ drivers/usb/dwc3/host.c | 37 + 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index a45b4f1..47bb2f3 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE endchoice +config USB_DWC3_HOST_SUSPEND + bool "Choose if the DWC3 host (xhci) can be suspend/resume" + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y >> >> why do you need another Kconfig for this? Just enable it already :-p > > I assume some platforms may do not need this feature. But okay, I can > remove this kconfig and enable it. thanks :-) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 9a4a5e4..7ad4bc3 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev) pm_runtime_use_autosuspend(dev); pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY); pm_runtime_enable(dev); + pm_suspend_ignore_children(dev, true); >> >> why do you need this? > > Since the dwc3 device is the parent deive of xhci device, if we want > to suspend dwc3 device, we need to suspend child device (xhci device) > manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In > pm_runtime_put_sync(), it will check if the children (xhci device) of > dwc3 device have been in suspend state, if not we will suspend dwc3 > device failed. > > We get/put the child device manually in parent device's runtime > callback, we need to ignore the child device's runtime state, or it > will failed due to the dependency. I see. Good explanation :-) There's one thing though, if you want to runtime suspend the gadget and dwc3 is working on peripheral mode, host side (XHCI) should already be runtime suspended because there's nothing attached to it. Why isn't it runtime suspended? -- balbi signature.asc Description: PGP signature
Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi,
On 8 December 2016 at 17:40, Felipe Balbi wrote:
>
> Hi,
>
> Baolin Wang writes:
>> Hi Felipe,
>>
>> On 28 November 2016 at 14:43, Baolin Wang wrote:
>>> For some mobile devices with strict power management, we also want to
>>> suspend
>>> the host when the slave is detached for power saving. Thus we add the host
>>> suspend/resume functions to support this requirement.
>>>
>>> Signed-off-by: Baolin Wang
>>> ---
>>> Changes since v3:
>>> - No updates.
>>>
>>> Changes since v2:
>>> - Remove pm_children_suspended() and other unused macros.
>>>
>>> Changes since v1:
>>>- Add pm_runtime.h head file to avoid kbuild error.
>>> ---
>>> drivers/usb/dwc3/Kconfig |7 +++
>>> drivers/usb/dwc3/core.c | 26 +-
>>> drivers/usb/dwc3/core.h | 15 +++
>>> drivers/usb/dwc3/host.c | 37 +
>>> 4 files changed, 84 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>>> index a45b4f1..47bb2f3 100644
>>> --- a/drivers/usb/dwc3/Kconfig
>>> +++ b/drivers/usb/dwc3/Kconfig
>>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE
>>>
>>> endchoice
>>>
>>> +config USB_DWC3_HOST_SUSPEND
>>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume"
>>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y
>
> why do you need another Kconfig for this? Just enable it already :-p
I assume some platforms may do not need this feature. But okay, I can
remove this kconfig and enable it.
>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 9a4a5e4..7ad4bc3 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev)
>>> pm_runtime_use_autosuspend(dev);
>>> pm_runtime_set_autosuspend_delay(dev,
>>> DWC3_DEFAULT_AUTOSUSPEND_DELAY);
>>> pm_runtime_enable(dev);
>>> + pm_suspend_ignore_children(dev, true);
>
> why do you need this?
Since the dwc3 device is the parent deive of xhci device, if we want
to suspend dwc3 device, we need to suspend child device (xhci device)
manually by issuing pm_runtime_put_sync() in dwc3_host_suspend(). In
pm_runtime_put_sync(), it will check if the children (xhci device) of
dwc3 device have been in suspend state, if not we will suspend dwc3
device failed.
We get/put the child device manually in parent device's runtime
callback, we need to ignore the child device's runtime state, or it
will failed due to the dependency.
>
>>> @@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev)
>>> static int dwc3_suspend_common(struct dwc3 *dwc)
>>> {
>>> unsigned long flags;
>>> + int ret;
>>>
>>> switch (dwc->dr_mode) {
>>> case USB_DR_MODE_PERIPHERAL:
>>> + spin_lock_irqsave(&dwc->lock, flags);
>>> + dwc3_gadget_suspend(dwc);
>>> + spin_unlock_irqrestore(&dwc->lock, flags);
>>> + break;
>>> case USB_DR_MODE_OTG:
>>> + ret = dwc3_host_suspend(dwc);
>>> + if (ret)
>>> + return ret;
>>> +
>>> spin_lock_irqsave(&dwc->lock, flags);
>>> dwc3_gadget_suspend(dwc);
>>> spin_unlock_irqrestore(&dwc->lock, flags);
>>> break;
>>> case USB_DR_MODE_HOST:
>>> + ret = dwc3_host_suspend(dwc);
>>> + if (ret)
>>> + return ret;
>>> default:
>>> /* do nothing */
>>> break;
>>> @@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc)
>>>
>>> switch (dwc->dr_mode) {
>>> case USB_DR_MODE_PERIPHERAL:
>>> + spin_lock_irqsave(&dwc->lock, flags);
>>> + dwc3_gadget_resume(dwc);
>>> + spin_unlock_irqrestore(&dwc->lock, flags);
>>> + break;
>>> case USB_DR_MODE_OTG:
>>> + ret = dwc3_host_resume(dwc);
>>> + if (ret)
>>> + return ret;
>>> +
>>> spin_lock_irqsave(&dwc->lock, flags);
>>> dwc3_gadget_resume(dwc);
>>> spin_unlock_irqrestore(&dwc->lock, flags);
>>> - /* FALLTHROUGH */
>>> + break;
>>> case USB_DR_MODE_HOST:
>>> + ret = dwc3_host_resume(dwc);
>>> + if (ret)
>>> + return ret;
>>> default:
>>> /* do nothing */
>>> break;
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index b585a30..db41908 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
>>> { }
>>> #endif
>>>
>>> +#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND)
>>> +int dwc3_host_suspend(struct dwc3 *dwc);
>>> +int
Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi,
Baolin Wang writes:
> Hi Felipe,
>
> On 28 November 2016 at 14:43, Baolin Wang wrote:
>> For some mobile devices with strict power management, we also want to suspend
>> the host when the slave is detached for power saving. Thus we add the host
>> suspend/resume functions to support this requirement.
>>
>> Signed-off-by: Baolin Wang
>> ---
>> Changes since v3:
>> - No updates.
>>
>> Changes since v2:
>> - Remove pm_children_suspended() and other unused macros.
>>
>> Changes since v1:
>>- Add pm_runtime.h head file to avoid kbuild error.
>> ---
>> drivers/usb/dwc3/Kconfig |7 +++
>> drivers/usb/dwc3/core.c | 26 +-
>> drivers/usb/dwc3/core.h | 15 +++
>> drivers/usb/dwc3/host.c | 37 +
>> 4 files changed, 84 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index a45b4f1..47bb2f3 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE
>>
>> endchoice
>>
>> +config USB_DWC3_HOST_SUSPEND
>> + bool "Choose if the DWC3 host (xhci) can be suspend/resume"
>> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y
why do you need another Kconfig for this? Just enable it already :-p
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 9a4a5e4..7ad4bc3 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev)
>> pm_runtime_use_autosuspend(dev);
>> pm_runtime_set_autosuspend_delay(dev,
>> DWC3_DEFAULT_AUTOSUSPEND_DELAY);
>> pm_runtime_enable(dev);
>> + pm_suspend_ignore_children(dev, true);
why do you need this?
>> @@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev)
>> static int dwc3_suspend_common(struct dwc3 *dwc)
>> {
>> unsigned long flags;
>> + int ret;
>>
>> switch (dwc->dr_mode) {
>> case USB_DR_MODE_PERIPHERAL:
>> + spin_lock_irqsave(&dwc->lock, flags);
>> + dwc3_gadget_suspend(dwc);
>> + spin_unlock_irqrestore(&dwc->lock, flags);
>> + break;
>> case USB_DR_MODE_OTG:
>> + ret = dwc3_host_suspend(dwc);
>> + if (ret)
>> + return ret;
>> +
>> spin_lock_irqsave(&dwc->lock, flags);
>> dwc3_gadget_suspend(dwc);
>> spin_unlock_irqrestore(&dwc->lock, flags);
>> break;
>> case USB_DR_MODE_HOST:
>> + ret = dwc3_host_suspend(dwc);
>> + if (ret)
>> + return ret;
>> default:
>> /* do nothing */
>> break;
>> @@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc)
>>
>> switch (dwc->dr_mode) {
>> case USB_DR_MODE_PERIPHERAL:
>> + spin_lock_irqsave(&dwc->lock, flags);
>> + dwc3_gadget_resume(dwc);
>> + spin_unlock_irqrestore(&dwc->lock, flags);
>> + break;
>> case USB_DR_MODE_OTG:
>> + ret = dwc3_host_resume(dwc);
>> + if (ret)
>> + return ret;
>> +
>> spin_lock_irqsave(&dwc->lock, flags);
>> dwc3_gadget_resume(dwc);
>> spin_unlock_irqrestore(&dwc->lock, flags);
>> - /* FALLTHROUGH */
>> + break;
>> case USB_DR_MODE_HOST:
>> + ret = dwc3_host_resume(dwc);
>> + if (ret)
>> + return ret;
>> default:
>> /* do nothing */
>> break;
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index b585a30..db41908 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
>> { }
>> #endif
>>
>> +#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND)
>> +int dwc3_host_suspend(struct dwc3 *dwc);
>> +int dwc3_host_resume(struct dwc3 *dwc);
>> +#else
>> +static inline int dwc3_host_suspend(struct dwc3 *dwc)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int dwc3_host_resume(struct dwc3 *dwc)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +
>> #endif /* __DRIVERS_USB_DWC3_CORE_H */
>> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
>> index ed82464..8e5309d6 100644
>> --- a/drivers/usb/dwc3/host.c
>> +++ b/drivers/usb/dwc3/host.c
>> @@ -16,6 +16,7 @@
>> */
>>
>> #include
>> +#include
>>
>> #include "core.h"
>>
>> @@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc)
>> dev_name(&dwc->xhci->dev));
>> platform_device_unregister(dwc->xhci);
>> }
>> +
>> +#ifdef CONFIG_USB_DWC3_HOST_SUSPEND
>> +int dwc3_host_suspend(struct dwc3 *dwc)
>> +{
>> +
Re: [PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
Hi Felipe,
On 28 November 2016 at 14:43, Baolin Wang wrote:
> For some mobile devices with strict power management, we also want to suspend
> the host when the slave is detached for power saving. Thus we add the host
> suspend/resume functions to support this requirement.
>
> Signed-off-by: Baolin Wang
> ---
> Changes since v3:
> - No updates.
>
> Changes since v2:
> - Remove pm_children_suspended() and other unused macros.
>
> Changes since v1:
>- Add pm_runtime.h head file to avoid kbuild error.
> ---
> drivers/usb/dwc3/Kconfig |7 +++
> drivers/usb/dwc3/core.c | 26 +-
> drivers/usb/dwc3/core.h | 15 +++
> drivers/usb/dwc3/host.c | 37 +
> 4 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index a45b4f1..47bb2f3 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE
>
> endchoice
>
> +config USB_DWC3_HOST_SUSPEND
> + bool "Choose if the DWC3 host (xhci) can be suspend/resume"
> + depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y
> + help
> + We can suspend the host when the slave is detached for power saving,
> + and resume the host when one slave is attached.
> +
> comment "Platform Glue Driver Support"
>
> config USB_DWC3_OMAP
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 9a4a5e4..7ad4bc3 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev)
> pm_runtime_use_autosuspend(dev);
> pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);
> pm_runtime_enable(dev);
> + pm_suspend_ignore_children(dev, true);
> ret = pm_runtime_get_sync(dev);
> if (ret < 0)
> goto err1;
> @@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev)
> static int dwc3_suspend_common(struct dwc3 *dwc)
> {
> unsigned long flags;
> + int ret;
>
> switch (dwc->dr_mode) {
> case USB_DR_MODE_PERIPHERAL:
> + spin_lock_irqsave(&dwc->lock, flags);
> + dwc3_gadget_suspend(dwc);
> + spin_unlock_irqrestore(&dwc->lock, flags);
> + break;
> case USB_DR_MODE_OTG:
> + ret = dwc3_host_suspend(dwc);
> + if (ret)
> + return ret;
> +
> spin_lock_irqsave(&dwc->lock, flags);
> dwc3_gadget_suspend(dwc);
> spin_unlock_irqrestore(&dwc->lock, flags);
> break;
> case USB_DR_MODE_HOST:
> + ret = dwc3_host_suspend(dwc);
> + if (ret)
> + return ret;
> default:
> /* do nothing */
> break;
> @@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc)
>
> switch (dwc->dr_mode) {
> case USB_DR_MODE_PERIPHERAL:
> + spin_lock_irqsave(&dwc->lock, flags);
> + dwc3_gadget_resume(dwc);
> + spin_unlock_irqrestore(&dwc->lock, flags);
> + break;
> case USB_DR_MODE_OTG:
> + ret = dwc3_host_resume(dwc);
> + if (ret)
> + return ret;
> +
> spin_lock_irqsave(&dwc->lock, flags);
> dwc3_gadget_resume(dwc);
> spin_unlock_irqrestore(&dwc->lock, flags);
> - /* FALLTHROUGH */
> + break;
> case USB_DR_MODE_HOST:
> + ret = dwc3_host_resume(dwc);
> + if (ret)
> + return ret;
> default:
> /* do nothing */
> break;
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index b585a30..db41908 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
> { }
> #endif
>
> +#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND)
> +int dwc3_host_suspend(struct dwc3 *dwc);
> +int dwc3_host_resume(struct dwc3 *dwc);
> +#else
> +static inline int dwc3_host_suspend(struct dwc3 *dwc)
> +{
> + return 0;
> +}
> +
> +static inline int dwc3_host_resume(struct dwc3 *dwc)
> +{
> + return 0;
> +}
> +#endif
> +
> #endif /* __DRIVERS_USB_DWC3_CORE_H */
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index ed82464..8e5309d6 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -16,6 +16,7 @@
> */
>
> #include
> +#include
>
> #include "core.h"
>
> @@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc)
> dev_name(&dwc->xhci->dev));
> platform_device_unregister(dwc->xhci);
> }
> +
> +#ifdef CONFIG_USB_DWC3_HOST_SUSPEND
>
[PATCH v4 2/2] usb: dwc3: core: Support the dwc3 host suspend/resume
For some mobile devices with strict power management, we also want to suspend
the host when the slave is detached for power saving. Thus we add the host
suspend/resume functions to support this requirement.
Signed-off-by: Baolin Wang
---
Changes since v3:
- No updates.
Changes since v2:
- Remove pm_children_suspended() and other unused macros.
Changes since v1:
- Add pm_runtime.h head file to avoid kbuild error.
---
drivers/usb/dwc3/Kconfig |7 +++
drivers/usb/dwc3/core.c | 26 +-
drivers/usb/dwc3/core.h | 15 +++
drivers/usb/dwc3/host.c | 37 +
4 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index a45b4f1..47bb2f3 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -47,6 +47,13 @@ config USB_DWC3_DUAL_ROLE
endchoice
+config USB_DWC3_HOST_SUSPEND
+ bool "Choose if the DWC3 host (xhci) can be suspend/resume"
+ depends on USB_DWC3_HOST=y || USB_DWC3_DUAL_ROLE=y
+ help
+ We can suspend the host when the slave is detached for power saving,
+ and resume the host when one slave is attached.
+
comment "Platform Glue Driver Support"
config USB_DWC3_OMAP
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9a4a5e4..7ad4bc3 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1091,6 +1091,7 @@ static int dwc3_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(dev);
pm_runtime_set_autosuspend_delay(dev, DWC3_DEFAULT_AUTOSUSPEND_DELAY);
pm_runtime_enable(dev);
+ pm_suspend_ignore_children(dev, true);
ret = pm_runtime_get_sync(dev);
if (ret < 0)
goto err1;
@@ -1215,15 +1216,27 @@ static int dwc3_remove(struct platform_device *pdev)
static int dwc3_suspend_common(struct dwc3 *dwc)
{
unsigned long flags;
+ int ret;
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
+ spin_lock_irqsave(&dwc->lock, flags);
+ dwc3_gadget_suspend(dwc);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ break;
case USB_DR_MODE_OTG:
+ ret = dwc3_host_suspend(dwc);
+ if (ret)
+ return ret;
+
spin_lock_irqsave(&dwc->lock, flags);
dwc3_gadget_suspend(dwc);
spin_unlock_irqrestore(&dwc->lock, flags);
break;
case USB_DR_MODE_HOST:
+ ret = dwc3_host_suspend(dwc);
+ if (ret)
+ return ret;
default:
/* do nothing */
break;
@@ -1245,12 +1258,23 @@ static int dwc3_resume_common(struct dwc3 *dwc)
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
+ spin_lock_irqsave(&dwc->lock, flags);
+ dwc3_gadget_resume(dwc);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+ break;
case USB_DR_MODE_OTG:
+ ret = dwc3_host_resume(dwc);
+ if (ret)
+ return ret;
+
spin_lock_irqsave(&dwc->lock, flags);
dwc3_gadget_resume(dwc);
spin_unlock_irqrestore(&dwc->lock, flags);
- /* FALLTHROUGH */
+ break;
case USB_DR_MODE_HOST:
+ ret = dwc3_host_resume(dwc);
+ if (ret)
+ return ret;
default:
/* do nothing */
break;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index b585a30..db41908 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1226,4 +1226,19 @@ static inline void dwc3_ulpi_exit(struct dwc3 *dwc)
{ }
#endif
+#if IS_ENABLED(CONFIG_USB_DWC3_HOST_SUSPEND)
+int dwc3_host_suspend(struct dwc3 *dwc);
+int dwc3_host_resume(struct dwc3 *dwc);
+#else
+static inline int dwc3_host_suspend(struct dwc3 *dwc)
+{
+ return 0;
+}
+
+static inline int dwc3_host_resume(struct dwc3 *dwc)
+{
+ return 0;
+}
+#endif
+
#endif /* __DRIVERS_USB_DWC3_CORE_H */
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index ed82464..8e5309d6 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -16,6 +16,7 @@
*/
#include
+#include
#include "core.h"
@@ -130,3 +131,39 @@ void dwc3_host_exit(struct dwc3 *dwc)
dev_name(&dwc->xhci->dev));
platform_device_unregister(dwc->xhci);
}
+
+#ifdef CONFIG_USB_DWC3_HOST_SUSPEND
+int dwc3_host_suspend(struct dwc3 *dwc)
+{
+ struct device *xhci = &dwc->xhci->dev;
+ int ret;
+
+ /*
+* Note: if we get the -EBUSY, which means the xHCI children devices are
+* not in suspend state yet, the glue layer need to wait for a while and
+* try to suspend xHCI device again.
+
