Re: [PATCH] appletouch powersaving - please apply for 2.6.23-rc1take #3

From: Soeren Sonnenburg
Date: Tue Jul 17 2007 - 14:16:35 EST


On Tue, 2007-07-17 at 11:01 -0400, Dmitry Torokhov wrote:
> Hi,
>
> On 7/17/07, Soeren Sonnenburg <kernel@xxxxxx> wrote:
> >
> > err_free_buffer:
> > @@ -656,6 +699,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,
> >
>
> This should go into atp_close() and I think you need to do
> cancel_work_sync after calling usb_kill_urb() otherwise you risk it
> being submitted while you gettingto kill the urb.

good catch. modified patch accordingly+attached.

> How many boxes did you try this patch on?

Mine plus 1 other. However please note that Matthews patch has been
(which is what this patch is based on) is in the mactel-patches
repository for quite some time now and that the not-yet-cleanup up
variant of this patch was posted to mactel-devel...

So the modeswitch part should work...

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.
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e321526..7f180f7 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:
@@ -480,6 +549,7 @@ static void atp_close(struct input_dev *input)
struct atp *dev = input_get_drvdata(input);

usb_kill_urb(dev->urb);
+ cancel_work_sync(&dev->work);
dev->open = 0;
}

@@ -528,40 +598,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 +676,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: