Re: patch to autosuspend mice

Am Donnerstag, 1. Februar 2007 15:51 schrieb Alan Stern:
> On Wed, 31 Jan 2007, Oliver Neukum wrote:
> 
> > This would call mod_timer() for every completed in-URB. Are you
> > sure this is better than an approach maintaining a periodical timer
> > and check for IO in the meantime?
> 
> I think we can have it both ways.  Add an atomic bitflag to the usb_device
> structure and have the autosuspend work routine check whether the flag is
> set.  If it is, just restart the timer.  The HID driver can simply set the
> flag whenever an URB is received.

This is exactly what I am doing in the HID driver, just with a private
timer. It is true that the timer could be switched off if other interfaces
preclude suspending the device anyhow, but apart from that I just
don't see the point in putting a specialised mechanism into generic code.

> There are other changes we should make that would go along with this.
> 
> 	The existing bitfields in usb_device should be changed over
> 	to atomic bitflags, to make them SMP-safe.

Is this needed?

> 	The default autosuspend delay should become a usbcore module
> 	parameter rather than a constant.  0 can mean "don't autosuspend".

Good idea.

> 	Your new autosuspend attribute file should be renamed to
> 	autosuspend_delay and it should expose an unsigned short instead
> 	of a boolean: the number of seconds for the device's autosuspend
> 	timer.  0 can mean "don't autosuspend".

Roger.

> 	We should add a similar "suspend" attribute file.  Writing
> 	"1" would immediately suspend and "0" would immediately resume
> 	the device.  This will allow userspace to micro-manage the
> 	device state and will replace the deprecated power/state file.

In which way will this avoid that interface's principal problems?
Currently, we know that a driver has either requested autosuspend
and thus should be prepared to suspend or the guarantees for
a system wide freeze are valid. We'd need to audit all drivers.

> 	We should write sysfs_add_to_group(), so that these two
> 	attributes can be added to the existing device/power
> 	subdirectory.  (sysfs_remove_from_group() doesn't seem to
> 	be needed, but it could be written as well.)

I will not comment on sysfs stuff without prior discussion with the
sysfs people. I am simply not competent to do so.

> 	We could add an unsigned short value (autosuspend_time) to
> 	struct usb_interface as well as to struct usb_device, so that
> 	individual interface drivers can specify a timeout value.  Or
> 	we could just let them modify the value in usb_device, which
> 	would be a lot simpler.
> 
> This leaves open the question of what timeout value we should actually
> use.  If we have interface-specific values then clearly we want to use
> their maximum.  But should a value set via sysfs always take precedence?

To the last question: yes.

> How does this sound?

A bit too complex with too much change in very important generic code.

	Regards
		Oliver

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel