On Wed, 21 Sep 2005, David Brownell wrote:

> Another way to look at it:  "hub" is the only driver in the whole
> tree that's so confused about the difference between "device" and
> "interface".  It's not IMO worth the pile of special cases needed to
> maintain that confusion, scattering hub class code through device
> and hcd infrastructure.

I would have said that the confusion was over whether a hub's downstream 
ports belong to the device or to the interface.  If we pick one answer -- 
the interface -- and stick with it, then the special-case code should be 
minimal.

(Incidentally, another symptom of the design error is the fact that the 
children[] array is part of usb_device rather than usb_hub, so we're 
allocating wasted space for every non-hub USB device.)

> > (BTW, is there any possibility of changing things so that downstream
> > devices really _are_ children of the hub interface instead of children of
> > the hub device?  This would have repercussions for userspace, since the
> > sysfs pathnames would change -- which makes me suspect it's not feasible
> > politically.)
> 
> This is what I was alluding to when I pointed out that the pm_parent
> mechanism "should" eventually be able to solve problems for us.  One
> logical structure would have devices' pm_parents be some facet of the
> hub interface, with PM suspend/resume operations using that information
> through some TBS calls ... without making such sysfs name changes.

We shouldn't need to use the pm_parent mechanism for this.  It's not a 
very good mechanism anyway, because it only allows one extra parent per 
device.  A small amount of coding around the illogical device links should 
suffice here.

> On the other hand, don't you just want to _try_ changing that parentage
> without solving that pm_parent issue, and hearing the screams?  ;)

Actually, I'd like to try changing it and let somebody else listen to the 
screams...  :-)


> > The hub.c:hub_suspend routine should be changed so that for PM_FREEZE and
> > PM_SUSPEND events it skips checking the state of the child devices.  We
> > can trust the PM core to have already put them in the correct state.
> 
> No we can't.  There are two counter-examples today.  One is sysfs,
> which should arguably be changed to do some of that checking (and
> stop clobbering the device power state recorded by device drivers)
> to the extent it can be abstracted.  Another is root hub autosuspend.

The sysfs interface needs to be changed too.  It should issue events of 
type PM_RUNTIME, since it is a runtime PM interface, not system PM.  Of 
course, PM_RUNTIME has to be defined first!

Root hub autosuspend is a bit of a poser, because each HCD does it 
differently.  Regardless, it shouldn't use PM_SUSPEND or PM_FREEZE 
either.  When these changes are made, what I said will be correct.


> > When you suspend an external hub's interface but not the hub itself (i.e.,
> > not its upstream port), the hub won't generate remote wakeup requests.
> 
> The key point is that usbcore won't be watching the notifications;
> they stop inside the hub's port status bits unless the upstream link
> is suspended.  They're actually still being issued...

"Issued" isn't really the right word.  They're being stored up inside the
hub.  They never make it to the computer.

> > Port status changes would show up only in the status URB, and the hub
> > driver will have unlinked that URB.  But with a root hub, calling
> > hcd->hub_suspend _does_ enable remote wakeup.  
> 
> Strictly speaking, it wouldn't _need_ to enable it.  Then it'd act the
> same as with an external hub in that state:  a semi-responsive lump.
> 
> In practice we _do_ want to enable wakeup, if the root hub supports it.
> The way to disable a USB bus is to unbind its HCD, not to play games
> with awkward PM states.

Yes, wakeups wouldn't be enabled if the wakeup_enable bit wasn't set.  
But normally it would be set, and the point remains true.  As for
disabling a USB bus, when you do it you might also want to power down the
HC.  Is the PCI core capable of suspending a device with no driver?  If it
isn't, you would want the HCD to remain bound.  And what about non-PCI
controllers?  Ideally, I guess all drivers and buses should be written to
leave devices in a low-power state when they are unbound...

> > So hcd->hub_suspend should 
> > be called as part of the hub device suspend, not part of the hub interface
> > suspend.
> 
> Or wakeups could be enabled in root_hub_dev->suspend rather than as part
> of the downstream link suspension.  That'd give a closer match for
> external hubs ... but how closely should the two mimic each other?

I don't think that's a good way to do it.  You would get mixed up in
issues like PME# or other sorts of wakeup signalling, things that were
intended for system PM rather than runtime PM.  Better to remain
consistent: Calling usb_suspend_device enables remote wakeup (unless
wakeup_enabled is clear) whereas merely suspending interfaces does not.


For the sake of your analogy...  We know that usb_suspend_device does its
work by suspending a downstream port on the device's parent hub.  Of
course, root hubs don't have parents with downstream ports, but it makes
sense to imagine that this port corresponds to the wiring inside the HC
that connects its PCI circuitry to its USB serializers.  Then the analogy
says that hcd->hub_suspend suspends that wiring and the serializers, but
it doesn't suspend the HC's connection to the PCI bus -- that's the job of
root_hub_dev->suspend.

To put it more explicitly: I would consider a host controller to comprise
a part (A) that manages the connection to the PCI (or other system-level)  
bus, plus a part that manages the connection to the USB bus and which acts
like a hub.  This second part includes both the upstream (B) and
downstream (C) halves of the root hub, as laid out in Chapter 11.  That's
in contrast to what you have written; you essentially said that you viewed
the (A) part as the upstream half of the root hub.

My way corresponds better to the different kinds of resume signals
available from the HC.  There's PME# or the equivalent, which comes from
the non-USB (A) part; the Resume-Detect bit in the USB Status register,
which comes from the (B) part; and the Resume-Detect bits in the Port
Status registers, which come from the (C) part.  The last two are of
course analogous to the Remote Wakeup signal and PORT_C_SUSPEND bits
coming from an external hub.  That's why I say (B) should map to the 
upstream half-hub and (A) shouldn't map to any part of the hub.

My way also means that usb_suspend_device does the same thing for root and 
external hubs: It suspends both halves of the hub.


> > Having said that, I should also point out that in the long run, the
> > difference will be minimal.  As part of the runtime PM design I'm working
> > on, USB devices will automatically be suspended as soon as all their
> > interfaces are -- that's the upward propagation of PM state changes.  So
> > suspending a hub's interface will _cause_ the hub device to be suspended.
> 
> Right, I'm looking forward to seeing that.  My next patchset will
> smooth out most of the differences appearing with USB_SUSPEND, and
> make various remote wakeup scenarios work again.  So it'll go well
> with that mechanism.

Good.  I've been waiting for Greg to apply your patches before writing any 
of this new stuff, so there will be a common base to work from.

Alan Stern



-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to