[PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Henrik Rydberg]

BCM5974: This driver adds support for the multitouch trackpad on the new
Apple Macbook Air and Macbook Pro Penryn laptops. It replaces the
appletouch driver on those computers, and integrates well with the
synaptics driver of the Xorg system.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---

This is version bcm5974-0.58. Changes since 0.57 are in short:

  * Name changes; use usbhid definitions, rename atp to bcm5974
  * Input syncing moved to report functions, event type setup broken out
  * Traffic functions added for open/close and suspend/resume logic
  * Open/close and suspend/resume transition logic corrected
  * Open/close serialized with respect to suspend/resume

Thanks to Dmitry Torokhov for reviewing and clearing up the resume functionality,
and Alan Stern for the resume error state handling.

This patch is against linux-next v2.6.26-9281-gb475aa5.

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 7bbea09..956cc4f 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -130,6 +130,29 @@ config MOUSE_APPLETOUCH
 	  To compile this driver as a module, choose M here: the
 	  module will be called appletouch.

+config MOUSE_BCM5974
+	tristate "Apple USB BCM5974 Multitouch trackpad support"
+	depends on USB_ARCH_HAS_HCD
+	depends on USB
+	help
+	  Say Y here if you have an Apple USB BCM5974 Multitouch
+	  trackpad.
+	
+	  The BCM5974 is the multitouch trackpad found in the Macbook
+	  Air (JAN2008) and Macbook Pro Penryn (FEB2008) laptops.
+	
+	  It is also found in the IPhone (2007) and Ipod Touch (2008).
+	
+	  This driver provides multitouch functionality together with
+	  the synaptics X11 driver.
+	
+	  The interface is currently identical to the appletouch interface,
+	  for further information, see
+	  <file:Documentation/input/appletouch.txt>.
+	
+	  To compile this driver as a module, choose M here: the
+	  module will be called bcm5974.
+
 config MOUSE_INPORT
 	tristate "InPort/MS/ATIXL busmouse"
 	depends on ISA
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index 9e6e363..d4d2025 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -6,6 +6,7 @@

 obj-$(CONFIG_MOUSE_AMIGA)	+= amimouse.o
 obj-$(CONFIG_MOUSE_APPLETOUCH)	+= appletouch.o
