Skip to contentSkip to conversation

xhci: fixes for 3.14

PATCH New Mathias Nyman 4 participants 6 messages Mar 3, 2014 – Mar 4, 2014 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover xhci: fixes for 3.14

Hi Greg,

Here's a small xhci fix for 3.14 rc

Only one patch that prevents a panic on xhci_suspend() during xhci initialization on some specific intel platforms if autosuspend is set to 0.

We're already at rc5 and I'm not sure what is acceptable anymore at this stage in the cycle.

-Mathias

Mathias Nyman (1):
xhci: Prevent runtime pm from autosuspending during initialization

 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
Signature · 1 line
1.8.1.2
#1 lore raw

PATCH v3 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.

---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-pci.c +10 -1
index 04f986d..753857f 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 runtime suspending between USB-2 and USB-3 initialization */
+	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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#3 lore raw

Re: [PATCH v3 1/1] xhci: Prevent runtime pm from autosuspending during initialization

Quoted 7 lines · Greg KH
Quoted 4 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.

What is the result if that happens?

Crashes. Null pointer dereference in xhci_suspend() when touching xhci->shared_hcd before it's initialized. More info here:

http://marc.info/?l=linux-usb&m=138914518219334&w=2

Quoted 3 lines

Is this a regression from 3.13? Or something new for 3.14?

According to reporter its been around since 3.7

commit 596d789a211d134dc5f94d1e5957248c204ef850
USB: set hub's default autosuspend delay as 0

But nobody else than the reporter is able to trigger it.

Quoted 1 line

What platform(s) are affected by this?

David, the reporter (added to cc), mentioned
"This bug happened in a platform with 1 usb3 host controller + 1 usb3 OTG controller" run by some Intel internal group

http://marc.info/?l=linux-usb&m=138915969822029&w=2

David, can you elaborate on the platform?

Quoted 6 lines
Quoted 2 lines

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.

I didn't think we had any in-kernel users of the xhci-platform driver, has that changed and I missed a new platform being added?

Not that I'm aware of. I just wanted to point out that this issue is only a matter on pci enumerated xhci hosts.

-Mathias

maintainer
#4 lore raw

Re: [PATCH v3 1/1] xhci: Prevent runtime pm from autosuspending during initialization

Quoted 18 lines · Mathias Nyman
Quoted 7 lines · Greg KH
Quoted 4 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.

What is the result if that happens?

Crashes. Null pointer dereference in xhci_suspend() when touching xhci->shared_hcd before it's initialized. More info here:

http://marc.info/?l=linux-usb&m=138914518219334&w=2

Quoted 3 lines

Is this a regression from 3.13? Or something new for 3.14?

Quoted 6 lines

According to reporter its been around since 3.7

commit 596d789a211d134dc5f94d1e5957248c204ef850
USB: set hub's default autosuspend delay as 0

But nobody else than the reporter is able to trigger it.

Then it can wait for 3.15-rc1, and then go back to the stable trees at that time, right? I'd prefer that as it's not a regression and not common.

Quoted 7 lines
Quoted 1 line

What platform(s) are affected by this?

David, the reporter (added to cc), mentioned
"This bug happened in a platform with 1 usb3 host controller + 1 usb3 OTG controller" run by some Intel internal group

http://marc.info/?l=linux-usb&m=138915969822029&w=2

That sounds like a "not shipping platform" to me :)

Please resend this with patches for 3.15-rc1.

thanks,

greg k-h

#5 lore raw

Re: [PATCH v3 1/1] xhci: Prevent runtime pm from autosuspending during initialization

Hi Greg,

Quoted 32 lines · Greg KH
Quoted 18 lines · Mathias Nyman
Quoted 7 lines · Greg KH
Quoted 4 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.

What is the result if that happens?

Crashes. Null pointer dereference in xhci_suspend() when touching xhci->shared_hcd before it's initialized. More info here:

http://marc.info/?l=linux-usb&m=138914518219334&w=2

Quoted 3 lines

Is this a regression from 3.13? Or something new for 3.14?

Quoted 6 lines

According to reporter its been around since 3.7

commit 596d789a211d134dc5f94d1e5957248c204ef850
USB: set hub's default autosuspend delay as 0

But nobody else than the reporter is able to trigger it.

Then it can wait for 3.15-rc1, and then go back to the stable trees at that time, right? I'd prefer that as it's not a regression and not common.

Quoted 2 lines
Quoted 1 line

What platform(s) are affected by this?

Quoted 7 lines
Quoted 5 lines

David, the reporter (added to cc), mentioned
"This bug happened in a platform with 1 usb3 host controller + 1 usb3 OTG controller" run by some Intel internal group

http://marc.info/?l=linux-usb&m=138915969822029&w=2

That sounds like a "not shipping platform" to me :)

Not quite. You'd find this configuration on Asus T100T. I said "not quite" because I don't think we can run Linux smoothly on it yet. But...

Quoted 2 lines

Please resend this with patches for 3.15-rc1.

...that sounds fine :)

Br, David

Quoted 4 lines

thanks,

greg k-h

#6 lore raw

Re: [PATCH v3 1/1] xhci: Prevent runtime pm from autosuspending during initialization

In reply to Greg KH
Quoted 21 lines · Greg KH
Quoted 20 lines · Mathias Nyman
Quoted 7 lines · Greg KH
Quoted 4 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.

What is the result if that happens?

Crashes. Null pointer dereference in xhci_suspend() when touching xhci->shared_hcd before it's initialized. More info here:

http://marc.info/?l=linux-usb&m=138914518219334&w=2

Quoted 3 lines

Is this a regression from 3.13? Or something new for 3.14?

According to reporter its been around since 3.7

Quoted 8 lines
Quoted 4 lines

commit 596d789a211d134dc5f94d1e5957248c204ef850
USB: set hub's default autosuspend delay as 0

But nobody else than the reporter is able to trigger it.

Then it can wait for 3.15-rc1, and then go back to the stable trees at that time, right? I'd prefer that as it's not a regression and not common.

Ok, I'll queue it with the other features for 3.15.

Sarah Sharp