[PACTH v3 0/2] usb: xhci: plat: Enable PM, async resume/suspend

[PACTH v3 0/2] usb: xhci: plat: Enable PM, async resume/suspend

[robert.foss]

From: Robert Foss <robert.foss@collabora.com>

This series enables runtime PM and asynchronous resume/suspend support for
xhci-plat devices.

This is a resumbmission of v3.

Changes since v1:
- Added Signed-off-by: Robert Foss <robert.foss@collabora.com>
- Added proper metadata tags to series.

Changes since v2:
- Added missing changelog to cover-letter.
- Added error checking to pm_runtime_get_sync().

Andrew Bresticker (2):
  usb: xhci: plat: Enable runtime PM
  usb: xhci: plat: Enable async suspend/resume

 drivers/usb/host/xhci-plat.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

-- 
2.7.4

[PACTH v3 1/2] usb: xhci: plat: Enable runtime PM

[robert.foss]

From: Andrew Bresticker <abrestic@chromium.org>

Enable runtime PM for the xhci-plat device so that the parent device
may implement runtime PM.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 drivers/usb/host/xhci-plat.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..4f39d4e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto dealloc_usb2_hcd;
 
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
 	return 0;
 
 
@@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct clk *clk = xhci->clk;
 
+	pm_runtime_disable(&dev->dev);
+
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_phy_shutdown(hcd->usb_phy);
 
@@ -292,7 +297,11 @@ static int xhci_plat_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
 
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		return ret;
 	/*
 	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
 	 * to do wakeup during suspend. Since xhci_plat_suspend is currently
@@ -301,15 +310,26 @@ static int xhci_plat_suspend(struct device *dev)
 	 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
 	 * also applies to runtime suspend.
 	 */
-	return xhci_suspend(xhci, device_may_wakeup(dev));
+	ret = xhci_suspend(xhci, device_may_wakeup(dev));
+	pm_runtime_put(dev);
+
+	return ret;
 }
 
 static int xhci_plat_resume(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		return ret;
 
-	return xhci_resume(xhci, 0);
+	ret = xhci_resume(xhci, 0);
+	pm_runtime_put(dev);
+
+	return ret;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
-- 
2.7.4

[PACTH v3 2/2] usb: xhci: plat: Enable async suspend/resume

[robert.foss]

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>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 drivers/usb/host/xhci-plat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 4f39d4e..89aef0e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -248,6 +248,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
+	device_enable_async_suspend(&pdev->dev);
 
 	return 0;
 
-- 
2.7.4

Re: [PACTH v3 1/2] usb: xhci: plat: Enable runtime PM

[Felipe Balbi]

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


Hi,

robert.foss@collabora.com writes:
> From: Andrew Bresticker <abrestic@chromium.org>
>
> Enable runtime PM for the xhci-plat device so that the parent device
> may implement runtime PM.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Robert Foss <robert.foss@collabora.com>
> Signed-off-by: Robert Foss <robert.foss@collabora.com>
> ---
>  drivers/usb/host/xhci-plat.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ed56bf9..4f39d4e 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto dealloc_usb2_hcd;
>  
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +
>  	return 0;
>  
>  
> @@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
>  	struct clk *clk = xhci->clk;
>  
> +	pm_runtime_disable(&dev->dev);
> +
>  	usb_remove_hcd(xhci->shared_hcd);
>  	usb_phy_shutdown(hcd->usb_phy);
>  
> @@ -292,7 +297,11 @@ static int xhci_plat_suspend(struct device *dev)
>  {
>  	struct usb_hcd	*hcd = dev_get_drvdata(dev);
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> +	int ret;
>  
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0)
> +		return ret;

you're leaving unbalanced PM usage_counter here. If pm_runtime_get*()
fails, you still have to decrement usage_counter.

>  	/*
>  	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
>  	 * to do wakeup during suspend. Since xhci_plat_suspend is currently
> @@ -301,15 +310,26 @@ static int xhci_plat_suspend(struct device *dev)
>  	 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
>  	 * also applies to runtime suspend.
>  	 */
> -	return xhci_suspend(xhci, device_may_wakeup(dev));
> +	ret = xhci_suspend(xhci, device_may_wakeup(dev));
> +	pm_runtime_put(dev);
> +
> +	return ret;
>  }
>  
>  static int xhci_plat_resume(struct device *dev)
>  {
>  	struct usb_hcd	*hcd = dev_get_drvdata(dev);
>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0)
> +		return ret;