+obj-$(CONFIG_MOUSE_BCM5974)	+= bcm5974.o
 obj-$(CONFIG_MOUSE_ATARI)	+= atarimouse.o
 obj-$(CONFIG_MOUSE_RISCPC)	+= rpcmouse.o
 obj-$(CONFIG_MOUSE_INPORT)	+= inport.o
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
new file mode 100644
index 0000000..55c1f60
--- /dev/null
+++ b/drivers/input/mouse/bcm5974.c
@@ -0,0 +1,673 @@
+/*
+ * Apple USB BCM5974 (Macbook Air and Penryn Macbook Pro) multitouch driver
+ *
+ * Copyright (C) 2008	   Henrik Rydberg (rydberg@euromail.se)
+ *
+ * The USB initialization and package decoding was made by
+ * Scott Shawcroft as part of the touchd user-space driver project:
+ * Copyright (C) 2008	   Scott Shawcroft (scott.shawcroft@gmail.com)
+ *
+ * The BCM5974 driver is based on the appletouch driver:
+ * Copyright (C) 2001-2004 Greg Kroah-Hartman (greg@kroah.com)
+ * Copyright (C) 2005      Johannes Berg (johannes@sipsolutions.net)
+ * Copyright (C) 2005	   Stelian Pop (stelian@popies.net)
+ * Copyright (C) 2005	   Frank Arnold (frank@scirocco-5v-turbo.de)
+ * Copyright (C) 2005	   Peter Osterlund (petero2@telia.com)
+ * Copyright (C) 2005	   Michael Hanselmann (linux-kernel@hansmi.ch)
+ * Copyright (C) 2006	   Nicolas Boichat (nicolas@boichat.ch)
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/usb/input.h>
+#include <linux/hid.h>
+#include <linux/mutex.h>
+
+#define USB_VENDOR_ID_APPLE		0x05ac
+
+/* MacbookAir, aka wellspring */
+#define USB_DEVICE_ID_APPLE_WELLSPRING_ANSI	0x0223
+#define USB_DEVICE_ID_APPLE_WELLSPRING_ISO	0x0224
+#define USB_DEVICE_ID_APPLE_WELLSPRING_JIS	0x0225
+/* MacbookProPenryn, aka wellspring2 */
+#define USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI	0x0230
+#define USB_DEVICE_ID_APPLE_WELLSPRING2_ISO	0x0231
+#define USB_DEVICE_ID_APPLE_WELLSPRING2_JIS	0x0232
+
+#define BCM5974_DEVICE(prod) {					\
+	.match_flags = (USB_DEVICE_ID_MATCH_DEVICE |		\
+			USB_DEVICE_ID_MATCH_INT_CLASS |		\
+			USB_DEVICE_ID_MATCH_INT_PROTOCOL),	\
+	.idVendor = USB_VENDOR_ID_APPLE,			\
+	.idProduct = (prod),					\
+	.bInterfaceClass = USB_INTERFACE_CLASS_HID,		\
+	.bInterfaceProtocol = USB_INTERFACE_PROTOCOL_MOUSE	\
+}
+
+/* table of devices that work with this driver */
+static const struct usb_device_id bcm5974_table [] = {
+	/* MacbookAir1.1 */
+	BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING_ANSI),
+	BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING_ISO),
+	BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING_JIS),
+	/* MacbookProPenryn */
+	BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI),
+	BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING2_ISO),
+	BCM5974_DEVICE(USB_DEVICE_ID_APPLE_WELLSPRING2_JIS),
+	/* Terminating entry */
+	{}
+};
+MODULE_DEVICE_TABLE(usb, bcm5974_table);
+
+MODULE_AUTHOR("Henrik Rydberg");
+MODULE_DESCRIPTION("Apple USB BCM5974 multitouch driver");
+MODULE_LICENSE("GPL");
+
+#define dprintk(level, format, a...)\
+	{ if (debug >= level) printk(KERN_DEBUG format, ##a); }
+
+static int debug = 1;
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "Activate debugging output");
+
+/* button data structure */
+struct bt_data {
+	u8 unknown1;		/* constant */
+	u8 button;		/* left button */
+	u8 rel_x;		/* relative x coordinate */
+	u8 rel_y;		/* relative y coordinate */
+};
+
+/* trackpad header structure */
+struct tp_header {
+	u8 unknown1[16];	/* constants, timers, etc */
+	u8 fingers;		/* number of fingers on trackpad */
+	u8 unknown2[9];		/* constants, timers, etc */
+};
+
+/* trackpad finger structure */
+struct tp_finger {
+	__le16 origin;		/* left/right origin? */
+	__le16 abs_x;		/* absolute x coodinate */
+	__le16 abs_y;		/* absolute y coodinate */
+	__le16 rel_x;		/* relative x coodinate */
+	__le16 rel_y;		/* relative y coodinate */
+	__le16 size_major;	/* finger size, major axis? */
+	__le16 size_minor;	/* finger size, minor axis? */
+	__le16 orientation;	/* 16384 when point, else 15 bit angle */
+	__le16 force_major;	/* trackpad force, major axis? */
+	__le16 force_minor;	/* trackpad force, minor axis? */
+	__le16 unused[3];	/* zeros */
+	__le16 multi;		/* one finger: varies, more fingers: constant */
+};
+
+/* trackpad data structure, empirically at least ten fingers */
+struct tp_data {
+	struct tp_header header;
+	struct tp_finger finger[16];
+};
+
+/* device-specific parameters */
+struct bcm5974_param {
+	int dim;		/* logical dimension */
+	int fuzz;		/* logical noise value */
+	int devmin;		/* device minimum reading */
+	int devmax;		/* device maximum reading */
+};
+
+/* device-specific configuration */
+struct bcm5974_config {
+	int ansi, iso, jis;	/* the product id of this device */
+	int bt_ep;		/* the endpoint of the button interface */
+	int bt_datalen;		/* data length of the button interface */
+	int tp_ep;		/* the endpoint of the trackpad interface */
+	int tp_datalen;		/* data length of the trackpad interface */
+	struct bcm5974_param p;	/* finger pressure limits */
+	struct bcm5974_param w;	/* finger width limits */
+	struct bcm5974_param x;	/* horizontal limits */
+	struct bcm5974_param y;	/* vertical limits */
+};
+
+/* logical device structure */
+struct bcm5974 {
+	char phys[64];
+	struct usb_device *udev;	/* usb device */
+	struct input_dev *input;	/* input dev */
+	struct bcm5974_config cfg;	/* device configuration */
+	struct mutex mutex;		/* serialize access to open/suspend */
+	int opened;			/* >0: opened, else closed */
+	int manually_suspended;		/* >0: manually suspended */
+	struct urb *bt_urb;		/* button usb request block */
+	struct bt_data *bt_data;	/* button transferred data */
+	struct urb *tp_urb;		/* trackpad usb request block */
+	struct tp_data *tp_data;	/* trackpad transferred data */
+};
+
+/* logical dimensions */
+#define DIM_PRESSURE	256		/* maximum finger pressure */
+#define DIM_WIDTH	16		/* maximum finger width */
+#define DIM_X		1280		/* maximum trackpad x value */
+#define DIM_Y		800		/* maximum trackpad y value */
+
+/* logical signal quality */
+#define SN_PRESSURE	45		/* pressure signal-to-noise ratio */
+#define SN_WIDTH	100		/* width signal-to-noise ratio */
+#define SN_COORD	250		/* coordinate signal-to-noise ratio */
+
+/* device constants */
+static const struct bcm5974_config bcm5974_config_table[] = {
+	{
+		USB_DEVICE_ID_APPLE_WELLSPRING_ANSI,
+		USB_DEVICE_ID_APPLE_WELLSPRING_ISO,
+		USB_DEVICE_ID_APPLE_WELLSPRING_JIS,
+		0x84, sizeof(struct bt_data),
+		0x81, sizeof(struct tp_data),
+		{ DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 256 },
+		{ DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
+		{ DIM_X, DIM_X / SN_COORD, -4824, 5342 },
+		{ DIM_Y, DIM_Y / SN_COORD, -172, 5820 }
+	},
+	{
+		USB_DEVICE_ID_APPLE_WELLSPRING2_ANSI,
+		USB_DEVICE_ID_APPLE_WELLSPRING2_ISO,
+		USB_DEVICE_ID_APPLE_WELLSPRING2_JIS,
+		0x84, sizeof(struct bt_data),
+		0x81, sizeof(struct tp_data),
+		{ DIM_PRESSURE, DIM_PRESSURE / SN_PRESSURE, 0, 256 },
+		{ DIM_WIDTH, DIM_WIDTH / SN_WIDTH, 0, 2048 },
+		{ DIM_X, DIM_X / SN_COORD, -4824, 4824 },
+		{ DIM_Y, DIM_Y / SN_COORD, -172, 4290 }
+	},
+	{}
+};
+
+/* return the device-specific configuration by device */
+static const struct bcm5974_config *bcm5974_get_config(struct usb_device *udev)
+{
+	u16 id = le16_to_cpu(udev->descriptor.idProduct);
+	const struct bcm5974_config *cfg;
+	for (cfg = bcm5974_config_table; cfg->ansi; ++cfg)
+		if (cfg->ansi == id || cfg->iso == id || cfg->jis == id)
+			return cfg;
+	return bcm5974_config_table;
+}
+
+/* convert 16-bit little endian to signed integer */
+static inline int raw2int(__le16 x)
+{
+	return (signed short)le16_to_cpu(x);
+}
+
+/* scale device data to logical dimensions (asserts devmin < devmax) */
+static inline int int2scale(const struct bcm5974_param *p, int x)
+{
+	return x * p->dim / (p->devmax - p->devmin);
+}
+
+/* all logical value ranges are [0,dim). */
+static inline int int2bound(const struct bcm5974_param *p, int x)
+{
+	int s = int2scale(p, x);
+	return s < 0 ? 0 : s >= p->dim ? p->dim - 1 : s;
+}
+
+/* setup which logical events to report */
+static void setup_events_to_report(struct input_dev *input_dev,
+	const struct bcm5974_config *cfg)
+{
+	set_bit(EV_ABS, input_dev->evbit);
+	input_set_abs_params(input_dev, ABS_PRESSURE,
+		0, cfg->p.dim, cfg->p.fuzz, 0);
+	input_set_abs_params(input_dev, ABS_TOOL_WIDTH,
+		0, cfg->w.dim, cfg->w.fuzz, 0);
+	input_set_abs_params(input_dev, ABS_X,
+		0, cfg->x.dim, cfg->x.fuzz, 0);
+	input_set_abs_params(input_dev, ABS_Y,
+		0, cfg->y.dim, cfg->y.fuzz, 0);
+
+	set_bit(EV_KEY, input_dev->evbit);
+	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);
+}
+
+/* report button data as logical button state */
+static int report_bt_state(struct bcm5974 *dev, int size)
+{
+	if (size != sizeof(struct bt_data))
+		return -EIO;
+
+	input_report_key(dev->input, BTN_LEFT, dev->bt_data->button);
+	input_sync(dev->input);
+
+	return 0;
+}
+
+/* report trackpad data as logical trackpad state */
+static int report_tp_state(struct bcm5974 *dev, int size)
+{
+	const struct bcm5974_config *c = &dev->cfg;
+	const struct tp_finger *f = dev->tp_data->finger;
+	const int fingers = (size - 26) / 28;
+	int p, w, x, y, n;
+
+	if (size < 26 || (size - 26) % 28 != 0)
+		return -EIO;
+
+	if (!fingers) {
+		input_report_abs(dev->input, ABS_PRESSURE, 0);
+		input_report_key(dev->input, BTN_TOOL_FINGER, false);
+		input_report_key(dev->input, BTN_TOOL_DOUBLETAP, false);
+		input_report_key(dev->input, BTN_TOOL_TRIPLETAP, false);
+		input_sync(dev->input);
+		return 0;
+	}
+
+	p = raw2int(f->force_major);
+	w = raw2int(f->size_major);
+	x = raw2int(f->abs_x);
+	y = raw2int(f->abs_y);
+	n = p > 0 ? fingers : 0;
+
+	dprintk(9, "bcm5974: p: %+05d w: %+05d x: %+05d y: %+05d n: %d\n",
+		p, w, x, y, n);
+
+	input_report_abs(dev->input, ABS_PRESSURE, int2bound(&c->p, p));
+	input_report_abs(dev->input, ABS_TOOL_WIDTH, int2bound(&c->w, w));
+	input_report_abs(dev->input, ABS_X, int2bound(&c->x, x - c->x.devmin));
+	input_report_abs(dev->input, ABS_Y, int2bound(&c->y, c->y.devmax - y));
+	input_report_key(dev->input, BTN_TOOL_FINGER, n == 1);
+	input_report_key(dev->input, BTN_TOOL_DOUBLETAP, n == 2);
+	input_report_key(dev->input, BTN_TOOL_TRIPLETAP, n > 2);
+	input_sync(dev->input);
+	return 0;
+}
+
+/* Wellspring initialization constants */
+#define BCM5974_WELLSPRING_MODE_READ_REQUEST_ID		1
+#define BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID	9
+#define BCM5974_WELLSPRING_MODE_REQUEST_VALUE		0x300
+#define BCM5974_WELLSPRING_MODE_REQUEST_INDEX		0
+#define BCM5974_WELLSPRING_MODE_VENDOR_VALUE		0x01
+
+static int bcm5974_wellspring_mode(struct bcm5974 *dev)
+{
+	char *data = kmalloc(8, GFP_KERNEL);
+	int error = 0, size;
+
+	if (!data) {
+		err("bcm5974: out of memory");
+		error = -ENOMEM;
+		goto error;
+	}
+
+	/* read configuration */
+	size = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
+		BCM5974_WELLSPRING_MODE_READ_REQUEST_ID,
+		USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+		BCM5974_WELLSPRING_MODE_REQUEST_VALUE,
+		BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000);
+
+	if (size != 8) {
+		err("bcm5974: could not read from device");
+		error = -EIO;
+		goto error;
+	}
+
+	/* apply the mode switch */
+	data[0] = BCM5974_WELLSPRING_MODE_VENDOR_VALUE;
+
+	/* write configuration */
+	size = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
+		BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID,
+		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+		BCM5974_WELLSPRING_MODE_REQUEST_VALUE,
+		BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000);
+
+	if (size != 8) {
+		err("bcm5974: could not write to device");
+		error = -EIO;
+		goto error;
+	}
+
+	dprintk(2, "bcm5974: switched to wellspring mode.\n");
+
+	kfree(data);
+	return 0;
+
+error:
+	kfree(data);
+	return error;
+}
+
+static void irq_button(struct urb *urb)
+{
+	struct bcm5974 *dev = urb->context;
+	int error;
+
+	switch (urb->status) {
+	case 0:
+		break;
+	case -EOVERFLOW:
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		dbg("bcm5974: button urb shutting down: %d", urb->status);
+		return;
+	default:
+		dbg("bcm5974: button urb status: %d", urb->status);
+		goto exit;
+	}
+
+	if (report_bt_state(dev, dev->bt_urb->actual_length))
+		dprintk(1, "bcm5974: bad button package, length: %d\n",
+			dev->bt_urb->actual_length);
+
+exit:
+	error = usb_submit_urb(dev->bt_urb, GFP_ATOMIC);
+	if (error)
+		err("bcm5974: button urb failed: %d", error);
+}
+
+static void irq_trackpad(struct urb *urb)
+{
+	struct bcm5974 *dev = urb->context;
+	int error;
+
+	switch (urb->status) {
+	case 0:
+		break;
+	case -EOVERFLOW:
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+		dbg("bcm5974: trackpad urb shutting down: %d", urb->status);
+		return;
+	default:
+		dbg("bcm5974: trackpad urb status: %d", urb->status);
+		goto exit;
+	}
+
+	/* control response ignored */
+	if (dev->tp_urb->actual_length == 2)
+		goto exit;
+
+	if (report_tp_state(dev, dev->tp_urb->actual_length))
+		dprintk(1, "bcm5974: bad trackpad package, length: %d\n",
+			dev->tp_urb->actual_length);
+
+exit:
+	error = usb_submit_urb(dev->tp_urb, GFP_ATOMIC);
+	if (error)
+		err("bcm5974: trackpad urb failed: %d", error);
+}
+
+/*
+ * The Wellspring trackpad, like many recent Apple trackpads, share
+ * the usb device with the keyboard. Since keyboards are usually
+ * handled by the HID system, the device ends up being handled by two
+ * modules. Setting up the device therefore becomes slightly
+ * complicated. To enable multitouch features, a mode switch is
+ * required, which is usually applied via the control interface of the
+ * device.  It can be argued where this switch should take place. In
+ * some drivers, like appletouch, the switch is made during
+ * probe. However, the hid module may also alter the state of the
+ * device, resulting in trackpad malfunction under certain
+ * circumstances. To get around this problem, there is at least one
+ * example that utilizes the USB_QUIRK_RESET_RESUME quirk in order to
+ * recieve a reset_resume request rather than the normal resume.
+ * Since the implementation of reset_resume is equal to mode switch
+ * plus start_traffic, it seems easier to always do the switch when
+ * starting traffic on the device.
+ */
+static int start_traffic(struct bcm5974 *dev)
+{
+	if (bcm5974_wellspring_mode(dev)) {
+		dprintk(1, "bcm5974: mode switch failed\n");
+		goto error;
+	}
+	if (usb_submit_urb(dev->bt_urb, GFP_KERNEL))
+		goto error;
+	if (usb_submit_urb(dev->tp_urb, GFP_KERNEL))
+		goto err_kill_bt;
+
+	return 0;
+err_kill_bt:
+	usb_kill_urb(dev->bt_urb);
+error:
+	return -EIO;
+}
+
+static void pause_traffic(struct bcm5974 *dev)
+{
+	usb_kill_urb(dev->tp_urb);
+	usb_kill_urb(dev->bt_urb);
+}
+
+/*
+ * The code below implements open/close and manual suspend/resume.
+ * All functions may be called in random order.
+ *
+ * Opening a suspended device fails with EACCES - permission denied.
+ *
+ * Failing a resume leaves the device resumed but closed.
+ */
+static int bcm5974_open(struct input_dev *input)
+{
+	struct bcm5974 *dev = input_get_drvdata(input);
+	int error = 0;
+
+	mutex_lock(&dev->mutex);
+	if (dev->manually_suspended)
+		error = -EACCES;
+	else if (!dev->opened)
+		error = start_traffic(dev);
+	dev->opened = !error;
+	mutex_unlock(&dev->mutex);
+
+	return error;
+}
+
+static void bcm5974_close(struct input_dev *input)
+{
+	struct bcm5974 *dev = input_get_drvdata(input);
+
+	mutex_lock(&dev->mutex);
+	if (!dev->manually_suspended)
+		pause_traffic(dev);
+	dev->opened = 0;
+	mutex_unlock(&dev->mutex);
+}
+
+static int bcm5974_suspend(struct usb_interface *iface, pm_message_t message)
+{
+	struct bcm5974 *dev = usb_get_intfdata(iface);
+
+	if (dev) {
+		mutex_lock(&dev->mutex);
+		if (dev->opened && !dev->manually_suspended)
+			pause_traffic(dev);
+		dev->manually_suspended++;
+		mutex_unlock(&dev->mutex);
+	}
+
+	return 0;
+}
+
+static int bcm5974_resume(struct usb_interface *iface)
+{
+	struct bcm5974 *dev = usb_get_intfdata(iface);
+	int error = 0;
+
+	if (dev) {
+		mutex_lock(&dev->mutex);
+		if (dev->manually_suspended)
+			dev->manually_suspended--;
+		if (dev->opened && !dev->manually_suspended)
+			error = start_traffic(dev);
+		if (error)
+			dev->opened = 0;
+		mutex_unlock(&dev->mutex);
+	}
+
+	return error;
+}
+
+static int bcm5974_probe(struct usb_interface *iface,
+		     const struct usb_device_id *id)
+{
+	struct usb_device *udev = interface_to_usbdev(iface);
+	const struct bcm5974_config *cfg;
+	struct bcm5974 *dev;
+	struct input_dev *input_dev;
+	int error = -ENOMEM;
+
+	/* find the product index */
+	cfg = bcm5974_get_config(udev);
+
+	/* allocate memory for our device state and initialize it */
+	dev = kzalloc(sizeof(struct bcm5974), GFP_KERNEL);
+	input_dev = input_allocate_device();
+	if (!dev || !input_dev) {
+		err("bcm5974: out of memory");
+		goto err_free_devs;
+	}
+
+	dev->udev = udev;
+	dev->input = input_dev;
+	dev->cfg = *cfg;
+	mutex_init(&dev->mutex);
+
+	/* setup urbs */
+	dev->bt_urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!dev->bt_urb)
+		goto err_free_devs;
+
+	dev->tp_urb = usb_alloc_urb(0, GFP_KERNEL);
+	if (!dev->tp_urb)
+		goto err_free_bt_urb;
+
+	dev->bt_data = usb_buffer_alloc(dev->udev,
+		dev->cfg.bt_datalen, GFP_KERNEL,
+		&dev->bt_urb->transfer_dma);
+	if (!dev->bt_data)
+		goto err_free_urb;
+
+	dev->tp_data = usb_buffer_alloc(dev->udev,
+		dev->cfg.tp_datalen, GFP_KERNEL,
+		&dev->tp_urb->transfer_dma);
+	if (!dev->tp_data)
+		goto err_free_bt_buffer;
+
+	usb_fill_int_urb(dev->bt_urb, udev,
+		usb_rcvintpipe(udev, cfg->bt_ep),
+		dev->bt_data, dev->cfg.bt_datalen,
+		irq_button, dev, 1);
+
+	usb_fill_int_urb(dev->tp_urb, udev,
+		usb_rcvintpipe(udev, cfg->tp_ep),
+		dev->tp_data, dev->cfg.tp_datalen,
+		irq_trackpad, dev, 1);
+
+	/* create bcm5974 device */
+	usb_make_path(udev, dev->phys, sizeof(dev->phys));
+	strlcat(dev->phys, "/input0", sizeof(dev->phys));
+
+	input_dev->name = "bcm5974";
+	input_dev->phys = dev->phys;
+	usb_to_input_id(dev->udev, &input_dev->id);
+	input_dev->dev.parent = &iface->dev;
+
+	input_set_drvdata(input_dev, dev);
+
+	input_dev->open = bcm5974_open;
+	input_dev->close = bcm5974_close;
+
+	setup_events_to_report(input_dev, cfg);
+
+	error = input_register_device(dev->input);
+	if (error)
+		goto err_free_buffer;
+
+	/* save our data pointer in this interface device */
+	usb_set_intfdata(iface, dev);
+
+	return 0;
+
+err_free_buffer:
+	usb_buffer_free(dev->udev, dev->cfg.tp_datalen,
+		dev->tp_data, dev->tp_urb->transfer_dma);
+err_free_bt_buffer:
+	usb_buffer_free(dev->udev, dev->cfg.bt_datalen,
+		dev->bt_data, dev->bt_urb->transfer_dma);
+err_free_urb:
+	usb_free_urb(dev->tp_urb);
+err_free_bt_urb:
+	usb_free_urb(dev->bt_urb);
+err_free_devs:
+	usb_set_intfdata(iface, NULL);
+	input_free_device(input_dev);
+	kfree(dev);
+	return error;
+}
+
+static void bcm5974_disconnect(struct usb_interface *iface)
+{
+	struct bcm5974 *dev = usb_get_intfdata(iface);
+
+	usb_set_intfdata(iface, NULL);
+
+	input_unregister_device(dev->input);
+	usb_buffer_free(dev->udev, dev->cfg.tp_datalen,
+		dev->tp_data, dev->tp_urb->transfer_dma);
+	usb_buffer_free(dev->udev, dev->cfg.bt_datalen,
+		dev->bt_data, dev->bt_urb->transfer_dma);
+	usb_free_urb(dev->tp_urb);
+	usb_free_urb(dev->bt_urb);
+	kfree(dev);
+
+	printk(KERN_INFO "bcm5974: disconnected\n");
+}
+
+static struct usb_driver bcm5974_driver = {
+	.name = "bcm5974",
+	.probe = bcm5974_probe,
+	.disconnect = bcm5974_disconnect,
+	.suspend = bcm5974_suspend,
+	.resume = bcm5974_resume,
+	.reset_resume = bcm5974_resume,
+	.id_table = bcm5974_table,
+};
+
+static int __init bcm5974_init(void)
+{
+	return usb_register(&bcm5974_driver);
+}
+
+static void __exit bcm5974_exit(void)
+{
+	usb_deregister(&bcm5974_driver);
+}
+
+module_init(bcm5974_init);
+module_exit(bcm5974_exit);
+

Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Dmitry Torokhov]

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

