Re: xhci: suspend/resume issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 18, 2010 at 11:54:29AM -0500, Alan Stern wrote:
> On Wed, 17 Nov 2010, Sarah Sharp wrote:
> 
> > On Wed, Nov 17, 2010 at 11:14:36AM -0500, Alan Stern wrote:
> > > On Tue, 16 Nov 2010, Sarah Sharp wrote:
> > > > At this point, the device has migrated from port 1 (a SuperSpeed port)
> > > > to port 3 (a High Speed port):
> > > 
> > > Surely this is the real problem.  Once the device has changed speeds 
> > > like this, what's the right way to get it to change back to SuperSpeed?  
> > > Reset the high-speed port?
> > 
> > Yes, the High Speed port needs to get reset.  Then the USB device will
> > go searching for SuperSpeed terminations.  If it doesn't find those,
> > it's supposed to drop back to high speed, but apparently this device
> > loses its mind and stops responding to descriptor requests.
> 
> So we definitely need to handle the LS/FS/HS root-hub ports before
> resuming the SS ports.  You might be able to make that work by
> numbering the ports properly (put all the SS ports at the end).  This 
> may fix part of Don's problem without the full-blown root-hub split.

Hmm, ok, I'll look into that after Don tests the first patchset.  All
the xHCI host controllers I've seen advertise the SS ports first.

> Do you have any idea why the device switched over to high speed in the 
> first place?

I'd have to watch with a USB 3.0 bus analyzer to see what's really going
on.  Since Don doesn't have any other USB 3.0 devices, we can't
determine whether it's just one buggy device, or a host-side issue.

> > You might be thinking of John Youn's USB 3.0 hub patch.  There was a bit
> > in there to make hub_port_disable() return if it was called with a USB
> > 3.0 hub:
> > 
> > +static inline int hub_is_superspeed(struct usb_device *hdev)
> > +{
> > +       return (hdev->descriptor.bDeviceProtocol == 3);
> > +}
> > @@ -607,7 +632,7 @@ static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
> >         if (hdev->children[port1-1] && set_state)
> >                 usb_set_device_state(hdev->children[port1-1],
> >                                 USB_STATE_NOTATTACHED);
> > -       if (!hub->error)
> > +       if (!hub->error && !hub_is_superspeed(hub->hdev))
> >                 ret = clear_port_feature(hdev, port1, USB_PORT_FEAT_ENABLE);
> >         if (ret)
> >                 dev_err(hub->intfdev, "cannot disable port %d (err = %d)\n",
> > 
> > But the problem with that patch and the current state of the xHCI driver
> > is then the USB core is disallowed from disabling all the roothub ports
> > (including HS/FS/LS) under an xHCI host.  Ideally, we would just prevent
> > it from disabling SS ports.
> 
> Yet another reason for the root-hub split.
> 
> > > We had a discussion about this issue some time back, and as I recall, 
> > > the outcome was that we decided there aren't any situations where 
> > > usbcore should need to disable a SuperSpeed port.  I don't remember if 
> > > we decided anything about non-SuperSpeed xHCI ports.
> > 
> > Yeah, I don't think anything was decided.  I'm still not sure why the
> > persist code disables a port if the device disappears from under it.
> 
> Well, "disappears" is too strong a word.  The device may well still be 
> there and still be active -- it's just that for whatever reason, the 
> kernel thinks something bad has happened to it.  If we were to assign 
> the same address to a different device without disabling the port, we 
> could end up in a situation where two active devices are using the same 
> address.  Not good.

Oh, I see.  That shouldn't be an issue with USB 3.0 devices, since
packets are routed, not broadcast, on the USB 3.0 wires.  But that's
definitely an argument for allowing the USB core to disable USB 2.0
ports under an xHCI host.

> > So the two extra patches I send Don dig around in the xHCI Extended
> > Capabilities registers and look for the registers that tell the driver
> > which speeds the ports are supposed to be.  Then the xHCI driver simply
> > refuses to disable SS ports.  But it feels like that code should really
> > be in the USB core instead.
> 
> Agreed.  On the other hand, this business of digging through the
> Extended Capabilities registers sounds like something you would need to
> do anyway before you could split up the root hub.

Yes, I had already created that patch for the split root hub work.

> Why on earth didn't the designers just specify that ports 1 - N would
> always be SuperSpeed and ports (N+1) - 2N would always be the
> corresponding LS/FS/HS connections?

Because they wanted to allow designers to put more of one type of
connector than the other on the box.  For example, you could expose only
two SuperSpeed ports and six LS/FS/HS ports.  Then the xHCI host
controller would have 2 PORTSC registers with SuperSpeed capabilities,
and eight LS/FS/HS PORTSC registers (two of which correspond to the SS
ports, but you wouldn't be able to tell which ones).

> > I'm also wondering if the roothub splitting patches will help deal with
> > the other issue Don is having.  Ideally, the USB core would look at the
> > HS/FS/LS ports first, reset any connected devices under them, and then
> > go process the SS ports.  Then any SS devices that originally connected
> > as HS would migrate over during the reset.
> > 
> > I'm not sure how to do this with the current combined roothub and the
> > current USB core code.  But if the roothub were split instead, couldn't
> > we make the SS roothub wait for the HS roothub?
> 
> Yes.
> 
> >  How does the current
> > resume code handle companion controllers?  Doesn't UHCI or OHCI have to
> > get resumed first, before EHCI?
> 
> That's right.  There's special code in core/hcd-pci.c to handle this,
> both matching up controllers with their companions and waiting during
> resume.  (The implication, by the way, is that when you register your
> two usb_hcd structures, the USB-2 structure must be registered before
> the USB-3 structure so that it will be resumed first.)

Ok, I'll have to look into that.  The code originally registered the USB
3.0 roothub first, to ensure all resources got allocated to it, and the
PCI device stored that usb_hcd in its private pointer.  But I hadn't
gotten to the PCI suspend/resume yet, and I guess that will have to
change.

> There's also special code in core/driver.c:usb_resume_device(); 
> non-root devices on a LS/FS bus must not be resumed until after the 
> root hub on the companion HS bus.  I don't know if you will need 
> anything comparable.

I'm not sure yet.  I haven't tried testing suspend with a USB 3.0 hub
yet.  But I suspect if the USB 3.0 portion of the hub isn't resumed
before the USB 2.0 portion, then the USB 3.0 devices under it will show
up as High Speed devices.  They'll come back from suspend, not find the
SuperSpeed terminations, and then switch over to HS.  But if the USB 3.0
hub is resumed first, it will turn on terminations to all downstream
ports and the devices will connect at SS.  I'm going to ask the hub guy
at Intel about it.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux