linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] input: Synaptics USB device driver
@ 2012-01-03 18:40 Jan Steinhoff
  2012-01-04  8:25 ` Oliver Neukum
  2012-01-04  8:55 ` Jiri Kosina
  0 siblings, 2 replies; 16+ messages in thread
From: Jan Steinhoff @ 2012-01-03 18:40 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: Dmitry Torokhov, linux-input, Jiri Kosina, linux-usb, linux-kernel

From: Jan Steinhoff <mail@jan-steinhoff.de>

This patch adds a driver for Synaptics USB touchpad or pointing stick
devices. These USB devices emulate an USB mouse by default, so one can
also use the usbhid driver. However, in combination with special user
space drivers this kernel driver allows one to customize the behaviour
of the device.

An extended version of this driver with support for the cPad background
display can be found at
<http://jan-steinhoff.de/linux/synaptics-usb.html>.

Signed-off-by: Jan Steinhoff <mail@jan-steinhoff.de>
---
 drivers/input/mouse/Kconfig         |   14 +
 drivers/input/mouse/Makefile        |    1 +
 drivers/input/mouse/synaptics_usb.c |  593 +++++++++++++++++++++++++++++++++++
 drivers/input/mouse/synaptics_usb.h |   66 ++++
 4 files changed, 674 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/mouse/synaptics_usb.c
 create mode 100644 drivers/input/mouse/synaptics_usb.h

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 9c1e6ee..f258089 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -322,4 +322,18 @@ config MOUSE_SYNAPTICS_I2C
 	  To compile this driver as a module, choose M here: the
 	  module will be called synaptics_i2c.
 
+config MOUSE_SYNAPTICS_USB
+	tristate "Synaptics USB device support"
+	depends on USB
+	help
+	  Say Y here if you want to use a Synaptics USB touchpad or pointing stick.
+	  These Synaptics USB devices emulate an USB mouse by default, so you can
+	  also use the usbhid driver. However, in combination with special X.Org
+	  drivers this kernel driver allows you to customize the behaviour of the
+	  device. More information can be found at:
+	  <http://jan-steinhoff.de/linux/synaptics-usb.html>
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called synaptics_usb.
+
 endif
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 570c84a4..4718eff 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_MOUSE_PXA930_TRKBALL)	+= pxa930_trkball.o
 obj-$(CONFIG_MOUSE_RISCPC)		+= rpcmouse.o
 obj-$(CONFIG_MOUSE_SERIAL)		+= sermouse.o
 obj-$(CONFIG_MOUSE_SYNAPTICS_I2C)	+= synaptics_i2c.o