Hi Henrik,

On Thu, Jul 24, 2008 at 09:37:02AM +0200, Henrik Rydberg wrote:
> 
> This is version bcm5974-0.58. Changes since 0.57 are in short:
> 
>   * Name changes; use usbhid definitions, rename atp to bcm5974
>   * Input syncing moved to report functions, event type setup broken out
>   * Traffic functions added for open/close and suspend/resume logic
>   * Open/close and suspend/resume transition logic corrected
>   * Open/close serialized with respect to suspend/resume
> 

Thank you for the changes.  You don't have to track whether device is
manually suspended or not - usb_submit_urb will fail and that is it.

Attached are 2 patches. First cleans suspend state tracking as it is not
really needed and does some formatting and other minor changes and 2nd
implements runtime pm for the device. Could you please try them and if
everything still works I will apply the driver.

Thanks!

-- 
Dmitry

[-- Attachment #2: bcm5974-cleanups.patch --]
[-- Type: text/plain, Size: 13648 bytes --]

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

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


---

 drivers/input/mouse/Kconfig   |   12 +-
 drivers/input/mouse/bcm5974.c |  228 ++++++++++++++++++++---------------------
 2 files changed, 118 insertions(+), 122 deletions(-)

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 2eab073..f996546 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -133,23 +133,23 @@ config MOUSE_APPLETOUCH
 config MOUSE_BCM5974
 	tristate "Apple USB BCM5974 Multitouch trackpad support"
 	depends on USB_ARCH_HAS_HCD
-	depends on USB
+	select USB
 	help
 	  Say Y here if you have an Apple USB BCM5974 Multitouch
 	  trackpad.
-	
+
 	  The BCM5974 is the multitouch trackpad found in the Macbook
 	  Air (JAN2008) and Macbook Pro Penryn (FEB2008) laptops.
-	
+
 	  It is also found in the IPhone (2007) and Ipod Touch (2008).
-	
+
 	  This driver provides multitouch functionality together with
 	  the synaptics X11 driver.
-	
+
 	  The interface is currently identical to the appletouch interface,
 	  for further information, see
 	  <file:Documentation/input/appletouch.txt>.
-	
+
 	  To compile this driver as a module, choose M here: the
 	  module will be called bcm5974.
 
diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index 55c1f60..6f85278 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -152,9 +152,8 @@ struct bcm5974 {
 	struct usb_device *udev;	/* usb device */
 	struct input_dev *input;	/* input dev */
 	struct bcm5974_config cfg;	/* device configuration */
-	struct mutex mutex;		/* serialize access to open/suspend */
-	int opened;			/* >0: opened, else closed */
-	int manually_suspended;		/* >0: manually suspended */
+	struct mutex pm_mutex;		/* serialize access to open/suspend */
+	int opened;			/* 1: opened, 0: closed */
 	struct urb *bt_urb;		/* button usb request block */
 	struct bt_data *bt_data;	/* button transferred data */
 	struct urb *tp_urb;		/* trackpad usb request block */
@@ -204,9 +203,11 @@ static const struct bcm5974_config *bcm5974_get_config(struct usb_device *udev)
 {
 	u16 id = le16_to_cpu(udev->descriptor.idProduct);
 	const struct bcm5974_config *cfg;
+
 	for (cfg = bcm5974_config_table; cfg->ansi; ++cfg)
 		if (cfg->ansi == id || cfg->iso == id || cfg->jis == id)
 			return cfg;
+
 	return bcm5974_config_table;
 }
 
@@ -226,28 +227,30 @@ static inline int int2scale(const struct bcm5974_param *p, int x)
 static inline int int2bound(const struct bcm5974_param *p, int x)
 {
 	int s = int2scale(p, x);
-	return s < 0 ? 0 : s >= p->dim ? p->dim - 1 : s;
+
+	return clamp_val(s, 0, p->dim - 1);
 }
 
 /* setup which logical events to report */
 static void setup_events_to_report(struct input_dev *input_dev,
-	const struct bcm5974_config *cfg)
+				   const struct bcm5974_config *cfg)
 {
-	set_bit(EV_ABS, input_dev->evbit);
+	__set_bit(EV_ABS, input_dev->evbit);
+
 	input_set_abs_params(input_dev, ABS_PRESSURE,
-		0, cfg->p.dim, cfg->p.fuzz, 0);
+				0, cfg->p.dim, cfg->p.fuzz, 0);
 	input_set_abs_params(input_dev, ABS_TOOL_WIDTH,
-		0, cfg->w.dim, cfg->w.fuzz, 0);
+				0, cfg->w.dim, cfg->w.fuzz, 0);
 	input_set_abs_params(input_dev, ABS_X,
-		0, cfg->x.dim, cfg->x.fuzz, 0);
+				0, cfg->x.dim, cfg->x.fuzz, 0);
 	input_set_abs_params(input_dev, ABS_Y,
-		0, cfg->y.dim, cfg->y.fuzz, 0);
+				0, cfg->y.dim, cfg->y.fuzz, 0);
 
-	set_bit(EV_KEY, input_dev->evbit);
-	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(EV_KEY, input_dev->evbit);
+	__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);
 }
 
 /* report button data as logical button state */
@@ -267,38 +270,37 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 {
 	const struct bcm5974_config *c = &dev->cfg;
 	const struct tp_finger *f = dev->tp_data->finger;
+	struct input_dev *input = dev->input;
 	const int fingers = (size - 26) / 28;
-	int p, w, x, y, n;
+	int p = 0, w, x, y, n = 0;
 
 	if (size < 26 || (size - 26) % 28 != 0)
 		return -EIO;
 
-	if (!fingers) {
-		input_report_abs(dev->input, ABS_PRESSURE, 0);
-		input_report_key(dev->input, BTN_TOOL_FINGER, false);
-		input_report_key(dev->input, BTN_TOOL_DOUBLETAP, false);
-		input_report_key(dev->input, BTN_TOOL_TRIPLETAP, false);
-		input_sync(dev->input);
-		return 0;
+	if (fingers) {
+		p = raw2int(f->force_major);
+		w = raw2int(f->size_major);
+		x = raw2int(f->abs_x);
+		y = raw2int(f->abs_y);
+		n = p > 0 ? fingers : 0;
+
+		dprintk(9,
+			"bcm5974: p: %+05d w: %+05d x: %+05d y: %+05d n: %d\n",
+			p, w, x, y, n);
+
+		input_report_abs(input, ABS_TOOL_WIDTH, int2bound(&c->w, w));
+		input_report_abs(input, ABS_X, int2bound(&c->x, x - c->x.devmin));
+		input_report_abs(input, ABS_Y, int2bound(&c->y, c->y.devmax - y));
 	}
 
-	p = raw2int(f->force_major);
-	w = raw2int(f->size_major);
-	x = raw2int(f->abs_x);
-	y = raw2int(f->abs_y);
-	n = p > 0 ? fingers : 0;
-
-	dprintk(9, "bcm5974: p: %+05d w: %+05d x: %+05d y: %+05d n: %d\n",
-		p, w, x, y, n);
-
-	input_report_abs(dev->input, ABS_PRESSURE, int2bound(&c->p, p));
-	input_report_abs(dev->input, ABS_TOOL_WIDTH, int2bound(&c->w, w));
-	input_report_abs(dev->input, ABS_X, int2bound(&c->x, x - c->x.devmin));
-	input_report_abs(dev->input, ABS_Y, int2bound(&c->y, c->y.devmax - y));
-	input_report_key(dev->input, BTN_TOOL_FINGER, n == 1);
-	input_report_key(dev->input, BTN_TOOL_DOUBLETAP, n == 2);
-	input_report_key(dev->input, BTN_TOOL_TRIPLETAP, n > 2);
-	input_sync(dev->input);
+	input_report_abs(input, ABS_PRESSURE, int2bound(&c->p, p));
+
+	input_report_key(input, BTN_TOOL_FINGER, n == 1);
+	input_report_key(input, BTN_TOOL_DOUBLETAP, n == 2);
+	input_report_key(input, BTN_TOOL_TRIPLETAP, n > 2);
+
+	input_sync(input);
+
 	return 0;
 }
 
@@ -312,25 +314,25 @@ static int report_tp_state(struct bcm5974 *dev, int size)
 static int bcm5974_wellspring_mode(struct bcm5974 *dev)
 {
 	char *data = kmalloc(8, GFP_KERNEL);
-	int error = 0, size;
+	int retval = 0, size;
 
 	if (!data) {
 		err("bcm5974: out of memory");
-		error = -ENOMEM;
-		goto error;
+		retval = -ENOMEM;
+		goto out;
 	}
 
 	/* read configuration */
 	size = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
-		BCM5974_WELLSPRING_MODE_READ_REQUEST_ID,
-		USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-		BCM5974_WELLSPRING_MODE_REQUEST_VALUE,
-		BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000);
+			BCM5974_WELLSPRING_MODE_READ_REQUEST_ID,
+			USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			BCM5974_WELLSPRING_MODE_REQUEST_VALUE,
+			BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000);
 
 	if (size != 8) {
 		err("bcm5974: could not read from device");
-		error = -EIO;
-		goto error;
+		retval = -EIO;
+		goto out;
 	}
 
 	/* apply the mode switch */
