Re: [PATCH 2/2] Fix console handling during suspend/resume

[Adam Belay <ambx1@neo.rr.com>]

On Fri, Jun 23, 2006 at 09:04:10PM -0700, David Brownell wrote:
> On Friday 23 June 2006 8:12 pm, you wrote:
> 
> > For example, on the run-time management, if we shut things down not as a 
> > "pci_device" but as a "network device" (which just happens to be _bound_ 
> > to a pci device), we could very easily do the highlevel network device 
> > crap to make sure that we don't get entered that way _first_. And do it in 
> > just one place.
> 
> Heh, I said as much in a recent note.  The issue is that the network
> stack doesn't know suspend from joe.  If "eth0" had a real "struct device",
> that solution should work ... and simplify lots of driver suspend and
> resume methods.  Backwards compat would be an issue though.
> 
> 
> > > One thing that might help us get there is if we passed a suspend notification
> > > to the class devices (i.e. the higher level subsystems).
> > 
> > Good point. We probably should. That really really makes sense, and that 
> > also automagically solves the "network device" issue.
> 
> I'm not sure doing that with class devcies is the right idea, at least
> until they show up in the driver model tree as physical children of the
> parent hardware (so that the driver model tree automatically handles
> sequence constraints.

I agree totally, class devices should be the real children of their physical
device instances.  It's really all about representing how the drivers are
_actually_ layered.  In the PCI network device case, the code always follows
this structure:

PCI Device -> Network Device Driver (e.g. e1000) -> Network Device Class

Therefore, I think the driver model parent-child relationship should match the
above exactly.  Currently we don't model driver instances at all and there is
a lot of unneeded asymmetry between class devices and normal devices.  I've
added Kay to CC as he's posted some interesting patches in the past that work
toward changing this.

Now for why this is relevant to suspend/resume...  If the driver model
framework exposes the correct layered structure of device drivers, then we
can just walk the device tree and call the suspend functions at each stage
of the suspend process with no special exceptions.  Currently, the device
drivers (notice that it's the middle layer in the above example) is the only
entry point for suspend notifications.  As a result, all of the burden of
quiescing the device falls on their shoulders, even though this is almost
always a higher-order subsystem issue.  In the end, we get large ammounts of
duplicated code and a the potential for added complexity.

However, it's also interesting that these device drivers have full
responsibility for enabling PME generation and entering lower PCI power states
during a suspend transition.  Let's remember, this is entirely a
PCI-specific issue, and more often than not, every device driver is doing
the exact same thing:

pci_disable_device(dev);
pci_save_state(dev);
pci_set_power_state(dev, PCI_D3);

So the PCI device instance itself could also stand to recieve these suspend
callbacks.  Not only that, but entering the correct PCI D-state is actually
a very complicated decision, often involving platform specific data (e.g.
ACPI) and it's generally very dependent on the target system-level suspend
state.  The horribly broken pci_choose_state() interface we have today doesn't
even come close to handling this correctly.  So again, we have large ammounts
of duplicated code, much of which isn't even correct.

However, if we pass along suspend notifications to every logical device driver
layer, then each layer only has to worry about issues that are important to
the specific hardware abstraction level it's entrusted to control.  To most
device drivers this means things become dead simple (possibly some won't have
to do anything at all).  Also, we can put in the time and effort to make sure
that some of the more tricky code paths (i.e. higher layers) work well because
they will always be called in a consistent dependable manner and there is only
one entry point.  Finally, it becomes a lot easier to make revisions to each
individual driver layer suspend routine without breaking code in others.

The driver model today, in many ways, is far from providing this sort of
abstraction.  However, we can certainly work toward it gradually.  Linus's
patch to add suspend/resume callbacks at the "struct device_class" level does
exactly that.

Thanks,
Adam

P.S.:  Linus, what are your thoughts on passing a mirror image of the suspend
callbacks we provide (or will provide) for the device interface to the class
device interface?  In other words, allow it to also get suspend_prepare(),
resume_finish(), etc. to encourage the sort of abstraction suggested above.