[PATCH 0/1] xhci: fixes for 3.14

[PATCH 0/1] xhci: fixes for 3.14

[Mathias Nyman]

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(-)

-- 
1.8.1.2

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

[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 <mathias.nyman@linux.intel.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/usb/host/xhci-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 04f986d..753857f 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -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;
 }
 
-- 
1.8.1.2

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

[Mathias Nyman]

On 03/03/2014 08:37 PM, Greg KH wrote:
> On Mon, Mar 03, 2014 at 07:30:17PM +0200, Mathias Nyman wrote:
>> 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

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

> 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?

>> 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

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

[Greg KH]

On Tue, Mar 04, 2014 at 01:50:55PM +0200, Mathias Nyman wrote:
> On 03/03/2014 08:37 PM, Greg KH wrote:
> >On Mon, Mar 03, 2014 at 07:30:17PM +0200, Mathias Nyman wrote:
> >>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
> 
> >
> >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.

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.

> >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

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

[David Cohen]

Hi Greg,

On Tue, Mar 04, 2014 at 09:04:58AM -0800, Greg KH wrote:
> On Tue, Mar 04, 2014 at 01:50:55PM +0200, Mathias Nyman wrote:
> > On 03/03/2014 08:37 PM, Greg KH wrote:
> > >On Mon, Mar 03, 2014 at 07:30:17PM +0200, Mathias Nyman wrote:
> > >>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
> > 
> > >
> > >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.
> 
> 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.
> 
> > >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 :)

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

> 
> Please resend this with patches for 3.15-rc1.

...that sounds fine :)

Br, David

> 
> thanks,
> 
> greg k-h

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

[Sarah Sharp]

On Tue, Mar 04, 2014 at 09:04:58AM -0800, Greg KH wrote:
> On Tue, Mar 04, 2014 at 01:50:55PM +0200, Mathias Nyman wrote:
> > On 03/03/2014 08:37 PM, Greg KH wrote:
> > >On Mon, Mar 03, 2014 at 07:30:17PM +0200, Mathias Nyman wrote:
> > >>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
> > 
> > >
> > >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.
> 
> 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