@@ -338,28 +340,25 @@ static int bcm5974_wellspring_mode(struct bcm5974 *dev)
 
 	/* write configuration */
 	size = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
-		BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID,
-		USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
-		BCM5974_WELLSPRING_MODE_REQUEST_VALUE,
-		BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000);
+			BCM5974_WELLSPRING_MODE_WRITE_REQUEST_ID,
+			USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			BCM5974_WELLSPRING_MODE_REQUEST_VALUE,
+			BCM5974_WELLSPRING_MODE_REQUEST_INDEX, data, 8, 5000);
 
 	if (size != 8) {
 		err("bcm5974: could not write to device");
-		error = -EIO;
-		goto error;
+		retval = -EIO;
+		goto out;
 	}
 
 	dprintk(2, "bcm5974: switched to wellspring mode.\n");
 
+ out:
 	kfree(data);
-	return 0;
-
-error:
-	kfree(data);
-	return error;
+	return retval;
 }
 
-static void irq_button(struct urb *urb)
+static void bcm5974_irq_button(struct urb *urb)
 {
 	struct bcm5974 *dev = urb->context;
 	int error;
@@ -388,7 +387,7 @@ exit:
 		err("bcm5974: button urb failed: %d", error);
 }
 
-static void irq_trackpad(struct urb *urb)
+static void bcm5974_irq_trackpad(struct urb *urb)
 {
 	struct bcm5974 *dev = urb->context;
 	int error;
@@ -439,25 +438,28 @@ exit:
  * plus start_traffic, it seems easier to always do the switch when
  * starting traffic on the device.
  */
-static int start_traffic(struct bcm5974 *dev)
+static int bcm5974_start_traffic(struct bcm5974 *dev)
 {
 	if (bcm5974_wellspring_mode(dev)) {
 		dprintk(1, "bcm5974: mode switch failed\n");
 		goto error;
 	}
+
 	if (usb_submit_urb(dev->bt_urb, GFP_KERNEL))
 		goto error;
+
 	if (usb_submit_urb(dev->tp_urb, GFP_KERNEL))
 		goto err_kill_bt;
 
 	return 0;
+
 err_kill_bt:
 	usb_kill_urb(dev->bt_urb);
 error:
 	return -EIO;
 }
 
-static void pause_traffic(struct bcm5974 *dev)
+static void bcm5974_pause_traffic(struct bcm5974 *dev)
 {
 	usb_kill_urb(dev->tp_urb);
 	usb_kill_urb(dev->bt_urb);
@@ -474,15 +476,15 @@ static void pause_traffic(struct bcm5974 *dev)
 static int bcm5974_open(struct input_dev *input)
 {
 	struct bcm5974 *dev = input_get_drvdata(input);
-	int error = 0;
+	int error;
 
-	mutex_lock(&dev->mutex);
-	if (dev->manually_suspended)
-		error = -EACCES;
-	else if (!dev->opened)
-		error = start_traffic(dev);
-	dev->opened = !error;
-	mutex_unlock(&dev->mutex);
+	mutex_lock(&dev->pm_mutex);
+
+	error = bcm5974_start_traffic(dev);
+	if (!error)
+		dev->opened = 1;
+
+	mutex_unlock(&dev->pm_mutex);
 
 	return error;
 }
@@ -491,24 +493,24 @@ static void bcm5974_close(struct input_dev *input)
 {
 	struct bcm5974 *dev = input_get_drvdata(input);
 
-	mutex_lock(&dev->mutex);
-	if (!dev->manually_suspended)
-		pause_traffic(dev);
+	mutex_lock(&dev->pm_mutex);
+
+	bcm5974_pause_traffic(dev);
 	dev->opened = 0;
-	mutex_unlock(&dev->mutex);
+
+	mutex_unlock(&dev->pm_mutex);
 }
 
 static int bcm5974_suspend(struct usb_interface *iface, pm_message_t message)
 {
 	struct bcm5974 *dev = usb_get_intfdata(iface);
 
-	if (dev) {
-		mutex_lock(&dev->mutex);
-		if (dev->opened && !dev->manually_suspended)
-			pause_traffic(dev);
-		dev->manually_suspended++;
-		mutex_unlock(&dev->mutex);
-	}
+	mutex_lock(&dev->pm_mutex);
+
+	if (dev->opened)
+		bcm5974_pause_traffic(dev);
+
+	mutex_unlock(&dev->pm_mutex);
 
 	return 0;
 }
@@ -518,22 +520,18 @@ static int bcm5974_resume(struct usb_interface *iface)
 	struct bcm5974 *dev = usb_get_intfdata(iface);
 	int error = 0;
 
-	if (dev) {
-		mutex_lock(&dev->mutex);
-		if (dev->manually_suspended)
-			dev->manually_suspended--;
-		if (dev->opened && !dev->manually_suspended)
-			error = start_traffic(dev);
-		if (error)
-			dev->opened = 0;
-		mutex_unlock(&dev->mutex);
-	}
+	mutex_lock(&dev->pm_mutex);
+
+	if (dev->opened)
+		error = bcm5974_start_traffic(dev);
+
+	mutex_unlock(&dev->pm_mutex);
 
 	return error;
 }
 
 static int bcm5974_probe(struct usb_interface *iface,
-		     const struct usb_device_id *id)
+			 const struct usb_device_id *id)
 {
 	struct usb_device *udev = interface_to_usbdev(iface);
 	const struct bcm5974_config *cfg;
@@ -555,7 +553,7 @@ static int bcm5974_probe(struct usb_interface *iface,
 	dev->udev = udev;
 	dev->input = input_dev;
 	dev->cfg = *cfg;
-	mutex_init(&dev->mutex);
+	mutex_init(&dev->pm_mutex);
 
 	/* setup urbs */
 	dev->bt_urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -567,26 +565,26 @@ static int bcm5974_probe(struct usb_interface *iface,
 		goto err_free_bt_urb;
 
 	dev->bt_data = usb_buffer_alloc(dev->udev,
-		dev->cfg.bt_datalen, GFP_KERNEL,
-		&dev->bt_urb->transfer_dma);
+					dev->cfg.bt_datalen, GFP_KERNEL,
+					&dev->bt_urb->transfer_dma);
 	if (!dev->bt_data)
 		goto err_free_urb;
 
 	dev->tp_data = usb_buffer_alloc(dev->udev,
-		dev->cfg.tp_datalen, GFP_KERNEL,
-		&dev->tp_urb->transfer_dma);
+					dev->cfg.tp_datalen, GFP_KERNEL,
+					&dev->tp_urb->transfer_dma);
 	if (!dev->tp_data)
 		goto err_free_bt_buffer;
 
 	usb_fill_int_urb(dev->bt_urb, udev,
-		usb_rcvintpipe(udev, cfg->bt_ep),
-		dev->bt_data, dev->cfg.bt_datalen,
-		irq_button, dev, 1);
+			 usb_rcvintpipe(udev, cfg->bt_ep),
+			 dev->bt_data, dev->cfg.bt_datalen,
+			 bcm5974_irq_button, dev, 1);
 
 	usb_fill_int_urb(dev->tp_urb, udev,
-		usb_rcvintpipe(udev, cfg->tp_ep),
-		dev->tp_data, dev->cfg.tp_datalen,
-		irq_trackpad, dev, 1);
+			 usb_rcvintpipe(udev, cfg->tp_ep),
+			 dev->tp_data, dev->cfg.tp_datalen,
+			 bcm5974_irq_trackpad, dev, 1);
 
 	/* create bcm5974 device */
 	usb_make_path(udev, dev->phys, sizeof(dev->phys));
@@ -638,24 +636,22 @@ static void bcm5974_disconnect(struct usb_interface *iface)
 
 	input_unregister_device(dev->input);
 	usb_buffer_free(dev->udev, dev->cfg.tp_datalen,
-		dev->tp_data, dev->tp_urb->transfer_dma);
+			dev->tp_data, dev->tp_urb->transfer_dma);
 	usb_buffer_free(dev->udev, dev->cfg.bt_datalen,
-		dev->bt_data, dev->bt_urb->transfer_dma);
+			dev->bt_data, dev->bt_urb->transfer_dma);
 	usb_free_urb(dev->tp_urb);
 	usb_free_urb(dev->bt_urb);
 	kfree(dev);
-
-	printk(KERN_INFO "bcm5974: disconnected\n");
 }
 
 static struct usb_driver bcm5974_driver = {
-	.name = "bcm5974",
-	.probe = bcm5974_probe,
-	.disconnect = bcm5974_disconnect,
-	.suspend = bcm5974_suspend,
-	.resume = bcm5974_resume,
-	.reset_resume = bcm5974_resume,
-	.id_table = bcm5974_table,
+	.name			= "bcm5974",
+	.probe			= bcm5974_probe,
+	.disconnect		= bcm5974_disconnect,
+	.suspend		= bcm5974_suspend,
+	.resume			= bcm5974_resume,
+	.reset_resume		= bcm5974_resume,
+	.id_table		= bcm5974_table,
 };
 
 static int __init bcm5974_init(void)

[-- Attachment #3: bcm5974-autopm.patch --]
[-- Type: text/plain, Size: 1909 bytes --]

Input: bcm5974 - implement autosuspend support

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

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

 drivers/input/mouse/bcm5974.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index 6f85278..2ec921b 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -150,6 +150,7 @@ struct bcm5974_config {
 struct bcm5974 {
 	char phys[64];
 	struct usb_device *udev;	/* usb device */
+	struct usb_interface *intf;	/* our interface */
 	struct input_dev *input;	/* input dev */
 	struct bcm5974_config cfg;	/* device configuration */
 	struct mutex pm_mutex;		/* serialize access to open/suspend */
@@ -478,6 +479,10 @@ static int bcm5974_open(struct input_dev *input)
 	struct bcm5974 *dev = input_get_drvdata(input);
 	int error;
 
+	error = usb_autopm_get_interface(dev->intf);
+	if (error)
+		return error;
+
 	mutex_lock(&dev->pm_mutex);
 
 	error = bcm5974_start_traffic(dev);
@@ -486,6 +491,9 @@ static int bcm5974_open(struct input_dev *input)
 
 	mutex_unlock(&dev->pm_mutex);
 
+	if (error)
+		usb_autopm_put_interface(dev->intf);
+
 	return error;
 }
 
@@ -499,6 +507,8 @@ static void bcm5974_close(struct input_dev *input)
 	dev->opened = 0;
 
 	mutex_unlock(&dev->pm_mutex);
+
+	usb_autopm_put_interface(dev->intf);
 }
 
 static int bcm5974_suspend(struct usb_interface *iface, pm_message_t message)
@@ -551,6 +561,7 @@ static int bcm5974_probe(struct usb_interface *iface,
 	}
 
 	dev->udev = udev;
+	dev->intf = iface;
 	dev->input = input_dev;
 	dev->cfg = *cfg;
 	mutex_init(&dev->pm_mutex);
@@ -652,6 +663,7 @@ static struct usb_driver bcm5974_driver = {
 	.resume			= bcm5974_resume,
 	.reset_resume		= bcm5974_resume,
 	.id_table		= bcm5974_table,
+	.supports_autosuspend	= 1,
 };
 
 static int __init bcm5974_init(void)

Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Henrik Rydberg]

Hello Dmitry,
> 
> Thank you for the changes.  You don't have to track whether device is
> manually suspended or not - usb_submit_urb will fail and that is it.
> 

Perfect.

> Attached are 2 patches. First cleans suspend state tracking as it is not
> really needed and does some formatting and other minor changes and 2nd
> implements runtime pm for the device. Could you please try them and if
> everything still works I will apply the driver.

Patch 1:

  Looking good, just a few comments:

  * I notice that clamp_val is new since 2.6.24.3
  * Regarding the open-fails comment: is EACCES correct?

Patch 2:

  Perfect.

Thank you so much, Dmitry - it's a GO!

Henrik

Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Dmitry Torokhov]

On Fri, Jul 25, 2008 at 08:25:29PM +0200, Henrik Rydberg wrote:
> Hello Dmitry,
> > 
> > Thank you for the changes.  You don't have to track whether device is
> > manually suspended or not - usb_submit_urb will fail and that is it.
> > 
> 
> Perfect.
> 
> > Attached are 2 patches. First cleans suspend state tracking as it is not
> > really needed and does some formatting and other minor changes and 2nd
> > implements runtime pm for the device. Could you please try them and if
> > everything still works I will apply the driver.
> 
> Patch 1:
> 
>   Looking good, just a few comments:
> 
>   * I notice that clamp_val is new since 2.6.24.3

Probably, I am not sure...

>   * Regarding the open-fails comment: is EACCES correct?

We don't return EACCESS anymore, I don't think. We will return what
bcm5974_start_traffic returns which in turn returns result of
usb_submit_urb and the likes.

I don't think returning -EACCESS was proper - it usually indicates
permisison issues, not the fact that the hardware is unavailable.

> 
> Patch 2:
> 
>   Perfect.
> 
> Thank you so much, Dmitry - it's a GO!
> 

Great, thank you!

-- 
Dmitry

Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Henrik Rydberg]

Hi Dmitry,

one thing about the drivers/input/mouse/Kconfig patch:

The original version contained "select USB", which was later changed
to "depends on USB" by Andrew Morton. I saw it reappear as "select USB"
in your latest patch, and simply considered it a change back, but
maybe it was an oversight?

Cheers,
Henrik

Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Dmitry Torokhov]

Hi Henrik,

On Sat, Jul 26, 2008 at 09:49:38AM +0200, Henrik Rydberg wrote:
> Hi Dmitry,
> 
> one thing about the drivers/input/mouse/Kconfig patch:
> 
> The original version contained "select USB", which was later changed
> to "depends on USB" by Andrew Morton. I saw it reappear as "select USB"
> in your latest patch, and simply considered it a change back, but
> maybe it was an oversight?
> 

I am of the opinion that it is OK to "select" high-level subsystems that
don't have additional dependencies, especially if they are "past" the
original driver in menuconfig. I don't really like the idea of forcing
users revisiting earlier sub-menus after they selected a new subsystem
to see if there are any new options. Plus, if user wants driver for his
touchpad he does not really care whether it is PS/2 or USB, [s]he just
wants it to work.

So the change was intentional on my part.

-- 
Dmitry

Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Johannes Berg]

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

On Tue, 2008-07-29 at 01:41 -0400, Dmitry Torokhov wrote:

> > one thing about the drivers/input/mouse/Kconfig patch:
> > 
> > The original version contained "select USB", which was later changed
> > to "depends on USB" by Andrew Morton. I saw it reappear as "select USB"
> > in your latest patch, and simply considered it a change back, but
> > maybe it was an oversight?
> > 
> 
> I am of the opinion that it is OK to "select" high-level subsystems that
> don't have additional dependencies, especially if they are "past" the
> original driver in menuconfig. I don't really like the idea of forcing
> users revisiting earlier sub-menus after they selected a new subsystem
> to see if there are any new options. Plus, if user wants driver for his
> touchpad he does not really care whether it is PS/2 or USB, [s]he just
> wants it to work.

USB tends to have implicit dependencies like the ehci/ohci/uhci
controller though, so it's not going to work like that anyway starting
from a minimal config, and all defconfigs probably have USB anyway.

Therefore, I personally would keep it as "depends on" rather than
"select". YMMV.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

[Dmitry Torokhov]

On Tue, Jul 29, 2008 at 08:46:13AM +0200, Johannes Berg wrote:
> On Tue, 2008-07-29 at 01:41 -0400, Dmitry Torokhov wrote:
> 
> > > one thing about the drivers/input/mouse/Kconfig patch:
> > > 
> > > The original version contained "select USB", which was later changed
> > > to "depends on USB" by Andrew Morton. I saw it reappear as "select USB"
> > > in your latest patch, and simply considered it a change back, but
> > > maybe it was an oversight?
> > > 
> > 
> > I am of the opinion that it is OK to "select" high-level subsystems that
> > don't have additional dependencies, especially if they are "past" the
> > original driver in menuconfig. I don't really like the idea of forcing
> > users revisiting earlier sub-menus after they selected a new subsystem
> > to see if there are any new options. Plus, if user wants driver for his
> > touchpad he does not really care whether it is PS/2 or USB, [s]he just
> > wants it to work.
> 
> USB tends to have implicit dependencies like the ehci/ohci/uhci
> controller though, so it's not going to work like that anyway starting
> from a minimal config, and all defconfigs probably have USB anyway.
> 
> Therefore, I personally would keep it as "depends on" rather than
> "select". YMMV.
> 

It should work fine I think. The full dependancy of most USB input
devices is:

        depends on USB_ARCH_HAS_HCD
        select USB

You do need to select the host controller, that is correct, but you
don't need to go to USB menu, select HCD, then go back to input menu
(which you already visited) and select your USB input device.

-- 
Dmitry