[ second half ]
> All right. Having said all of that, I'll respond to some of your
> comments.
I'll shrink this response by removing most points of agreement. :)
> > A different driver controls the
> > upstream link ... maybe a different instance of the hub driver (for
> > USB links), maybe an HCD (for links using some other bus).
>
> Yes. But controlling the upstream link is not the same as controlling the
> device's response to messages sent over that link. (Even though
> usb_suspend_device resides inside hub.c, it's not called by khubd.)
Doesn't matter where that routine lives. There are lots of routines
that would be better fits for different sources files. :)
> > That means it would be nonsensical (illogical, inconsistent) to support
> > access to *any* downstream ports/devices when the hub driver is suspended.
> >
> > Because such access would mean it's not the hub driver which is actually
> > managing those ports ... and if it isn't, who is? And why do we have a
> > hub driver then? Something would need to vanish in a puff of illogic!!
>
> ("Oh dear, I hadn't thought of that!" says God.) :-)
The worm Ouroboros swallows its tail without vanishing though;
not as illogical a notion of creation, I guess!
> Agreed. But there can be times when usbcore's idea of the port state
> differs from the actual state; see (3) above. And we don't need to call
> hcd->hub_suspend to disable access to the root hub ports; it suffices to
> suspend them individually.
But that issue doesn't affect which software is there to manage the
ports ... and since hcd->hub_suspend() doesn't suspend those ports,
I don't see what you're implying by that.
> > (Where remote wakeup mechanisms are only one part of the stack that
> > needs to work. On PCs, it needs to cascade through S1/S3/... through
> > ACPI to Linux. I've seen the wakeups arriving, but then ACPI borks;
> > that's actually progress from the last time I tested this.)
>
> You left out a third type of wakeup signal, assuming the HC isn't
> suspended: an interrupt from the HC with the Resume Detect bit set in some
> status register. This is in fact precisely the signal we would get when
> the root hub is suspended, the HC isn't, and a port status change occurs.
> These signals should be controllable by the same sort of wakeup_enable
> flag as the others.
Those events don't go upstream. They're internal to the HCD.
So in that "system" sense they're not really wakeup events ... for
all that they use those hardware wakeup signaling. (At least,
they do if the root hub doesn't have the bugs that some have...)
> > There's no way you can monitor the D+ and D- signals upstream of any
> > root hub. Since the definition of the USB suspend state is in terms
> > of those signals (within a VBUS power session), usb_suspend_device()
> > can't do that to a root hub.
>
> Root hubs don't have to obey the definition of the USB suspend state
> because they aren't USB devices. However they should act in much the same
> manner as USB devices when we call dpm_runtime_suspend.
"Much" the same, sure. Identical, no; they're not USB devices.
Their cojones are bigger or something.
> > > I would be a lot happier if you renamed hcd->hub_suspend to
> > > hcd->bus_suspend and explicitly defined it as leaving remote wakeup turned
> > > off. Then there would have to be a new callback, hcd->rh_suspend, which
> > > would do nothing but enable remote wakeup,
> >
> > I guess I don't see any confusion. The hub driver's suspend method works
> > only on the resources it manages: downstream ports. Same for all hubs
> > (root or external). As part of entering USB_STATE_SUSPEND, the device
> > on the other end of that link may be told to enable remote wakeup.
>
> The confusion is over whether the "hub" in "hcd->hub_suspend" is analogous
> to the "hub" in "hub_suspend" or to the "hub" in "root hub"; that is,
> whether it refers to a (root) hub interface/driver or a (root) hub device.
I chose my words carefully: the only resources that a hub
driver controls are the downstream ports. No difference;
no confusion.
> All along you've apparently thought of it as the HCD's component of an
> interface suspend -- just as hub_suspend is the hub driver's component --
I ended up thinking that, as the most consistent approach.
> whereas I've thought of it as the HCD's component of the device suspend.
> By definition (see above), the first means it gets called at the same time
> as hub_suspend and the second means it gets called as part of
> usb_suspend_device.
> Or to put it another way, is it called "hcd->hub_suspend" because it is
> logically part of the hub driver's hub_suspend, or because it suspends the
> root hub?
The former. ;)
> That's why I prefer "bus_suspend", to make it clear that the routine
> suspends an entire bus. Or even "global_suspend", since that's
> appropriate, although I know you don't like it.
I think I already said I'd have no problem with renaming it bus_suspend;
it's not actually "global" and that's why I don't like that phrase.
(It was chosen way back when having two USB controllers was bizarre!)
> And I still say that we need to separate out remote-wakeup-enable for the
> root hub from suspending the root hub's interface.
I guess I just don't see any point to that. Though you may be focussing
more on _external_ software trying to manage the root hub power than
the autosuspend/autoresume mechanism already working in OHCI. (I came
across a note from someone who said that made a notable difference in
battery life on their laptop, FWIW.)
- Dave
p.s. Here are the script I mentioned in my previous note.
============================
#!/bin/bash
# pm-wake ... list devices with wakeup capability
# note: /proc/acpi/wakeup devices aren't yet integrated with the
# driver model tree.
# devtype $PATH ==> $type
devtype ()
{
local F T
# fixed length, currently ten spaces
type=""
for F in $(cd $1; echo *:*)
do
case $F in
# ignore device names by recognizing specific patterns
net:eth*) type="lan "; return;;
net:*) type="net "; return;;
usb_host:*) type="usb_host "; return;;
sound:*) type="modem "; return;;
i2c-dev:*) type="i2c "; return;;
# avoid really generic labels
usb_device:*)
read T < $1/maxchild
if [ 0 -lt $T ]
then
type="hub "
return
fi
# REVISIT this ignores Ethernet adapters...
for T in $(cd $1; echo */input:*)
do
type="input "
return
done
type="(usb) "
continue;;
esac
done
if [ "$type" = "" ]
then
for T in $(cd $1; echo fw-host*/ieee1394_host:*)
do
type="firewire "
return
done
fi
if [ "$type" = "" ]
then
type=" "
fi
}
cd /sys/devices
for F in $(find * -name 'wakeup')
do
# F=.../power/wakeup
read value < $F
if [ "$value" = "" ]
then
continue
fi
# F=...
F=$(dirname $(dirname $F))
devtype $F
# for each entry that actually supports wakeup, one line with:
# - wake on/OFF
# - device type (if recognized)
# - /sys/devices/... path
case "$value" in
"disabled") echo "OFF $type $F" ;;
"enabled") echo "on $type $F" ;;
esac
done
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
