xhci: Prevent runtime pm from autosuspending during initialization

RFC PATCH Superseded Mathias Nyman 5 participants 7 messages Feb 24, 2014 – Feb 28, 2014

Subsystems (2)

USB USB xHCI driver

lore

Share & tools

lore

Mathias Nyman

author

2014-02-24 16:19 UTC #0 lore raw

RFC PATCH v2 1/1 xhci: Prevent runtime pm from autosuspending during initialization

xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready.

Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready.

xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe.

Signed-off-by Mathias Nyman <[email protected]>

---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

drivers/usb/host/xhci-pci.c +10 -1

index 04f986d..ea7158b 100644
@@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	struct usb_hcd *hcd;
 
 	driver = (struct hc_driver *)id->driver_data;
+
+	/* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */
+	pm_runtime_get_noresume(&dev->dev);
+
 	/* Register the USB 2.0 roothub.
 	 * FIXME: USB core must know to register the USB 2.0 roothub first.
 	 * This is sort of silly, because we could just set the HCD driver flags

@@ -199,7 +203,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	retval = usb_hcd_pci_probe(dev, id);
 
 	if (retval)
-		return retval;
+		goto put_runtime_pm;
 
 	/* USB 2.0 roothub is stored in the PCI device now. */
 	hcd = dev_get_drvdata(&dev->dev);

@@ -228,12 +232,17 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (xhci->quirks & XHCI_LPM_SUPPORT)
 		hcd_to_bus(xhci->shared_hcd)->root_hub->lpm_capable = 1;
 
+	/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
+	pm_runtime_put_noidle(&dev->dev);
+
 	return 0;
 
 put_usb3_hcd:
 	usb_put_hcd(xhci->shared_hcd);
 dealloc_usb2_hcd:
 	usb_hcd_pci_remove(dev);
+put_runtime_pm:
+	pm_runtime_put_noidle(&dev->dev);
 	return retval;
 }

Signature · 1 line

1.8.1.2

Dan Williams

Acked-by on v2 1/1

2014-02-24 16:47 UTC #1 lore raw

Quoted 5 lines · Mathias Nyman

xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready.

Quoted 8 lines

Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready.
xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe.
Signed-off-by Mathias Nyman <[email protected]>

Acked-by Dan Williams <[email protected]>

Alan Stern

Acked-by on v2 1/1

2014-02-24 17:44 UTC #2 lore raw

In reply to Mathias Nyman

Quoted 22 lines · Mathias Nyman

xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready.
Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready.
xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe.
Signed-off-by Mathias Nyman <[email protected]>
---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +0 -0
index 04f986d..ea7158b 100644

drivers/usb/host/xhci-pci.c static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) · line 195

Quoted 6 lines

drivers/usb/host/xhci-pci.c +3 -0

@@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	struct usb_hcd *hcd;
 
 	driver = (struct hc_driver *)id->driver_data;
+
+	/* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */
+	pm_runtime_get_noresume(&dev->dev);

Strictly speaking, this prevents the _controller_ from going into runtime suspend -- not the root hub.

Quoted 20 lines

drivers/usb/host/xhci-pci.c +5 -1

+
 	/* Register the USB 2.0 roothub.
 	 * FIXME: USB core must know to register the USB 2.0 roothub first.
 	 * This is sort of silly, because we could just set the HCD driver flags

@@ -199,7 +203,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	retval = usb_hcd_pci_probe(dev, id);
 
 	if (retval)
-		return retval;
+		goto put_runtime_pm;
 
 	/* USB 2.0 roothub is stored in the PCI device now. */
 	hcd = dev_get_drvdata(&dev->dev);

@@ -228,12 +232,17 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	if (xhci->quirks & XHCI_LPM_SUPPORT)
 		hcd_to_bus(xhci->shared_hcd)->root_hub->lpm_capable = 1;
 
+	/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
+	pm_runtime_put_noidle(&dev->dev);
+

drivers/usb/host/xhci-pci.c static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) · line 247

Quoted 10 lines

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

 	return 0;
 
 put_usb3_hcd:
 	usb_put_hcd(xhci->shared_hcd);
 dealloc_usb2_hcd:
 	usb_hcd_pci_remove(dev);
+put_runtime_pm:
+	pm_runtime_put_noidle(&dev->dev);
 	return retval;
 }

Apart from nit about the comment,

Acked-by Alan Stern <[email protected]>

Sarah Sharp

2014-02-28 20:32 UTC #3 lore raw

Quoted 26 lines · Alan Stern

Quoted 24 lines · Mathias Nyman
xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready.
Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready.
xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe.
Signed-off-by Mathias Nyman <[email protected]>
---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +0 -0
index 04f986d..ea7158b 100644
@@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	struct usb_hcd *hcd;
 

