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

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Fri, 2006-06-16 at 11:45 -0700, Linus Torvalds wrote:
> 
> On Fri, 16 Jun 2006, Linus Torvalds wrote:
> > 
> > ie I think it's literally an "outl()" followed by a "udelay()", and there 
> > is basically _zero_ room for problems. The "unfreeze" is then just 
> > setting the "run controller" bit again.
> 
> Something like
> 
> 	/*
> 	 * Used to temporarily stop all activity.
> 	 */
> 	static void freeze_uhci(struct uhci_hcd *uhci)
> 	{
> 		u16 cmd;
> 
> 		if (uhci->is_stopped)
> 			return;
> 		cmd = inw(uhci->io_addr + USBCMD) & ~USBCMD_RS;
> 		outw(cmd, uhci->io_addr + USBCMD);
> 		udelay(1);
> 	}
> 
> 	static void unfreeze_uhci(struct uhci_hcd *uhci)
> 	{
> 		u16 cmd;
> 		if (uhci->is_stopped)
> 			return;
> 		cmd = inw(uhci->io_addr + USBCMD) | USBCMD_RS;
> 		outw(cmd, uhci->io_addr + USBCMD);
> 	}
> 
> would seem to be enough, if the caller also guarantees that interrupts are 
> off during this (which you'd also want regardless, I assume).

Well, you also need to synchronize with other things trying to re-enable
queue processing (I don't know secifically about UHCI here but there may
be issues with OHCI) and other things like that...  (root hub activity,
urb processing, etc....)

Depending on the device, the "frozen" state may cause all sort of
troubles if requests come in and no protection against that are taken.
Granted freezing userland helps for some of that (though one would have
to freeze also things like kernel nfs server, prevent filesystem
read-ahead, etc...), but you know how intricated some drivers can be... 

Thus you end up with something quite similar to a full suspend ...
except the power off part. That is you stop processing of queues and
stop the hardware from DMA'ing. That is something simple for network
drivers and more complicated for various others...

That's why having a simple parameter to suspend() indicating wether you
want a full suspend or just a freeze works well in most cases: The
driver author doesn't have to think too much about it and can default to
suspend (suboptimal but works). I think it makes things easier on the
driver side of things.

In fact, if we implement the prepare() step we discussed and we also
make sure, as I proposed, that "bus drivers" do not hotplug new devices
in between prepare() and finish(), that will handle part of the problem
for STD as well: the hub driver of USB would be esssentially "stopped"
by prepare() (at least stopped from a device insertion point of view),
thus limiting the issues with both suspend and freeze later on
(sycnhronisation with the root hub for example has been typically
annoying to deal with in the past). 

> For a number of simple devices, just disabling interrupts guarantees that 
> they won't do anything, but busmasters obviously need to be told to stop 
> their BM engine (which is what the above should do for UHCI).

Yes but various code path in drivers tend to re-enable interrupts or
re-enabling DMA processing, it's not _that_ simple... in the end, as I
said, the necessary driver code to acheive that end up being very
similar if not identical to what is needed for suspend.

> Of course, these days EHCI etc is probably more interesting than UHCI, but 
> I only personally worked with UHCI, so I don't know the details, but I 
> assume it has a similar "run" bit in some command register.
> 
> My point is, this has nothing to do with _suspending_ the device.

No, but it's about suspending the _driver_. My point is that suspending
the device and suspending the driver are 2 different things. STR
involves both, STD involves only the driver. However, because of the
dependency on parent devices, they always have to be done at the same
time.

Ben.