On Mon, 15 Nov 2004, David Brownell wrote:
> That is: they'll get the "new" behavior of suspend()/resume()
> callbacks, so the devices enter USB_STATE_SUSPEND after all
> the interfaces' drivers have been suspended.
Yes. That's the real problem with not setting CONFIG_USB_SUSPEND:
drivers' suspend/resume callbacks will never be invoked, but the devices
will nevertheless get suspended from time to time. Definitely a potential
source of trouble.
> > > > 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.
>
> I'd call that "completing normally".
Except that they don't complete...
> That's not what we do for
> requests going to devices in USB_STATE_SUSPEND...
No. And that's an argument in favor of setting the device states even
when CONFIG_USB_SUSPEND isn't on. And don't forget, we have to worry not
just about URBs submitted after the suspend but also URBs submitted
beforehand that have not yet completed. Normally a driver's suspend
callback would unlink such URBs, but it can't if the callback isn't
invoked...
> > 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.
>
> That's what I was thinking, except that "later" could be "never".
> More consistently: insist the HCD turn it on in the first place.
That makes sense for all controllers capable of interrupting when a port
change event occurs; they don't need any polling. It also would let
drivers handle weird cases like controllers that can signal change events
but only when they are suspended, not when they are running (which was how
my PIIX4 UHCI controller worked, last time I checked).
> > 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?
>
> The difference seeming to be partially necessary, since the whole
> reason to treat that as an error is that the drivers _have_ been
> told to suspend().
It's not just the drivers; there's also sysfs and usbfs. They haven't
been told anything but they still need to receive those errors.
> > 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.
>
> Except when some ancestor is in the FREEZE state? Autoresuming
> would be wrong during system sleep transitions, right?
It certainly would. Which raises an interesting question: Do we want to
allow autoresume during a selective suspend? Presumably yes -- but how is
a driver supposed to know whether a suspend call is selective or
system-wide?
The question about ancestors being frozen/suspended applies to autoresume
in general. It's why I once suggested adding a PM routine to resume a
device plus all its ancestors.
> But that gets us back to FREEZE being orthogonal to power saving
> modes: drivers could be { full power, FROZEN } during some sleep
> state transitions, or { low power, THAWED } during routine operation
> to save power.
The current PM setup doesn't recognize "low power" as such. There's just
frozen and suspended (which implies frozen). I don't think anything's
wrong with that; wouldn't a device in a low-power mode to save power
during routine operation also be frozen?
What we do need is a way to specify whether autoresume is enabled. This
concept is closely tied in with remote wakeup. I'm tempted to say they
can be treated as the same thing, but that's not necessarily so: There may
be devices (like the Genesys host controller) that are capable of remote
wakeup but not capable of resume signalling.
> see above re going back to FREEZE/THAW being transitions
> orthogonal to sleep states. Did we see an API proposal
> for that yet?
No.
> That's why I'd rather just plan to get rid of CONFIG_USB_SUSPEND
> once we settle on how the "normal" suspend logic should work, and
> it's reasonably well debugged. (And we get usbcore out of the
> business of enforcing the hierarchical stuff that pmcore isn't
> doing for us... except during system sleep transitions.)
There's a lot to worry about. Without CONFIG_USB_SUSPEND we have to
test each of the following:
Freeze/suspend HC for system sleep transition,
Selective suspend of HC,
Autosuspend of root hub by HCD.
With CONFIG_USB_SUSPEND we will also have to test:
Suspend root hub for system sleep transition,
Selective suspend of root hub,
Autosuspend of root hub by hub driver (not yet written).
> This was in conjunction with a "Variable Scheduling Timeout" (VST)
> patch, so the timer precision is still 1/HZ but the interrupt
> rate is less ... if the next scheduled timeout is 1 second in
> the future, the timer IRQ wouldn't fire until then. The S/390
> folk contributed some infrastructure a while back that makes
> VST trivial. The basic deal is that CPU can stay in the idle
> "loop" a lot longer.
How does jiffies get updated when VST is in effect and no timeouts are
scheduled for a while?
> > What have I left out? Most of these things can be handled using alternate
> > mechanisms; they don't really need hcd->state.
>
> But that "alternate mechanism" is in most cases udev->state
> being USB_STATE_SUSPEND, either for the specific device or
> for all of the ones on that particular bus.
In other words, for the root hub. That seems okay to me;
USB_STATE_SUSPEND is a good thing to check for.
Alan Stern
