Re: PATCH/RFC: usbcore wakeup updates (3/4)

[David Brownell <david-b@pacbell.net>]

On Thursday 07 October 2004 2:19 pm, Pavel Machek wrote:
> 
> Hmm, perhaps it is wrong thing to tell the devices what state to enter
> in the first place.

As a rule, yes.  (I think you've made that point before!)

During system suspend transitions, pmcore should just
be telling drivers to "get compatible with system state X",
after "idle everything" and (for swsusp/pmdisk) "write
this image to swap" steps.  PMcore has no business
telling drivers which device-specific modes to use; it
isn't supposed to know any hardware that well.  It
wouldn't know, for example, that when wakeup is
enabled, certain states are ruled out ...

Though I don't see anything obviously broken about
(for example) using sysfs to force some devices into
PCI_D3hot state ... or with maintaining compatibility
with today's PCI API, which talks in terms of power state.


> > Do you think adding those two bits to per-device PM state
> > is basically a good way to introduce their wakeup capabilities
> > to the PM core?  Suggestions on the next step?
> 
> It did not look overly ugly to me... so it is probably okay. Not sure
> what the next step is 

Merging into some tree... presumably after 2.6.10 opens!  :)


> -- you'll probably want some sysfs interface for 
> suspending single devices?

Other than "echo -n $(IDENTIFIER) > power/state" ?  No,
we have sysfs support already!  But problems with it include:

 - Minor annoyance:  "echo" doesn't work, "echo -n" does!

 - The sysfs code needs to handle suspending a _tree_ not
   just a single device ... it'll have children if the device is a
   bus adapter (HCD), bridge (hub), or just the floor of a stack
   of virtualized drivers (usb-storage hotplugging SCSI hosts and
   disks on the fly, network adapters, etc).  This needs to do
   bottom-up-suspend and top-down-resume -- but it doesn't.
   (And USB has some workarounds, but they may need to cross
   from USB into other driver stacks ...)

 - Semantics of $(IDENTIFIER).  I think those need to include
   driver-specific values, and that means ripping out code that
   "knows" otherwise.  That includes in PMcore code.  And
   what makes most sense to me there involves two different
   sets of state identifiers, identified as meaningful strings not
   cryptic digits:  (a) a handful of generic states like "idle",
   "lowpower", "on", and "off", plus (b) device-specific states
   that might borrow from the bus (PCI_D1, PCI_D3hot, etc)
   but which can be customized to match the hardware.

   For some devices, "idle" and "lowpower" may both map
   to "off"; for some, "lowpower" may mean "cut power when
   idle" (instead of "full power all the time").

Near term, I'd hope for tree suspend/resume but start to clean
up the mess involved with $(IDENTIFIER).  (Plus make the
wakeup mechanisms work better.)


> > > Introducing enums where PCI suspend level is stored in u32
> > > would be welcome... 
> > 
> > I'm not averse to enums, especially once sparse does good things
> > with them, but I still think that sort of change would just be nibbling
> > around the edges of a larger problem.  (Which should be addressed
> > by different patches making device power states/policies, like G0/D1
> > or "idle yourself", be types that are fully distinct from system power
> > states like G1/S3, and for which abuse creates compiler errors.)
> 
> I'm not sure we want to move to anything complicated than simple enum.

I'm pretty sure we should.  If for no other reason than to force
all the drivers to change.  They disagree about what $IDENTIFIER
means because enums are basically un-typechecked integers,
and that's unlikely to change.

But also, since a typed struct pointer can support lots of other
policy structures, including things analagous to "cpufreq"
governors.  A set of drivers can agree (maybe because they
share the same bus, or are otherwise related) that the
pointer gets container_of() treatment to morph to something
packaginging much more interesting power policies than a
simple PCI bus needs.  Like for example suspending four
devices, on different busses, together -- or not at all.  Or
understanding that when these devices all suspend, one
of the power supplies (or clocks) can be disabled.

- Dave


> 								Pavel
> -- 
> People were complaining that M$ turns users into beta-testers...
> ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
>