ditto

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PACTH v3 1/2] usb: xhci: plat: Enable runtime PM

[Robert Foss]

On 2016-08-09 05:19 AM, Felipe Balbi wrote:
>
> Hi,
>
> robert.foss@collabora.com writes:
>> From: Andrew Bresticker <abrestic@chromium.org>
>>
>> Enable runtime PM for the xhci-plat device so that the parent device
>> may implement runtime PM.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> Tested-by: Robert Foss <robert.foss@collabora.com>
>> Signed-off-by: Robert Foss <robert.foss@collabora.com>
>> ---
>>  drivers/usb/host/xhci-plat.c | 24 ++++++++++++++++++++++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index ed56bf9..4f39d4e 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>  	if (ret)
>>  		goto dealloc_usb2_hcd;
>>
>> +	pm_runtime_set_active(&pdev->dev);
>> +	pm_runtime_enable(&pdev->dev);
>> +
>>  	return 0;
>>
>>
>> @@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
>>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
>>  	struct clk *clk = xhci->clk;
>>
>> +	pm_runtime_disable(&dev->dev);
>> +
>>  	usb_remove_hcd(xhci->shared_hcd);
>>  	usb_phy_shutdown(hcd->usb_phy);
>>
>> @@ -292,7 +297,11 @@ static int xhci_plat_suspend(struct device *dev)
>>  {
>>  	struct usb_hcd	*hcd = dev_get_drvdata(dev);
>>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
>> +	int ret;
>>
>> +	ret = pm_runtime_get_sync(dev);
>> +	if (ret < 0)
>> +		return ret;
>
> you're leaving unbalanced PM usage_counter here. If pm_runtime_get*()
> fails, you still have to decrement usage_counter.
>
>>  	/*
>>  	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
>>  	 * to do wakeup during suspend. Since xhci_plat_suspend is currently
>> @@ -301,15 +310,26 @@ static int xhci_plat_suspend(struct device *dev)
>>  	 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
>>  	 * also applies to runtime suspend.
>>  	 */
>> -	return xhci_suspend(xhci, device_may_wakeup(dev));
>> +	ret = xhci_suspend(xhci, device_may_wakeup(dev));
>> +	pm_runtime_put(dev);
>> +
>> +	return ret;
>>  }
>>
>>  static int xhci_plat_resume(struct device *dev)
>>  {
>>  	struct usb_hcd	*hcd = dev_get_drvdata(dev);
>>  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
>> +	int ret;
>> +
>> +	ret = pm_runtime_get_sync(dev);
>> +	if (ret < 0)
>> +		return ret;
>
> ditto
>

Thanks Filipe!
I'll be submitting a v4 as soon as testing has completed.

[PACTH v3 1/2] usb: xhci: plat: Enable runtime PM

[robert.foss]

From: Andrew Bresticker <abrestic@chromium.org>

Enable runtime PM for the xhci-plat device so that the parent device
may implement runtime PM.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Robert Foss <robert.foss@collabora.com>
Signed-off-by: Robert Foss <robert.foss@collabora.com>
---
 drivers/usb/host/xhci-plat.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index ed56bf9..4f39d4e 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto dealloc_usb2_hcd;
 
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
 	return 0;
 
 
@@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
 	struct clk *clk = xhci->clk;
 
+	pm_runtime_disable(&dev->dev);
+
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_phy_shutdown(hcd->usb_phy);
 
@@ -292,7 +297,11 @@ static int xhci_plat_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
 
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		return ret;
 	/*
 	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
 	 * to do wakeup during suspend. Since xhci_plat_suspend is currently
@@ -301,15 +310,26 @@ static int xhci_plat_suspend(struct device *dev)
 	 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
 	 * also applies to runtime suspend.
 	 */
-	return xhci_suspend(xhci, device_may_wakeup(dev));
+	ret = xhci_suspend(xhci, device_may_wakeup(dev));
+	pm_runtime_put(dev);
+
+	return ret;
 }
 
 static int xhci_plat_resume(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0)
+		return ret;
 
-	return xhci_resume(xhci, 0);
+	ret = xhci_resume(xhci, 0);
+	pm_runtime_put(dev);
+
+	return ret;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
-- 
2.7.4