On Sun, 19 Sep 2004, David Brownell wrote:

> Well, there's some concept of selective suspend, but not much of one.
> 
> So for example PCI has the concept, as does USB, but as you've
> noticed:  things misbehave, at very basic levels like not guaranteeing
> children suspend before parents ... and devices that are already
> suspended getting woken up in order to suspend the whole system.
> (Pointlessly woken up, that is.)

For some unknown reason the rule seems to be that transitions are allowed 
between fully-powered and any of the low-power states, but not between 
different low-power states.  This seems like a mistake; the PM core 
shouldn't make any such assumptions about the capabilities of different 
devices.  If some device can't make these transitions, its driver should 
interpolate the intermediate power-on state automatically or should inform 
the PM core somehow.

> Did you know that the swsusp and pmdisk versions are merging?
> Last I looked, those changes didn't affect pmcore.  However the
> "swsusp2" work does have some fixes to that.

I haven't paid any attention to that stuff.  It's hard enough following 
all the USB problems!

> > I also found a few bugs in the USB code.  The problems can be summed up 
> > like this: Who is responsible, the caller or the callee, for checking that 
> > old_state != new_state (i.e., whether the call is necessary), and who is 
> > responsible for setting dev->power.power_state to new_state?  The code is 
> > inconsistent in its answers to these questions, leading to various weird 
> > behaviors.
> 
> Yes, I have some fixes in that area that I've not yet submitted.  Of course,
> the PM core code should be making the same tests too, and doesn't;
> at least not consistently.

Looking more at the USB code, I found some rhyme & reason behind the
behavior.  Power state levels are sent to the interface drivers, but for
the whole usb_devices they aren't sent (because USB only defines two power
levels, on or suspended).

That brings up another question: Why do we have all these
suspend()/resume() pairs?  Why not just a single set_power() routine, that
takes the desired new power level as an argument and maybe also a flag
stating whether the actual new power level must be <=, ==, or >= the
desired level?  Is this historical baggage?

> Another example of "PM core design is confused" is that it's not clear
> who _interprets_ the states.  So for example USB only has one suspend
> state.  Yet when the PM core code checks those states, it wrongly thinks
> that USB has multiple suspend states ... so it tries to "suspend more deeply"
> by first resuming (the whole subtree!!) then suspending it again (to the
> same level).  Bleech.

Just so -- an example of the error I mentioned above.

> Oh yes, and there's also the fact that the PM core currently insists
> there be a flat namespace of state numbers.  As Linus has observed,
> given that assumption, and the number of PM-aware drivers in Linux,
> there is only one suitable answer:  the numers must match what
> PCI uses.  Yet it doesn't.  (See OSDL bugid 2886.)

To fix these problems will require getting a consensus among a reasonably 
large number of people.  Do you know of anyone actively working on it?


> > For instance, interface drivers' resume() method can be called either from
> > finish_port_resume() or from usb_generic_resume().  One checks whether the
> > interface has already been resumed and the other doesn't.  One of the
> > callees, hub_resume(), doesn't check.  Hence there can be multiple resumes 
> > of a hub.
> > 
> > As another example, the bus->op->hub_suspend() call in 
> > __usb_suspend_device() doesn't have to set power_state because the code 
> > following the call does so.  But the corresponding call to 
> > bus->op->hub_resume() in usb_resume_device() does have to set power_state.
> 
> And there are others.  Some of those fixes are in my patchset.

Would you care to post your fixes?

> > Even the suspend_device() and resume_device() routines in 
> > drivers/base/power/{suspend,resume}.c are inconsistent about checking 
> > whether the device is already suspended/resumed.  At least they are 
> > consistent in relying on the callee to set power_state.
> > 
> > Presumably the callee is always supposed to set power_state, rather than 
> > the caller -- that is the majority behavior.  But who's supposed to weed 
> > out unnecessary calls?
> 
> There are several levels of caller and callee ... too many ways into
> suspend()/resume() methods to rely exclusively on a single answer
> there, I'm afraid, given the current PM core.
> 
> For the moment I'm thinking that protection in both places in usbcore
> is safest.  Let's try to keep individual drivers from needing to check; we
> can do that, since we know there's only one suspend state.  Once that
> mess starts to behave sanely, we'll at least be armored against the worst
> bogosity from the PM core code, and can clean things up later when
> the PM framework makes sense.

That makes sense.

> Unfortunately I think the PM core code still needs to be rewritten, but I'd
> rather have USB behaving better -- notably, integrated so that it works
> OK with system-wide suspend not just USB suspend/wake/resume --  before
> any of that starts.

I think that should be very doable, and it shouldn't even need very much 
work -- apart from the root hub implementations (subject of a different 
thread).

Alan Stern



-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to