On Sunday 14 November 2004 18:08, Alan Stern wrote:
> 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 ..
>
> So the difference isn't in the root hub behavior, it's in the way
> we treat all the other USB devices.
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.
> > > 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". That's not what we do for
requests going to devices in USB_STATE_SUSPEND...
> > 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 combination "usbcore + X-hcd" won't necessarily act the
same for all values of X. Minimally there are the differences
in how hardware reports a given fault, but there are other ways
the behavior can differ.
> > 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.
That's what I was thinking, except that "later" could be "never".
More consistently: insist the HCD turn it on in the first place.
> > > 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?
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().
> 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?
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.
> 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.
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.)
> > 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()...
Just like you don't want wakeup kicking in then either...
see above re going back to FREEZE/THAW being transitions
orthogonal to sleep states. Did we see an API proposal
for that yet?
> > 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?
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.
> > > 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;
The IRQ check is only to detect the "dead HCD" case; IRQs shouldn't
otherwise be trouble.
> 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.
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.
- Dave
> Alan Stern
>
>