Quoted 7 lines

drivers/usb/host/xhci-pci.c static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) · line 195
Quoted 4 lines
drivers/usb/host/xhci-pci.c +3 -0
 	driver = (struct hc_driver *)id->driver_data;
+
+	/* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */
+	pm_runtime_get_noresume(&dev->dev);
Strictly speaking, this prevents the _controller_ from going into runtime suspend -- not the root hub.

Signed-off-by Sarah Sharp <[email protected]>

Mathias, want to fix this comment and queue this up to Greg? It should probably be marked for stable.

Quoted 4 lines

Apart from nit about the comment,
Acked-by Alan Stern <[email protected]>

Dan Williams

2014-02-28 20:41 UTC #4 lore raw

Quoted 35 lines · Sarah Sharp

Quoted 26 lines · Alan Stern
Quoted 24 lines · Mathias Nyman
xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready.
Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready.
xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe.
Signed-off-by Mathias Nyman <[email protected]>
---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +0 -0
index 04f986d..ea7158b 100644
@@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
    struct usb_hcd *hcd;
 
Quoted 7 lines
drivers/usb/host/xhci-pci.c static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) · line 195
Quoted 4 lines
drivers/usb/host/xhci-pci.c +3 -0
    driver = (struct hc_driver *)id->driver_data;
+
+   /* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */
+   pm_runtime_get_noresume(&dev->dev);
Strictly speaking, this prevents the _controller_ from going into runtime suspend -- not the root hub.

Quoted 1 line

Signed-off-by Sarah Sharp <[email protected]>

Pardon the nitpick, but this is an acked-by if Mathias is submitting.

Greg KH

2014-02-28 21:56 UTC #5 lore raw

Quoted 37 lines · Dan Williams

Quoted 35 lines · Sarah Sharp
Quoted 33 lines · Alan Stern
Quoted 22 lines · Mathias Nyman
xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready.
Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready.
xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe.
Signed-off-by Mathias Nyman <[email protected]>
---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +0 -0
index 04f986d..ea7158b 100644
drivers/usb/host/xhci-pci.c static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) · line 195
Quoted 6 lines
drivers/usb/host/xhci-pci.c +3 -0
@@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
    struct usb_hcd *hcd;
 
    driver = (struct hc_driver *)id->driver_data;
+
+   /* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */
+   pm_runtime_get_noresume(&dev->dev);
Strictly speaking, this prevents the _controller_ from going into runtime suspend -- not the root hub.

Quoted 3 lines

Quoted 1 line
Signed-off-by Sarah Sharp <[email protected]>
Pardon the nitpick, but this is an acked-by if Mathias is submitting.

No, it's a "chain of signed-off-by's" and this is allowed just fine (it used to be the default-thing before we came up with the "acked-by" line...)

Dan Williams

2014-02-28 22:02 UTC #6 lore raw

Quoted 38 lines · Greg KH

Quoted 37 lines · Dan Williams
Quoted 35 lines · Sarah Sharp
Quoted 33 lines · Alan Stern
Quoted 22 lines · Mathias Nyman
xHCI driver has its own pci probe function that will call usb_hcd_pci_probe to register its usb-2 bus, and then continue to manually register the usb-3 bus. usb_hcd_pci_probe does a pm_runtime_put_noidle at the end and might thus trigger a runtime suspend before the usb-3 bus is ready.
Prevent the runtime suspend by increasing the usage count in the beginning of xhci_pci_probe, and decrease it once the usb-3 bus is ready.
xhci-platform driver is not using usb_hcd_pci_probe to set up busses and should not need to have it's usage count increased during probe.
Signed-off-by Mathias Nyman <[email protected]>
---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +0 -0
index 04f986d..ea7158b 100644
drivers/usb/host/xhci-pci.c static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) · line 195
Quoted 6 lines
drivers/usb/host/xhci-pci.c +3 -0
@@ -190,6 +190,10 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
    struct usb_hcd *hcd;
 
    driver = (struct hc_driver *)id->driver_data;
+
+   /* Prevent USB-2 roothub runtime suspend until USB-3 is initialized. */
+   pm_runtime_get_noresume(&dev->dev);
Strictly speaking, this prevents the _controller_ from going into runtime suspend -- not the root hub.

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Sarah Sharp <[email protected]>
Pardon the nitpick, but this is an acked-by if Mathias is submitting.
No, it's a "chain of signed-off-by's" and this is allowed just fine (it used to be the default-thing before we came up with the "acked-by" line...)

Ok. jejb picked on me about it once, but I'm more than willing to drop this off my list of things to nitpick.