Re: [PATCH] appletouch powersaving - please apply for 2.6.23-rc1

[Soeren Sonnenburg <kernel@nn7.de>]

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

On Tue, 2007-07-17 at 15:03 +0200, Johannes Berg wrote:
> Hi,
> 
> Good stuff :)
> 
> > +	int                     idlecount;      /* number of empty packets */
> 
> should probably use tabs here.

fixed.

> > +	size = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
> > +			       ATP_GEYSER3_MODE_WRITE_REQUEST_ID,
> > +			       USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
> > +			       ATP_GEYSER3_MODE_REQUEST_VALUE,
> > +			       ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
> > +	<eol>
> 
> trailing whitespace.

fixed
 
> > +	input_report_key(dev->input, BTN_LEFT, dev->data[dev->datalen-1] & 1);
> > +
> 
> 
> > @@ -449,10 +511,19 @@ static void atp_complete(struct urb* urb)
> >  
> >  		/* reset the accumulator on release */
> >  		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
> > -	}
> >  
> > -	input_report_key(dev->input, BTN_LEFT,
> > -			 !!dev->data[dev->datalen - 1]);
> 
> Any hint as to why you move this? The different test, yes, ok, you
> explained that, but moving it?

OK, Sven Anders also asked why the move... and well the reason was that
when I was trying to figure out what goes wrong I memset everything
including dev->data to zero which required the move ...

Anyway as there is no goto/return inbetween I fail to see that this will
make any difference. So I moved the code back down where it was. 

The new patch containing these cleanups is attached.

Best,
Soeren
-- 
Sometimes, there's a moment as you're waking, when you become aware of
the real world around you, but you're still dreaming.

[-- Attachment #2: appletouch2.patch --]
[-- Type: text/x-patch, Size: 5078 bytes --]

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e321526..34c1eca 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -155,6 +155,8 @@ struct atp {
 	int			xy_acc[ATP_XSENSORS + ATP_YSENSORS];
 	int			overflowwarn;	/* overflow warning printed? */
 	int			datalen;	/* size of an USB urb transfer */
+	int			idlecount;      /* number of empty packets */
+	struct work_struct      work;
 };
 
 #define dbg_dump(msg, tab) \
@@ -208,6 +210,63 @@ static inline int atp_is_geyser_3(struct atp *dev)
 		(productId == GEYSER4_JIS_PRODUCT_ID);
 }
 
+/*
+ * By default Geyser 3 device sends standard USB HID mouse
+ * packets (Report ID 2). This code changes device mode, so it
+ * sends raw sensor reports (Report ID 5).
+ */
+static int atp_geyser3_init(struct usb_device *udev)
+{
+	char data[8];
+	int size;
+	int i;
+
+	size = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
+				ATP_GEYSER3_MODE_READ_REQUEST_ID,
+				USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+				ATP_GEYSER3_MODE_REQUEST_VALUE,
+				ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
+
+	if (size != 8) {
+		printk("appletouch atp_geyser3_init READ error\n");
+		for (i=0; i<8; i++)
+			printk("appletouch[%d]: %d\n", i, (int) data[i]);
+
+		err("Could not do mode read request from device"
+		    " (Geyser 3 mode)");
+		return -EIO;
+	}
+
+	/* Apply the mode switch */
+	data[0] = ATP_GEYSER3_MODE_VENDOR_VALUE;
+
+	size = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				ATP_GEYSER3_MODE_WRITE_REQUEST_ID,
+				USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+				ATP_GEYSER3_MODE_REQUEST_VALUE,
+				ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
+
+	if (size != 8) {
+		printk("appletouch atp_geyser3_init WRITE error\n");
+		for (i=0; i<8; i++)
+			printk("appletouch[%d]: %d\n", i, (int) data[i]);
+		err("Could not do mode write request to device"
+		    " (Geyser 3 mode)");
+		return -EIO;
+	}
+	return 0;
+}
+
+/* Reinitialise the device if it's a geyser 3 */
+static void atp_reinit(struct work_struct *work)
+{
+	struct atp *dev = container_of(work, struct atp, work);
+	struct usb_device *udev = dev->udev;
+
+	dev->idlecount = 0;
+	atp_geyser3_init(udev);
+}
+
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
 			     int *z, int *fingers)
 {
@@ -449,11 +508,21 @@ static void atp_complete(struct urb* urb)
 
 		/* reset the accumulator on release */
 		memset(dev->xy_acc, 0, sizeof(dev->xy_acc));
-	}
 
-	input_report_key(dev->input, BTN_LEFT,
-			 !!dev->data[dev->datalen - 1]);
+		/* Geyser 3 will continue to send packets continually after
+		   the first touch unless reinitialised. Do so if it's been
+		   idle for a while in order to avoid waking the kernel up
+		   several hundred times a second */
+		if (atp_is_geyser_3(dev)) {
+			dev->idlecount++;
+			if (dev->idlecount == 10) {
+				dev->valid=0;
+				schedule_work (&dev->work);
+			}
+		}
+	}
 
+	input_report_key(dev->input, BTN_LEFT, dev->data[dev->datalen-1] & 1);
 	input_sync(dev->input);
 
 exit:
@@ -528,40 +597,10 @@ static int atp_probe(struct usb_interface *iface, const struct usb_device_id *id
 		dev->datalen = 81;
 
 	if (atp_is_geyser_3(dev)) {
-		/*
-		 * By default Geyser 3 device sends standard USB HID mouse
-		 * packets (Report ID 2). This code changes device mode, so it
-		 * sends raw sensor reports (Report ID 5).
-		 */
-		char data[8];
-		int size;
-
-		size = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
-			ATP_GEYSER3_MODE_READ_REQUEST_ID,
-			USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			ATP_GEYSER3_MODE_REQUEST_VALUE,
-			ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
-
-		if (size != 8) {
-			err("Could not do mode read request from device"
-							" (Geyser 3 mode)");
+		/* switch to raw sensor mode */
+		if (atp_geyser3_init(udev))
 			goto err_free_devs;
-		}
-
-		/* Apply the mode switch */
-		data[0] = ATP_GEYSER3_MODE_VENDOR_VALUE;
-
-		size = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-			ATP_GEYSER3_MODE_WRITE_REQUEST_ID,
-			USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-			ATP_GEYSER3_MODE_REQUEST_VALUE,
-			ATP_GEYSER3_MODE_REQUEST_INDEX, &data, 8, 5000);
 
-		if (size != 8) {
-			err("Could not do mode write request to device"
-							" (Geyser 3 mode)");
-			goto err_free_devs;
-		}
 		printk("appletouch Geyser 3 inited.\n");
 	}
 
@@ -636,6 +675,8 @@ static int atp_probe(struct usb_interface *iface, const struct usb_device_id *id
 	/* save our data pointer in this interface device */
 	usb_set_intfdata(iface, dev);
 
+	INIT_WORK(&dev->work, atp_reinit);
+
 	return 0;
 
  err_free_buffer:
@@ -656,6 +697,7 @@ static void atp_disconnect(struct usb_interface *iface)
 
 	usb_set_intfdata(iface, NULL);
 	if (dev) {
+		cancel_work_sync(&dev->work);
 		usb_kill_urb(dev->urb);
 		input_unregister_device(dev->input);
 		usb_buffer_free(dev->udev, dev->datalen,