usb: xhci: plat: Enable runtime PM

PATCH Superseded Robert Foss 2 participants 7 messages Dec 1, 2016 – Dec 14, 2016

Subsystems (2)

USB USB xHCI driver

lore

Share & tools

lore

Robert Foss

author

2016-12-01 21:46 UTC #0 lore raw

v7 cover usb: xhci: plat: Enable runtime PM

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

Changes since v1:
- Added Signed-off-by: Robert Foss <[email protected]>
- Added proper metadata tags to series

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

Changes since v3:
- Decrement usage_counter for failed pm_runtime_get*() calls

Changes since v4:
- Added missing brackets

Changes since v5:
- Switched out atomic_dec() calls with pm_runtime_put() calls

Changes since v6:
- Rebased on v4.9-final

Andrew Bresticker (1):
usb: xhci: plat: Enable async suspend/resume

Robert Foss (1):
usb: xhci: plat: Enable runtime PM

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

Signature · 1 line

2.11.0

Robert Foss

author

2016-12-01 21:46 UTC #1 lore raw

PATCH v7 1/2 usb: xhci: plat: Enable runtime PM

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

Signed-off-by Robert Foss <[email protected]>

Tested-by Robert Foss <[email protected]>

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

drivers/usb/host/xhci-plat.c +27 -2