+obj-$(CONFIG_MOUSE_SYNAPTICS_USB)	+= synaptics_usb.o
 obj-$(CONFIG_MOUSE_VSXXXAA)		+= vsxxxaa.o
 
 psmouse-objs := psmouse-base.o synaptics.o
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
new file mode 100644
index 0000000..717bfe8
--- /dev/null
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -0,0 +1,593 @@
+/****
+ * USB Synaptics device driver
+ *
+ *  Copyright (c) 2002 Rob Miller (rob@inpharmatica . co . uk)
+ *  Copyright (c) 2003 Ron Lee (ron@debian.org)
+ *	cPad driver for kernel 2.4
+ *
+ *  Copyright (c) 2004 Jan Steinhoff (cpad@jan-steinhoff . de)
+ *  Copyright (c) 2004 Ron Lee (ron@debian.org)
+ *	rewritten for kernel 2.6
+ *
+ *  cPad display character device part is not included. It can be found at
+ *  http://jan-steinhoff.de/linux/synaptics-usb.html
+ *
+ * Bases on:	usb_skeleton.c v2.2 by Greg Kroah-Hartman
+ *		drivers/hid/usbhid/usbmouse.c by Vojtech Pavlik
+ *		drivers/input/mouse/synaptics.c by Peter Osterlund
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * Trademarks are the property of their respective owners.
+ */
+
+/****
+ * There are three different types of Synaptics USB devices: Touchpads,
+ * touchsticks (or trackpoints), and touchscreens. Touchpads are well supported
+ * by this driver, touchstick support has not been tested much yet, and
+ * touchscreens have not been tested at all. The only difference between pad
+ * and stick seems to be that the x and y finger positions are unsigned 13 bit
+ * integers in the first case, but are signed ones in the second case.
+ *
+ * Up to three alternate settings are possible:
+ *	setting 0: one int endpoint for relative movement (used by usbhid.ko)
+ *	setting 1: one int endpoint for absolute finger position
+ *	setting 2 (cPad only): one int endpoint for absolute finger position and
+ *		   two bulk endpoints for the display (in/out)
+ * This driver uses setting 1.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kref.h>
+#include <linux/usb.h>
+#include <linux/input.h>
+#include <linux/usb/input.h>
+#include <linux/workqueue.h>
+
+#include "synaptics_usb.h"
+
+
+/*
+ * input device code
+ */
+
+MODULE_PARM_DESC(xmin, "minimal horizontal finger position for touchpads");
+MODULE_PARM_DESC(xmax, "maximal horizontal finger position for touchpads");
+MODULE_PARM_DESC(ymin, "minimal vertical finger position for touchpads");
+MODULE_PARM_DESC(ymax, "maximal vertical finger position for touchpads");
+static int xmin = 1472;
+static int xmax = 5472;
+static int ymin = 1408;
+static int ymax = 4448;
+module_param(xmin, int, 0444);
+module_param(xmax, int, 0444);
+module_param(ymin, int, 0444);
+module_param(ymax, int, 0444);
+
+MODULE_PARM_DESC(btn_middle, "if set, cPad menu button is reported "
+			     "as a middle button");
+static int btn_middle = 1;
+module_param(btn_middle, int, 0644);
+
+static const char synusb_pad_name[] = "Synaptics USB TouchPad";
+static const char synusb_stick_name[] = "Synaptics USB Styk";
+static const char synusb_screen_name[] = "Synaptics USB TouchScreen";
+
+static const char *synusb_get_name(struct synusb *synusb)
+{
+	switch (synusb->input_type) {
+	case SYNUSB_PAD:
+		return synusb_pad_name;
+	case SYNUSB_STICK:
+		return synusb_stick_name;
+	case SYNUSB_SCREEN:
+		return synusb_screen_name;
+	}
+	return NULL;
+}
+
+/* report tool_width for touchpads */
+static void synusb_report_width(struct input_dev *idev, int pressure, int w)
+{
+	int num_fingers, tool_width;
+
+	if (pressure > 0) {
+		num_fingers = 1;
+		tool_width = 5;
+		switch (w) {
+		case 0 ... 1:
+			num_fingers = 2 + w;
+			break;
+		case 2:	                /* pen, pretend its a finger */
+			break;
+		case 4 ... 15:
+			tool_width = w;
+			break;
+		}
+	} else {
+		num_fingers = 0;
+		tool_width = 0;
+	}
+
+	input_report_abs(idev, ABS_TOOL_WIDTH, tool_width);
+
+	input_report_key(idev, BTN_TOOL_FINGER, num_fingers == 1);
+	input_report_key(idev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
+	input_report_key(idev, BTN_TOOL_TRIPLETAP, num_fingers == 3);
+}
+
+/* convert signed or unsigned 13 bit number to int */
+static inline int us13_to_int(u8 high, u8 low, int has_sign)
+{
+	int res;
+
+	res = ((int)(high & 0x1f) << 8) | low;
+	if (has_sign && (high & 0x10))
+		res -= 0x2000;
+
+	return res;
+}
+
+static void synusb_input_callback(struct urb *urb)
+{
+	struct synusb *synusb = (struct synusb *)urb->context;
+	u8 *data = urb->transfer_buffer;
+	struct input_dev *idev = synusb->idev;
+	int res, x, y, pressure;
+	int is_stick = (synusb->input_type == SYNUSB_STICK) ? 1 : 0;
+
+	if (urb->status) {
+		if (synusb_urb_status_error(urb)) {
+			synusb_err(synusb, "nonzero int in status received: %d",
+				   urb->status);
+			/* an error occured, try to resubmit */
+			goto resubmit;
+		}
+
+		/* unlink urb, do not resubmit */
+		return;
+	}
+
+	pressure = data[6];
+	x = us13_to_int(data[2], data[3], is_stick);
+	y = us13_to_int(data[4], data[5], is_stick);
+
+	if (is_stick) {
+		y = -y;
+		if (pressure > 6)
+			input_report_key(idev, BTN_TOUCH, 1);
+		if (pressure < 5)
+			input_report_key(idev, BTN_TOUCH, 0);
+		input_report_key(idev, BTN_TOOL_FINGER, pressure ? 1 : 0);
+	} else {
+		if (pressure > 30)
+			input_report_key(idev, BTN_TOUCH, 1);
+		if (pressure < 25)
+			input_report_key(idev, BTN_TOUCH, 0);
+		synusb_report_width(idev, pressure, data[0] & 0x0f);
+		y = ymin + ymax - y;
+	}
+
+	if (pressure > 0) {
+		input_report_abs(idev, ABS_X, x);
+		input_report_abs(idev, ABS_Y, y);
+	}
+	input_report_abs(idev, ABS_PRESSURE, pressure);
+
+	input_report_key(idev, BTN_LEFT, data[1] & 0x04);
+	input_report_key(idev, BTN_RIGHT, data[1] & 0x01);
+	input_report_key(idev, BTN_MIDDLE, data[1] & 0x02);
+	if (synusb->has_display)
+		input_report_key(idev, btn_middle ? BTN_MIDDLE : BTN_MISC,
+				 data[1] & 0x08);
+
+	input_sync(idev);
+resubmit:
+	res = usb_submit_urb(urb, GFP_ATOMIC);
+	if (res)
+		synusb_err(synusb, "submit int in urb failed with result %d",
+			   res);
+}
+
+/* Data must always be fetched from the int endpoint, otherwise the device
+ * would reconnect to force driver reload. So this is always scheduled by probe.
+ */
+static void synusb_submit_int(struct work_struct *work)
+{
+	struct synusb *synusb = container_of(work, struct synusb, isubmit.work);
+	int res;
+
+	res = usb_submit_urb(synusb->iurb, GFP_KERNEL);
+	if (res)
+		synusb_err(synusb, "submit int in urb failed with result %d",
+			   res);
+}
+
+static int synusb_init_input(struct synusb *synusb)
+{
+	struct input_dev *idev;
+	struct usb_device *udev = synusb->udev;
+	int is_stick = (synusb->input_type == SYNUSB_STICK) ? 1 : 0;
+	int retval = -ENOMEM;
+
+	idev = input_allocate_device();
+	if (idev == NULL) {
+		synusb_err(synusb, "Can not allocate input device");
+		goto error;
+	}
+
+	__set_bit(EV_ABS, idev->evbit);
+	__set_bit(EV_KEY, idev->evbit);
+
+	if (is_stick) {
+		input_set_abs_params(idev, ABS_X, -512, 512, 0, 0);
+		input_set_abs_params(idev, ABS_Y, -512, 512, 0, 0);
+		input_set_abs_params(idev, ABS_PRESSURE, 0, 127, 0, 0);
+	} else {
+		input_set_abs_params(idev, ABS_X, xmin, xmax, 0, 0);
+		input_set_abs_params(idev, ABS_Y, ymin, ymax, 0, 0);
+		input_set_abs_params(idev, ABS_PRESSURE, 0, 255, 0, 0);
+		input_set_abs_params(idev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
+		__set_bit(BTN_TOOL_DOUBLETAP, idev->keybit);
+		__set_bit(BTN_TOOL_TRIPLETAP, idev->keybit);
+	}
+
+	__set_bit(BTN_TOUCH, idev->keybit);
+	__set_bit(BTN_TOOL_FINGER, idev->keybit);
+	__set_bit(BTN_LEFT, idev->keybit);
+	__set_bit(BTN_RIGHT, idev->keybit);
+	__set_bit(BTN_MIDDLE, idev->keybit);
+	if (synusb->has_display)
+		__set_bit(BTN_MISC, idev->keybit);
+
+	usb_make_path(udev, synusb->iphys, sizeof(synusb->iphys));
+	strlcat(synusb->iphys, "/input0", sizeof(synusb->iphys));
+	idev->phys = synusb->iphys;
+	idev->name = synusb_get_name(synusb);
+	usb_to_input_id(udev, &idev->id);
+	idev->dev.parent = &synusb->interface->dev;
+	input_set_drvdata(idev, synusb);
+
+	retval = input_register_device(idev);
+	if (retval) {
+		synusb_err(synusb, "Can not register input device");
+		goto error;
+	}
+	synusb->idev = idev;
+
+	synusb->interface->needs_remote_wakeup = 1;
+
+	return 0;
+error:
+	if (idev)
+		input_free_device(idev);
+
+	return retval;
+}
+
+
+/*
+ * initialization of usb data structures
+ */
+
+static int synusb_setup_iurb(struct synusb *synusb,
+			     struct usb_endpoint_descriptor *endpoint)
+{
+	char *buf;
+
+	if (endpoint->wMaxPacketSize < 8)
+		return 0;
+	if (synusb->iurb) {
+		synusb_warn(synusb, "Found more than one possible "
+				    "int in endpoint");
+		return 0;
+	}
+	synusb->iurb = usb_alloc_urb(0, GFP_KERNEL);
+	if (synusb->iurb == NULL)
+		return -ENOMEM;
+	buf = usb_alloc_coherent(synusb->udev, 8, GFP_ATOMIC,
+				 &synusb->iurb->transfer_dma);
+	if (buf == NULL)
+		return -ENOMEM;
+	usb_fill_int_urb(synusb->iurb, synusb->udev,
+			 usb_rcvintpipe(synusb->udev,
+					endpoint->bEndpointAddress),
+			 buf, 8, synusb_input_callback,
+			 synusb, endpoint->bInterval);
+	synusb->iurb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+	return 0;
+}
+
+static inline int synusb_match_endpoint(struct usb_endpoint_descriptor *ep)
+{
+	if (((ep->bEndpointAddress & USB_DIR_IN) == USB_DIR_IN) &&
+	    ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
+			== USB_ENDPOINT_XFER_INT))
+		return 0;
+
+	return -ENODEV;
+}
+
+static int synusb_setup_endpoints(struct synusb *synusb)
+{
+	struct usb_host_interface *iface_desc;
+	struct usb_endpoint_descriptor *endpoint;
+	int int_num = synusb->interface->cur_altsetting->desc.bInterfaceNumber;
+	unsigned altsetting;
+	int i, res;
+
+	altsetting = min((unsigned) 1, synusb->interface->num_altsetting);
+	res = usb_set_interface(synusb->udev, int_num, altsetting);
+	if (res) {
+		synusb_err(synusb, "Can not set alternate setting to %i, "
+				   "error: %i", altsetting, res);
+		return res;
+	}
+
+	/* go through all endpoints */
+	iface_desc = synusb->interface->cur_altsetting;
+	for (i = 0; i < iface_desc->desc.bNumEndpoints; i++) {
+		endpoint = &iface_desc->endpoint[i].desc;
+		if (synusb_match_endpoint(endpoint) == 0) {
+			res = synusb_setup_iurb(synusb, endpoint);
+			if (res)
+				return res;
+		}
+	}
+
+	/* check whether an int in endpoint has been set up */
+	if (!synusb->iurb)
+		return -ENODEV;
+
+	return 0;
+}
+
+/* disable stick support by default, as the device becomes unusable without
+ * corresponding user space driver otherwise */
+MODULE_PARM_DESC(enable_stick, "enable trackpoint support");
+static int enable_stick;
+module_param(enable_stick, int, 0644);
+
+static int synusb_detect_type(struct synusb *synusb,
+			      const struct usb_device_id *id)
+{
+	int int_num = synusb->interface->cur_altsetting->desc.bInterfaceNumber;
+
+	synusb->has_display = 0;
+	if (id->idVendor == USB_VID_SYNAPTICS) {
+		switch (id->idProduct) {
+		case USB_DID_SYN_TS:
+			synusb->input_type = SYNUSB_SCREEN;
+			break;
+		case USB_DID_SYN_STICK:
+			synusb->input_type = SYNUSB_STICK;
+			break;
+		case USB_DID_SYN_COMP_TP:
+			if (int_num == 1)
+				synusb->input_type = SYNUSB_STICK;
+			else
+				synusb->input_type = SYNUSB_PAD;
+			break;
+		case USB_DID_SYN_CPAD:
+			synusb->has_display = 1;
+		default:
+			synusb->input_type = SYNUSB_PAD;
+		}
+	} else {
+		synusb->input_type = SYNUSB_PAD;
+	}
+
+	if ((synusb->input_type == SYNUSB_STICK) && !enable_stick)
+		return -ENODEV;
+	if (synusb->input_type == SYNUSB_SCREEN)
+		synusb_warn(synusb, "driver has not been tested "
+				    "with touchscreens!");
+
+	return 0;
+}
+
+static void synusb_free_urb(struct urb *urb)
+{
+	if (urb == NULL)
+		return;
+	usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+			  urb->transfer_buffer, urb->transfer_dma);
+	usb_free_urb(urb);
+}
+
+static void synusb_delete(struct kref *kref)
+{
+	struct synusb *synusb = container_of(kref, struct synusb, kref);
+
+	synusb_free_urb(synusb->iurb);
+	if (synusb->idev)
+		input_unregister_device(synusb->idev);
+
+	usb_put_dev(synusb->udev);
+	kfree(synusb);
+}
+
+static int synusb_probe(struct usb_interface *interface,
+			const struct usb_device_id *id)
+{
+	struct synusb *synusb = NULL;
+	struct usb_device *udev = interface_to_usbdev(interface);
+	int retval = -ENOMEM;
+
+	synusb = kzalloc(sizeof(*synusb), GFP_KERNEL);
+	if (synusb == NULL) {
+		dev_err(&interface->dev, "Out of memory in synusb_probe\n");
+		goto error;
+	}
+
+	synusb->udev = usb_get_dev(udev);
+	synusb->interface = interface;
+	kref_init(&synusb->kref);
+	usb_set_intfdata(interface, synusb);
+
+	if (synusb_detect_type(synusb, id))
+		goto error;
+
+	retval = synusb_setup_endpoints(synusb);
+	if (retval) {
+		synusb_err(synusb, "Can not set up endpoints, error: %i",
+			   retval);
+		goto error;
+	}
+
+	retval = synusb_init_input(synusb);
+	if (retval)
+		goto error;
+
+	/* submit the int in urb */
+	INIT_DELAYED_WORK(&synusb->isubmit, synusb_submit_int);
+	schedule_delayed_work(&synusb->isubmit, msecs_to_jiffies(10));
+
+	return 0;
+
+error:
+	if (synusb) {
+		usb_set_intfdata(interface, NULL);
+		kref_put(&synusb->kref, synusb_delete);
+	}
+	return retval;
+}
+
+static void synusb_disconnect(struct usb_interface *interface)
+{
+	struct synusb *synusb;
+
+	synusb = usb_get_intfdata(interface);
+	usb_set_intfdata(interface, NULL);
+
+	cancel_delayed_work_sync(&synusb->isubmit);
+
+	usb_kill_urb(synusb->iurb);
+	input_unregister_device(synusb->idev);
+	synusb->idev = NULL;
+
+	kref_put(&synusb->kref, synusb_delete);
+
+	dev_info(&interface->dev, "Synaptics device disconnected\n");
+}
+
+
+/*
+ * suspend code
+ */
+
+static void synusb_draw_down(struct synusb *synusb)
+{
+	cancel_delayed_work_sync(&synusb->isubmit);
+	usb_kill_urb(synusb->iurb);
+}
+
+static int synusb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+
+	if (synusb == NULL)
+		return 0;
+
+	synusb_draw_down(synusb);
+
+	return 0;
+}
+
+static int synusb_resume(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	int res;
+
+	res = usb_submit_urb(synusb->iurb, GFP_ATOMIC);
+	if (res)
+		synusb_err(synusb, "submit int in urb failed during resume "
+				   "with result %d", res);
+
+	return res;
+}
+
+static int synusb_pre_reset(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+
+	synusb_draw_down(synusb);
+
+	return 0;
+}
+
+static int synusb_post_reset(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	int res;
+
+	res = usb_submit_urb(synusb->iurb, GFP_ATOMIC);
+	if (res)
+		synusb_err(synusb, "submit int in urb failed in during "
+				   "post_reset with result %d", res);
+
+	return res;
+}
+
+static int synusb_reset_resume(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	int res;
+
+	res = usb_submit_urb(synusb->iurb, GFP_ATOMIC);
+	if (res)
+		synusb_err(synusb, "submit int in urb failed during "
+				   "reset-resume with result %d", res);
+
+	return res;
+}
+
+/* the id table is filled via sysfs, so usbhid is always the default driver */
+static struct usb_device_id synusb_idtable[] = { { } };
+MODULE_DEVICE_TABLE(usb, synusb_idtable);
+
+static struct usb_driver synusb_driver = {
+	.name =		"synaptics_usb",
+	.probe =	synusb_probe,
+	.disconnect =	synusb_disconnect,
+	.id_table =	synusb_idtable,
+	.suspend =	synusb_suspend,
+	.resume =	synusb_resume,
+	.pre_reset =	synusb_pre_reset,
+	.post_reset =	synusb_post_reset,
+	.reset_resume = synusb_reset_resume,
+	.supports_autosuspend = 1,
+};
+
+static int __init synusb_init(void)
+{
+	int result;
+
+	result = usb_register(&synusb_driver);
+	if (result)
+		err("usb_register failed. Error number %d", result);
+	else
+		pr_info(DRIVER_DESC " " DRIVER_VERSION "\n");
+
+	return result;
+}
+
+static void __exit synusb_exit(void)
+{
+	usb_deregister(&synusb_driver);
+}
+
+module_init(synusb_init);
+module_exit(synusb_exit);
+
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE("GPL");
diff --git a/drivers/input/mouse/synaptics_usb.h b/drivers/input/mouse/synaptics_usb.h
new file mode 100644
index 0000000..f42a1e1
--- /dev/null
+++ b/drivers/input/mouse/synaptics_usb.h
@@ -0,0 +1,66 @@
+#ifndef _SYNAPTICS_USB_H
+#define _SYNAPTICS_USB_H
+
+#include <linux/usb.h>
+#include <linux/input.h>
+#include <linux/workqueue.h>
+
+#define DRIVER_VERSION	"v1.5"
+#define DRIVER_AUTHOR	"Rob Miller (rob@inpharmatica . co . uk), "\
+			"Ron Lee (ron@debian.org), "\
+			"Jan Steinhoff (cpad@jan-steinhoff . de)"
+#define DRIVER_DESC	"USB Synaptics device driver"
+
+/* vendor and device IDs */
+#define USB_VID_SYNAPTICS	0x06cb	/* Synaptics vendor ID */
+#define USB_DID_SYN_TP		0x0001	/* Synaptics USB TouchPad */
+#define USB_DID_SYN_INT_TP	0x0002	/* Synaptics Integrated USB TouchPad */
+#define USB_DID_SYN_CPAD	0x0003	/* Synaptics cPad */
+#define USB_DID_SYN_TS		0x0006	/* Synaptics TouchScreen */
+#define USB_DID_SYN_STICK	0x0007	/* Synaptics USB Styk */
+#define USB_DID_SYN_WP		0x0008	/* Synaptics USB WheelPad */
+#define USB_DID_SYN_COMP_TP	0x0009	/* Synaptics Composite USB TouchPad */
+#define USB_DID_SYN_WTP		0x0010	/* Synaptics Wireless TouchPad */
+#define USB_DID_SYN_DP		0x0013	/* Synaptics DisplayPad */
+
+/* structure to hold all of our device specific data */
+struct synusb {
+	struct usb_device	*udev;
+	struct usb_interface	*interface;
+	struct kref		kref;
+
+	/* characteristics of the device */
+	unsigned int		input_type:8;
+	unsigned int		buttons:8;
+	unsigned int		has_display:1;
+
+	/* input device related data structures */
+	struct input_dev	*idev;
+	char			iphys[64];
+	struct delayed_work	isubmit;
+	struct urb		*iurb;
+};
+
+/* possible values for synusb.input_type */
+#define SYNUSB_PAD		0
+#define SYNUSB_STICK		1
+#define SYNUSB_SCREEN		2
+
+static inline int synusb_urb_status_error(struct urb *urb)
+{
+	/* sync/async unlink faults aren't errors */
+	if (urb->status == -ENOENT ||
+	    urb->status == -ECONNRESET ||
+	    urb->status == -ESHUTDOWN)
+		return 0;
+	else
+		return urb->status;
+}
+
+/* helper functions for printk */
+#define synusb_err(_synusb, format, arg...)	\
+	dev_err(&(_synusb)->interface->dev, format "\n", ## arg)
+#define synusb_warn(_synusb, format, arg...)	\
+	dev_warn(&(_synusb)->interface->dev, format "\n", ## arg)
+
+#endif /* _SYNAPTICS_USB_H */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-03 18:40 [PATCH] input: Synaptics USB device driver Jan Steinhoff
@ 2012-01-04  8:25 ` Oliver Neukum
  2012-01-05  2:46   ` Jan Steinhoff
  2012-01-04  8:55 ` Jiri Kosina
  1 sibling, 1 reply; 16+ messages in thread
From: Oliver Neukum @ 2012-01-04  8:25 UTC (permalink / raw)
  To: Jan Steinhoff
  Cc: Alessandro Rubini, Dmitry Torokhov, linux-input, Jiri Kosina,
	linux-usb, linux-kernel

Am Dienstag, 3. Januar 2012, 19:40:33 schrieb Jan Steinhoff:
> From: Jan Steinhoff <mail@jan-steinhoff.de>
> 
> This patch adds a driver for Synaptics USB touchpad or pointing stick
> devices. These USB devices emulate an USB mouse by default, so one can
> also use the usbhid driver. However, in combination with special user
> space drivers this kernel driver allows one to customize the behaviour
> of the device.

Hi,

thank you for this driver. There are a few issues which I have commented on
in the text.

	Regards
			Oliver

[..]
> +	input_report_abs(idev, ABS_PRESSURE, pressure);
> +
> +	input_report_key(idev, BTN_LEFT, data[1] & 0x04);
> +	input_report_key(idev, BTN_RIGHT, data[1] & 0x01);
> +	input_report_key(idev, BTN_MIDDLE, data[1] & 0x02);
> +	if (synusb->has_display)
> +		input_report_key(idev, btn_middle ? BTN_MIDDLE : BTN_MISC,
> +				 data[1] & 0x08);
> +
> +	input_sync(idev);
> +resubmit:
> +	res = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (res)

You are racing with disconnect, suspend and pre_reset. This may lead
to a spurious error message. The correct check is (res && res != -EPERM)

[..]
> +
> +/*
> + * initialization of usb data structures
> + */
> +
> +static int synusb_setup_iurb(struct synusb *synusb,
> +			     struct usb_endpoint_descriptor *endpoint)
> +{
> +	char *buf;
> +
> +	if (endpoint->wMaxPacketSize < 8)
> +		return 0;

How could this happen?

> +	if (synusb->iurb) {
> +		synusb_warn(synusb, "Found more than one possible "
> +				    "int in endpoint");
> +		return 0;
> +	}
> +	synusb->iurb = usb_alloc_urb(0, GFP_KERNEL);
> +	if (synusb->iurb == NULL)
> +		return -ENOMEM;
> +	buf = usb_alloc_coherent(synusb->udev, 8, GFP_ATOMIC,

No need for an atomic allocation here.

> +				 &synusb->iurb->transfer_dma);
> +	if (buf == NULL)
> +		return -ENOMEM;
> +	usb_fill_int_urb(synusb->iurb, synusb->udev,
> +			 usb_rcvintpipe(synusb->udev,
> +					endpoint->bEndpointAddress),
> +			 buf, 8, synusb_input_callback,
> +			 synusb, endpoint->bInterval);
> +	synusb->iurb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +	return 0;
> +}
> +
> +static inline int synusb_match_endpoint(struct usb_endpoint_descriptor *ep)
> +{
> +	if (((ep->bEndpointAddress & USB_DIR_IN) == USB_DIR_IN) &&
> +	    ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK)
> +			== USB_ENDPOINT_XFER_INT))

We have macros for such checks. Please use them.

> +		return 0;
> +
> +	return -ENODEV;
> +}
> +

[..]
> +static void synusb_disconnect(struct usb_interface *interface)
> +{
> +	struct synusb *synusb;
> +
> +	synusb = usb_get_intfdata(interface);
> +	usb_set_intfdata(interface, NULL);
> +
> +	cancel_delayed_work_sync(&synusb->isubmit);
> +
> +	usb_kill_urb(synusb->iurb);

Code duplication. If you define draw_down(), use it.

> +	input_unregister_device(synusb->idev);
> +	synusb->idev = NULL;
> +
> +	kref_put(&synusb->kref, synusb_delete);
> +
> +	dev_info(&interface->dev, "Synaptics device disconnected\n");
> +}
> +
> +
> +/*
> + * suspend code
> + */
> +
> +static void synusb_draw_down(struct synusb *synusb)
> +{
> +	cancel_delayed_work_sync(&synusb->isubmit);
> +	usb_kill_urb(synusb->iurb);
> +}
> +
> +static int synusb_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> +	struct synusb *synusb = usb_get_intfdata(intf);
> +
> +	if (synusb == NULL)
> +		return 0;

How can this happen?

> +
> +	synusb_draw_down(synusb);
> +
> +	return 0;
> +}
> +
> +static int synusb_resume(struct usb_interface *intf)
> +{
> +	struct synusb *synusb = usb_get_intfdata(intf);
> +	int res;
> +
> +	res = usb_submit_urb(synusb->iurb, GFP_ATOMIC);

GFP_NOIO

> +	if (res)
> +		synusb_err(synusb, "submit int in urb failed during resume "
> +				   "with result %d", res);
> +
> +	return res;
> +}
> +
> +static int synusb_pre_reset(struct usb_interface *intf)
> +{
> +	struct synusb *synusb = usb_get_intfdata(intf);
> +
> +	synusb_draw_down(synusb);
> +
> +	return 0;
> +}
> +
> +static int synusb_post_reset(struct usb_interface *intf)
> +{
> +	struct synusb *synusb = usb_get_intfdata(intf);
> +	int res;
> +
> +	res = usb_submit_urb(synusb->iurb, GFP_ATOMIC);

GFP_NOIO

> +	if (res)
> +		synusb_err(synusb, "submit int in urb failed in during "
> +				   "post_reset with result %d", res);
> +
> +	return res;
> +}
> +
> +static int synusb_reset_resume(struct usb_interface *intf)
> +{
> +	struct synusb *synusb = usb_get_intfdata(intf);
> +	int res;
> +
> +	res = usb_submit_urb(synusb->iurb, GFP_ATOMIC);

GFP_NOIO

> +	if (res)
> +		synusb_err(synusb, "submit int in urb failed during "
> +				   "reset-resume with result %d", res);
> +
> +	return res;
> +}
> +
> +/* the id table is filled via sysfs, so usbhid is always the default driver */
> +static struct usb_device_id synusb_idtable[] = { { } };
> +MODULE_DEVICE_TABLE(usb, synusb_idtable);
> +
> +static struct usb_driver synusb_driver = {
> +	.name =		"synaptics_usb",
> +	.probe =	synusb_probe,
> +	.disconnect =	synusb_disconnect,
> +	.id_table =	synusb_idtable,
> +	.suspend =	synusb_suspend,
> +	.resume =	synusb_resume,
> +	.pre_reset =	synusb_pre_reset,
> +	.post_reset =	synusb_post_reset,
> +	.reset_resume = synusb_reset_resume,
> +	.supports_autosuspend = 1,

Yet you do not manage the busy state. Do you really want to play
ping-pong with the power state?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-03 18:40 [PATCH] input: Synaptics USB device driver Jan Steinhoff
  2012-01-04  8:25 ` Oliver Neukum
@ 2012-01-04  8:55 ` Jiri Kosina
  2012-01-04  9:20   ` Oliver Neukum
  1 sibling, 1 reply; 16+ messages in thread
From: Jiri Kosina @ 2012-01-04  8:55 UTC (permalink / raw)
  To: Jan Steinhoff
  Cc: Alessandro Rubini, Dmitry Torokhov, linux-input, linux-usb, linux-kernel

On Tue, 3 Jan 2012, Jan Steinhoff wrote:

[ ... snip ... ]
> +static int synusb_setup_iurb(struct synusb *synusb,
> +			     struct usb_endpoint_descriptor *endpoint)
> +{
> +	char *buf;
> +
> +	if (endpoint->wMaxPacketSize < 8)
> +		return 0;
> +	if (synusb->iurb) {
> +		synusb_warn(synusb, "Found more than one possible "
> +				    "int in endpoint");
> +		return 0;
> +	}
> +	synusb->iurb = usb_alloc_urb(0, GFP_KERNEL);
> +	if (synusb->iurb == NULL)
> +		return -ENOMEM;
> +	buf = usb_alloc_coherent(synusb->udev, 8, GFP_ATOMIC,
> +				 &synusb->iurb->transfer_dma);
> +	if (buf == NULL)
> +		return -ENOMEM;

You seem to leak synusb->iurb here.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-04  8:55 ` Jiri Kosina
@ 2012-01-04  9:20   ` Oliver Neukum
  2012-01-04  9:41     ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Oliver Neukum @ 2012-01-04  9:20 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Jan Steinhoff, Alessandro Rubini, Dmitry Torokhov, linux-input,
	linux-usb, linux-kernel

Am Mittwoch, 4. Januar 2012, 09:55:19 schrieb Jiri Kosina:
> On Tue, 3 Jan 2012, Jan Steinhoff wrote:
> 
> [ ... snip ... ]
> > +static int synusb_setup_iurb(struct synusb *synusb,
> > +                          struct usb_endpoint_descriptor *endpoint)
> > +{
> > +     char *buf;
> > +
> > +     if (endpoint->wMaxPacketSize < 8)
> > +             return 0;
> > +     if (synusb->iurb) {
> > +             synusb_warn(synusb, "Found more than one possible "
> > +                                 "int in endpoint");
> > +             return 0;
> > +     }
> > +     synusb->iurb = usb_alloc_urb(0, GFP_KERNEL);
> > +     if (synusb->iurb == NULL)
> > +             return -ENOMEM;
> > +     buf = usb_alloc_coherent(synusb->udev, 8, GFP_ATOMIC,
> > +                              &synusb->iurb->transfer_dma);
> > +     if (buf == NULL)
> > +             return -ENOMEM;
> 
> You seem to leak synusb->iurb here.

Seems so, but doesn't. It gets freed when the refcount hits 0.
Now, whether this is a good technique is another question.

	Regards
		Oliver
-- 
- - - 
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 
Maxfeldstraße 5                         
90409 Nürnberg 
Germany 
- - - 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-04  9:20   ` Oliver Neukum
@ 2012-01-04  9:41     ` Dmitry Torokhov
  2012-01-04  9:56       ` Oliver Neukum
  2012-01-05  5:39       ` Jan Steinhoff
  0 siblings, 2 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2012-01-04  9:41 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Jiri Kosina, Jan Steinhoff, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Wed, Jan 04, 2012 at 10:20:41AM +0100, Oliver Neukum wrote:
> Am Mittwoch, 4. Januar 2012, 09:55:19 schrieb Jiri Kosina:
> > On Tue, 3 Jan 2012, Jan Steinhoff wrote:
> > 
> > [ ... snip ... ]
> > > +static int synusb_setup_iurb(struct synusb *synusb,
> > > +                          struct usb_endpoint_descriptor *endpoint)
> > > +{
> > > +     char *buf;
> > > +
> > > +     if (endpoint->wMaxPacketSize < 8)
> > > +             return 0;
> > > +     if (synusb->iurb) {
> > > +             synusb_warn(synusb, "Found more than one possible "
> > > +                                 "int in endpoint");
> > > +             return 0;
> > > +     }
> > > +     synusb->iurb = usb_alloc_urb(0, GFP_KERNEL);
> > > +     if (synusb->iurb == NULL)
> > > +             return -ENOMEM;
> > > +     buf = usb_alloc_coherent(synusb->udev, 8, GFP_ATOMIC,
> > > +                              &synusb->iurb->transfer_dma);
> > > +     if (buf == NULL)
> > > +             return -ENOMEM;
> > 
> > You seem to leak synusb->iurb here.
> 
> Seems so, but doesn't. It gets freed when the refcount hits 0.
> Now, whether this is a good technique is another question.

Om, so I have been looking at the driver and the following seems to be
still working with my Lenovo composite touchpad/stick.

Changes:

- the stick is reported in relative mode; we might consider addig
  sensitivity control similar to trackpoint driver. Press-to-select -
  maybe in driver, or in client; undecided.

- got rid of BTN_MISC/BTN_MIDDLE option (at least for now);

- added devices to HID blacklist;

- runtime PM (hopefully I got it right);

- open/close.

Still TODO:

- query device for supported rages instead of having module parameters.

Thanks.

-- 
Dmitry


Input: Synaptics USB device driver

From: Jan Steinhoff <mail@jan-steinhoff.de>

This patch adds a driver for Synaptics USB touchpad or pointing stick
devices. These USB devices emulate an USB mouse by default, so one can
also use the usbhid driver. However, in combination with special user
space drivers this kernel driver allows one to customize the behaviour
of the device.

An extended version of this driver with support for the cPad background
display can be found at
<http://jan-steinhoff.de/linux/synaptics-usb.html>.

Signed-off-by: Jan Steinhoff <mail@jan-steinhoff.de>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/hid/hid-core.c              |    9 +
 drivers/hid/hid-ids.h               |   11 +
 drivers/input/mouse/Kconfig         |   14 +
 drivers/input/mouse/Makefile        |    1 
 drivers/input/mouse/synaptics_usb.c |  529 +++++++++++++++++++++++++++++++++++
 5 files changed, 564 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/mouse/synaptics_usb.c


diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index af35384..75f207c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1892,6 +1892,15 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_PANJIT, 0x0004) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_PHILIPS, USB_DEVICE_ID_PHILIPS_IEEE802154_DONGLE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_POWERCOM, USB_DEVICE_ID_POWERCOM_UPS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_TP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_INT_TP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_CPAD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_TS) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_STICK) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_WP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_COMP_TP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_WTP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_DPAD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_VERNIER, USB_DEVICE_ID_VERNIER_LABPRO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_VERNIER, USB_DEVICE_ID_VERNIER_GOTEMP) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_VERNIER, USB_DEVICE_ID_VERNIER_SKIP) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4a441a6..b48a64b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -633,6 +633,17 @@
 #define USB_DEVICE_ID_SYMBOL_SCANNER_1	0x0800
 #define USB_DEVICE_ID_SYMBOL_SCANNER_2	0x1300
 
