On Thu, 5 Apr 2007, David Brownell wrote:

> On Thursday 05 April 2007 1:04 pm, Alan Stern wrote:
> > This patch (as880) strives to keep the PM core's idea of a USB
> > interface's power state in synch with usbcore's own idea.  In the end
> > this doesn't really matter, but it's better to be consistent.
> 
> ISTR trying this originally and watching it break things in some
> of the test scenarios I tried.  Quite a surprise actually.  And as
> I recall, it interacted with the issue in your patch #7/11 ...

Did you run all those tests before usb_suspend_both() and
usb_resume_both() were written?  If you did, these things work very
differently now.

To tell the truth, I no longer remember exactly why I wrote this patch!  
It probably was for some trivial reason, like preventing some annoying 
messages from showing up in the system log.  The actual functional impact 
should be nil.

> What was your test matrix?
> 
>       - USB_SUSPEND on and off?
>       - All host controller drivers?
>       - Devices with remote wakeup enabled?  HCDs?  Both, using it?
>       - ...
> 
> I remember identifying at least half a dozen factors that needed
> testing to make sure all code paths got hit ... some worked this
> way, some  didn't.  My test matrix covered each factor on/off, and
> various populat combinations.  Retesting after each bugfix was a
> PITA ... 20 minutes per test, if they succeeded; sigh.
> 
> I think that USB_SUSPEND was the key factor I saw, or remote wakeup.

Such testing is no longer necessary, or no longer _as_ necessary as
before.  The value of intf->dev.power.power_state is not read anywhere in
usbcore, only written.  (There is an apparently unnecessary reference in
usbtest.c, but it shouldn't be affected by this patch.  It also has a
race...)

BTW, the same is true for udev->dev.power.power_state, with two 
exceptions:

        At one point in hub.c the value is checked in lieu of looking
        at udev->state when CONFIG_USB_SUSPEND isn't set;

        At one place in hcd-pci.c the value for a root hub is checked --
        this really should be replaced with a check for USB_STATE_SUSPENDED.

So the only possible impact would lie in the PM core.  There the only uses 
are to control whether certain messages are logged and to control whether 
or not the suspend and resume methods actually get called.  Since the 
suspend and resume methods for USB interfaces don't do anything, again 
this won't matter.

(There also are some uses in runtime.c, but that whole piece of code is 
fundamentally broken anyway.)

Alan Stern


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
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