[More and more this looks like it belongs back in linux-usb-devel... Oh well, you can change it if you want.] On Sun, 14 Nov 2004, David Brownell wrote: > On Saturday 13 November 2004 08:38, Alan Stern wrote: > > > > Maybe things aren't quite as bad as I thought at first. Our model should > > be that when CONFIG_PM is set and CONFIG_USB_SUSPEND isn't then suspending > > the HC does a "bus-wide" suspend. > > That's the model in both cases though; what are you thinking > ought to be different about root hub behavior? The behavior of the root hubs is indeed the same in the two cases. However when CONFIG_USB_SUSPEND is set, a suspend operation will recursively do a set-port-feature(SUSPEND) for each device below the root hub. When it isn't set none of those ports will be suspended, only the entire bus will be. That is, if we have root-hub ---> hub H ---> device D then even if the ports leading to H and D aren't suspended, when the root hub is shut down H and D will suspend. Remember, the USB spec differentiates between suspended and non-suspended downstream ports in a hub, even when the hub itself is suspended. So the difference isn't in the root hub behavior, it's in the way we treat all the other USB devices. > > Packets won't be sent, but URBs don't > > have to be unlinked -- they can just sit in the schedule waiting until > > things start up again. Each usb_device->state can remain as CONFIGURED > > (since the core won't support USB_STATE_SUSPENDED), including the root hub > > device. HCDs should be able to enqueue and dequeue URBs, but none will > > complete normally. > > That is, they'll all report errors? No, they won't do anything at all. They'll just sit in the HCD's data structures until they are explicitly unlinked or the HC is resumed. > If you can make usbcore > behave consistently for all HCDs, preserving the ability of > the root hubs to deeply autosuspend, then I'm in favor of it. ? Doesn't usbcore behave consistently for all HCDs? I don't recall seeing any place where the behavior changes depending on which driver is bound to a controller. (Apart from things like PCI resource allocation in hcd-pci.c.) > > The root hub status timer will continue to fire four > > times per second but it shouldn't call the HCD's hub_status_data routine. > > The root hub timers get in the way here sometimes; I think > root hubs need to be more in control there. Here's a suggestion: Export an interface from the core to allow controller drivers to turn off the timer when they are suspended and turn it back on later. > > Not so clear is what to do about URBs submitted for the root hub. Right > > now the code rejects some > > "Some" sounds like a bug; all or none would be appropriate. > What's a better code to report the "I'm suspended and you > shouldn't have called me" error? We already have -EHOSTUNREACH. The real problem is that we don't have a clear idea just how cognizant the core should be about suspended devices when CONFIG_USB_SUSPEND isn't set. My impression has been that the core is totally unaware that it's possible for a device to be suspended, but this leads to exactly the issue you mention: How to report an error because the root hub is suspended when you don't recognize the existence of a suspended state? If root hubs worked like real USB devices there would be no need to report an error; requests simply wouldn't complete until the root hub was resumed. That's hard to do, though, because URBs are submitted to root hubs synchronously. Hence my suggestion for autoresuming when an URB is submitted. Another approach would be to recursively set all the device states from the root hub on down to USB_STATE_SUSPENDED when the HC is suspended, if CONFIG_USB_SUSPEND isn't set. In general this makes more sense, but it might confuse some drivers. > > of them with -EAGAIN, a rather inappropriate > > response. Perhaps the best course is to accept them, and resume the HC if > > it's necessary to call the hub_control routine. Under normal > > circumstances, such as preparing for a system suspend, there shouldn't be > > any such URBs. (Although there might be a few since the suspend call > > won't be synchronized with the hub driver.) > > I'd rather have these act the same regardless of > whether USB_SUSPEND is set. Autoresume sounds good, > going along with autosuspend mechanisms. But you don't want autoresume kicking in during a system suspend! It's not obvious how to prevent such a thing, apart from guaranteeing that no URBs are submitted. Maybe this is a job for in_system_suspend()... > It's a larger version of the problem where the DMA polling > from a periodic transfer (usb mouse, etc) keeps the Intel > CPU from entering C3 state. The timescale is larger, but > the result is the same: needless activity keeps the system > too busy to enter some states. > > If the system in question takes, say, 1/2 second to resume > from a particular low power mode, a 1/4 second timer will > prevent using that low power mode. How does that work? I would expect that in the low power mode the normal 1/HZ clock interrupt would be turned off. Hence the 1/4 second timer wouldn't fire, since it's triggered from the clock IRQ handler. But maybe I'm wrong and the clock interrupts do arrive every 1/HZ second -- surely that's already a lot worse than a routine waking up four times a second? > > > That's the right thing to do. I _think_ that last set of > > > changes makes a decent milestone, and would be happy if > > > we can avoid changing usbcore PM support for a while. > > > > But we will have to change it to match the FREEZE vs. SUSPEND semantics. > > Not yet in Linus' tree we don't ... :) Not yet, but it won't be long... > > I've started thinking about hcd->state and to what extent we really need > > it. More on this later -- but maybe it could be made mostly private to > > the HCDs and ignored by the core. > > Relying on that less would be good. But that'd mean > being able to use udev->state == USB_STATE_SUSPENDED > instead, in some cases. I haven't had time to look at it yet. Here are the places I can think of offhand where the core uses hcd->state: check whether to allow an URB to be queued; check whether to call the hub_status routine during the status timer handler; check whether to forward an IRQ to the handler routine; check whether the controller is currently suspended; check whether the controller has died. What have I left out? Most of these things can be handled using alternate mechanisms; they don't really need hcd->state. Alan Stern