+#define USB_VENDOR_ID_SYNAPTICS		0x06cb
+#define USB_DEVICE_ID_SYNAPTICS_TP	0x0001
+#define USB_DEVICE_ID_SYNAPTICS_INT_TP	0x0002
+#define USB_DEVICE_ID_SYNAPTICS_CPAD	0x0003
+#define USB_DEVICE_ID_SYNAPTICS_TS	0x0006
+#define USB_DEVICE_ID_SYNAPTICS_STICK	0x0007
+#define USB_DEVICE_ID_SYNAPTICS_WP	0x0008
+#define USB_DEVICE_ID_SYNAPTICS_COMP_TP	0x0009
+#define USB_DEVICE_ID_SYNAPTICS_WTP	0x0010
+#define USB_DEVICE_ID_SYNAPTICS_DPAD	0x0013
+
 #define USB_VENDOR_ID_THRUSTMASTER	0x044f
 
 #define USB_VENDOR_ID_TOPSEED		0x0766
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 9c1e6ee..f258089 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -322,4 +322,18 @@ config MOUSE_SYNAPTICS_I2C
 	  To compile this driver as a module, choose M here: the
 	  module will be called synaptics_i2c.
 
+config MOUSE_SYNAPTICS_USB
+	tristate "Synaptics USB device support"
+	depends on USB
+	help
+	  Say Y here if you want to use a Synaptics USB touchpad or pointing stick.
+	  These Synaptics USB devices emulate an USB mouse by default, so you can
+	  also use the usbhid driver. However, in combination with special X.Org
+	  drivers this kernel driver allows you to customize the behaviour of the
+	  device. More information can be found at:
+	  <http://jan-steinhoff.de/linux/synaptics-usb.html>
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called synaptics_usb.
+
 endif
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 570c84a4..4718eff 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_MOUSE_PXA930_TRKBALL)	+= pxa930_trkball.o
 obj-$(CONFIG_MOUSE_RISCPC)		+= rpcmouse.o
 obj-$(CONFIG_MOUSE_SERIAL)		+= sermouse.o
 obj-$(CONFIG_MOUSE_SYNAPTICS_I2C)	+= synaptics_i2c.o
