Hi! > > It's Ok if it involves a drive change, so long as its an optional change, which > > means that it shouldn't affect the interface very much (i.e. the calling > > convention). That's why it'd be good to augment and/or modify pm_message_t > > to implement the changes, so we wouldn't have to change every single driver > > again.. > > I'll post more patches after I sort out some oddness -- why is swsusp_suspend() > leaving preempt_count() == 1, code I was nowhere near? -- but 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. > @@ -143,29 +143,61 @@ typedef struct pm_message { > } pm_message_t; > > /* > - * There are 4 important states driver can be in: > - * ON -- driver is working > - * FREEZE -- stop operations and apply whatever policy is applicable to a > - * suspended driver of that class, freeze queues for block like IDE > - * does, drop packets for ethernet, etc... stop DMA engine too etc... > - * so a consistent image can be saved; but do not power any hardware > - * down. > - * SUSPEND - like FREEZE, but hardware is doing as much powersaving as > - * possible. Roughly pci D3. > - * > - * Unfortunately, current drivers only recognize numeric values 0 (ON) and 3 > - * (SUSPEND). We'll need to fix the drivers. So yes, putting 3 to all different > - * defines is intentional, and will go away as soon as drivers are fixed. Also > - * note that typedef is neccessary, we'll probably want to switch to > - * typedef struct pm_message_t { int event; int flags; } pm_message_t > - * or something similar soon. > + * Several driver power state transitions are externally visible, affecting > + * the state of pending I/O queues and (for drivers that touch hardware) > + * interrupts, wakeups, and other hardware state. There may also be > + * internal transitions to various low power modes, which are transparent > + * to the rest of the driver stack (such as a driver that's ON gating off > + * clocks which are not in active use). > + * > + * One transition is triggered by resume(), after a suspend() call; the > + * message is implicit: > + * > + * ON Driver starts working again, responding to hardware events > + * and software requests. The hardware may have gone through > + * a power-off reset, or it may have maintained state from the > + * previous suspend() which the driver will rely on while > + * resuming. On most platforms, there are no restrictions on > + * availability of resources like clocks during resume(). > + * > + * 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? This will result in if (message == FREEZE || message == PRETHAW) that is kind-of ugly. Now, you are right that you only needed to fix 16 places, so it is not _that_ bad... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html