index ed56bf9ed885..ba4efe74f537 100644
@@ -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,6 +297,13 @@ 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) {
+		pm_runtime_put(dev);
+		return ret;
+	}
 
 	/*
 	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed

@@ -301,15 +313,28 @@ 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;
 
-	return xhci_resume(xhci, 0);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		pm_runtime_put(dev);
+		return ret;
+	}
+
+	ret = xhci_resume(xhci, 0);
+	pm_runtime_put(dev);
+
+	return ret;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {

Signature · 1 line

2.11.0

Baolin Wang

2016-12-12 05:21 UTC #2 lore raw

Hi Robert,

Quoted 15 lines · Robert Foss

Enable runtime PM for the xhci-plat device so that the parent device may implement runtime PM.
Signed-off-by Robert Foss <[email protected]>
Tested-by Robert Foss <[email protected]>
---
 drivers/usb/host/xhci-plat.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-plat.c +0 -0
index ed56bf9ed885..ba4efe74f537 100644

drivers/usb/host/xhci-plat.c static int xhci_plat_probe(struct platform_device *pdev) · line 250

Quoted 5 lines

drivers/usb/host/xhci-plat.c +2 -0

@@ -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);

You did not implement the runtime callbacks of xHCI device, then how can the parent device control the resume/suspend of xHCI device by runtime PM mechanism? Could you see my previous patch which implement the runtime callbacks for xHCI device?
https://lkml.org/lkml/2016/11/28/25

Quoted 61 lines

drivers/usb/host/xhci-plat.c +25 -3

+
        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,6 +297,13 @@ 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) {
+               pm_runtime_put(dev);
+               return ret;
+       }
 
        /*
         * xhci_suspend() needs `do_wakeup` to know whether host is allowed

@@ -301,15 +313,28 @@ 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;
 
-       return xhci_resume(xhci, 0);
+       ret = pm_runtime_get_sync(dev);
+       if (ret < 0) {
+               pm_runtime_put(dev);
+               return ret;
+       }
+
+       ret = xhci_resume(xhci, 0);
+       pm_runtime_put(dev);
+
+       return ret;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
--

2.11.0

Signature · 2 lines

Baolin.wang
Best Regards

Robert Foss

author

2016-12-14 06:43 UTC #3 lore raw

Hey Baolin,

Quoted 21 lines · Baolin Wang

Hi Robert,
Quoted 18 lines · Robert Foss
Enable runtime PM for the xhci-plat device so that the parent device may implement runtime PM.
Signed-off-by Robert Foss <[email protected]>
Tested-by Robert Foss <[email protected]>
---
 drivers/usb/host/xhci-plat.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-plat.c +0 -0
index ed56bf9ed885..ba4efe74f537 100644
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
        if (ret)
                goto dealloc_usb2_hcd;
 

Quoted 9 lines

drivers/usb/host/xhci-plat.c static int xhci_plat_probe(struct platform_device *pdev) · line 250
Quoted 2 lines
drivers/usb/host/xhci-plat.c +2 -0
+       pm_runtime_set_active(&pdev->dev);
+       pm_runtime_enable(&pdev->dev);
You did not implement the runtime callbacks of xHCI device, then how can the parent device control the resume/suspend of xHCI device by runtime PM mechanism? Could you see my previous patch which implement the runtime callbacks for xHCI device?
https://lkml.org/lkml/2016/11/28/25

Pardon my ignorance, but which exact functions need to be implemented?
Also, does the lkml link you provided contain an example implementation of those functions?

Quoted 64 lines

Quoted 61 lines

drivers/usb/host/xhci-plat.c +25 -3

+
        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,6 +297,13 @@ 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) {
+               pm_runtime_put(dev);
+               return ret;
+       }
 
        /*
         * xhci_suspend() needs `do_wakeup` to know whether host is allowed

@@ -301,15 +313,28 @@ 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;
 
-       return xhci_resume(xhci, 0);
+       ret = pm_runtime_get_sync(dev);
+       if (ret < 0) {
+               pm_runtime_put(dev);
+               return ret;
+       }
+
+       ret = xhci_resume(xhci, 0);
+       pm_runtime_put(dev);
+
+       return ret;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
--

2.11.0

Baolin Wang

2016-12-14 09:27 UTC #4 lore raw

Hi Robert,

Quoted 39 lines · Robert Foss

Hey Baolin,
Quoted 25 lines · Baolin Wang
Hi Robert,
Quoted 20 lines · Robert Foss
Enable runtime PM for the xhci-plat device so that the parent device may implement runtime PM.
Signed-off-by Robert Foss <[email protected]>
Tested-by Robert Foss <[email protected]>
---
 drivers/usb/host/xhci-plat.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-plat.c +0 -0
index ed56bf9ed885..ba4efe74f537 100644
@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device
 *pdev)
        if (ret)
                goto dealloc_usb2_hcd;
 
Quoted 10 lines
drivers/usb/host/xhci-plat.c static int xhci_plat_probe(struct platform_device *pdev) · line 250
Quoted 2 lines
drivers/usb/host/xhci-plat.c +2 -0
+       pm_runtime_set_active(&pdev->dev);
+       pm_runtime_enable(&pdev->dev);
You did not implement the runtime callbacks of xHCI device, then how can the parent device control the resume/suspend of xHCI device by runtime PM mechanism? Could you see my previous patch which implement the runtime callbacks for xHCI device?
https://lkml.org/lkml/2016/11/28/25

Quoted 3 lines

Pardon my ignorance, but which exact functions need to be implemented? Also, does the lkml link you provided contain an example implementation of those functions?

Please check below link:
https://lkml.org/lkml/2016/12/13/32
https://lkml.org/lkml/2016/12/13/34

Quoted 70 lines

Quoted 69 lines

Quoted 66 lines

drivers/usb/host/xhci-plat.c +10 -0

+
        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,6 +297,13 @@ 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) {
+               pm_runtime_put(dev);
+               return ret;
+       }
 
        /*
         * xhci_suspend() needs `do_wakeup` to know whether host is

allowed

drivers/usb/host/xhci-plat.c +15 -2

@@ -301,15 +313,28 @@ 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;
 
-       return xhci_resume(xhci, 0);
+       ret = pm_runtime_get_sync(dev);
+       if (ret < 0) {
+               pm_runtime_put(dev);
+               return ret;
+       }
+
+       ret = xhci_resume(xhci, 0);
+       pm_runtime_put(dev);
+
+       return ret;
 }

static const struct dev_pm_ops xhci_plat_pm_ops = {
--
2.11.0

Signature · 2 lines

Baolin.wang
Best Regards

Robert Foss

author

2016-12-01 21:46 UTC #5 lore raw

PATCH v7 2/2 usb: xhci: plat: Enable async suspend/resume

In reply to Robert Foss

From: Andrew Bresticker <[email protected]>

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 <[email protected]>

Tested-by Andrew Bresticker <[email protected]>

Tested-by Robert Foss <[email protected]>

Signed-off-by Robert Foss <[email protected]>

---
 drivers/usb/host/xhci-plat.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/host/xhci-plat.c +1 -0

index ba4efe74f537..c35b7fe3c999 100644
@@ -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;

Signature · 1 line

2.11.0

Robert Foss

author

2016-12-09 17:18 UTC #6 lore raw

In reply to Robert Foss

Hey,

I was hoping to get an ACK on v7, if there is anything I can to do expedite the process please let me know!

Rob.

Quoted 33 lines · Robert Foss

This series enables runtime PM and asynchronous resume/suspend support for xhci-plat devices.
Changes since v1:
- Added Signed-off-by: Robert Foss <[email protected]>
- Added proper metadata tags to series
Changes since v2:
- Added missing changelog to cover-letter
- Added error checking to pm_runtime_get_sync() calls
Changes since v3:
- Decrement usage_counter for failed pm_runtime_get*() calls
Changes since v4:
- Added missing brackets
Changes since v5:
- Switched out atomic_dec() calls with pm_runtime_put() calls
Changes since v6:
- Rebased on v4.9-final
Andrew Bresticker (1):
usb: xhci: plat: Enable async suspend/resume
Robert Foss (1):
usb: xhci: plat: Enable runtime PM
 drivers/usb/host/xhci-plat.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)