converting appletouch to usb autosuspend again...

[Soeren Sonnenburg <kernel@nn7.de>]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

Dear all,

I wonder whether anyone else tried to get appletouch usb autosuspend to
work. I've seen Oliver's patch but as it kept oopsing on me and I
couldn't parse the usb_mark_last_busy logic I came up with the attached
patch against (current git; at least didn't oops for me over the last
several days and suspend/resume cycles).

However things I don't understand, 

a) is autosuspend on the appletouch driver used at all (how can I find
out)

b) do I need dev->intf->needs_remote_wakeup = 1; ?

c) is line 33 in the patch safe to do ?

And while we are at it I am still seeing these X falls back to hid mouse
mode, only switching to console and back resolves this (looks like this
only worked in previous kernels as resume from s2ram took a lot longer
due to the IDE driver doing a couple of resets) - is there anything one
could do about it?

Soeren


[-- Attachment #2: appletouch_autosuspend.diff --]
[-- Type: text/x-patch, Size: 3921 bytes --]

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 1f41ae9..19003c8 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/usb/input.h>
+#include <linux/mutex.h>
 
 /* Type of touchpad */
 enum atp_touchpad_type {
@@ -140,10 +141,12 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 struct atp {
 	char			phys[64];
 	struct usb_device	*udev;		/* usb device */
+	struct usb_interface	*intf;		/* our interface */
 	struct urb		*urb;		/* usb request block */
 	signed char		*data;		/* transferred data */
 	struct input_dev	*input;		/* input dev */
 	enum atp_touchpad_type	type;		/* type of touchpad */
+	struct mutex		pm_mutex;	/* serialize access to open/suspend */
 	bool			open;
 	bool			valid;		/* are the samples valid? */
 	bool			size_detect_done;
@@ -259,6 +262,11 @@ static void atp_reinit(struct work_struct *work)
 	if (retval)
 		err("atp_reinit: usb_submit_urb failed with error %d",
 		    retval);
+
+	/* device is idle and available for autosuspend */
+	mutex_lock(&dev->pm_mutex);
+	usb_autopm_disable(dev->intf);
+	mutex_unlock(&dev->pm_mutex);
 }
 
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
@@ -547,21 +555,37 @@ static void atp_complete(struct urb *urb)
 static int atp_open(struct input_dev *input)
 {
 	struct atp *dev = input_get_drvdata(input);
+	int error = usb_autopm_get_interface(dev->intf);
+	if (error)
+		return error;
 
-	if (usb_submit_urb(dev->urb, GFP_ATOMIC))
-		return -EIO;
+	mutex_lock(&dev->pm_mutex);
+	error=usb_submit_urb(dev->urb, GFP_ATOMIC);
 
-	dev->open = 1;
-	return 0;
+	if (!error)
+	    dev->open = 1;
+
+	mutex_unlock(&dev->pm_mutex);
+
+	if (error)
+		usb_autopm_put_interface(dev->intf);
+
+	return error;
 }
 
 static void atp_close(struct input_dev *input)
 {
 	struct atp *dev = input_get_drvdata(input);
 
+	mutex_lock(&dev->pm_mutex);
+
 	usb_kill_urb(dev->urb);
 	cancel_work_sync(&dev->work);
 	dev->open = 0;
+
+	mutex_unlock(&dev->pm_mutex);
+
+	usb_autopm_put_interface(dev->intf);
 }
 
 static int atp_handle_geyser(struct atp *dev)
@@ -615,9 +639,12 @@ static int atp_probe(struct usb_interface *iface,
 	}
 
 	dev->udev = udev;
+	dev->intf = iface;
 	dev->input = input_dev;
 	dev->type = id->driver_info;
 	dev->overflow_warned = false;
+	mutex_init(&dev->pm_mutex);
+
 	if (dev->type == ATP_FOUNTAIN || dev->type == ATP_GEYSER1)
 		dev->datalen = 81;
 	else
@@ -750,27 +777,41 @@ static int atp_suspend(struct usb_interface *iface, pm_message_t message)
 {
 	struct atp *dev = usb_get_intfdata(iface);
 
+	mutex_lock(&dev->pm_mutex);
+
 	usb_kill_urb(dev->urb);
 	dev->valid = false;
 
+	mutex_unlock(&dev->pm_mutex);
+
 	return 0;
 }
 
 static int atp_resume(struct usb_interface *iface)
 {
 	struct atp *dev = usb_get_intfdata(iface);
+	int error = 0;
 
-	if (dev->open && usb_submit_urb(dev->urb, GFP_ATOMIC))
-		return -EIO;
+	mutex_lock(&dev->pm_mutex);
 
-	return 0;
+	if (dev->open)
+	    error = usb_submit_urb(dev->urb, GFP_ATOMIC);
+
+	mutex_unlock(&dev->pm_mutex);
+
+	return error;
 }
 
 static int atp_reset_resume(struct usb_interface *iface)
 {
 	struct atp *dev = usb_get_intfdata(iface);
+	int error = 0;
 
-	return atp_recover(dev);
+	mutex_lock(&dev->pm_mutex);
+	error = atp_recover(dev);
+	mutex_unlock(&dev->pm_mutex);
+
+	return error;
 }
 
 static struct usb_driver atp_driver = {
@@ -781,6 +822,7 @@ static struct usb_driver atp_driver = {
 	.resume		= atp_resume,
 	.reset_resume	= atp_reset_resume,
 	.id_table	= atp_table,
+	.supports_autosuspend	= 1,
 };
 
 static int __init atp_init(void)
@@ -795,3 +837,4 @@ static void __exit atp_exit(void)
 
 module_init(atp_init);
 module_exit(atp_exit);
+