[RFC 1/5] usb: ohci-exynos: enable async suspend/resume

[RFC 1/5] usb: ohci-exynos: enable async suspend/resume

[Yuvaraj Kumar C D]

From: Andrew Bresticker <abrestic@chromium.org>

USB host controllers can take a significant amount of time to suspend
and resume, adding several hundred miliseconds to the kernel resume
time. Since the Exynos OHCI controller has no outside dependencies
(other than clocks, which are suspended late/resumed early), allow it to
suspend and resume asynchronously.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 drivers/usb/host/ohci-exynos.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 68588d8..faad2bdc 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -137,6 +137,8 @@ skip_phy:
 	if (exynos_ohci->otg)
 		exynos_ohci->otg->set_host(exynos_ohci->otg, &hcd->self);
 
+	device_enable_async_suspend(&pdev->dev);
+
 	platform_set_drvdata(pdev, hcd);
 
 	exynos_ohci_phy_enable(pdev);
-- 
1.7.9.5

[RFC 2/5] usb: ehci-s5p: enable async suspend/resume

[Yuvaraj Kumar C D]

From: Andrew Bresticker <abrestic@chromium.org>

USB host controllers can take a significant amount of time to suspend
and resume, adding several hundred miliseconds to the kernel resume
time. Since the Exynos EHCI controller has no outside dependencies
(other than clocks, which are suspended late/resumed early), allow it to
suspend and resume asynchronously.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 drivers/usb/host/ehci-exynos.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index f7ce8e2..e5125cd 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -165,6 +165,8 @@ skip_phy:
 	}
 	device_wakeup_enable(hcd->self.controller);
 
+	device_enable_async_suspend(&pdev->dev);
+
 	platform_set_drvdata(pdev, hcd);
 
 	return 0;
-- 
1.7.9.5

[RFC 3/5] usb: xhci-plat: enable async suspend/resume

[Yuvaraj Kumar C D]

From: Andrew Bresticker <abrestic@chromium.org>

USB host controllers can take a significant amount of time to suspend
and resume, adding several hundred miliseconds to the kernel resume
time. Since the XHCI controller has no outside dependencies (other than
clocks, which are suspended late/resumed early), allow it to suspend and
resume asynchronously.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 drivers/usb/host/xhci-plat.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 8abda5c..1bc1565 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -162,6 +162,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto put_usb3_hcd;
 
+	device_enable_async_suspend(&pdev->dev);
+
 	return 0;
 
 put_usb3_hcd:
-- 
1.7.9.5

[RFC 4/5] usb: dwc3-exynos: enable async suspend/resume

[Yuvaraj Kumar C D]

From: Andrew Bresticker <abrestic@chromium.org>

In addition to enabling async suspend/resume on the xhci-plat device,
we must enable it for the dwc3-exynos platform device in order to make
the full USB stack resume asynchronously.  Like the xhci-plat, ehci-s5p,
and ohci-exynos drivers, there are no outside dependencies which would
make resuming the dwc3-exynos driver asynchronously unsafe.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 drivers/usb/dwc3/dwc3-exynos.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 8b20c70..57431b7 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -155,6 +155,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
 		goto err2;
 	}
 
+	device_enable_async_suspend(dev);
+
 	return 0;
 
 err2:
-- 
1.7.9.5

[RFC 5/5] usb: dwc3: enable async suspend/resume

[Yuvaraj Kumar C D]

From: Andrew Bresticker <abrestic@chromium.org>

In addition to enabling async suspend/resume on the xhci-plat device,
we must enable it for the dwc3 device (the parent of xhci-plat) in order
to make the full USB stack resume asynchronously.  Like the xhci-plat,
ehci-s5p, and ohci-exynos drivers, there are no outside dependencies
which would make resuming the dwc3 driver asynchronously unsafe.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
---
 drivers/usb/dwc3/core.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 59bb8d2..9c8a273 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -586,6 +586,8 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	pm_runtime_allow(dev);
 
+	device_enable_async_suspend(dev);
+
 	return 0;
 
 err3:
-- 
1.7.9.5

Re: [RFC 5/5] usb: dwc3: enable async suspend/resume

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]

On Wed, Dec 18, 2013 at 04:09:34PM +0530, Yuvaraj Kumar C D wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
> 
> In addition to enabling async suspend/resume on the xhci-plat device,
> we must enable it for the dwc3 device (the parent of xhci-plat) in order
> to make the full USB stack resume asynchronously.  Like the xhci-plat,
> ehci-s5p, and ohci-exynos drivers, there are no outside dependencies
> which would make resuming the dwc3 driver asynchronously unsafe.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Julius Werner <jwerner@chromium.org>
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> ---
>  drivers/usb/dwc3/core.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 59bb8d2..9c8a273 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -586,6 +586,8 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	pm_runtime_allow(dev);
>  
> +	device_enable_async_suspend(dev);

how has this series been tested ? Let me rephrase this: was this series
tested at all ? Note that if we are in gadget mode, we will disable
physical endpoints 0 and 1, which will break USB communication requiring
a new enumeration later. On top of that, for versions of the core which
are configured without hibernation support - in fact for all cores
currently, since we don't have hibernation support implemented in this
driver -, we will loose communication with the far end (be it a host or
a device).

You mention there are no external dependencies to make async suspend
work on this driver, but that's far from the truth. As it is today, this
driver needs a lot of work to learn about all the details about all
different versions of this IP when it comes to supporting async PM.

I suppose this was tested with 500 other out-of-tree patches and you
simply cherry-picked this patch to send upstream ? Am I right ? It
certainly looks like it.

Please let us know how has this been tested ? Did you run USB30CV ? Did
you make sure to run through USB Certification interoperability tests ?

Did you leave some stress test running for weeks before sending this
patch out ? Is Exynos 5 working fine out of mainline ?

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 4/5] usb: dwc3-exynos: enable async suspend/resume

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1197 bytes --]

On Wed, Dec 18, 2013 at 04:09:33PM +0530, Yuvaraj Kumar C D wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
> 
> In addition to enabling async suspend/resume on the xhci-plat device,
> we must enable it for the dwc3-exynos platform device in order to make
> the full USB stack resume asynchronously.  Like the xhci-plat, ehci-s5p,
> and ohci-exynos drivers, there are no outside dependencies which would
> make resuming the dwc3-exynos driver asynchronously unsafe.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Julius Werner <jwerner@chromium.org>
> Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com>
> ---
>  drivers/usb/dwc3/dwc3-exynos.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 8b20c70..57431b7 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -155,6 +155,8 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>  		goto err2;
>  	}
>  
> +	device_enable_async_suspend(dev);

sure that clk_disable() in your ->suspend() callback will cause no
issues at all ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]