Re: [linux-usb-devel] Re: [patch/rft 2.6.17-rc2] swsusp resume must not device_suspend()

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

On Friday 26 May 2006 4:16 pm, Pavel Machek wrote:
> 
> > 		 the patch appended
> > here shows what I'm pursuing.  Same calling convention, new PRETHAW message
> > that "pm-naive" drivers (most of them!) can handle just like FREEZE.
> > 
> > Other than this, it affects about 20 files with about 2/3 being drivers; say
> > that maybe 5% of all in-tree drivers needed trivial changes, most of which
> > could count as bugfixes _before_ defining the new message.
> 
> Okay, so you changed the interface, and it needed fixing at 16 places.

Not what I said ... in many of those places, the driver code was already dubious.
Making the event code __bitwise would have helped catch those errors.


> > + * Other transitions are triggered by messages sent using suspend().  All
> > + * these transitions quiesce the driver, so that I/O queues are inactive.
> > + * That commonly entails turning off IRQs and DMA; there may be rules
> > + * about how to quiesce that are specific to the bus or the device's type.
> > + * (For example, network drivers mark the link state.)  Other details may
> > + * differ according to the message:
> > + *
> > + * SUSPEND	Quiesce, enter a low power device state appropriate for
> > + * 		the upcoming system state (such as PCI_D3hot), and enable
> > + * 		wakeup events as appropriate.
> > + *
> > + * FREEZE	Quiesce operations so that a consistent image can be saved;
> > + * 		but do NOT otherwise enter a low power device state, and do
> > + * 		NOT emit system wakeup events.
> > + *
> > + * PRETHAW	Quiesce as if for FREEZE; additionally, prepare for restoring
> > + * 		the system from a snapshot taken after an earlier FREEZE.
> > + * 		Some drivers will need to reset their hardware state instead
> > + * 		of preserving it, to ensure that it's never mistaken for the
> > + * 		state which that earlier snapshot had set up.
> 
> And you *do* realize that PRETHAW is like a FREEZE... So... can we use
> FREEZE, and add aditional flag field that says it is preTHAW?

Of course, FREEZE is like a SUSPEND, and SUSPEND is the only behavior that's
required of all drivers, or which most drivers understand.  So why did you not
implement FREEZE as a flag in the first place, if you like that model?  :)

Most drivers treat all suspend() messages as SUSPEND anyway; mesg.event was
already little more than a fancy flag (SUSPEND vs FREEZE).  We only need
three transition types (so far), not four (flag x flag ==> 4 states).

Plus, I'd never add flags to that structure.  I've elaborated some of the reasons
why not in the past, and I won't repeat that here; it's basically a bad idea,
wastes data and code space, is an error prone and ugly state machine design
practice, and so forth.


> This will result in if (message == FREEZE || message == PRETHAW) that
> is kind-of ugly.

switch (mesg.event) { ... } for the few drivers that actually care about
more than one of the transitions.  Only a handful need to care about more
than whether it's a real SUSPEND or not.

- Dave