+obj-$(CONFIG_MOUSE_SYNAPTICS_USB)	+= synaptics_usb.o
 obj-$(CONFIG_MOUSE_VSXXXAA)		+= vsxxxaa.o
 
 psmouse-objs := psmouse-base.o synaptics.o
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
new file mode 100644
index 0000000..a414fa2
--- /dev/null
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -0,0 +1,529 @@
+/****
+ * USB Synaptics device driver
+ *
+ *  Copyright (c) 2002 Rob Miller (rob@inpharmatica . co . uk)
+ *  Copyright (c) 2003 Ron Lee (ron@debian.org)
+ *	cPad driver for kernel 2.4
+ *
+ *  Copyright (c) 2004 Jan Steinhoff (cpad@jan-steinhoff . de)
+ *  Copyright (c) 2004 Ron Lee (ron@debian.org)
+ *	rewritten for kernel 2.6
+ *
+ *  cPad display character device part is not included. It can be found at
+ *  http://jan-steinhoff.de/linux/synaptics-usb.html
+ *
+ * Bases on:	usb_skeleton.c v2.2 by Greg Kroah-Hartman
+ *		drivers/hid/usbhid/usbmouse.c by Vojtech Pavlik
+ *		drivers/input/mouse/synaptics.c by Peter Osterlund
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * Trademarks are the property of their respective owners.
+ */
+
+/****
+ * There are three different types of Synaptics USB devices: Touchpads,
+ * touchsticks (or trackpoints), and touchscreens. Touchpads are well supported
+ * by this driver, touchstick support has not been tested much yet, and
+ * touchscreens have not been tested at all. The only difference between pad
+ * and stick seems to be that the x and y finger positions are unsigned 13 bit
+ * integers in the first case, but are signed ones in the second case.
+ *
+ * Up to three alternate settings are possible:
+ *	setting 0: one int endpoint for relative movement (used by usbhid.ko)
+ *	setting 1: one int endpoint for absolute finger position
+ *	setting 2 (cPad only): one int endpoint for absolute finger position and
+ *		   two bulk endpoints for the display (in/out)
+ * This driver uses setting 1.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/usb.h>
+#include <linux/input.h>
+#include <linux/usb/input.h>
+
+#define USB_VENDOR_ID_SYNAPTICS	0x06cb
+#define USB_DEVICE_ID_SYNAPTICS_TP	0x0001	/* Synaptics USB TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_INT_TP	0x0002	/* Integrated USB TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_CPAD	0x0003	/* Synaptics cPad */
+#define USB_DEVICE_ID_SYNAPTICS_TS	0x0006	/* Synaptics TouchScreen */
+#define USB_DEVICE_ID_SYNAPTICS_STICK	0x0007	/* Synaptics USB Styk */
+#define USB_DEVICE_ID_SYNAPTICS_WP	0x0008	/* Synaptics USB WheelPad */
+#define USB_DEVICE_ID_SYNAPTICS_COMP_TP	0x0009	/* Composite USB TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_WTP	0x0010	/* Wireless TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_DPAD	0x0013	/* DisplayPad */
+
+#define SYNUSB_TOUCHPAD			(1 << 0)
+#define SYNUSB_STICK			(1 << 1)
+#define SYNUSB_TOUCHSCREEN		(1 << 2)
+#define SYNUSB_AUXDISPLAY		(1 << 3) /* For cPad */
+
+#define USB_DEVICE_SYNAPTICS(prod, kind)		\
+	USB_DEVICE(USB_VENDOR_ID_SYNAPTICS,		\
+		   USB_DEVICE_ID_SYNAPTICS_##prod),	\
+	.driver_info = (kind),
+
+#define SYNUSB_RECV_SIZE	8
+
+MODULE_PARM_DESC(xmin, "minimal horizontal finger position for touchpads");
+MODULE_PARM_DESC(xmax, "maximal horizontal finger position for touchpads");
+MODULE_PARM_DESC(ymin, "minimal vertical finger position for touchpads");
+MODULE_PARM_DESC(ymax, "maximal vertical finger position for touchpads");
+static int xmin = 1472;
+static int xmax = 5472;
+static int ymin = 1408;
+static int ymax = 4448;
+module_param(xmin, int, 0444);
+module_param(xmax, int, 0444);
+module_param(ymin, int, 0444);
+module_param(ymax, int, 0444);
+
+struct synusb {
+	struct usb_device *udev;
+	struct usb_interface *intf;
+	struct urb *urb;
+	unsigned char *data;
+
+	/* input device related data structures */
+	struct input_dev *input;
+	char name[128];
+	char phys[64];
+
+	/* characteristics of the device */
+	unsigned long flags;
+};
+
+static void synusb_report_buttons(struct synusb *synusb)
+{
+	struct input_dev *input_dev = synusb->input;
+
+	input_report_key(input_dev, BTN_LEFT, synusb->data[1] & 0x04);
+	input_report_key(input_dev, BTN_RIGHT, synusb->data[1] & 0x01);
+	input_report_key(input_dev, BTN_MIDDLE, synusb->data[1] & 0x02);
+	if (synusb->flags & SYNUSB_AUXDISPLAY)
+		input_report_key(input_dev, BTN_MISC, synusb->data[1] & 0x08);
+}
+
+static void synusb_report_stick(struct synusb *synusb)
+{
+	struct input_dev *input_dev = synusb->input;
+	int x, y;
+	unsigned int pressure;
+
+	pressure = synusb->data[6];
+	x = (s16)(be16_to_cpup((__be16 *)&synusb->data[2]) << 3) >> 7;
+	y = (s16)(be16_to_cpup((__be16 *)&synusb->data[4]) << 3) >> 7;
+
+	if (pressure > 0) {
+		input_report_rel(input_dev, REL_X, x);
+		input_report_rel(input_dev, REL_Y, -y);
+	}
+
+	input_report_abs(input_dev, ABS_PRESSURE, pressure);
+
+	synusb_report_buttons(synusb);
+
+	input_sync(input_dev);
+}
+
+static void synusb_report_touchpad(struct synusb *synusb)
+{
+	struct input_dev *input_dev = synusb->input;
+	unsigned int num_fingers, tool_width;
+	unsigned int x, y;
+	unsigned int pressure, w;
+
+	pressure = synusb->data[6];
+	x = be16_to_cpup((__be16 *)&synusb->data[2]);
+	y = be16_to_cpup((__be16 *)&synusb->data[4]);
+	w = synusb->data[0] & 0x0f;
+
+	if (pressure > 0) {
+		num_fingers = 1;
+		tool_width = 5;
+		switch (w) {
+		case 0 ... 1:
+			num_fingers = 2 + w;
+			break;
+
+		case 2:	                /* pen, pretend its a finger */
+			break;
+
+		case 4 ... 15:
+			tool_width = w;
+			break;
+		}
+	} else {
+		num_fingers = 0;
+		tool_width = 0;
+	}
+
+	/*
+	 * Post events
+	 * BTN_TOUCH has to be first as mousedev relies on it when doing
+	 * absolute -> relative conversion
+	 */
+
+	if (pressure > 30)
+		input_report_key(input_dev, BTN_TOUCH, 1);
+	if (pressure < 25)
+		input_report_key(input_dev, BTN_TOUCH, 0);
+
+	if (num_fingers > 0) {
+		input_report_abs(input_dev, ABS_X, x);
+		input_report_abs(input_dev, ABS_Y, ymin + ymax - y);
+	}
+
+	input_report_abs(input_dev, ABS_PRESSURE, pressure);
+	input_report_abs(input_dev, ABS_TOOL_WIDTH, tool_width);
+
+	input_report_key(input_dev, BTN_TOOL_FINGER, num_fingers == 1);
+	input_report_key(input_dev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
+	input_report_key(input_dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);
+
+	synusb_report_buttons(synusb);
+
+	input_sync(input_dev);
+}
+
+static void synusb_irq(struct urb *urb)
+{
+	struct synusb *synusb = urb->context;
+	int error;
+
+	/* Check our status in case we need to bail out early. */
+	switch (urb->status) {
+	case 0:
+		usb_mark_last_busy(synusb->udev);
+		break;
+
+	case -EOVERFLOW:
+		dev_err(&synusb->intf->dev,
+			"%s: OVERFLOW with data length %d, actual length is %d\n",
+			__func__, SYNUSB_RECV_SIZE, urb->actual_length);
+
+	/* Device went away so don't keep trying to read from it. */
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		return;
+
+	default:
+		goto resubmit;
+		break;
+	}
+
+	if (synusb->flags & SYNUSB_STICK)
+		synusb_report_stick(synusb);
+	else
+		synusb_report_touchpad(synusb);
+
+resubmit:
+	error = usb_submit_urb(urb, GFP_ATOMIC);
+	if (error && error != -EPERM)
+		dev_err(&synusb->intf->dev,
+			"%s - usb_submit_urb failed with result: %d",
+			__func__, error);
+}
+
+static struct usb_endpoint_descriptor *
+synusb_get_in_endpoint(struct usb_host_interface *iface)
+{
+
+	struct usb_endpoint_descriptor *endpoint;
+	int i;
+
+	for (i = 0; i < iface->desc.bNumEndpoints; ++i) {
+		endpoint = &iface->endpoint[i].desc;
+
+		if (usb_endpoint_is_int_in(endpoint)) {
+			/* we found our interrupt in endpoint */
+			return endpoint;
+		}
+	}
+
+	return NULL;
+}
+
+static int synusb_open(struct input_dev *dev)
+{
+	struct synusb *synusb = input_get_drvdata(dev);
+	int retval = 0;
+
+	dev_err(&synusb->intf->dev, "%s\n", __func__);
+
+	if (usb_autopm_get_interface(synusb->intf) < 0)
+		return -EIO;
+
+	if (usb_submit_urb(synusb->urb, GFP_KERNEL)) {
+		dev_err(&synusb->intf->dev,
+			"%s - usb_submit_urb failed", __func__);
+		retval = -EIO;
+		goto out;
+	}
+
+	synusb->intf->needs_remote_wakeup = 1;
+
+out:
+	usb_autopm_put_interface(synusb->intf);
+	dev_err(&synusb->intf->dev, "%s done: %d\n", __func__, retval);
+	return retval;
+}
+
+static void synusb_close(struct input_dev *dev)
+{
+	struct synusb *synusb = input_get_drvdata(dev);
+	int autopm_error;
+
+	autopm_error = usb_autopm_get_interface(synusb->intf);
+
+	usb_kill_urb(synusb->urb);
+	synusb->intf->needs_remote_wakeup = 0;
+
+	if (!autopm_error)
+		usb_autopm_put_interface(synusb->intf);
+}
+
+static int synusb_probe(struct usb_interface *intf,
+			const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *ep;
+	struct synusb *synusb;
+	struct input_dev *input_dev;
+	unsigned int intf_num = intf->cur_altsetting->desc.bInterfaceNumber;
+	unsigned int altsetting = min(intf->num_altsetting, 1U);
+	int error;
+
+	error = usb_set_interface(udev, intf_num, altsetting);
+	if (error) {
+		dev_err(&udev->dev,
+			"Can not set alternate setting to %i, error: %i",
+			altsetting, error);
+		return error;
+	}
+
+	ep = synusb_get_in_endpoint(intf->cur_altsetting);
+	if (!ep)
+		return -ENODEV;
+
+	synusb = kzalloc(sizeof(*synusb), GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!synusb || !input_dev) {
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	synusb->udev = udev;
+	synusb->intf = intf;
+	synusb->input = input_dev;
+
+	synusb->flags = id->driver_info;
+	if (test_bit(SYNUSB_STICK, &synusb->flags) &&
+	    test_bit(SYNUSB_STICK, &synusb->flags)) {
+		/*
+		 * This is a combo device, we need to leave only proper
+		 * capability, depending on the interface.
+		 */
+		clear_bit(intf_num == 1 ? SYNUSB_TOUCHPAD : SYNUSB_STICK,
+			  &synusb->flags);
+	}
+
+	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!synusb->urb) {
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	synusb->data = usb_alloc_coherent(udev, SYNUSB_RECV_SIZE, GFP_KERNEL,
+					  &synusb->urb->transfer_dma);
+	if (!synusb->data) {
+		error = -ENOMEM;
+		goto err_free_urb;
+	}
+
+	usb_fill_int_urb(synusb->urb, udev,
+			 usb_rcvintpipe(udev, ep->bEndpointAddress),
+			 synusb->data, SYNUSB_RECV_SIZE,
+			 synusb_irq, synusb,
+			 ep->bInterval);
+	synusb->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+	if (udev->manufacturer)
+		strlcpy(synusb->name, udev->manufacturer,
+			sizeof(synusb->name));
+
+	if (udev->product) {
+		if (udev->manufacturer)
+			strlcat(synusb->name, " ", sizeof(synusb->name));
+		strlcat(synusb->name, udev->product, sizeof(synusb->name));
+	}
+
+	if (!strlen(synusb->name))
+		snprintf(synusb->name, sizeof(synusb->name),
+			 "USB Synaptics Device %04x:%04x",
+			 le16_to_cpu(udev->descriptor.idVendor),
+			 le16_to_cpu(udev->descriptor.idProduct));
+
+	if (synusb->flags & SYNUSB_STICK)
+		strlcat(synusb->name, " (Stick) ", sizeof(synusb->name));
+
+	usb_make_path(udev, synusb->phys, sizeof(synusb->phys));
+	strlcat(synusb->phys, "/input0", sizeof(synusb->phys));
+
+	input_dev->name = synusb->name; // synusb_get_name(synusb);
+	input_dev->phys = synusb->phys;
+	usb_to_input_id(udev, &input_dev->id);
+	input_dev->dev.parent = &synusb->intf->dev;
+
+	input_dev->open = synusb_open;
+	input_dev->close = synusb_close;
+
+	input_set_drvdata(input_dev, synusb);
+
+	__set_bit(EV_ABS, input_dev->evbit);
+	__set_bit(EV_KEY, input_dev->evbit);
+
+	if (synusb->flags & SYNUSB_STICK) {
+		__set_bit(EV_REL, input_dev->evbit);
+		__set_bit(REL_X, input_dev->relbit);
+		__set_bit(REL_Y, input_dev->relbit);
+		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 127, 0, 0);
+	} else {
+		input_set_abs_params(input_dev, ABS_X, xmin, xmax, 0, 0);
+		input_set_abs_params(input_dev, ABS_Y, ymin, ymax, 0, 0);
+		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 255, 0, 0);
+		input_set_abs_params(input_dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
+		__set_bit(BTN_TOUCH, input_dev->keybit);
+		__set_bit(BTN_TOOL_FINGER, input_dev->keybit);
+		__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
+		__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
+	}
+
+	__set_bit(BTN_LEFT, input_dev->keybit);
+	__set_bit(BTN_RIGHT, input_dev->keybit);
+	__set_bit(BTN_MIDDLE, input_dev->keybit);
+	if (synusb->flags & SYNUSB_AUXDISPLAY)
+		__set_bit(BTN_MISC, input_dev->keybit);
+
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(&udev->dev, "Failed to register input device, error %d\n",
+			error);
+		goto err_free_dma;
+	}
+
+	usb_set_intfdata(intf, synusb);
+	return 0;
+
+err_free_dma:
+	usb_free_coherent(udev, SYNUSB_RECV_SIZE, synusb->data,
+			  synusb->urb->transfer_dma);
+err_free_urb:
+	usb_free_urb(synusb->urb);
+err_free_mem:
+	input_free_device(input_dev);
+	kfree(synusb);
+
+	return error;
+}
+
+static void synusb_disconnect(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct usb_device *udev = interface_to_usbdev(intf);
+
+	usb_set_intfdata(intf, NULL);
+
+	input_unregister_device(synusb->input);
+
+	usb_free_coherent(udev, SYNUSB_RECV_SIZE, synusb->data,
+			  synusb->urb->transfer_dma);
+	usb_free_urb(synusb->urb);
+
+	kfree(synusb);
+}
+
+static int synusb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct input_dev *input_dev = synusb->input;
+
+	mutex_lock(&input_dev->mutex);
+	usb_kill_urb(synusb->urb);
+	mutex_unlock(&input_dev->mutex);
+
+	return 0;
+}
+
+static int synusb_resume(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct input_dev *input_dev = synusb->input;
+	int rv = 0;
+
+	mutex_lock(&input_dev->mutex);
+
+	if (input_dev->users && usb_submit_urb(synusb->urb, GFP_NOIO) < 0)
+		rv = -EIO;
+
+	mutex_unlock(&input_dev->mutex);
+
+	return rv;
+}
+
+static int synusb_reset_resume(struct usb_interface *intf)
+{
+	return synusb_resume(intf);
+}
+
+static struct usb_device_id synusb_idtable[] = {
+	{ USB_DEVICE_SYNAPTICS(TP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(INT_TP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(CPAD, SYNUSB_TOUCHPAD | SYNUSB_AUXDISPLAY) },
+	{ USB_DEVICE_SYNAPTICS(TS, SYNUSB_TOUCHSCREEN) },
+	{ USB_DEVICE_SYNAPTICS(STICK, SYNUSB_STICK) },
+	{ USB_DEVICE_SYNAPTICS(WP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(COMP_TP, SYNUSB_TOUCHPAD | SYNUSB_STICK) },
+	{ USB_DEVICE_SYNAPTICS(WTP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(DPAD, SYNUSB_TOUCHPAD) },
+	{ }
+};
+MODULE_DEVICE_TABLE(usb, synusb_idtable);
+
+static struct usb_driver synusb_driver = {
+	.name		= "synaptics_usb",
+	.probe		= synusb_probe,
+	.disconnect	= synusb_disconnect,
+	.id_table	= synusb_idtable,
+	.suspend	= synusb_suspend,
+	.resume		= synusb_resume,
+	.reset_resume	= synusb_reset_resume,
+	.supports_autosuspend = 1,
+};
+
+static int __init synusb_init(void)
+{
+	return usb_register(&synusb_driver);
+}
+
+static void __exit synusb_exit(void)
+{
+	usb_deregister(&synusb_driver);
+}
+
+module_init(synusb_init);
+module_exit(synusb_exit);
+
+MODULE_AUTHOR("Rob Miller <rob@inpharmatica.co.uk>, "
+              "Ron Lee <ron@debian.org>, "
+              "Jan Steinhoff <cpad@jan-steinhoff.de>");
+MODULE_DESCRIPTION("Synaptics USB device driver");
+MODULE_LICENSE("GPL");

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-04  9:41     ` Dmitry Torokhov
@ 2012-01-04  9:56       ` Oliver Neukum
  2012-01-04 10:05         ` Dmitry Torokhov
  2012-01-05  5:39       ` Jan Steinhoff
  1 sibling, 1 reply; 16+ messages in thread
From: Oliver Neukum @ 2012-01-04  9:56 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Jiri Kosina, Jan Steinhoff, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

Am Mittwoch, 4. Januar 2012, 10:41:03 schrieb Dmitry Torokhov:


> +static void synusb_irq(struct urb *urb)
> +{
> +	struct synusb *synusb = urb->context;
> +	int error;
> +
> +	/* Check our status in case we need to bail out early. */
> +	switch (urb->status) {
> +	case 0:
> +		usb_mark_last_busy(synusb->udev);
> +		break;
> +
> +	case -EOVERFLOW:
> +		dev_err(&synusb->intf->dev,
> +			"%s: OVERFLOW with data length %d, actual length is %d\n",
> +			__func__, SYNUSB_RECV_SIZE, urb->actual_length);

Do you really want to fall through here?

> +
> +	/* Device went away so don't keep trying to read from it. */
> +	case -ECONNRESET:
> +	case -ENOENT:
> +	case -ESHUTDOWN:
> +		return;
> +
> +	default:
> +		goto resubmit;
> +		break;
> +	}
> +
> +	if (synusb->flags & SYNUSB_STICK)
> +		synusb_report_stick(synusb);
> +	else
> +		synusb_report_touchpad(synusb);
> +
> +resubmit:
> +	error = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (error && error != -EPERM)
> +		dev_err(&synusb->intf->dev,
> +			"%s - usb_submit_urb failed with result: %d",
> +			__func__, error);
> +}
> +

[..]
> +static int synusb_open(struct input_dev *dev)
> +{
> +	struct synusb *synusb = input_get_drvdata(dev);
> +	int retval = 0;
> +
> +	dev_err(&synusb->intf->dev, "%s\n", __func__);
> +
> +	if (usb_autopm_get_interface(synusb->intf) < 0)
> +		return -EIO;
> +
> +	if (usb_submit_urb(synusb->urb, GFP_KERNEL)) {
> +		dev_err(&synusb->intf->dev,
> +			"%s - usb_submit_urb failed", __func__);
> +		retval = -EIO;
> +		goto out;
> +	}
> +
> +	synusb->intf->needs_remote_wakeup = 1;
> +
> +out:
> +	usb_autopm_put_interface(synusb->intf);
> +	dev_err(&synusb->intf->dev, "%s done: %d\n", __func__, retval);
> +	return retval;
> +}
> +
> +static void synusb_close(struct input_dev *dev)
> +{
> +	struct synusb *synusb = input_get_drvdata(dev);
> +	int autopm_error;
> +
> +	autopm_error = usb_autopm_get_interface(synusb->intf);
> +
> +	usb_kill_urb(synusb->urb);
> +	synusb->intf->needs_remote_wakeup = 0;
> +
> +	if (!autopm_error)
> +		usb_autopm_put_interface(synusb->intf);
> +}
> +
> +static int synusb_probe(struct usb_interface *intf,
> +			const struct usb_device_id *id)
> +{
> +	struct usb_device *udev = interface_to_usbdev(intf);
> +	struct usb_endpoint_descriptor *ep;
> +	struct synusb *synusb;
> +	struct input_dev *input_dev;
> +	unsigned int intf_num = intf->cur_altsetting->desc.bInterfaceNumber;
> +	unsigned int altsetting = min(intf->num_altsetting, 1U);
> +	int error;
> +
> +	error = usb_set_interface(udev, intf_num, altsetting);
> +	if (error) {
> +		dev_err(&udev->dev,
> +			"Can not set alternate setting to %i, error: %i",
> +			altsetting, error);
> +		return error;
> +	}
> +
> +	ep = synusb_get_in_endpoint(intf->cur_altsetting);
> +	if (!ep)
> +		return -ENODEV;
> +
> +	synusb = kzalloc(sizeof(*synusb), GFP_KERNEL);
> +	input_dev = input_allocate_device();
> +	if (!synusb || !input_dev) {
> +		error = -ENOMEM;
> +		goto err_free_mem;
> +	}
> +
> +	synusb->udev = udev;
> +	synusb->intf = intf;
> +	synusb->input = input_dev;
> +
> +	synusb->flags = id->driver_info;
> +	if (test_bit(SYNUSB_STICK, &synusb->flags) &&
> +	    test_bit(SYNUSB_STICK, &synusb->flags)) {

??? Voodoo ???

> +		/*
> +		 * This is a combo device, we need to leave only proper
> +		 * capability, depending on the interface.
> +		 */
> +		clear_bit(intf_num == 1 ? SYNUSB_TOUCHPAD : SYNUSB_STICK,
> +			  &synusb->flags);
> +	}
> +
> +	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
> +	if (!synusb->urb) {
> +		error = -ENOMEM;
> +		goto err_free_mem;
> +	}
> +
> +	synusb->data = usb_alloc_coherent(udev, SYNUSB_RECV_SIZE, GFP_KERNEL,
> +					  &synusb->urb->transfer_dma);
> +	if (!synusb->data) {
> +		error = -ENOMEM;
> +		goto err_free_urb;
> +	}
> +
> +	usb_fill_int_urb(synusb->urb, udev,
> +			 usb_rcvintpipe(udev, ep->bEndpointAddress),
> +			 synusb->data, SYNUSB_RECV_SIZE,
> +			 synusb_irq, synusb,
> +			 ep->bInterval);
> +	synusb->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;

According to the comment in the original driver you must submit the URB.
Are you sure not doing so is save?

> +
> +	if (udev->manufacturer)
> +		strlcpy(synusb->name, udev->manufacturer,
> +			sizeof(synusb->name));
> +
> +	if (udev->product) {
> +		if (udev->manufacturer)
> +			strlcat(synusb->name, " ", sizeof(synusb->name));
> +		strlcat(synusb->name, udev->product, sizeof(synusb->name));
> +	}
> +
> +	if (!strlen(synusb->name))
> +		snprintf(synusb->name, sizeof(synusb->name),
> +			 "USB Synaptics Device %04x:%04x",
> +			 le16_to_cpu(udev->descriptor.idVendor),
> +			 le16_to_cpu(udev->descriptor.idProduct));
> +
> +	if (synusb->flags & SYNUSB_STICK)
> +		strlcat(synusb->name, " (Stick) ", sizeof(synusb->name));
> +
> +	usb_make_path(udev, synusb->phys, sizeof(synusb->phys));
> +	strlcat(synusb->phys, "/input0", sizeof(synusb->phys));
> +
> +	input_dev->name = synusb->name; // synusb_get_name(synusb);
> +	input_dev->phys = synusb->phys;
> +	usb_to_input_id(udev, &input_dev->id);
> +	input_dev->dev.parent = &synusb->intf->dev;
> +
> +	input_dev->open = synusb_open;
> +	input_dev->close = synusb_close;
> +
> +	input_set_drvdata(input_dev, synusb);
> +
> +	__set_bit(EV_ABS, input_dev->evbit);
> +	__set_bit(EV_KEY, input_dev->evbit);
> +
> +	if (synusb->flags & SYNUSB_STICK) {
> +		__set_bit(EV_REL, input_dev->evbit);
> +		__set_bit(REL_X, input_dev->relbit);
> +		__set_bit(REL_Y, input_dev->relbit);
> +		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 127, 0, 0);
> +	} else {
> +		input_set_abs_params(input_dev, ABS_X, xmin, xmax, 0, 0);
> +		input_set_abs_params(input_dev, ABS_Y, ymin, ymax, 0, 0);
> +		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 255, 0, 0);
> +		input_set_abs_params(input_dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> +		__set_bit(BTN_TOUCH, input_dev->keybit);
> +		__set_bit(BTN_TOOL_FINGER, input_dev->keybit);
> +		__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
> +		__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
> +	}
> +
> +	__set_bit(BTN_LEFT, input_dev->keybit);
> +	__set_bit(BTN_RIGHT, input_dev->keybit);
> +	__set_bit(BTN_MIDDLE, input_dev->keybit);
> +	if (synusb->flags & SYNUSB_AUXDISPLAY)
> +		__set_bit(BTN_MISC, input_dev->keybit);
> +
> +	error = input_register_device(input_dev);
> +	if (error) {
> +		dev_err(&udev->dev, "Failed to register input device, error %d\n",
> +			error);
> +		goto err_free_dma;
> +	}
> +
> +	usb_set_intfdata(intf, synusb);
> +	return 0;
> +
> +err_free_dma:
> +	usb_free_coherent(udev, SYNUSB_RECV_SIZE, synusb->data,
> +			  synusb->urb->transfer_dma);
> +err_free_urb:
> +	usb_free_urb(synusb->urb);
> +err_free_mem:
> +	input_free_device(input_dev);
> +	kfree(synusb);
> +
> +	return error;
> +}
> +

[..]
> +static struct usb_driver synusb_driver = {
> +	.name		= "synaptics_usb",
> +	.probe		= synusb_probe,
> +	.disconnect	= synusb_disconnect,
> +	.id_table	= synusb_idtable,
> +	.suspend	= synusb_suspend,
> +	.resume		= synusb_resume,
> +	.reset_resume	= synusb_reset_resume,

You've killed the support for reset. That is not cool.

> +	.supports_autosuspend = 1,
> +};

	Regards
		Oliver

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-04  9:56       ` Oliver Neukum
@ 2012-01-04 10:05         ` Dmitry Torokhov
  2012-01-05  6:01           ` Jan Steinhoff
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2012-01-04 10:05 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Jiri Kosina, Jan Steinhoff, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Wed, Jan 04, 2012 at 10:56:20AM +0100, Oliver Neukum wrote:
> Am Mittwoch, 4. Januar 2012, 10:41:03 schrieb Dmitry Torokhov:
> 
> 
> > +static void synusb_irq(struct urb *urb)
> > +{
> > +	struct synusb *synusb = urb->context;
> > +	int error;
> > +
> > +	/* Check our status in case we need to bail out early. */
> > +	switch (urb->status) {
> > +	case 0:
> > +		usb_mark_last_busy(synusb->udev);
> > +		break;
> > +
> > +	case -EOVERFLOW:
> > +		dev_err(&synusb->intf->dev,
> > +			"%s: OVERFLOW with data length %d, actual length is %d\n",
> > +			__func__, SYNUSB_RECV_SIZE, urb->actual_length);
> 
> Do you really want to fall through here?

Probably not.

> 
> > +
> > +	/* Device went away so don't keep trying to read from it. */
> > +	case -ECONNRESET:
> > +	case -ENOENT:
> > +	case -ESHUTDOWN:
> > +		return;
> > +
> > +	default:
> > +		goto resubmit;
> > +		break;
> > +	}
> > +
> > +	if (synusb->flags & SYNUSB_STICK)
> > +		synusb_report_stick(synusb);
> > +	else
> > +		synusb_report_touchpad(synusb);
> > +
> > +resubmit:
> > +	error = usb_submit_urb(urb, GFP_ATOMIC);
> > +	if (error && error != -EPERM)
> > +		dev_err(&synusb->intf->dev,
> > +			"%s - usb_submit_urb failed with result: %d",
> > +			__func__, error);
> > +}
> > +
> 
> [..]
> > +static int synusb_open(struct input_dev *dev)
> > +{
> > +	struct synusb *synusb = input_get_drvdata(dev);
> > +	int retval = 0;
> > +
> > +	dev_err(&synusb->intf->dev, "%s\n", __func__);
> > +
> > +	if (usb_autopm_get_interface(synusb->intf) < 0)
> > +		return -EIO;
> > +
> > +	if (usb_submit_urb(synusb->urb, GFP_KERNEL)) {
> > +		dev_err(&synusb->intf->dev,
> > +			"%s - usb_submit_urb failed", __func__);
> > +		retval = -EIO;
> > +		goto out;
> > +	}
> > +
> > +	synusb->intf->needs_remote_wakeup = 1;
> > +
> > +out:
> > +	usb_autopm_put_interface(synusb->intf);
> > +	dev_err(&synusb->intf->dev, "%s done: %d\n", __func__, retval);
> > +	return retval;
> > +}
> > +
> > +static void synusb_close(struct input_dev *dev)
> > +{
> > +	struct synusb *synusb = input_get_drvdata(dev);
> > +	int autopm_error;
> > +
> > +	autopm_error = usb_autopm_get_interface(synusb->intf);
> > +
> > +	usb_kill_urb(synusb->urb);
> > +	synusb->intf->needs_remote_wakeup = 0;
> > +
> > +	if (!autopm_error)
> > +		usb_autopm_put_interface(synusb->intf);
> > +}
> > +
> > +static int synusb_probe(struct usb_interface *intf,
> > +			const struct usb_device_id *id)
> > +{
> > +	struct usb_device *udev = interface_to_usbdev(intf);
> > +	struct usb_endpoint_descriptor *ep;
> > +	struct synusb *synusb;
> > +	struct input_dev *input_dev;
> > +	unsigned int intf_num = intf->cur_altsetting->desc.bInterfaceNumber;
> > +	unsigned int altsetting = min(intf->num_altsetting, 1U);
> > +	int error;
> > +
> > +	error = usb_set_interface(udev, intf_num, altsetting);
> > +	if (error) {
> > +		dev_err(&udev->dev,
> > +			"Can not set alternate setting to %i, error: %i",
> > +			altsetting, error);
> > +		return error;
> > +	}
> > +
> > +	ep = synusb_get_in_endpoint(intf->cur_altsetting);
> > +	if (!ep)
> > +		return -ENODEV;
> > +
> > +	synusb = kzalloc(sizeof(*synusb), GFP_KERNEL);
> > +	input_dev = input_allocate_device();
> > +	if (!synusb || !input_dev) {
> > +		error = -ENOMEM;
> > +		goto err_free_mem;
> > +	}
> > +
> > +	synusb->udev = udev;
> > +	synusb->intf = intf;
> > +	synusb->input = input_dev;
> > +
> > +	synusb->flags = id->driver_info;
> > +	if (test_bit(SYNUSB_STICK, &synusb->flags) &&
> > +	    test_bit(SYNUSB_STICK, &synusb->flags)) {
> 
> ??? Voodoo ???

Yeah, 2nd should be test_bit(SYNUSB_TOUCHPAD, ...)
> 
> > +		/*
> > +		 * This is a combo device, we need to leave only proper
> > +		 * capability, depending on the interface.
> > +		 */
> > +		clear_bit(intf_num == 1 ? SYNUSB_TOUCHPAD : SYNUSB_STICK,
> > +			  &synusb->flags);
> > +	}
> > +
> > +	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
> > +	if (!synusb->urb) {
> > +		error = -ENOMEM;
> > +		goto err_free_mem;
> > +	}
> > +
> > +	synusb->data = usb_alloc_coherent(udev, SYNUSB_RECV_SIZE, GFP_KERNEL,
> > +					  &synusb->urb->transfer_dma);
> > +	if (!synusb->data) {
> > +		error = -ENOMEM;
> > +		goto err_free_urb;
> > +	}
> > +
> > +	usb_fill_int_urb(synusb->urb, udev,
> > +			 usb_rcvintpipe(udev, ep->bEndpointAddress),
> > +			 synusb->data, SYNUSB_RECV_SIZE,
> > +			 synusb_irq, synusb,
> > +			 ep->bInterval);
> > +	synusb->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> 
> According to the comment in the original driver you must submit the URB.
> Are you sure not doing so is save?

Seems to work here...

> 
> > +
> > +	if (udev->manufacturer)
> > +		strlcpy(synusb->name, udev->manufacturer,
> > +			sizeof(synusb->name));
> > +
> > +	if (udev->product) {
> > +		if (udev->manufacturer)
> > +			strlcat(synusb->name, " ", sizeof(synusb->name));
> > +		strlcat(synusb->name, udev->product, sizeof(synusb->name));
> > +	}
> > +
> > +	if (!strlen(synusb->name))
> > +		snprintf(synusb->name, sizeof(synusb->name),
> > +			 "USB Synaptics Device %04x:%04x",
> > +			 le16_to_cpu(udev->descriptor.idVendor),
> > +			 le16_to_cpu(udev->descriptor.idProduct));
> > +
> > +	if (synusb->flags & SYNUSB_STICK)
> > +		strlcat(synusb->name, " (Stick) ", sizeof(synusb->name));
> > +
> > +	usb_make_path(udev, synusb->phys, sizeof(synusb->phys));
> > +	strlcat(synusb->phys, "/input0", sizeof(synusb->phys));
> > +
> > +	input_dev->name = synusb->name; // synusb_get_name(synusb);
> > +	input_dev->phys = synusb->phys;
> > +	usb_to_input_id(udev, &input_dev->id);
> > +	input_dev->dev.parent = &synusb->intf->dev;
> > +
> > +	input_dev->open = synusb_open;
> > +	input_dev->close = synusb_close;
> > +
> > +	input_set_drvdata(input_dev, synusb);
> > +
> > +	__set_bit(EV_ABS, input_dev->evbit);
> > +	__set_bit(EV_KEY, input_dev->evbit);
> > +
> > +	if (synusb->flags & SYNUSB_STICK) {
> > +		__set_bit(EV_REL, input_dev->evbit);
> > +		__set_bit(REL_X, input_dev->relbit);
> > +		__set_bit(REL_Y, input_dev->relbit);
> > +		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 127, 0, 0);
> > +	} else {
> > +		input_set_abs_params(input_dev, ABS_X, xmin, xmax, 0, 0);
> > +		input_set_abs_params(input_dev, ABS_Y, ymin, ymax, 0, 0);
> > +		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 255, 0, 0);
> > +		input_set_abs_params(input_dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
> > +		__set_bit(BTN_TOUCH, input_dev->keybit);
> > +		__set_bit(BTN_TOOL_FINGER, input_dev->keybit);
> > +		__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
> > +		__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
> > +	}
> > +
> > +	__set_bit(BTN_LEFT, input_dev->keybit);
> > +	__set_bit(BTN_RIGHT, input_dev->keybit);
> > +	__set_bit(BTN_MIDDLE, input_dev->keybit);
> > +	if (synusb->flags & SYNUSB_AUXDISPLAY)
> > +		__set_bit(BTN_MISC, input_dev->keybit);
> > +
> > +	error = input_register_device(input_dev);
> > +	if (error) {
> > +		dev_err(&udev->dev, "Failed to register input device, error %d\n",
> > +			error);
> > +		goto err_free_dma;
> > +	}
> > +
> > +	usb_set_intfdata(intf, synusb);
> > +	return 0;
> > +
> > +err_free_dma:
> > +	usb_free_coherent(udev, SYNUSB_RECV_SIZE, synusb->data,
> > +			  synusb->urb->transfer_dma);
> > +err_free_urb:
> > +	usb_free_urb(synusb->urb);
> > +err_free_mem:
> > +	input_free_device(input_dev);
> > +	kfree(synusb);
> > +
> > +	return error;
> > +}
> > +
> 
> [..]
> > +static struct usb_driver synusb_driver = {
> > +	.name		= "synaptics_usb",
> > +	.probe		= synusb_probe,
> > +	.disconnect	= synusb_disconnect,
> > +	.id_table	= synusb_idtable,
> > +	.suspend	= synusb_suspend,
> > +	.resume		= synusb_resume,
> > +	.reset_resume	= synusb_reset_resume,
> 
> You've killed the support for reset. That is not cool.

OK.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-04  8:25 ` Oliver Neukum
@ 2012-01-05  2:46   ` Jan Steinhoff
  0 siblings, 0 replies; 16+ messages in thread
From: Jan Steinhoff @ 2012-01-05  2:46 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Alessandro Rubini, Dmitry Torokhov, linux-input, Jiri Kosina,
	linux-usb, linux-kernel

On Wed, 4 Jan 2012 09:25:59 +0100
Oliver Neukum <oneukum@suse.de> wrote:

> Am Dienstag, 3. Januar 2012, 19:40:33 schrieb Jan Steinhoff:  
> > From: Jan Steinhoff <mail@jan-steinhoff.de>
> > 
> > This patch adds a driver for Synaptics USB touchpad or pointing
> > stick devices. These USB devices emulate an USB mouse by default,
> > so one can also use the usbhid driver. However, in combination with
> > special user space drivers this kernel driver allows one to
> > customize the behaviour of the device.  
> 
> Hi,
> 
> thank you for this driver. There are a few issues which I have
> commented on in the text.  

Hi,

I agree with most of the comments, but I have some further
questions/remarks below.

Best,
Jan

[...]
> [..]  
> > +
> > +/*
> > + * initialization of usb data structures
> > + */
> > +
> > +static int synusb_setup_iurb(struct synusb *synusb,
> > +			     struct usb_endpoint_descriptor
> > *endpoint) +{
> > +	char *buf;
> > +
> > +	if (endpoint->wMaxPacketSize < 8)
> > +		return 0;  
> 
> How could this happen?  

The id table can be modified via sysfs, so it is easily possible to try
this driver with officially unsupported devices. (At least for Synaptics
devices, there is a good chance this driver works just the same.) The
condition I put there was the only one I could think of to select when
it is not worth trying.

[...]
> > +static int synusb_suspend(struct usb_interface *intf, pm_message_t
> > message) +{
> > +	struct synusb *synusb = usb_get_intfdata(intf);
> > +
> > +	if (synusb == NULL)
> > +		return 0;  
> 
> How can this happen?  

According to the Documentation this can not happen. However, this check
is present in usb-skeleton.c, so I put it there just in case I
overlooked something.

[...]
> > +static struct usb_driver synusb_driver = {
> > +	.name =		"synaptics_usb",
> > +	.probe =	synusb_probe,
> > +	.disconnect =	synusb_disconnect,
> > +	.id_table =	synusb_idtable,
> > +	.suspend =	synusb_suspend,
> > +	.resume =	synusb_resume,
> > +	.pre_reset =	synusb_pre_reset,
> > +	.post_reset =	synusb_post_reset,
> > +	.reset_resume = synusb_reset_resume,
> > +	.supports_autosuspend = 1,  
> 
> Yet you do not manage the busy state. Do you really want to play
> ping-pong with the power state?  

The last time I tested autosuspend its timer was reseted "automatically"
every time an int urb arrived. Isn't this all that is needed for an
input device?

The autosuspend delay should of course be set to a reasonably large
value. Then there should be no ping-pong with the power state while the
device is actually used/touched. On the other hand, if the pad is not
touched for a while, then no int urb arrives and the device will
autosuspend (and, thanks to remote wakeup, autoresume nicely when
touched again).

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-04  9:41     ` Dmitry Torokhov
  2012-01-04  9:56       ` Oliver Neukum
@ 2012-01-05  5:39       ` Jan Steinhoff
  2012-01-05  6:34         ` Dmitry Torokhov
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Steinhoff @ 2012-01-05  5:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Wed, 4 Jan 2012 01:41:03 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> Om, so I have been looking at the driver and the following seems to be
> still working with my Lenovo composite touchpad/stick.
> 
> Changes:
> 
> - the stick is reported in relative mode; we might consider addig
>   sensitivity control similar to trackpoint driver. Press-to-select -
>   maybe in driver, or in client; undecided.
> 
> - got rid of BTN_MISC/BTN_MIDDLE option (at least for now);

Please make BTN_MIDDLE the default in this case for the cPad. This
button is actually located where the middle button should be. Only if
one uses the cPad's background display it obtains a "special" meaning.
(Then pressing it should launch a menu on the background display and the
touchpad works as a touchscreen for this display.)

> - added devices to HID blacklist;

I think it is more flexible and save to use the manual driver binding
capability of the USB core instead:

http://lwn.net/Articles/143397/

Which driver to use for which device can then be set, e.g., with the
help of udev config files.

Blacklisting has the big disadvantage that the device will not work at
all if the synaptics-usb kernel module is not loaded/present, whereas
otherwise usbhid is used as the default. At least the corresponding
part in the blacklist should be enclosed in
#ifdef CONFIG_MOUSE_SYNAPTICS_USB ... #endif

Further, as noted in the initial comment, not all devices have been
tested with this driver. This is another reason to allow the user to
choose which driver to use.

> - runtime PM (hopefully I got it right);
> 
> - open/close.

I do not understand why needs_remote_wakeup is flipped in open/close.
As far as I know, the device can not autosuspend after
usb_autopm_get_interface was called anyway?

> Still TODO:
> 
> - query device for supported rages instead of having module
> parameters.

At least for the cPad the device does unfortunately not provide this
information. I tried to read the HID descriptors once but just got
errors. However, the defaults work quite well for all tested devices.
The module parameters are actually meant to just provide a simple
workaround in case one of the untested devices (especially the
touchscreen) might need them.

Thanks!

Jan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-04 10:05         ` Dmitry Torokhov
@ 2012-01-05  6:01           ` Jan Steinhoff
  2012-01-05  6:36             ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Steinhoff @ 2012-01-05  6:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Wed, 4 Jan 2012 02:05:53 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Wed, Jan 04, 2012 at 10:56:20AM +0100, Oliver Neukum wrote:
> > Am Mittwoch, 4. Januar 2012, 10:41:03 schrieb Dmitry Torokhov:
> > > +static int synusb_probe(struct usb_interface *intf,
> > > +			const struct usb_device_id *id)
> > > +{
[...]
> > > +	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
> > > +	if (!synusb->urb) {
> > > +		error = -ENOMEM;
> > > +		goto err_free_mem;
> > > +	}
> > > +
> > > +	synusb->data = usb_alloc_coherent(udev,
> > > SYNUSB_RECV_SIZE, GFP_KERNEL,
> > > +
> > > &synusb->urb->transfer_dma);
> > > +	if (!synusb->data) {
> > > +		error = -ENOMEM;
> > > +		goto err_free_urb;
> > > +	}
> > > +
> > > +	usb_fill_int_urb(synusb->urb, udev,
> > > +			 usb_rcvintpipe(udev,
> > > ep->bEndpointAddress),
> > > +			 synusb->data, SYNUSB_RECV_SIZE,
> > > +			 synusb_irq, synusb,
> > > +			 ep->bInterval);
> > > +	synusb->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> > 
> > According to the comment in the original driver you must submit the
> > URB. Are you sure not doing so is save?
> 
> Seems to work here...

Let me comment on this issue in more detail, so you can better judge
what is the "right" solution here.

Both ways actually work. But, at least for the cPad, the device will
pretend it got reconnected if the int urb is not fetched (while not
suspended, of course). This means it would disconnect if no input device
is opened (AFAIK most X.org input drivers close the devices when one
switches to console) and one touches the pad (actually, a slight breeze
of air might be enough, it is very sensitive!). Another annoying side
effect in the case of the cPad is that the background light flashes on
every reconnect.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-05  5:39       ` Jan Steinhoff
@ 2012-01-05  6:34         ` Dmitry Torokhov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2012-01-05  6:34 UTC (permalink / raw)
  To: Jan Steinhoff
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Thu, Jan 05, 2012 at 05:39:57AM +0000, Jan Steinhoff wrote:
> On Wed, 4 Jan 2012 01:41:03 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > Om, so I have been looking at the driver and the following seems to be
> > still working with my Lenovo composite touchpad/stick.
> > 
> > Changes:
> > 
> > - the stick is reported in relative mode; we might consider addig
> >   sensitivity control similar to trackpoint driver. Press-to-select -
> >   maybe in driver, or in client; undecided.
> > 
> > - got rid of BTN_MISC/BTN_MIDDLE option (at least for now);
> 
> Please make BTN_MIDDLE the default in this case for the cPad. This
> button is actually located where the middle button should be. Only if
> one uses the cPad's background display it obtains a "special" meaning.
> (Then pressing it should launch a menu on the background display and the
> touchpad works as a touchscreen for this display.)

OK, let's change it to MTN_MIDDLE. We may consider changing it to
BTN_MISC if cPad display support is merged and active.

> 
> > - added devices to HID blacklist;
> 
> I think it is more flexible and save to use the manual driver binding
> capability of the USB core instead:
> 
> http://lwn.net/Articles/143397/
> 
> Which driver to use for which device can then be set, e.g., with the
> help of udev config files.

However it is more work for distributions and users. As it stands now
adding blacklist and compiling synaptics_usb does not require any
additional userspace support (everyone packages and installs synaptics X
driver nowadays).

> 
> Blacklisting has the big disadvantage that the device will not work at
> all if the synaptics-usb kernel module is not loaded/present, whereas
> otherwise usbhid is used as the default. At least the corresponding
> part in the blacklist should be enclosed in
> #ifdef CONFIG_MOUSE_SYNAPTICS_USB ... #endif

This is a good idea, I'll add ifdefs.

> 
> Further, as noted in the initial comment, not all devices have been
> tested with this driver. This is another reason to allow the user to
> choose which driver to use.

I'll remove touchscreen from blacklist/device table as it really should
generate different set of events; the rest I hope we'll be able to fix
reasonably quickly if there are complaints.

> 
> > - runtime PM (hopefully I got it right);
> > 
> > - open/close.
> 
> I do not understand why needs_remote_wakeup is flipped in open/close.
> As far as I know, the device can not autosuspend after
> usb_autopm_get_interface was called anyway?

We do usb_autopm_put_interface as last action in open() so the device
will be suspended and needs wakeups.

> 
> > Still TODO:
> > 
> > - query device for supported rages instead of having module
> > parameters.
> 
> At least for the cPad the device does unfortunately not provide this
> information. I tried to read the HID descriptors once but just got
> errors. However, the defaults work quite well for all tested devices.
> The module parameters are actually meant to just provide a simple
> workaround in case one of the untested devices (especially the
> touchscreen) might need them.

OK, so why don't we replace module parameters with default ranges and
maybe later Synaptics guys will tell us how to query ranges properly?
Default ranges worked reasonably well for PS/2 version for several
years.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-05  6:01           ` Jan Steinhoff
@ 2012-01-05  6:36             ` Dmitry Torokhov
  2012-01-05 14:22               ` Jan Steinhoff
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2012-01-05  6:36 UTC (permalink / raw)
  To: Jan Steinhoff
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Thu, Jan 05, 2012 at 06:01:38AM +0000, Jan Steinhoff wrote:
> On Wed, 4 Jan 2012 02:05:53 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Jan 04, 2012 at 10:56:20AM +0100, Oliver Neukum wrote:
> > > Am Mittwoch, 4. Januar 2012, 10:41:03 schrieb Dmitry Torokhov:
> > > > +static int synusb_probe(struct usb_interface *intf,
> > > > +			const struct usb_device_id *id)
> > > > +{
> [...]
> > > > +	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
> > > > +	if (!synusb->urb) {
> > > > +		error = -ENOMEM;
> > > > +		goto err_free_mem;
> > > > +	}
> > > > +
> > > > +	synusb->data = usb_alloc_coherent(udev,
> > > > SYNUSB_RECV_SIZE, GFP_KERNEL,
> > > > +
> > > > &synusb->urb->transfer_dma);
> > > > +	if (!synusb->data) {
> > > > +		error = -ENOMEM;
> > > > +		goto err_free_urb;
> > > > +	}
> > > > +
> > > > +	usb_fill_int_urb(synusb->urb, udev,
> > > > +			 usb_rcvintpipe(udev,
> > > > ep->bEndpointAddress),
> > > > +			 synusb->data, SYNUSB_RECV_SIZE,
> > > > +			 synusb_irq, synusb,
> > > > +			 ep->bInterval);
> > > > +	synusb->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> > > 
> > > According to the comment in the original driver you must submit the
> > > URB. Are you sure not doing so is save?
> > 
> > Seems to work here...
> 
> Let me comment on this issue in more detail, so you can better judge
> what is the "right" solution here.
> 
> Both ways actually work. But, at least for the cPad, the device will
> pretend it got reconnected if the int urb is not fetched (while not
> suspended, of course). This means it would disconnect if no input device
> is opened (AFAIK most X.org input drivers close the devices when one
> switches to console) and one touches the pad (actually, a slight breeze
> of air might be enough, it is very sensitive!). Another annoying side
> effect in the case of the cPad is that the background light flashes on
> every reconnect.

OK, then maybe we should have cPad start transfers immediately and let
other devices do it in open. There is no reason to do work if nobody
interested in it...

Another option is to move configuring interface to open()?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-05  6:36             ` Dmitry Torokhov
@ 2012-01-05 14:22               ` Jan Steinhoff
  2012-01-10  9:43                 ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Steinhoff @ 2012-01-05 14:22 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Wed, 4 Jan 2012 22:36:11 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Thu, Jan 05, 2012 at 06:01:38AM +0000, Jan Steinhoff wrote:
> > On Wed, 4 Jan 2012 02:05:53 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > On Wed, Jan 04, 2012 at 10:56:20AM +0100, Oliver Neukum wrote:
> > > > Am Mittwoch, 4. Januar 2012, 10:41:03 schrieb Dmitry Torokhov:
> > > > > +static int synusb_probe(struct usb_interface *intf,
> > > > > +			const struct usb_device_id *id)
> > > > > +{
> > [...]
> > > > > +	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
> > > > > +	if (!synusb->urb) {
> > > > > +		error = -ENOMEM;
> > > > > +		goto err_free_mem;
> > > > > +	}
> > > > > +
> > > > > +	synusb->data = usb_alloc_coherent(udev,
> > > > > SYNUSB_RECV_SIZE, GFP_KERNEL,
> > > > > +
> > > > > &synusb->urb->transfer_dma);
> > > > > +	if (!synusb->data) {
> > > > > +		error = -ENOMEM;
> > > > > +		goto err_free_urb;
> > > > > +	}
> > > > > +
> > > > > +	usb_fill_int_urb(synusb->urb, udev,
> > > > > +			 usb_rcvintpipe(udev,
> > > > > ep->bEndpointAddress),
> > > > > +			 synusb->data, SYNUSB_RECV_SIZE,
> > > > > +			 synusb_irq, synusb,
> > > > > +			 ep->bInterval);
> > > > > +	synusb->urb->transfer_flags |=
> > > > > URB_NO_TRANSFER_DMA_MAP;
> > > > 
> > > > According to the comment in the original driver you must submit
> > > > the URB. Are you sure not doing so is save?
> > > 
> > > Seems to work here...
> > 
> > Let me comment on this issue in more detail, so you can better judge
> > what is the "right" solution here.
> > 
> > Both ways actually work. But, at least for the cPad, the device will
> > pretend it got reconnected if the int urb is not fetched (while not
> > suspended, of course). This means it would disconnect if no input
> > device is opened (AFAIK most X.org input drivers close the devices
> > when one switches to console) and one touches the pad (actually, a
> > slight breeze of air might be enough, it is very sensitive!).
> > Another annoying side effect in the case of the cPad is that the
> > background light flashes on every reconnect.
> 
> OK, then maybe we should have cPad start transfers immediately and let
> other devices do it in open. There is no reason to do work if nobody
> interested in it...

Given that the devices are very similar, I expect that also the other
devices may behave like this. So this is maybe not only a cPad
related problem. But I have never investigated this for other devices,
so I am not sure.

Regarding the extra amount of work, I thought it is finally less
expensive to run through the callback and drop the 8 bytes of data,
than to let the device trigger a disconnect, probe, and associated
device deletion/creation by udev every 60 sec or so (it maybe even
produces more USB traffic).

> Another option is to move configuring interface to open()?

The reconnect will always happen if the int endpoint is not "in use". So
configuring the device not in probe will not help, will it?

I think the best way would be to immediately suspend the device, if
there is an easy way to trigger this using the driver or usb core. (And
if there is no easy way, it is still possible to do it via sysfs if
someone is annoyed by this minor "reconnect bug".) Further, this should
be fine for all devices, whether they have this "reconnect bug" or not.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-05 14:22               ` Jan Steinhoff
@ 2012-01-10  9:43                 ` Dmitry Torokhov
  2012-01-12  0:08                   ` Jan Steinhoff
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2012-01-10  9:43 UTC (permalink / raw)
  To: Jan Steinhoff
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Thu, Jan 05, 2012 at 02:22:57PM +0000, Jan Steinhoff wrote:
> On Wed, 4 Jan 2012 22:36:11 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > On Thu, Jan 05, 2012 at 06:01:38AM +0000, Jan Steinhoff wrote:
> > > On Wed, 4 Jan 2012 02:05:53 -0800
> > > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > On Wed, Jan 04, 2012 at 10:56:20AM +0100, Oliver Neukum wrote:
> > > > > Am Mittwoch, 4. Januar 2012, 10:41:03 schrieb Dmitry Torokhov:
> > > > > > +static int synusb_probe(struct usb_interface *intf,
> > > > > > +			const struct usb_device_id *id)
> > > > > > +{
> > > [...]
> > > > > > +	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
> > > > > > +	if (!synusb->urb) {
> > > > > > +		error = -ENOMEM;
> > > > > > +		goto err_free_mem;
> > > > > > +	}
> > > > > > +
> > > > > > +	synusb->data = usb_alloc_coherent(udev,
> > > > > > SYNUSB_RECV_SIZE, GFP_KERNEL,
> > > > > > +
> > > > > > &synusb->urb->transfer_dma);
> > > > > > +	if (!synusb->data) {
> > > > > > +		error = -ENOMEM;
> > > > > > +		goto err_free_urb;
> > > > > > +	}
> > > > > > +
> > > > > > +	usb_fill_int_urb(synusb->urb, udev,
> > > > > > +			 usb_rcvintpipe(udev,
> > > > > > ep->bEndpointAddress),
> > > > > > +			 synusb->data, SYNUSB_RECV_SIZE,
> > > > > > +			 synusb_irq, synusb,
> > > > > > +			 ep->bInterval);
> > > > > > +	synusb->urb->transfer_flags |=
> > > > > > URB_NO_TRANSFER_DMA_MAP;
> > > > > 
> > > > > According to the comment in the original driver you must submit
> > > > > the URB. Are you sure not doing so is save?
> > > > 
> > > > Seems to work here...
> > > 
> > > Let me comment on this issue in more detail, so you can better judge
> > > what is the "right" solution here.
> > > 
> > > Both ways actually work. But, at least for the cPad, the device will
> > > pretend it got reconnected if the int urb is not fetched (while not
> > > suspended, of course). This means it would disconnect if no input
> > > device is opened (AFAIK most X.org input drivers close the devices
> > > when one switches to console) and one touches the pad (actually, a
> > > slight breeze of air might be enough, it is very sensitive!).
> > > Another annoying side effect in the case of the cPad is that the
> > > background light flashes on every reconnect.
> > 
> > OK, then maybe we should have cPad start transfers immediately and let
> > other devices do it in open. There is no reason to do work if nobody
> > interested in it...
> 
> Given that the devices are very similar, I expect that also the other
> devices may behave like this. So this is maybe not only a cPad
> related problem. But I have never investigated this for other devices,
> so I am not sure.
> 
> Regarding the extra amount of work, I thought it is finally less
> expensive to run through the callback and drop the 8 bytes of data,
> than to let the device trigger a disconnect, probe, and associated
> device deletion/creation by udev every 60 sec or so (it maybe even
> produces more USB traffic).

Right, we do not want to go through connect/disconnect cycle if we
can help it.

> 
> > Another option is to move configuring interface to open()?
> 
> The reconnect will always happen if the int endpoint is not "in use". So
> configuring the device not in probe will not help, will it?
> 
> I think the best way would be to immediately suspend the device, if
> there is an easy way to trigger this using the driver or usb core. (And
> if there is no easy way, it is still possible to do it via sysfs if
> someone is annoyed by this minor "reconnect bug".) Further, this should
> be fine for all devices, whether they have this "reconnect bug" or not.

Unfortunately forcing suspend requires kernel to be compiled with
suspend/autosuspend support which we can't guarantee. So I still decided
to go with a device flag instructing driver to start IO immediately.

The patch below seems to work well on my combo device (Lenovo keyboard
with integrated touchpad/trackpoint). Could you please tell me if it
works with your devices as well?

Thanks.

-- 
Dmitry

Input: add Synaptics USB device driver

From: Jan Steinhoff <mail@jan-steinhoff.de>

This patch adds a driver for Synaptics USB touchpad or pointing stick
devices. These USB devices emulate an USB mouse by default, so one can
also use the usbhid driver. However, in combination with special user
space drivers this kernel driver allows one to customize the behaviour
of the device.

An extended version of this driver with support for the cPad background
display can be found at
<http://jan-steinhoff.de/linux/synaptics-usb.html>.

Signed-off-by: Jan Steinhoff <mail@jan-steinhoff.de>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/hid/hid-core.c              |   10 +
 drivers/hid/hid-ids.h               |   11 +
 drivers/input/mouse/Kconfig         |   16 +
 drivers/input/mouse/Makefile        |    1 
 drivers/input/mouse/synaptics_usb.c |  589 +++++++++++++++++++++++++++++++++++
 5 files changed, 627 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/mouse/synaptics_usb.c


diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index af35384..221904f 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1892,6 +1892,16 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_PANJIT, 0x0004) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_PHILIPS, USB_DEVICE_ID_PHILIPS_IEEE802154_DONGLE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_POWERCOM, USB_DEVICE_ID_POWERCOM_UPS) },
+#if defined(CONFIG_MOUSE_SYNAPTICS_USB) || defined(CONFIG_MOUSE_SYNAPTICS_USB_MODULE)
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_TP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_INT_TP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_CPAD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_STICK) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_WP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_COMP_TP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_WTP) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SYNAPTICS, USB_DEVICE_ID_SYNAPTICS_DPAD) },
+#endif
 	{ HID_USB_DEVICE(USB_VENDOR_ID_VERNIER, USB_DEVICE_ID_VERNIER_LABPRO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_VERNIER, USB_DEVICE_ID_VERNIER_GOTEMP) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_VERNIER, USB_DEVICE_ID_VERNIER_SKIP) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4a441a6..b48a64b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -633,6 +633,17 @@
 #define USB_DEVICE_ID_SYMBOL_SCANNER_1	0x0800
 #define USB_DEVICE_ID_SYMBOL_SCANNER_2	0x1300
 
+#define USB_VENDOR_ID_SYNAPTICS		0x06cb
+#define USB_DEVICE_ID_SYNAPTICS_TP	0x0001
+#define USB_DEVICE_ID_SYNAPTICS_INT_TP	0x0002
+#define USB_DEVICE_ID_SYNAPTICS_CPAD	0x0003
+#define USB_DEVICE_ID_SYNAPTICS_TS	0x0006
+#define USB_DEVICE_ID_SYNAPTICS_STICK	0x0007
+#define USB_DEVICE_ID_SYNAPTICS_WP	0x0008
+#define USB_DEVICE_ID_SYNAPTICS_COMP_TP	0x0009
+#define USB_DEVICE_ID_SYNAPTICS_WTP	0x0010
+#define USB_DEVICE_ID_SYNAPTICS_DPAD	0x0013
+
 #define USB_VENDOR_ID_THRUSTMASTER	0x044f
 
 #define USB_VENDOR_ID_TOPSEED		0x0766
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 9c1e6ee..9df84b9 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -322,4 +322,20 @@ config MOUSE_SYNAPTICS_I2C
 	  To compile this driver as a module, choose M here: the
 	  module will be called synaptics_i2c.
 
+config MOUSE_SYNAPTICS_USB
+	tristate "Synaptics USB device support"
+	depends on USB
+	help
+	  Say Y here if you want to use a Synaptics USB touchpad or pointing
+	  stick.
+
+	  While these devices emulate an USB mouse by default and can be used
+	  with standard usbhid driver, this driver, together with its X.Org
+	  counterpart, allows you to fully utilize capabilities of the device.
+	  More information can be found at:
+	  <http://jan-steinhoff.de/linux/synaptics-usb.html>
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called synaptics_usb.
+
 endif
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 570c84a4..4718eff 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_MOUSE_PXA930_TRKBALL)	+= pxa930_trkball.o
 obj-$(CONFIG_MOUSE_RISCPC)		+= rpcmouse.o
 obj-$(CONFIG_MOUSE_SERIAL)		+= sermouse.o
 obj-$(CONFIG_MOUSE_SYNAPTICS_I2C)	+= synaptics_i2c.o
+obj-$(CONFIG_MOUSE_SYNAPTICS_USB)	+= synaptics_usb.o
 obj-$(CONFIG_MOUSE_VSXXXAA)		+= vsxxxaa.o
 
 psmouse-objs := psmouse-base.o synaptics.o
diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
new file mode 100644
index 0000000..216cf2b
--- /dev/null
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -0,0 +1,589 @@
+/*
+ * USB Synaptics device driver
+ *
+ *  Copyright (c) 2002 Rob Miller (rob@inpharmatica . co . uk)
+ *  Copyright (c) 2003 Ron Lee (ron@debian.org)
+ *	cPad driver for kernel 2.4
+ *
+ *  Copyright (c) 2004 Jan Steinhoff (cpad@jan-steinhoff . de)
+ *  Copyright (c) 2004 Ron Lee (ron@debian.org)
+ *	rewritten for kernel 2.6
+ *
+ *  cPad display character device part is not included. It can be found at
+ *  http://jan-steinhoff.de/linux/synaptics-usb.html
+ *
+ * Bases on:	usb_skeleton.c v2.2 by Greg Kroah-Hartman
+ *		drivers/hid/usbhid/usbmouse.c by Vojtech Pavlik
+ *		drivers/input/mouse/synaptics.c by Peter Osterlund
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * Trademarks are the property of their respective owners.
+ */
+
+/*
+ * There are three different types of Synaptics USB devices: Touchpads,
+ * touchsticks (or trackpoints), and touchscreens. Touchpads are well supported
+ * by this driver, touchstick support has not been tested much yet, and
+ * touchscreens have not been tested at all.
+ *
+ * Up to three alternate settings are possible:
+ *	setting 0: one int endpoint for relative movement (used by usbhid.ko)
+ *	setting 1: one int endpoint for absolute finger position
+ *	setting 2 (cPad only): one int endpoint for absolute finger position and
+ *		   two bulk endpoints for the display (in/out)
+ * This driver uses setting 1.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/usb.h>
+#include <linux/input.h>
+#include <linux/usb/input.h>
+
+#define USB_VENDOR_ID_SYNAPTICS	0x06cb
+#define USB_DEVICE_ID_SYNAPTICS_TP	0x0001	/* Synaptics USB TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_INT_TP	0x0002	/* Integrated USB TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_CPAD	0x0003	/* Synaptics cPad */
+#define USB_DEVICE_ID_SYNAPTICS_TS	0x0006	/* Synaptics TouchScreen */
+#define USB_DEVICE_ID_SYNAPTICS_STICK	0x0007	/* Synaptics USB Styk */
+#define USB_DEVICE_ID_SYNAPTICS_WP	0x0008	/* Synaptics USB WheelPad */
+#define USB_DEVICE_ID_SYNAPTICS_COMP_TP	0x0009	/* Composite USB TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_WTP	0x0010	/* Wireless TouchPad */
+#define USB_DEVICE_ID_SYNAPTICS_DPAD	0x0013	/* DisplayPad */
+
+#define SYNUSB_TOUCHPAD			(1 << 0)
+#define SYNUSB_STICK			(1 << 1)
+#define SYNUSB_TOUCHSCREEN		(1 << 2)
+#define SYNUSB_AUXDISPLAY		(1 << 3) /* For cPad */
+#define SYNUSB_COMBO			(1 << 4) /* Composite device (TP + stick) */
+#define SYNUSB_IO_ALWAYS		(1 << 5)
+
+#define USB_DEVICE_SYNAPTICS(prod, kind)		\
+	USB_DEVICE(USB_VENDOR_ID_SYNAPTICS,		\
+		   USB_DEVICE_ID_SYNAPTICS_##prod),	\
+	.driver_info = (kind),
+
+#define SYNUSB_RECV_SIZE	8
+
+#define XMIN_NOMINAL		1472
+#define XMAX_NOMINAL		5472
+#define YMIN_NOMINAL		1408
+#define YMAX_NOMINAL		4448
+
+struct synusb {
+	struct usb_device *udev;
+	struct usb_interface *intf;
+	struct urb *urb;
+	unsigned char *data;
+
+	/* input device related data structures */
+	struct input_dev *input;
+	char name[128];
+	char phys[64];
+
+	/* characteristics of the device */
+	unsigned long flags;
+};
+
+static void synusb_report_buttons(struct synusb *synusb)
+{
+	struct input_dev *input_dev = synusb->input;
+
+	input_report_key(input_dev, BTN_LEFT, synusb->data[1] & 0x04);
+	input_report_key(input_dev, BTN_RIGHT, synusb->data[1] & 0x01);
+	input_report_key(input_dev, BTN_MIDDLE, synusb->data[1] & 0x02);
+}
+
+static void synusb_report_stick(struct synusb *synusb)
+{
+	struct input_dev *input_dev = synusb->input;
+	int x, y;
+	unsigned int pressure;
+
+	pressure = synusb->data[6];
+	x = (s16)(be16_to_cpup((__be16 *)&synusb->data[2]) << 3) >> 7;
+	y = (s16)(be16_to_cpup((__be16 *)&synusb->data[4]) << 3) >> 7;
+
+	if (pressure > 0) {
+		input_report_rel(input_dev, REL_X, x);
+		input_report_rel(input_dev, REL_Y, -y);
+	}
+
+	input_report_abs(input_dev, ABS_PRESSURE, pressure);
+
+	synusb_report_buttons(synusb);
+
+	input_sync(input_dev);
+}
+
+static void synusb_report_touchpad(struct synusb *synusb)
+{
+	struct input_dev *input_dev = synusb->input;
+	unsigned int num_fingers, tool_width;
+	unsigned int x, y;
+	unsigned int pressure, w;
+
+	pressure = synusb->data[6];
+	x = be16_to_cpup((__be16 *)&synusb->data[2]);
+	y = be16_to_cpup((__be16 *)&synusb->data[4]);
+	w = synusb->data[0] & 0x0f;
+
+	if (pressure > 0) {
+		num_fingers = 1;
+		tool_width = 5;
+		switch (w) {
+		case 0 ... 1:
+			num_fingers = 2 + w;
+			break;
+
+		case 2:	                /* pen, pretend its a finger */
+			break;
+
+		case 4 ... 15:
+			tool_width = w;
+			break;
+		}
+	} else {
+		num_fingers = 0;
+		tool_width = 0;
+	}
+
+	/*
+	 * Post events
+	 * BTN_TOUCH has to be first as mousedev relies on it when doing
+	 * absolute -> relative conversion
+	 */
+
+	if (pressure > 30)
+		input_report_key(input_dev, BTN_TOUCH, 1);
+	if (pressure < 25)
+		input_report_key(input_dev, BTN_TOUCH, 0);
+
+	if (num_fingers > 0) {
+		input_report_abs(input_dev, ABS_X, x);
+		input_report_abs(input_dev, ABS_Y,
+				 YMAX_NOMINAL + YMIN_NOMINAL - y);
+	}
+
+	input_report_abs(input_dev, ABS_PRESSURE, pressure);
+	input_report_abs(input_dev, ABS_TOOL_WIDTH, tool_width);
+
+	input_report_key(input_dev, BTN_TOOL_FINGER, num_fingers == 1);
+	input_report_key(input_dev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
+	input_report_key(input_dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);
+
+	synusb_report_buttons(synusb);
+	if (synusb->flags & SYNUSB_AUXDISPLAY)
+		input_report_key(input_dev, BTN_MIDDLE, synusb->data[1] & 0x08);
+
+	input_sync(input_dev);
+}
+
+static void synusb_irq(struct urb *urb)
+{
+	struct synusb *synusb = urb->context;
+	int error;
+
+	/* Check our status in case we need to bail out early. */
+	switch (urb->status) {
+	case 0:
+		usb_mark_last_busy(synusb->udev);
+		break;
+
+	/* Device went away so don't keep trying to read from it. */
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		return;
+
+	default:
+		goto resubmit;
+		break;
+	}
+
+	if (synusb->flags & SYNUSB_STICK)
+		synusb_report_stick(synusb);
+	else
+		synusb_report_touchpad(synusb);
+
+resubmit:
+	error = usb_submit_urb(urb, GFP_ATOMIC);
+	if (error && error != -EPERM)
+		dev_err(&synusb->intf->dev,
+			"%s - usb_submit_urb failed with result: %d",
+			__func__, error);
+}
+
+static struct usb_endpoint_descriptor *
+synusb_get_in_endpoint(struct usb_host_interface *iface)
+{
+
+	struct usb_endpoint_descriptor *endpoint;
+	int i;
+
+	for (i = 0; i < iface->desc.bNumEndpoints; ++i) {
+		endpoint = &iface->endpoint[i].desc;
+
+		if (usb_endpoint_is_int_in(endpoint)) {
+			/* we found our interrupt in endpoint */
+			return endpoint;
+		}
+	}
+
+	return NULL;
+}
+
+static int synusb_open(struct input_dev *dev)
+{
+	struct synusb *synusb = input_get_drvdata(dev);
+	int retval;
+
+	if (synusb->flags & SYNUSB_IO_ALWAYS) {
+		/* We already started IO in synusb_probe() */
+		return 0;
+	}
+
+	retval = usb_autopm_get_interface(synusb->intf);
+	if (retval) {
+		dev_err(&synusb->intf->dev,
+			"%s - usb_autopm_get_interface failed, error: %d\n",
+			__func__, retval);
+		return retval;
+	}
+
+	retval = usb_submit_urb(synusb->urb, GFP_KERNEL);
+	if (retval) {
+		dev_err(&synusb->intf->dev,
+			"%s - usb_submit_urb failed, error: %d\n",
+			__func__, retval);
+		retval = -EIO;
+		goto out;
+	}
+
+	synusb->intf->needs_remote_wakeup = 1;
+
+out:
+	usb_autopm_put_interface(synusb->intf);
+	return retval;
+}
+
+static void synusb_close(struct input_dev *dev)
+{
+	struct synusb *synusb = input_get_drvdata(dev);
+
+	if (!(synusb->flags & SYNUSB_IO_ALWAYS)) {
+		int autopm_error = usb_autopm_get_interface(synusb->intf);
+
+		usb_kill_urb(synusb->urb);
+		synusb->intf->needs_remote_wakeup = 0;
+
+		if (!autopm_error)
+			usb_autopm_put_interface(synusb->intf);
+	}
+}
+
+static int synusb_probe(struct usb_interface *intf,
+			const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(intf);
+	struct usb_endpoint_descriptor *ep;
+	struct synusb *synusb;
+	struct input_dev *input_dev;
+	unsigned int intf_num = intf->cur_altsetting->desc.bInterfaceNumber;
+	unsigned int altsetting = min(intf->num_altsetting, 1U);
+	int error;
+
+	error = usb_set_interface(udev, intf_num, altsetting);
+	if (error) {
+		dev_err(&udev->dev,
+			"Can not set alternate setting to %i, error: %i",
+			altsetting, error);
+		return error;
+	}
+
+	ep = synusb_get_in_endpoint(intf->cur_altsetting);
+	if (!ep)
+		return -ENODEV;
+
+	synusb = kzalloc(sizeof(*synusb), GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!synusb || !input_dev) {
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	synusb->udev = udev;
+	synusb->intf = intf;
+	synusb->input = input_dev;
+
+	synusb->flags = id->driver_info;
+	if (synusb->flags & SYNUSB_COMBO) {
+		/*
+		 * This is a combo device, we need to set proper
+		 * capability, depending on the interface.
+		 */
+		synusb->flags |= intf_num == 1 ?
+					SYNUSB_STICK : SYNUSB_TOUCHPAD;
+	}
+
+	synusb->urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!synusb->urb) {
+		error = -ENOMEM;
+		goto err_free_mem;
+	}
+
+	synusb->data = usb_alloc_coherent(udev, SYNUSB_RECV_SIZE, GFP_KERNEL,
+					  &synusb->urb->transfer_dma);
+	if (!synusb->data) {
+		error = -ENOMEM;
+		goto err_free_urb;
+	}
+
+	usb_fill_int_urb(synusb->urb, udev,
+			 usb_rcvintpipe(udev, ep->bEndpointAddress),
+			 synusb->data, SYNUSB_RECV_SIZE,
+			 synusb_irq, synusb,
+			 ep->bInterval);
+	synusb->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+	if (udev->manufacturer)
+		strlcpy(synusb->name, udev->manufacturer,
+			sizeof(synusb->name));
+
+	if (udev->product) {
+		if (udev->manufacturer)
+			strlcat(synusb->name, " ", sizeof(synusb->name));
+		strlcat(synusb->name, udev->product, sizeof(synusb->name));
+	}
+
+	if (!strlen(synusb->name))
+		snprintf(synusb->name, sizeof(synusb->name),
+			 "USB Synaptics Device %04x:%04x",
+			 le16_to_cpu(udev->descriptor.idVendor),
+			 le16_to_cpu(udev->descriptor.idProduct));
+
+	if (synusb->flags & SYNUSB_STICK)
+		strlcat(synusb->name, " (Stick) ", sizeof(synusb->name));
+
+	usb_make_path(udev, synusb->phys, sizeof(synusb->phys));
+	strlcat(synusb->phys, "/input0", sizeof(synusb->phys));
+
+	input_dev->name = synusb->name;
+	input_dev->phys = synusb->phys;
+	usb_to_input_id(udev, &input_dev->id);
+	input_dev->dev.parent = &synusb->intf->dev;
+
+	input_dev->open = synusb_open;
+	input_dev->close = synusb_close;
+
+	input_set_drvdata(input_dev, synusb);
+
+	__set_bit(EV_ABS, input_dev->evbit);
+	__set_bit(EV_KEY, input_dev->evbit);
+
+	if (synusb->flags & SYNUSB_STICK) {
+		__set_bit(EV_REL, input_dev->evbit);
+		__set_bit(REL_X, input_dev->relbit);
+		__set_bit(REL_Y, input_dev->relbit);
+		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 127, 0, 0);
+	} else {
+		input_set_abs_params(input_dev, ABS_X,
+				     XMIN_NOMINAL, XMAX_NOMINAL, 0, 0);
+		input_set_abs_params(input_dev, ABS_Y,
+				     YMIN_NOMINAL, YMAX_NOMINAL, 0, 0);
+		input_set_abs_params(input_dev, ABS_PRESSURE, 0, 255, 0, 0);
+		input_set_abs_params(input_dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
+		__set_bit(BTN_TOUCH, input_dev->keybit);
+		__set_bit(BTN_TOOL_FINGER, input_dev->keybit);
+		__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
+		__set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit);
+	}
+
+	__set_bit(BTN_LEFT, input_dev->keybit);
+	__set_bit(BTN_RIGHT, input_dev->keybit);
+	__set_bit(BTN_MIDDLE, input_dev->keybit);
+
+	usb_set_intfdata(intf, synusb);
+
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(&udev->dev,
+			"Failed to register input device, error %d\n",
+			error);
+		goto err_free_dma;
+	}
+
+	if (synusb->flags & SYNUSB_IO_ALWAYS) {
+		error = usb_autopm_get_interface(synusb->intf);
+		if (error) {
+			dev_err(&udev->dev,
+				"%s - usb_autopm_get_interface failed, error: %d\n",
+				__func__, error);
+			goto err_unregister_input;
+		}
+
+		error = usb_submit_urb(synusb->urb, GFP_KERNEL);
+		if (error) {
+			dev_err(&synusb->intf->dev,
+				"%s - usb_submit_urb failed, error: %d\n",
+				__func__, error);
+			error = -EIO;
+			goto err_put_intf;
+		}
+	}
+
+	return 0;
+
+err_put_intf:
+	usb_autopm_put_interface(synusb->intf);
+err_unregister_input:
+	input_unregister_device(input_dev);
+	input_dev = NULL;
+err_free_dma:
+	usb_free_coherent(udev, SYNUSB_RECV_SIZE, synusb->data,
+			  synusb->urb->transfer_dma);
+err_free_urb:
+	usb_free_urb(synusb->urb);
+err_free_mem:
+	input_free_device(input_dev);
+	kfree(synusb);
+	usb_set_intfdata(intf, NULL);
+
+	return error;
+}
+
+static void synusb_disconnect(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct usb_device *udev = interface_to_usbdev(intf);
+
+	if (synusb->flags & SYNUSB_IO_ALWAYS) {
+		usb_kill_urb(synusb->urb);
+		usb_autopm_put_interface(synusb->intf);
+	}
+
+	input_unregister_device(synusb->input);
+
+	usb_free_coherent(udev, SYNUSB_RECV_SIZE, synusb->data,
+			  synusb->urb->transfer_dma);
+	usb_free_urb(synusb->urb);
+	kfree(synusb);
+
+	usb_set_intfdata(intf, NULL);
+}
+
+static int synusb_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct input_dev *input_dev = synusb->input;
+
+	mutex_lock(&input_dev->mutex);
+	usb_kill_urb(synusb->urb);
+	mutex_unlock(&input_dev->mutex);
+
+	return 0;
+}
+
+static int synusb_resume(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct input_dev *input_dev = synusb->input;
+	int retval = 0;
+
+	mutex_lock(&input_dev->mutex);
+
+	if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
+	    usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
+		retval = -EIO;
+	}
+
+	mutex_unlock(&input_dev->mutex);
+
+	return retval;
+}
+
+static int synusb_pre_reset(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct input_dev *input_dev = synusb->input;
+
+	mutex_lock(&input_dev->mutex);
+	usb_kill_urb(synusb->urb);
+
+	return 0;
+}
+
+static int synusb_post_reset(struct usb_interface *intf)
+{
+	struct synusb *synusb = usb_get_intfdata(intf);
+	struct input_dev *input_dev = synusb->input;
+	int retval = 0;
+
+	if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
+	    usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
+		retval = -EIO;
+	}
+
+	mutex_unlock(&input_dev->mutex);
+
+	return retval;
+}
+
+static int synusb_reset_resume(struct usb_interface *intf)
+{
+	return synusb_resume(intf);
+}
+
+static struct usb_device_id synusb_idtable[] = {
+	{ USB_DEVICE_SYNAPTICS(TP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(INT_TP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(CPAD,
+		SYNUSB_TOUCHPAD | SYNUSB_AUXDISPLAY | SYNUSB_IO_ALWAYS) },
+	{ USB_DEVICE_SYNAPTICS(TS, SYNUSB_TOUCHSCREEN) },
+	{ USB_DEVICE_SYNAPTICS(STICK, SYNUSB_STICK) },
+	{ USB_DEVICE_SYNAPTICS(WP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(COMP_TP, SYNUSB_COMBO) },
+	{ USB_DEVICE_SYNAPTICS(WTP, SYNUSB_TOUCHPAD) },
+	{ USB_DEVICE_SYNAPTICS(DPAD, SYNUSB_TOUCHPAD) },
+	{ }
+};
+MODULE_DEVICE_TABLE(usb, synusb_idtable);
+
+static struct usb_driver synusb_driver = {
+	.name		= "synaptics_usb",
+	.probe		= synusb_probe,
+	.disconnect	= synusb_disconnect,
+	.id_table	= synusb_idtable,
+	.suspend	= synusb_suspend,
+	.resume		= synusb_resume,
+	.pre_reset	= synusb_pre_reset,
+	.post_reset	= synusb_post_reset,
+	.reset_resume	= synusb_reset_resume,
+	.supports_autosuspend = 1,
+};
+
+static int __init synusb_init(void)
+{
+	return usb_register(&synusb_driver);
+}
+
+static void __exit synusb_exit(void)
+{
+	usb_deregister(&synusb_driver);
+}
+
+module_init(synusb_init);
+module_exit(synusb_exit);
+
+MODULE_AUTHOR("Rob Miller <rob@inpharmatica.co.uk>, "
+              "Ron Lee <ron@debian.org>, "
+              "Jan Steinhoff <cpad@jan-steinhoff.de>");
+MODULE_DESCRIPTION("Synaptics USB device driver");
+MODULE_LICENSE("GPL");

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-10  9:43                 ` Dmitry Torokhov
@ 2012-01-12  0:08                   ` Jan Steinhoff
  2012-01-18  5:25                     ` Dmitry Torokhov
  0 siblings, 1 reply; 16+ messages in thread
From: Jan Steinhoff @ 2012-01-12  0:08 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Tue, 10 Jan 2012 01:43:34 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> The patch below seems to work well on my combo device (Lenovo keyboard
> with integrated touchpad/trackpoint). Could you please tell me if it
> works with your devices as well?

Yes, it works. Thanks a lot for the cleanup and improvement!

Just a last remark: The reconnects do not appear while the cPad is
suspended, so it is save to allow it to autosuspend. Can the autopm
calls be removed from probe and disconnect, and needs_remote_wakeup be
set in probe instead?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] input: Synaptics USB device driver
  2012-01-12  0:08                   ` Jan Steinhoff
@ 2012-01-18  5:25                     ` Dmitry Torokhov
  0 siblings, 0 replies; 16+ messages in thread
From: Dmitry Torokhov @ 2012-01-18  5:25 UTC (permalink / raw)
  To: Jan Steinhoff
  Cc: Oliver Neukum, Jiri Kosina, Alessandro Rubini, linux-input,
	linux-usb, linux-kernel

On Thu, Jan 12, 2012 at 12:08:48AM +0000, Jan Steinhoff wrote:
> On Tue, 10 Jan 2012 01:43:34 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > The patch below seems to work well on my combo device (Lenovo keyboard
> > with integrated touchpad/trackpoint). Could you please tell me if it
> > works with your devices as well?
> 
> Yes, it works. Thanks a lot for the cleanup and improvement!
> 
> Just a last remark: The reconnects do not appear while the cPad is
> suspended, so it is save to allow it to autosuspend. Can the autopm
> calls be removed from probe and disconnect, and needs_remote_wakeup be
> set in probe instead?

OK, how about thie patch below then (on top of the previous one)?

Thanks.

-- 
Dmitry


Input: synaptics-use - rework starting of cPad-like devices

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/mouse/synaptics_usb.c |   65 ++++++++++++-----------------------
 1 files changed, 22 insertions(+), 43 deletions(-)


diff --git a/drivers/input/mouse/synaptics_usb.c b/drivers/input/mouse/synaptics_usb.c
index 216cf2b..e559a94 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -245,11 +245,6 @@ static int synusb_open(struct input_dev *dev)
 	struct synusb *synusb = input_get_drvdata(dev);
 	int retval;
 
-	if (synusb->flags & SYNUSB_IO_ALWAYS) {
-		/* We already started IO in synusb_probe() */
-		return 0;
-	}
-
 	retval = usb_autopm_get_interface(synusb->intf);
 	if (retval) {
 		dev_err(&synusb->intf->dev,
@@ -277,16 +272,15 @@ out:
 static void synusb_close(struct input_dev *dev)
 {
 	struct synusb *synusb = input_get_drvdata(dev);
+	int autopm_error;
 
-	if (!(synusb->flags & SYNUSB_IO_ALWAYS)) {
-		int autopm_error = usb_autopm_get_interface(synusb->intf);
+	autopm_error = usb_autopm_get_interface(synusb->intf);
 
-		usb_kill_urb(synusb->urb);
-		synusb->intf->needs_remote_wakeup = 0;
+	usb_kill_urb(synusb->urb);
+	synusb->intf->needs_remote_wakeup = 0;
 
-		if (!autopm_error)
-			usb_autopm_put_interface(synusb->intf);
-	}
+	if (!autopm_error)
+		usb_autopm_put_interface(synusb->intf);
 }
 
 static int synusb_probe(struct usb_interface *intf,
@@ -380,8 +374,10 @@ static int synusb_probe(struct usb_interface *intf,
 	usb_to_input_id(udev, &input_dev->id);
 	input_dev->dev.parent = &synusb->intf->dev;
 
-	input_dev->open = synusb_open;
-	input_dev->close = synusb_close;
+	if (!(synusb->flags & SYNUSB_IO_ALWAYS)) {
+		input_dev->open = synusb_open;
+		input_dev->close = synusb_close;
+	}
 
 	input_set_drvdata(input_dev, synusb);
 
@@ -412,40 +408,25 @@ static int synusb_probe(struct usb_interface *intf,
 
 	usb_set_intfdata(intf, synusb);
 
+	if (synusb->flags & SYNUSB_IO_ALWAYS) {
+		error = synusb_open(input_dev);
+		if (error)
+			goto err_free_dma;
+	}
+
 	error = input_register_device(input_dev);
 	if (error) {
 		dev_err(&udev->dev,
 			"Failed to register input device, error %d\n",
 			error);
-		goto err_free_dma;
-	}
-
-	if (synusb->flags & SYNUSB_IO_ALWAYS) {
-		error = usb_autopm_get_interface(synusb->intf);
-		if (error) {
-			dev_err(&udev->dev,
-				"%s - usb_autopm_get_interface failed, error: %d\n",
-				__func__, error);
-			goto err_unregister_input;
-		}
-
-		error = usb_submit_urb(synusb->urb, GFP_KERNEL);
-		if (error) {
-			dev_err(&synusb->intf->dev,
-				"%s - usb_submit_urb failed, error: %d\n",
-				__func__, error);
-			error = -EIO;
-			goto err_put_intf;
-		}
+		goto err_stop_io;
 	}
 
 	return 0;
 
-err_put_intf:
-	usb_autopm_put_interface(synusb->intf);
-err_unregister_input:
-	input_unregister_device(input_dev);
-	input_dev = NULL;
+err_stop_io:
+	if (synusb->flags & SYNUSB_IO_ALWAYS)
+		synusb_close(synusb->input);
 err_free_dma:
 	usb_free_coherent(udev, SYNUSB_RECV_SIZE, synusb->data,
 			  synusb->urb->transfer_dma);
@@ -464,10 +445,8 @@ static void synusb_disconnect(struct usb_interface *intf)
 	struct synusb *synusb = usb_get_intfdata(intf);
 	struct usb_device *udev = interface_to_usbdev(intf);
 
-	if (synusb->flags & SYNUSB_IO_ALWAYS) {
-		usb_kill_urb(synusb->urb);
-		usb_autopm_put_interface(synusb->intf);
-	}
+	if (synusb->flags & SYNUSB_IO_ALWAYS)
+		synusb_close(synusb->input);
 
 	input_unregister_device(synusb->input);
 

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-01-18  5:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-03 18:40 [PATCH] input: Synaptics USB device driver Jan Steinhoff
2012-01-04  8:25 ` Oliver Neukum
2012-01-05  2:46   ` Jan Steinhoff
2012-01-04  8:55 ` Jiri Kosina
2012-01-04  9:20   ` Oliver Neukum
2012-01-04  9:41     ` Dmitry Torokhov
2012-01-04  9:56       ` Oliver Neukum
2012-01-04 10:05         ` Dmitry Torokhov
2012-01-05  6:01           ` Jan Steinhoff
2012-01-05  6:36             ` Dmitry Torokhov
2012-01-05 14:22               ` Jan Steinhoff
2012-01-10  9:43                 ` Dmitry Torokhov
2012-01-12  0:08                   ` Jan Steinhoff
2012-01-18  5:25                     ` Dmitry Torokhov
2012-01-05  5:39       ` Jan Steinhoff
2012-01-05  6:34         ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).