Re: autosuspend for HID devices, take #2

On Tue, 22 May 2007, Oliver Neukum wrote:

> - Alan, do you have strong feelings about putting all conditions
>   for failing to suspend into one condition? I consider it conceptually
>   cleaner to have seperate branches for wanting to check for failure

It looks like you've got two conditions for failing to suspend:

	The driver is still busy doing something like error recovery
	when the autosuspend timer expires;

	I/O in progress or on the queue takes too long to complete.

Those are sufficiently different in nature that it makes sense to have 
separate branches.

On the other hand, I still think you'd be better off with only one
spin_lock_irq() call in hid_suspend().  After all, you always have to
synchronize with the error handler, no matter what kind of suspend it
is.  In addition there would be nothing really wrong with calling
usbhid_wait_io() for an autosuspend, since it should return right
away.  If you follow this advice, you'll find that the two branches
share quite a lot more code than they do now.

> TODO
> 
> - pre/post_reset is currently broken, it can no longer be a parasite
>   on suspend/resume

Why not?  The only difference I can see is that you're allowed to fail 
a suspend but not a pre_reset.

In fact, failing a non-auto suspend is not a good idea.  You could 
easily end up preventing somebody's laptop from hibernating when they 
close the lid and put it in their knapsack.

> - adapt to hibernate

What adaptations are needed?

> There is a principal issue. What is to be done with output requests?
> Starting the queue as soon as one arrives seems the safest thing to do,
> but it is fatal in a subset of situations, that is, it must not be done during
> snapshotting and when going for system suspend. The freezer is insufficient
> to prevent new requests from coming in.

Because new requests are generated as a result of interrupt processing.  

But if you kill the interrupt URB then there will be no more inputs and
hence nothing to generate any more output.  Thus, when suspending you
should kill the inputs and wait for the outputs to drain (or else
explicitly plug the output queue).  Then it will be safe to autoresume
whenever a new output queue entry arrives.

Come to think of it, it would be safest to have an explicit way of 
plugging the queues.  But those details are up to you.

> Alan, are you going to scream and leap with talons extended if I
> suggest yet another pair of methods for USB devices to facilitate
> that?

Probably.  Besides, it's not just a USB problem.  Other drivers -- and
also some kernel threads -- will need to know when a hibernation is
about to start.  Isn't there going to be a notifier chain added for 
this?

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel