On Thu, 2 Feb 2012, Sarah Sharp wrote: > > > Is there any reason why the USB core would be suspending a bus if all > > > attached devices were not suspended? > > > > Yes, as a matter of fact. This happens during hibernation, in the > > PMSG_FREEZE stage. The idea is that everything is supposed to be > > quiescent (no IRQs or DMA) but should remain at full power. The USB > > core calls the suspend methods for all the device drivers, so all URBs > > should be cancelled, but it doesn't actually suspend the devices. And > > it does call the bus_suspend methods for the HCDs, because the host > > controllers are what actually generate IRQs and do DMA. > > Why doesn't it actually suspend the devices? Because some of them might > react badly to suspend? No, because it would be a waste of time. They would just be powered back up again in a second or so. Also I forgot to mention it earlier, but the same thing happens when CONFIG_USB_SUSPEND isn't enabled. Suspend and bus_suspend methods get called, but no ports are put into suspend (or U3). > > Well, there are two things to worry about. The first is that you don't > > want xhci_bus_suspend to return successfully if wakeup is enabled and > > any ports have generated a wakeup event. The second is that you don't > > want the interrupt handler routine to be modifying port settings > > concurrently with xhci_bus_suspend. > > For the first concern, I think there will always be a race condition > between the xhci_bus_suspend and the port status interrupt handler. If > the USB 2.0 bus is active, and we're trying to suspend the USB 3.0 bus, > we drop the xhci->lock at the end of bus suspend, and it's possible to > get an interrupt with a device-initiated resume port status change > before xhci_bus_suspend returns. I don't see a way around that race. > > Initially, I wondered if this race condition was actually an issue. Say > we suspended both buses, but the interrupt for start of a > device-initiated resume crept in before we suspended the PCI device. > We'd clear the PLC bit, set the link state to U0 to stop the resume > signaling, and not kick the roothub, counting on the second PLC change > to generate an interrupt. What if the PCI device is suspended then? Does the second PLC-change interrupt even take place if the root hub isn't running? And is it safe to clear PLC and set the link state after bus_suspend is done? See the first two Notes in 5.4.8. > We would be basically suspending a host while it's in the middle of > reflecting device resume signaling, and I don't think there's any > guarantee we would get that second PLC bit. So I think we do need to > prevent xhci_suspend from running if a remote wakeup has been signaled. Maybe. But what if the controller isn't enabled for wakeup? In that case, a remote resume should not prevent the controller from suspending. > How about this: > > xhci_bus_suspend takes the xhci->lock, checks whether any port is > resuming (by checking the USB 2.0 resume bits or the USB 3.0 > remote_wakeup_bits), and fails if it finds that. Then it sets a new > flag, xhci->bus_state->suspended. > > When the port status handler grabs the xHCI lock and finds a USB 3.0 > port has a device-initiated resume in progress, it sets the > remote_wakeup_bits for that port. If it finds > xhci->bus_state->suspended is set, it attempts to kick the roothub. If > not, it skips the kick, and acts exactly an external USB 3.0 hub that > does not send an interrupt transfer on device-initiated resume. > > That may mean we kick the roothub when the USB 3.0 bus is suspended, but > the USB 2.0 bus is not (there is a per-roothub bus_state structure). > However, I think that's necessary to avoid having the USB core suspend > the USB 2.0 bus and PCI device before the end of the USB 3.0 > device-initiated resume causes the PLC to be set. I'm not sure that you have settled on the right approach yet, which means it's too early to start worrying about implementation details. > There's still the synchronization between the interrupt handler and the > PCI suspend function that needs to be taken care of. When we enter > xhci_suspend, there might be an interrupt in progress. If it hasn't had > time to grab the xHCI spinlock, xhci_suspend will stop and suspend the > PCI device before the interrupt handler has a chance to run. > > Table 10 lists when the xHC will issue a PME# due to a port event. > Unfortunately, it only lists an entry for when the port is in U3 and > resume signaling happens. It doesn't list when the port is in Resume > and the host is suspended. So we can't rely on the host controller > automatically bringing itself out of PCI suspend in this race condition. > (Although if I was a hardware engineer, I would signal a PME# if I saw > any resume signaling on a port.) > > But the interrupt handler will run after xhci_suspend drops the lock. > Right now, it will discover the status register reads as 0xfffffff > (since the PCI device is suspended), think the HCD was removed, and call > usb_hcd_died(). It probably should be changed to check whether the PCI > device is suspended, and call usb_hcd_resume_root_hub() or maybe kick > the roothub? I'm not quite sure what the right function to call is, but > basically I want the PCI device to be unsuspended. > > I think this solution will be OK, because the device will still be > signaling the resume until we set the link state to U0. So it will just > be signaling for a bit longer until we resume the PCI device. > > It might be clearer if I just put this into code, but how does it sound? I tend to find English clearer than code, for anything at all subtle. Code tells you what's happening but not why, and it doesn't explain the implications of the various possible design choices. Alan Stern -- 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
