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

David Brownell
2006-05-27T00:19:54Z

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