[patch 00/11] input: force feedback updates

[patch 00/11] input: force feedback updates

[Anssi Hannula]

Major update for the force feedback support, including a new force feedback
driver interface and two new HID ff drivers. 

--
Anssi Hannula

[patch 01/11] input: move fixp-arith.h to drivers/input

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-move-fixp-arith.diff --]
[-- Type: text/plain, Size: 6944 bytes --]

Move fixp-arith.h from drivers/usb/input to drivers/input, as the part of
force feedback support that requires trigonometric functions is being moved
there.

hid-lgff.c is temporarily modified to avoid breaking git-bisect.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/input/fixp-arith.h     |   90 +++++++++++++++++++++++++++++++++++++++++
 drivers/usb/input/fixp-arith.h |   90 -----------------------------------------
 drivers/usb/input/hid-lgff.c   |    6 +-
 3 files changed, 93 insertions(+), 93 deletions(-)

Index: linux-2.6.17-rc3-git12/drivers/input/fixp-arith.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.17-rc3-git12/drivers/input/fixp-arith.h	2006-05-13 23:16:02.000000000 +0300
@@ -0,0 +1,90 @@
+#ifndef _FIXP_ARITH_H
+#define _FIXP_ARITH_H
+
+/*
+ * $$
+ *
+ * Simplistic fixed-point arithmetics.
+ * Hmm, I'm probably duplicating some code :(
+ *
+ * Copyright (c) 2002 Johann Deneux
+ */
+
+/*
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Should you need to contact me, the author, you can do so by
+ * e-mail - mail your message to <deneux@ifrance.com>
+ */
+
+#include <linux/types.h>
+
+// The type representing fixed-point values
+typedef s16 fixp_t;
+
+#define FRAC_N 8
+#define FRAC_MASK ((1<<FRAC_N)-1)
+
+// Not to be used directly. Use fixp_{cos,sin}
+static const fixp_t cos_table[45] = {
+	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
+	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
+	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
+	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
+	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
+	0x002C,	0x0023,	0x001A,	0x0011,	0x0008
+};
+
+
+/* a: 123 -> 123.0 */
+static inline fixp_t fixp_new(s16 a)
+{
+	return a<<FRAC_N;
+}
+
+/* a: 0xFFFF -> -1.0
+      0x8000 -> 1.0
+      0x0000 -> 0.0
+*/
+static inline fixp_t fixp_new16(s16 a)
+{
+	return ((s32)a)>>(16-FRAC_N);
+}
+
+static inline fixp_t fixp_cos(unsigned int degrees)
+{
+	int quadrant = (degrees / 90) & 3;
+	unsigned int i = degrees % 90;
+
+	if (quadrant == 1 || quadrant == 3) {
+		i = 89 - i;
+	}
+
+	i >>= 1;
+
+	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
+}
+
+static inline fixp_t fixp_sin(unsigned int degrees)
+{
+	return -fixp_cos(degrees + 90);
+}
+
+static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
+{
+	return ((s32)(a*b))>>FRAC_N;
+}
+
+#endif
Index: linux-2.6.17-rc3-git12/drivers/usb/input/fixp-arith.h
===================================================================
--- linux-2.6.17-rc3-git12.orig/drivers/usb/input/fixp-arith.h	2006-05-13 23:15:00.000000000 +0300
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,90 +0,0 @@
-#ifndef _FIXP_ARITH_H
-#define _FIXP_ARITH_H
-
-/*
- * $$
- *
- * Simplistic fixed-point arithmetics.
- * Hmm, I'm probably duplicating some code :(
- *
- * Copyright (c) 2002 Johann Deneux
- */
-
-/*
- * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- * Should you need to contact me, the author, you can do so by
- * e-mail - mail your message to <deneux@ifrance.com>
- */
-
-#include <linux/types.h>
-
-// The type representing fixed-point values
-typedef s16 fixp_t;
-
-#define FRAC_N 8
-#define FRAC_MASK ((1<<FRAC_N)-1)
-
-// Not to be used directly. Use fixp_{cos,sin}
-static const fixp_t cos_table[45] = {
-	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
-	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
-	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
-	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
-	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
-	0x002C,	0x0023,	0x001A,	0x0011,	0x0008
-};
-
-
-/* a: 123 -> 123.0 */
-static inline fixp_t fixp_new(s16 a)
-{
-	return a<<FRAC_N;
-}
-
-/* a: 0xFFFF -> -1.0
-      0x8000 -> 1.0
-      0x0000 -> 0.0
-*/
-static inline fixp_t fixp_new16(s16 a)
-{
-	return ((s32)a)>>(16-FRAC_N);
-}
-
-static inline fixp_t fixp_cos(unsigned int degrees)
-{
-	int quadrant = (degrees / 90) & 3;
-	unsigned int i = degrees % 90;
-
-	if (quadrant == 1 || quadrant == 3) {
-		i = 89 - i;
-	}
-
-	i >>= 1;
-
-	return (quadrant == 1 || quadrant == 2)? -cos_table[i] : cos_table[i];
-}
-
-static inline fixp_t fixp_sin(unsigned int degrees)
-{
-	return -fixp_cos(degrees + 90);
-}
-
-static inline fixp_t fixp_mult(fixp_t a, fixp_t b)
-{
-	return ((s32)(a*b))>>FRAC_N;
-}
-
-#endif
Index: linux-2.6.17-rc3-git12/drivers/usb/input/hid-lgff.c
===================================================================
--- linux-2.6.17-rc3-git12.orig/drivers/usb/input/hid-lgff.c	2006-05-13 23:17:47.000000000 +0300
+++ linux-2.6.17-rc3-git12/drivers/usb/input/hid-lgff.c	2006-05-13 23:18:04.000000000 +0300
@@ -37,7 +37,7 @@
 #include <linux/circ_buf.h>
 
 #include "hid.h"
-#include "fixp-arith.h"
+//#include "fixp-arith.h"
 
 
 /* Periodicity of the update */
@@ -445,10 +445,10 @@ static void hid_lgff_timer(unsigned long
 			case FF_CONSTANT: {
 				//TODO: handle envelopes
 				int degrees = effect->effect.direction * 360 >> 16;
-				x += fixp_mult(fixp_sin(degrees),
+	/*			x += fixp_mult(fixp_sin(degrees),
 					       fixp_new16(effect->effect.u.constant.level));
 				y += fixp_mult(-fixp_cos(degrees),
-					       fixp_new16(effect->effect.u.constant.level));
+					       fixp_new16(effect->effect.u.constant.level));*/
 			}       break;
 			case FF_RUMBLE:
 				right += effect->effect.u.rumble.strong_magnitude;

--
Anssi Hannula

[patch 02/11] input: fix accuracy of fixp-arith.h

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-fixp-arith-accuracy.diff --]
[-- Type: text/plain, Size: 1403 bytes --]

Add the value of cos(90) = 0 to the table. This also moves the results so that
sin(x) == sin(180-x) is true as expected.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/input/fixp-arith.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6.16-git20/drivers/input/fixp-arith.h
===================================================================
--- linux-2.6.16-git20.orig/drivers/input/fixp-arith.h	2006-04-10 21:02:50.000000000 +0300
+++ linux-2.6.16-git20/drivers/input/fixp-arith.h	2006-04-10 21:03:45.000000000 +0300
@@ -38,13 +38,13 @@ typedef s16 fixp_t;
 #define FRAC_MASK ((1<<FRAC_N)-1)
 
 // Not to be used directly. Use fixp_{cos,sin}
-static const fixp_t cos_table[45] = {
+static const fixp_t cos_table[46] = {
 	0x0100,	0x00FF,	0x00FF,	0x00FE,	0x00FD,	0x00FC,	0x00FA,	0x00F8,
 	0x00F6,	0x00F3,	0x00F0,	0x00ED,	0x00E9,	0x00E6,	0x00E2,	0x00DD,
 	0x00D9,	0x00D4,	0x00CF,	0x00C9,	0x00C4,	0x00BE,	0x00B8,	0x00B1,
 	0x00AB,	0x00A4,	0x009D,	0x0096,	0x008F,	0x0087,	0x0080,	0x0078,
 	0x0070,	0x0068,	0x005F,	0x0057,	0x004F,	0x0046,	0x003D,	0x0035,
-	0x002C,	0x0023,	0x001A,	0x0011,	0x0008
+	0x002C,	0x0023,	0x001A,	0x0011,	0x0008, 0x0000
 };
 
 
@@ -69,7 +69,7 @@ static inline fixp_t fixp_cos(unsigned i
 	unsigned int i = degrees % 90;
 
 	if (quadrant == 1 || quadrant == 3) {
-		i = 89 - i;
+		i = 90 - i;
 	}
 
 	i >>= 1;

--
Anssi Hannula

[patch 03/11] input: new force feedback interface

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-new-interface.diff --]
[-- Type: text/plain, Size: 29696 bytes --]

Implement a new force feedback interface, in which all non-driver-specific
operations are separated to a common module. This module handles effect type
validations, effect timers, locking, etc.

As a result, support is added for gain and envelope for memoryless devices,
periodic => rumble conversion for memoryless devices and rumble => periodic
conversion for devices with periodic support instead of rumble support. Also
the effect memory of devices is not emptied if the root user opens and closes
the device while another user is using effects. This module also obsoletes
some flawed locking and timer code in few ff drivers.

The module is named ff-effects. If INPUT_FF_EFFECTS is enabled, the force
feedback drivers and interfaces (evdev) will be depending on it.

Userspace interface is left unaltered.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/input/Kconfig      |    9 
 drivers/input/Makefile     |    1 
 drivers/input/evdev.c      |   12 
 drivers/input/ff-effects.c |  751 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/input/input.c      |   17 -
 include/linux/input.h      |   50 ++
 6 files changed, 831 insertions(+), 9 deletions(-)

Index: linux-2.6.17-rc4-git1/drivers/input/ff-effects.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.17-rc4-git1/drivers/input/ff-effects.c	2006-05-14 00:14:44.000000000 +0300
@@ -0,0 +1,751 @@
+/*
+ *  Force feedback support for Linux input subsystem
+ *
+ *  Copyright (c) 2006 Anssi Hannula <anssi.hannula@gmail.com>
+ */
+
+/*
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/sched.h>
+
+#include "fixp-arith.h"
+
+/* #define DEBUG */
+
+#ifdef DEBUG
+#define debug(format, arg...) printk(KERN_DEBUG "ff-effects: " format "\n" , ## arg)
+#else
+#define debug(format, arg...) do {} while (0)
+#endif
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Anssi Hannula <anssi.hannula@gmail.com>");
+MODULE_DESCRIPTION("Force feedback support for input subsystem");
+
+/* Number of effects handled with memoryless devices */
+#define FF_MEMLESS_EFFECTS	16
+
+/* Envelope update interval in ms */
+#define FF_ENVELOPE_INTERVAL	50
+
+EXPORT_SYMBOL(input_ff_allocate);
+EXPORT_SYMBOL(input_ff_register);
+EXPORT_SYMBOL(input_ff_upload);
+EXPORT_SYMBOL(input_ff_erase);
+
+#define spin_ff_cond_lock(_ff, _flags)					  \
+	do {								  \
+		if (!_ff->driver->playback)				  \
+			spin_lock_irqsave(&_ff->atomiclock, _flags);	  \
+	} while (0);
+
+#define spin_ff_cond_unlock(_ff, _flags)					  \
+	do {								  \
+		if (!_ff->driver->playback)				  \
+			spin_unlock_irqrestore(&_ff->atomiclock, _flags); \
+	} while (0);
+
+static int input_ff_effect_access(struct input_dev *dev, int id, int override)
+{
+	struct ff_device *ff = dev->ff;
+	if (id < dev->ff_effects_max && id >= 0 && test_bit(FF_EFFECT_USED, ff->effects[id].flags))
+		if (override || ff->effects[id].owner == current->pid)
+			return 0;
+		else
+			return -EACCES;
+	else
+		return -EINVAL;
+}
+
+static int input_ff_envelope_time(struct ff_effect_status *effect, struct ff_envelope *envelope, unsigned long *event_time)
+{
+	unsigned long fade_start;
+	if (!envelope)
+		return 0;
+
+	if (envelope->attack_length && time_after(effect->play_at + msecs_to_jiffies(envelope->attack_length), effect->adj_at)) {
+		*event_time = effect->adj_at + msecs_to_jiffies(FF_ENVELOPE_INTERVAL);
+		return 1;
+	}
+
+	if (!envelope->fade_length || !effect->effect.replay.length)
+		return 0;
+
+	fade_start = effect->stop_at - msecs_to_jiffies(envelope->fade_length);
+
+	if (time_after(fade_start, effect->adj_at)) {
+		*event_time = fade_start;
+		return 1;
+	}
+
+	*event_time = effect->adj_at + msecs_to_jiffies(FF_ENVELOPE_INTERVAL);
+	if (time_after(*event_time, effect->stop_at))
+		*event_time = effect->stop_at;
+
+	return 1;
+}
+
+static void input_ff_calc_timer(struct ff_device *ff)
+{
+	int i;
+	int events = 0;
+	unsigned long next_time = 0;
+	debug("calculating next timer");
+	for (i = 0; i < FF_MEMLESS_EFFECTS; ++i) {
+		unsigned long event_time;
+		struct ff_envelope *envelope = NULL;
+		int event_set = 0;
+
+		if (!test_bit(FF_EFFECT_STARTED, ff->effects[i].flags)) {
+			if (test_bit(FF_EFFECT_PLAYING, ff->effects[i].flags)) {
+				event_time = ff->effects[i].stop_at = jiffies;
+				event_set = 1;
+			} else
+				continue;
+		} else {
+			if (!test_bit(FF_EFFECT_PLAYING, ff->effects[i].flags)) {
+				event_time = ff->effects[i].play_at;
+				event_set = 1;
+			} else {
+				if (ff->effects[i].effect.type == FF_PERIODIC)
+					envelope = &ff->effects[i].effect.u.periodic.envelope;
+				else if (ff->effects[i].effect.type == FF_CONSTANT)
+					envelope = &ff->effects[i].effect.u.constant.envelope;
+
+				event_set = input_ff_envelope_time(&ff->effects[i], envelope, &event_time);
+				if (!event_set && ff->effects[i].effect.replay.length) {
+					event_time = ff->effects[i].stop_at;
+					event_set = 1;
+				}
+			}
+		}
+
+		if (!event_set)
+			continue;
+		events++;
+
+		if (time_after(jiffies, event_time)) {
+			event_time = jiffies;
+			break;
+		}
+		if (events == 1)
+			next_time = event_time;
+		else {
+			if (time_after(next_time, event_time))
+				next_time = event_time;
+		}
+	}
+	if (!events) {
+		debug("no actions");
+		del_timer(&ff->timer);
+	} else {
+		debug("timer set");
+		mod_timer(&ff->timer, next_time);
+	}
+}
+
+static u16 input_ff_unsign(s16 value)
+{
+	if (value == -0x8000)
+		return 0x7fff;
+
+	return (value < 0 ? -value : value);
+}
+
+static int input_ff_envelope(struct ff_effect_status *effect, int value, struct ff_envelope *envelope)
+{
+	unsigned long current_time = jiffies;
+	unsigned int time_from_level;
+	unsigned int time_of_envelope;
+	unsigned int envelope_level;
+	int difference;
+
+	if (envelope->attack_length && time_after(effect->play_at + msecs_to_jiffies(envelope->attack_length),
+						  current_time)) {
+		debug("value = 0x%x, attack_level = 0x%x", value, envelope->attack_level);
+		time_from_level = current_time - effect->play_at;
+		time_of_envelope = msecs_to_jiffies(envelope->attack_length);
+		envelope_level = envelope->attack_level > 0x7fff ? 0x7fff : envelope->attack_level;
+	} else if (envelope->fade_length && effect->effect.replay.length &&
+			  time_after(current_time, effect->stop_at - msecs_to_jiffies(envelope->fade_length)) &&
+			  time_after(effect->stop_at, current_time)) {
+		time_from_level = effect->stop_at - current_time;
+		time_of_envelope = msecs_to_jiffies(envelope->fade_length);
+		envelope_level = envelope->fade_level > 0x7fff ? 0x7fff : envelope->fade_level;
+	} else {
+		return value;
+	}
+
+	difference = abs(value) - envelope_level;
+
+	debug("difference = %d", difference);
+	debug("time_from_level = 0x%x", time_from_level);
+	debug("time_of_envelope = 0x%x", time_of_envelope);
+	if (difference < 0)
+		difference = -((-difference) * time_from_level / time_of_envelope);
+	else
+		difference = difference * time_from_level / time_of_envelope;
+
+	debug("difference = %d", difference);
+
+	if (value < 0)
+		return -(difference + envelope_level);
+
+	return difference + envelope_level;
+}
+
+static int input_ff_emu_effect_type(struct input_dev *dev, int effect_type) {
+
+	if (test_bit(effect_type, dev->ff->flags))
+		return effect_type;
+
+	if (effect_type == FF_PERIODIC && test_bit(FF_RUMBLE, dev->ff->flags))
+		return FF_RUMBLE;
+
+	printk(KERN_ERR "ff-effects: invalid type in input_ff_emu_effect_type()\n");
+	return 0;
+}
+
+static int input_ff_safe_sum(int a, int b, int limit) {
+	int c;
+	if (!a)
+		return b;
+	c = a + b;
+	if (c > limit)
+		return limit;
+	return c;
+}
+
+static s8 input_ff_s8_sum(int a, int b) {
+	int c;
+	c = input_ff_safe_sum(a, b, 0x7f);
+	if (c < -0x80)
+		return -0x80;
+	return c;
+}
+
+static void input_ff_convert_effect(struct input_dev *dev, struct ff_effect *effect)
+{
+	int strong_magnitude, weak_magnitude;
+
+	if (effect->type == FF_RUMBLE && test_bit(FF_PERIODIC, dev->ff->flags)) {
+		/* Strong magnitude as 2/3 full and weak 1/3 full */
+		/* Also divide by 2 */
+		strong_magnitude = effect->u.rumble.strong_magnitude * 2 / 6;
+		weak_magnitude = effect->u.rumble.weak_magnitude / 6;
+
+		effect->type = FF_PERIODIC;
+
+		effect->u.periodic.waveform = FF_SINE;
+		effect->u.periodic.period = 50;
+		effect->u.periodic.magnitude = input_ff_safe_sum(strong_magnitude, weak_magnitude, 0x7fff);
+		effect->u.periodic.offset = 0;
+		effect->u.periodic.phase = 0;
+		effect->u.periodic.envelope.attack_length = 0;
+		effect->u.periodic.envelope.attack_level = 0;
+		effect->u.periodic.envelope.fade_length = 0;
+		effect->u.periodic.envelope.fade_level = 0;
+		return;
+	}
+
+
+	printk(KERN_ERR "ff-effects: invalid effect type in convert_effect()\n");
+}
+
+static void input_ff_sum_effect(struct ff_effect *effect, struct ff_effect_status *new, unsigned int gain)
+{
+	unsigned int strong, weak, i;
+	int x, y;
+	fixp_t level;
+	switch (new->effect.type) {
+		case FF_CONSTANT:
+			if (effect->type != FF_CONSTANT)
+				break;
+			i = new->effect.direction * 360 / 0xffff;
+			level = fixp_new16(input_ff_envelope(new, new->effect.u.constant.level,
+					   &new->effect.u.constant.envelope));
+			x = fixp_mult(fixp_sin(i), level) * gain / 0xffff;
+			y = fixp_mult(-fixp_cos(i), level) * gain / 0xffff;
+			/* here we abuse ff_ramp to hold x and y of constant force */
+			/* If in future any driver wants something else than
+			x and y in s8, this should be changed to something more generic */
+			effect->u.ramp.start_level = input_ff_s8_sum(effect->u.rumble.strong_magnitude, x);
+			effect->u.ramp.end_level = input_ff_s8_sum(effect->u.rumble.weak_magnitude, y);
+			return;
+		case FF_RUMBLE:
+			if (effect->type != FF_RUMBLE)
+				break;
+			strong = new->effect.u.rumble.strong_magnitude * gain / 0xffff;
+			weak = new->effect.u.rumble.weak_magnitude * gain / 0xffff;
+			effect->u.rumble.strong_magnitude =
+					input_ff_safe_sum(effect->u.rumble.strong_magnitude, strong, 0xffff);
+			effect->u.rumble.weak_magnitude =
+					input_ff_safe_sum(effect->u.rumble.weak_magnitude, weak, 0xffff);
+			return;
+		case FF_PERIODIC:
+			if (effect->type != FF_RUMBLE)
+				break;
+			/* very small period values should lessen the magnitude */
+			i = input_ff_unsign(new->effect.u.periodic.magnitude);
+			i = input_ff_envelope(new, i, &new->effect.u.periodic.envelope);
+			/* here we also scale it 0x7fff => 0xffff */
+			i = i * gain / 0x7fff;
+
+			effect->u.rumble.strong_magnitude =
+					input_ff_safe_sum(effect->u.rumble.strong_magnitude, i, 0xffff);
+			effect->u.rumble.weak_magnitude =
+					input_ff_safe_sum(effect->u.rumble.weak_magnitude, i, 0xffff);
+			return;
+	}
+
+	printk(KERN_ERR "ff-effects: invalid type in input_ff_sum_effect()\n");
+}
+
+static void input_ff_timer(unsigned long timer_data)
+{
+	struct input_dev *dev = (struct input_dev *) timer_data;
+	struct ff_device *ff = dev->ff;
+	struct ff_effect effect;
+	int i;
+	unsigned long flags;
+	int effects_pending;
+	unsigned long effect_handled[NBITS(FF_EFFECTS_MAX)];
+	int effect_type;
+	int safety;
+
+	debug("timer: updating effects");
+
+	spin_lock_irqsave(&ff->atomiclock, flags);
+
+	memset(effect_handled, 0, sizeof(effect_handled));
+	safety = 0;
+	effects_pending = 1;
+
+	while (effects_pending) {
+		if (safety++ > 50) {
+			printk(KERN_ERR "ff-effects: update aborted to avoid deadlock\n");
+			break;
+		}
+		effects_pending = 0;
+		memset(&effect, 0, sizeof(effect));
+		for (i = 0; i < dev->ff_effects_max; i++) {
+
+			if (test_bit(i, effect_handled))
+				continue;
+
+			if (!test_bit(FF_EFFECT_STARTED, ff->effects[i].flags)) {
+				if (!test_bit(FF_EFFECT_PLAYING, ff->effects[i].flags))
+					continue;
+				if (!effect.type) {
+					effect.type = input_ff_emu_effect_type(dev, ff->effects[i].effect.type);
+					clear_bit(FF_EFFECT_PLAYING, ff->effects[i].flags);
+					set_bit(i, effect_handled);
+					continue;
+				}
+				if (effect.type == input_ff_emu_effect_type(dev, ff->effects[i].effect.type)) {
+					clear_bit(FF_EFFECT_PLAYING, ff->effects[i].flags);
+					set_bit(i, effect_handled);
+					continue;
+				}
+				debug("effects pending: case 1");
+				effects_pending = 1;
+				continue;
+			}
+
+			if (time_after(ff->effects[i].play_at, jiffies)) {
+				set_bit(i, effect_handled);
+				continue;
+			}
+
+			effect_type = input_ff_emu_effect_type(dev, ff->effects[i].effect.type);
+
+			if (effect.type != effect_type) {
+				if (effect.type) {
+					debug("effects pending: case 2");
+					effects_pending = 1;
+					continue;
+				}
+				effect.type = effect_type;
+			}
+
+			if (ff->effects[i].effect.replay.length && time_after_eq(jiffies, ff->effects[i].stop_at)) {
+
+				clear_bit(FF_EFFECT_PLAYING, ff->effects[i].flags);
+
+				if (--ff->effects[i].count <= 0) {
+					clear_bit(FF_EFFECT_STARTED, ff->effects[i].flags);
+					set_bit(i, effect_handled);
+					continue;
+				} else {
+					ff->effects[i].play_at =
+							jiffies + msecs_to_jiffies(ff->effects[i].effect.replay.delay);
+					ff->effects[i].stop_at =
+							dev->ff->effects[i].play_at +
+							msecs_to_jiffies(ff->effects[i].effect.replay.length);
+					set_bit(i, effect_handled);
+					continue;
+				}
+			}
+
+			set_bit(FF_EFFECT_PLAYING, ff->effects[i].flags);
+
+			input_ff_sum_effect(&effect, &ff->effects[i], ff->gain);
+			ff->effects[i].adj_at = jiffies;
+			set_bit(i, effect_handled);
+		}
+
+		ff->driver->upload(dev, &effect, NULL);
+	}
+
+	input_ff_calc_timer(ff);
+	spin_unlock_irqrestore(&ff->atomiclock, flags);
+
+}
+
+static int _input_ff_erase(struct input_dev *dev, int id, int override)
+{
+	int ret = input_ff_effect_access(dev, id, override);
+	if (ret)
+		return ret;
+	if (dev->ff->driver->playback) {
+		dev->ff->driver->playback(dev, id, 0);
+		ret = dev->ff->driver->erase(dev, id);
+		if (!ret)
+			clear_bit(FF_EFFECT_USED, dev->ff->effects[id].flags);
+	} else {
+		if (test_and_clear_bit(FF_EFFECT_STARTED, dev->ff->effects[id].flags))
+			input_ff_calc_timer(dev->ff);
+		clear_bit(FF_EFFECT_USED, dev->ff->effects[id].flags);
+	}
+	return ret;
+}
+
+int input_ff_erase(struct input_dev *dev, int id)
+{
+	struct ff_device *ff;
+	unsigned long flags = 0;
+	int ret;
+	if (!test_bit(EV_FF, dev->evbit))
+		return -EINVAL;
+	mutex_lock(&dev->ff_lock);
+	ff = dev->ff;
+	if (!ff) {
+		mutex_unlock(&dev->ff_lock);
+		return -ENODEV;
+	}
+	spin_ff_cond_lock(ff, flags);
+	ret = _input_ff_erase(dev, id, current->pid == 0);
+	spin_ff_cond_unlock(ff, flags);
+
+	mutex_unlock(&dev->ff_lock);
+	return ret;
+}
+
+static int input_ff_flush(struct input_dev *dev, struct file *file)
+{
+	struct ff_device *ff;
+	unsigned long flags = 0;
+	int i;
+	debug("flushing now");
+	mutex_lock(&dev->ff_lock);
+	ff = dev->ff;
+	if (!ff) {
+		mutex_unlock(&dev->ff_lock);
+		return -ENODEV;
+	}
+	spin_ff_cond_lock(ff, flags);
+	for (i = 0; i < dev->ff_effects_max; i++) {
+		_input_ff_erase(dev, i, 0);
+	}
+	spin_ff_cond_unlock(ff, flags);
+	mutex_unlock(&dev->ff_lock);
+	return 0;
+}
+
+
+static int input_ff_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
+{
+
+	struct ff_device *ff = dev->ff;
+	int ret = 0;
+	unsigned long flags = 0;
+	if (value < 0)
+		return -EINVAL;
+	/* Mutex is already locked in input.c as input_ff_event address is
+	   stored in ff_device struct */
+	spin_ff_cond_lock(ff, flags);
+
+	if (code == FF_GAIN) {
+		int i;
+		if (!test_bit(FF_GAIN, dev->ffbit) || value > 0xffff || value < 0) {
+			ret = -EINVAL;
+		} else if (ff->driver->set_gain) {
+			ff->driver->set_gain(dev, value);
+		} else if (!ff->driver->playback) {
+			ff->gain = value;
+			for (i = 0; i < dev->ff_effects_max; i++)
+				clear_bit(FF_EFFECT_PLAYING, ff->effects[i].flags);
+			input_ff_calc_timer(ff);
+		} else {
+			ret = -ENOSYS;
+		}
+		spin_ff_cond_unlock(ff, flags);
+		return ret;
+	}
+
+	if (code == FF_AUTOCENTER) {
+		if (!test_bit(FF_AUTOCENTER, dev->ffbit) || value > 0xffff || value < 0) {
+			ret = -EINVAL;
+		} else if (ff->driver->set_autocenter) {
+			ff->driver->set_autocenter(dev, value);
+			ret = 0;
+		} else {
+			ret = -ENOSYS;
+		}
+		spin_ff_cond_unlock(ff, flags);
+		return ret;
+	}
+
+
+	ret = input_ff_effect_access(dev, code, 1);
+	if (ret) {
+		spin_ff_cond_unlock(ff, flags);
+		return ret;
+	}
+
+	if (ff->driver->playback) {
+		ret = ff->driver->playback(dev, code, value);
+		return ret;
+	}
+
+	if (value > 0) {
+		debug("initiated play");
+		set_bit(FF_EFFECT_STARTED, ff->effects[code].flags);
+		clear_bit(FF_EFFECT_PLAYING, ff->effects[code].flags);
+		ff->effects[code].count = value;
+		ff->effects[code].play_at = jiffies +
+				msecs_to_jiffies(ff->effects[code].effect.replay.delay);
+		ff->effects[code].stop_at = ff->effects[code].play_at +
+				msecs_to_jiffies(ff->effects[code].effect.replay.length);
+		ff->effects[code].adj_at = ff->effects[code].play_at;
+	} else {
+		debug("initiated stop");
+		clear_bit(FF_EFFECT_STARTED, ff->effects[code].flags);
+	}
+	input_ff_calc_timer(ff);
+
+	spin_unlock_irqrestore(&ff->atomiclock, flags);
+	return 0;
+}
+
+#define VALID_EFFECT_MIN	FF_RUMBLE
+#define VALID_EFFECT_MAX	FF_RAMP
+#define VALID_WFORM_MIN		FF_SQUARE
+#define VALID_WFORM_MAX		FF_CUSTOM
+static int input_ff_validate_effect(struct ff_effect *effect)
+{
+	if (effect->type < VALID_EFFECT_MIN || effect->type > VALID_EFFECT_MAX)
+		return -EINVAL;
+	if (effect->type == FF_PERIODIC &&
+		   (effect->u.periodic.waveform < VALID_WFORM_MIN ||
+		   effect->u.periodic.waveform > VALID_WFORM_MAX))
+		return -EINVAL;
+	return 0;
+}
+
+int input_ff_upload(struct input_dev *dev, struct ff_effect *effect)
+{
+	struct ff_device *ff;
+	unsigned long flags = 0;
+	int ret = 0; int id;
+
+	if (!test_bit(EV_FF, dev->evbit) || input_ff_validate_effect(effect) ||
+		    !test_bit(effect->type, dev->ffbit) ||
+		    (effect->type == FF_PERIODIC &&
+				    !test_bit(effect->u.periodic.waveform, dev->ffbit))) {
+		debug("invalid upload");
+		return -EINVAL;
+	}
+
+	mutex_lock(&dev->ff_lock);
+	ff = dev->ff;
+	if (!ff) {
+		mutex_unlock(&dev->ff_lock);
+		return -ENODEV;
+	}
+	spin_ff_cond_lock(ff, flags);
+
+	if (effect->id == -1) {
+		for (id = 0; id < dev->ff_effects_max && test_bit(FF_EFFECT_USED, ff->effects[id].flags); id++);
+
+		if (id >= dev->ff_effects_max) {
+			spin_ff_cond_unlock(ff, flags);
+			mutex_unlock(&dev->ff_lock);
+			return -ENOSPC;
+		}
+
+		effect->id = id;
+		ff->effects[id].owner = current->pid;
+		ff->effects[id].flags[0] = 0;
+		ff->effects[id].effect = *effect;
+
+		if (ff->driver->playback) {
+			if (!test_bit(effect->type, ff->flags))
+				input_ff_convert_effect(dev, effect);
+			ret = ff->driver->upload(dev, effect, NULL);
+			if (!ret)
+				set_bit(FF_EFFECT_USED, ff->effects[id].flags);
+			mutex_unlock(&dev->ff_lock);
+			return ret;
+		}
+		set_bit(FF_EFFECT_USED, ff->effects[id].flags);
+
+	} else {
+		id = effect->id;
+
+		ret = input_ff_effect_access(dev, id, 1);
+		if (ret) {
+			spin_ff_cond_unlock(ff, flags);
+			mutex_unlock(&dev->ff_lock);
+			return ret;
+		}
+
+		if (effect->type != ff->effects[id].effect.type ||
+				  (effect->type == FF_PERIODIC && effect->u.periodic.waveform !=
+				  ff->effects[id].effect.u.periodic.waveform)) {
+			spin_ff_cond_unlock(ff, flags);
+			mutex_unlock(&dev->ff_lock);
+			return -EINVAL;
+		}
+
+		if (ff->driver->playback) {
+			if (!test_bit(effect->type, ff->flags))
+				input_ff_convert_effect(dev, effect);
+			ret = ff->driver->upload(dev, effect, &ff->effects[id].effect);
+			ff->effects[id].effect = *effect;
+			mutex_unlock(&dev->ff_lock);
+			return ret;
+		}
+		ff->effects[id].effect = *effect;
+		clear_bit(FF_EFFECT_PLAYING, ff->effects[id].flags);
+
+	}
+
+	spin_unlock_irqrestore(&ff->atomiclock, flags);
+	mutex_unlock(&dev->ff_lock);
+	return ret;
+}
+
+int input_ff_allocate(struct input_dev *dev)
+{
+	debug("allocating device");
+	mutex_lock(&dev->ff_lock);
+	if (dev->ff)
+		printk(KERN_ERR "ff-effects: allocating to non-NULL pointer\n");
+	dev->ff = kzalloc(sizeof(*dev->ff), GFP_KERNEL);
+	if (!dev->ff) {
+		mutex_unlock(&dev->ff_lock);
+		return -ENOMEM;
+	}
+	spin_lock_init(&dev->ff->atomiclock);
+	init_timer(&dev->ff->timer);
+	dev->ff->timer.function = input_ff_timer;
+	dev->ff->timer.data = (unsigned long) dev;
+	dev->ff->event = input_ff_event;
+	mutex_unlock(&dev->ff_lock);
+	debug("ff allocated");
+	return 0;
+}
+
+int input_ff_register(struct input_dev *dev, struct ff_driver *driver)
+{
+	debug("registering device");
+	if (!dev->ff) {
+		printk(KERN_ERR "ff-effects: tried to register before allocate\n");
+		return -EPERM;
+	}
+	if (driver->playback && !dev->ff_effects_max) {
+		printk(KERN_ERR "ff-effects: cannot register a device with 0 effects\n");
+		return -EINVAL;
+	}
+	mutex_lock(&dev->ff_lock);
+
+	dev->flush = input_ff_flush;
+	dev->ff->driver = driver;
+
+	if (!dev->ff->driver->playback) {
+		dev->ff_effects_max = FF_MEMLESS_EFFECTS;
+		dev->ff->gain = 0xffff;
+		set_bit(FF_GAIN, dev->ffbit);
+	}
+
+	if (test_bit(FF_RUMBLE, dev->ff->flags)) {
+		debug("device has rumble");
+		set_bit(FF_RUMBLE, dev->ffbit);
+		if (!dev->ff->driver->playback) {
+			set_bit(FF_PERIODIC, dev->ffbit);
+			set_bit(FF_SINE, dev->ffbit);
+			set_bit(FF_TRIANGLE, dev->ffbit);
+			set_bit(FF_SQUARE, dev->ffbit);
+		}
+	}
+
+	if (test_bit(FF_PERIODIC, dev->ff->flags)) {
+		set_bit(FF_PERIODIC, dev->ffbit);
+		if (dev->ff->driver->playback)
+			set_bit(FF_RUMBLE, dev->ffbit);
+	}
+
+	if (test_bit(FF_CONSTANT, dev->ff->flags))
+		set_bit(FF_CONSTANT, dev->ffbit);
+	if (test_bit(FF_SPRING, dev->ff->flags))
+		set_bit(FF_SPRING, dev->ffbit);
+	if (test_bit(FF_FRICTION, dev->ff->flags))
+		set_bit(FF_FRICTION, dev->ffbit);
+	if (test_bit(FF_DAMPER, dev->ff->flags))
+		set_bit(FF_DAMPER, dev->ffbit);
+	if (test_bit(FF_INERTIA, dev->ff->flags))
+		set_bit(FF_INERTIA, dev->ffbit);
+	if (test_bit(FF_RAMP, dev->ff->flags))
+		set_bit(FF_RAMP, dev->ffbit);
+	if (test_bit(FF_SQUARE, dev->ff->flags))
+		set_bit(FF_SQUARE, dev->ffbit);
+	if (test_bit(FF_TRIANGLE, dev->ff->flags))
+		set_bit(FF_TRIANGLE, dev->ffbit);
+	if (test_bit(FF_SINE, dev->ff->flags))
+		set_bit(FF_SINE, dev->ffbit);
+	if (test_bit(FF_SAW_UP, dev->ff->flags))
+		set_bit(FF_SAW_UP, dev->ffbit);
+	if (test_bit(FF_SAW_DOWN, dev->ff->flags))
+		set_bit(FF_SAW_DOWN, dev->ffbit);
+	if (test_bit(FF_CUSTOM, dev->ff->flags))
+		set_bit(FF_CUSTOM, dev->ffbit);
+	if (test_bit(FF_GAIN, dev->ff->flags))
+		set_bit(FF_GAIN, dev->ffbit);
+	if (test_bit(FF_AUTOCENTER, dev->ff->flags))
+		set_bit(FF_AUTOCENTER, dev->ffbit);
+
+ 	set_bit(EV_FF, dev->evbit);
+	mutex_unlock(&dev->ff_lock);
+
+	return 0;
+}
+
Index: linux-2.6.17-rc4-git1/drivers/input/Kconfig
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/Kconfig	2006-03-20 07:53:29.000000000 +0200
+++ linux-2.6.17-rc4-git1/drivers/input/Kconfig	2006-05-14 02:28:42.000000000 +0300
@@ -24,6 +24,14 @@ config INPUT
 
 if INPUT
 
+config INPUT_FF_EFFECTS
+	tristate "Force feedback effects"
+	help
+	  Say Y here if you want to be able to play force feedback effects.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ff-effects.
+
 comment "Userland interfaces"
 
 config INPUT_MOUSEDEV
@@ -110,6 +118,7 @@ config INPUT_TSDEV_SCREEN_Y
 
 config INPUT_EVDEV
 	tristate "Event interface"
+	depends on INPUT_FF_EFFECTS || INPUT_FF_EFFECTS=n
 	help
 	  Say Y here if you want your input device events be accessible
 	  under char device 13:64+ - /dev/input/eventX in a generic way.
Index: linux-2.6.17-rc4-git1/drivers/input/Makefile
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/Makefile	2006-03-20 07:53:29.000000000 +0200
+++ linux-2.6.17-rc4-git1/drivers/input/Makefile	2006-05-14 00:14:44.000000000 +0300
@@ -5,6 +5,7 @@
 # Each configuration option enables a list of files.
 
 obj-$(CONFIG_INPUT)		+= input.o
+obj-$(CONFIG_INPUT_FF_EFFECTS)	+= ff-effects.o
 obj-$(CONFIG_INPUT_MOUSEDEV)	+= mousedev.o
 obj-$(CONFIG_INPUT_JOYDEV)	+= joydev.o
 obj-$(CONFIG_INPUT_EVDEV)	+= evdev.o
Index: linux-2.6.17-rc4-git1/include/linux/input.h
===================================================================
--- linux-2.6.17-rc4-git1.orig/include/linux/input.h	2006-05-13 19:59:03.000000000 +0300
+++ linux-2.6.17-rc4-git1/include/linux/input.h	2006-05-14 01:36:20.000000000 +0300
@@ -865,6 +865,9 @@ struct input_dev {
 	unsigned long sndbit[NBITS(SND_MAX)];
 	unsigned long ffbit[NBITS(FF_MAX)];
 	unsigned long swbit[NBITS(SW_MAX)];
+
+	struct ff_device *ff;
+	struct mutex ff_lock;
 	int ff_effects_max;
 
 	unsigned int keycodemax;
@@ -1000,6 +1003,53 @@ struct input_handle {
 #define to_handle(n) container_of(n,struct input_handle,d_node)
 #define to_handle_h(n) container_of(n,struct input_handle,h_node)
 
+#define FF_EFFECTS_MAX 64
+
+struct ff_driver {
+	int (*upload)(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old);
+	int (*erase)(struct input_dev *dev, int effect_id);
+	int (*playback)(struct input_dev *dev, int effect_id, int value);
+	void (*set_gain)(struct input_dev *dev, u16 gain);
+	void (*set_autocenter)(struct input_dev *dev, u16 magnitude);
+};
+
+#define FF_EFFECT_USED		0
+#define FF_EFFECT_STARTED	1
+#define FF_EFFECT_PLAYING	2
+
+struct ff_effect_status {
+	pid_t owner;
+	struct ff_effect effect;
+	unsigned long flags[1];
+	int count;
+	unsigned long play_at;
+	unsigned long stop_at;
+	unsigned long adj_at;
+};
+
+struct ff_device {
+	int (*event)(struct input_dev *dev, unsigned int type, unsigned int code, int value);
+	unsigned long flags[NBITS(FF_MAX)];
+	u16 gain;
+	spinlock_t atomiclock;
+	struct ff_driver *driver;
+	struct timer_list timer;
+	struct ff_effect_status effects[FF_EFFECTS_MAX];
+};
+
+#if defined(CONFIG_INPUT_FF_EFFECTS_MODULE) || defined(CONFIG_INPUT_FF_EFFECTS)
+int input_ff_allocate(struct input_dev *dev);
+int input_ff_register(struct input_dev *dev, struct ff_driver *driver);
+int input_ff_upload(struct input_dev *dev, struct ff_effect *effect);
+int input_ff_erase(struct input_dev *dev, int id);
+#else
+static inline int input_ff_allocate(struct input_dev *dev) { return -ENOSYS; }
+static inline int input_ff_register(struct input_dev *dev, struct ff_driver *driver) { return -ENOSYS; }
+static inline int input_ff_upload(struct input_dev *dev, struct ff_effect *effect) { return -EINVAL; }
+static inline int input_ff_erase(struct input_dev *dev, int id) { return -EINVAL; }
+#endif
+
+
 static inline void init_input_dev(struct input_dev *dev)
 {
 	INIT_LIST_HEAD(&dev->h_list);
Index: linux-2.6.17-rc4-git1/drivers/input/input.c
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/input.c	2006-05-13 19:58:22.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/input/input.c	2006-05-14 00:14:44.000000000 +0300
@@ -172,7 +172,10 @@ void input_event(struct input_dev *dev, 
 			break;
 
 		case EV_FF:
-			if (dev->event) dev->event(dev, type, code, value);
+			mutex_lock(&dev->ff_lock);
+			if (dev->ff)
+				dev->ff->event(dev, type, code, value);
+			mutex_unlock(&dev->ff_lock);
 			break;
 	}
 
@@ -733,6 +736,17 @@ static void input_dev_release(struct cla
 {
 	struct input_dev *dev = to_input_dev(class_dev);
 
+	if (dev->ff) {
+		struct ff_device *ff = dev->ff;
+		clear_bit(EV_FF, dev->evbit);
+		mutex_lock(&dev->ff_lock);
+		del_timer_sync(&ff->timer);
+		dev->flush = NULL;
+		dev->ff = NULL;
+		kfree(ff);
+		mutex_unlock(&dev->ff_lock);
+	}
+
 	kfree(dev);
 	module_put(THIS_MODULE);
 }
@@ -874,6 +888,7 @@ struct input_dev *input_allocate_device(
 		class_device_initialize(&dev->cdev);
 		INIT_LIST_HEAD(&dev->h_list);
 		INIT_LIST_HEAD(&dev->node);
+		mutex_init(&dev->ff_lock);
 	}
 
 	return dev;
Index: linux-2.6.17-rc4-git1/drivers/input/evdev.c
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/evdev.c	2006-05-13 19:58:22.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/input/evdev.c	2006-05-14 00:14:44.000000000 +0300
@@ -458,24 +458,20 @@ static long evdev_ioctl_handler(struct f
 			return 0;
 
 		case EVIOCSFF:
-			if (dev->upload_effect) {
+			{
 				struct ff_effect effect;
 				int err;
 
 				if (copy_from_user(&effect, p, sizeof(effect)))
 					return -EFAULT;
-				err = dev->upload_effect(dev, &effect);
+				err = input_ff_upload(dev, &effect);
 				if (put_user(effect.id, &(((struct ff_effect __user *)p)->id)))
 					return -EFAULT;
 				return err;
-			} else
-				return -ENOSYS;
+			}
 
 		case EVIOCRMFF:
-			if (!dev->erase_effect)
-				return -ENOSYS;
-
-			return dev->erase_effect(dev, (int)(unsigned long) p);
+			return input_ff_erase(dev, (int)(unsigned long) p);
 
 		case EVIOCGEFFECTS:
 			if (put_user(dev->ff_effects_max, ip))

--
Anssi Hannula

Re: [patch 03/11] input: new force feedback interface

[Andrew Morton]

Anssi Hannula <anssi.hannula@gmail.com> wrote:
>
> Implement a new force feedback interface, in which all non-driver-specific
> operations are separated to a common module. This module handles effect type
> validations, effect timers, locking, etc.
> 
> As a result, support is added for gain and envelope for memoryless devices,
> periodic => rumble conversion for memoryless devices and rumble => periodic
> conversion for devices with periodic support instead of rumble support. Also
> the effect memory of devices is not emptied if the root user opens and closes
> the device while another user is using effects. This module also obsoletes
> some flawed locking and timer code in few ff drivers.
> 
> The module is named ff-effects. If INPUT_FF_EFFECTS is enabled, the force
> feedback drivers and interfaces (evdev) will be depending on it.
> 
> Userspace interface is left unaltered.
> 

Nice-looking patches.

>
> +#define spin_ff_cond_lock(_ff, _flags)					  \
> +	do {								  \
> +		if (!_ff->driver->playback)				  \
> +			spin_lock_irqsave(&_ff->atomiclock, _flags);	  \
> +	} while (0);
> +
> +#define spin_ff_cond_unlock(_ff, _flags)					  \
> +	do {								  \
> +		if (!_ff->driver->playback)				  \
> +			spin_unlock_irqrestore(&_ff->atomiclock, _flags); \
> +	} while (0);

Making these static inline functions would deuglify them a bit.

> +static int input_ff_effect_access(struct input_dev *dev, int id, int override)
> +{
> +	struct ff_device *ff = dev->ff;
> +	if (id < dev->ff_effects_max && id >= 0 && test_bit(FF_EFFECT_USED, ff->effects[id].flags))

Kernel does have an 80-columns rule, but input seems to have always spurned it.

> +static int input_ff_envelope_time(struct ff_effect_status *effect, struct ff_envelope *envelope, unsigned long *event_time)
> +{
> +	unsigned long fade_start;
> +	if (!envelope)
> +		return 0;
> +
> +	if (envelope->attack_length && time_after(effect->play_at + msecs_to_jiffies(envelope->attack_length), effect->adj_at)) {

Try using an 80-column wondow for a while ;)

> +		return value;
> +	}
> +
> +	difference = abs(value) - envelope_level;
> +
> +	debug("difference = %d", difference);
> +	debug("time_from_level = 0x%x", time_from_level);
> +	debug("time_of_envelope = 0x%x", time_of_envelope);
> +	if (difference < 0)
> +		difference = -((-difference) * time_from_level / time_of_envelope);
> +	else
> +		difference = difference * time_from_level / time_of_envelope;

You've checked there's no possibility of divide-by-zero here?

> +
> +static int input_ff_safe_sum(int a, int b, int limit) {

The opening brace goes in column zero, please.

> +	int c;
> +	if (!a)
> +		return b;
> +	c = a + b;
> +	if (c > limit)
> +		return limit;
> +	return c;
> +}
> +
> +static s8 input_ff_s8_sum(int a, int b) {

dittoes.

> +	int c;
> +	c = input_ff_safe_sum(a, b, 0x7f);
> +	if (c < -0x80)
> +		return -0x80;
> +	return c;
> +}
>
> ...
>
> +static void input_ff_timer(unsigned long timer_data)
> +{
> +	struct input_dev *dev = (struct input_dev *) timer_data;
> +	struct ff_device *ff = dev->ff;
> +	struct ff_effect effect;
> +	int i;
> +	unsigned long flags;
> +	int effects_pending;
> +	unsigned long effect_handled[NBITS(FF_EFFECTS_MAX)];

DECLARE_BITMAP would be more usual.  (Yes, it should have been called
DEFINE_BITMAP).

> +	int effect_type;
> +	int safety;
> +
> +	debug("timer: updating effects");
> +
> +	spin_lock_irqsave(&ff->atomiclock, flags);
> +
> +	memset(effect_handled, 0, sizeof(effect_handled));

You could take the lock after the memset.

> +int input_ff_erase(struct input_dev *dev, int id)
> +{
> +	struct ff_device *ff;
> +	unsigned long flags = 0;
> +	int ret;
> +	if (!test_bit(EV_FF, dev->evbit))
> +		return -EINVAL;
> +	mutex_lock(&dev->ff_lock);
> +	ff = dev->ff;
> +	if (!ff) {
> +		mutex_unlock(&dev->ff_lock);
> +		return -ENODEV;
> +	}
> +	spin_ff_cond_lock(ff, flags);
> +	ret = _input_ff_erase(dev, id, current->pid == 0);
> +	spin_ff_cond_unlock(ff, flags);
> +
> +	mutex_unlock(&dev->ff_lock);
> +	return ret;
> +}

Perhaps you meant `current->uid == 0' here.  There's no way in which pid
0 will call this code.

What's happening here anyway?  Why does this code need to know about pids?

Checking for uid==0 woud be a fishy thing to do as well.

> +static int input_ff_flush(struct input_dev *dev, struct file *file)
> +{
> +	struct ff_device *ff;
> +	unsigned long flags = 0;
> +	int i;
> +	debug("flushing now");
> +	mutex_lock(&dev->ff_lock);
> +	ff = dev->ff;
> +	if (!ff) {
> +		mutex_unlock(&dev->ff_lock);
> +		return -ENODEV;
> +	}
> +	spin_ff_cond_lock(ff, flags);
> +	for (i = 0; i < dev->ff_effects_max; i++) {
> +		_input_ff_erase(dev, i, 0);
> +	}

Unneeded braces.

> +	spin_ff_cond_unlock(ff, flags);
> +	mutex_unlock(&dev->ff_lock);
> +	return 0;
> +}
> +
> +
> +		ff->effects[id].flags[0] = 0;
> +		ff->effects[id].effect = *effect;
> +
> +		if (ff->driver->playback) {
> +			if (!test_bit(effect->type, ff->flags))
> +				input_ff_convert_effect(dev, effect);
> +			ret = ff->driver->upload(dev, effect, NULL);
> +			if (!ret)
> +				set_bit(FF_EFFECT_USED, ff->effects[id].flags);
> +			mutex_unlock(&dev->ff_lock);
> +			return ret;
> +		}
> +		set_bit(FF_EFFECT_USED, ff->effects[id].flags);
> +
> +	} else {
> +		id = effect->id;
> +
> +		ret = input_ff_effect_access(dev, id, 1);
> +		if (ret) {
> +			spin_ff_cond_unlock(ff, flags);
> +			mutex_unlock(&dev->ff_lock);
> +			return ret;
> +		}
> +
> +		if (effect->type != ff->effects[id].effect.type ||
> +				  (effect->type == FF_PERIODIC && effect->u.periodic.waveform !=
> +				  ff->effects[id].effect.u.periodic.waveform)) {
> +			spin_ff_cond_unlock(ff, flags);
> +			mutex_unlock(&dev->ff_lock);
> +			return -EINVAL;
> +		}
> +
> +		if (ff->driver->playback) {
> +			if (!test_bit(effect->type, ff->flags))
> +				input_ff_convert_effect(dev, effect);
> +			ret = ff->driver->upload(dev, effect, &ff->effects[id].effect);
> +			ff->effects[id].effect = *effect;
> +			mutex_unlock(&dev->ff_lock);
> +			return ret;

I think we're missing a spin_ff_cond_unlock() here?

> +		}
> +		ff->effects[id].effect = *effect;
> +		clear_bit(FF_EFFECT_PLAYING, ff->effects[id].flags);
> +
> +	}
> +
> +	spin_unlock_irqrestore(&ff->atomiclock, flags);
> +	mutex_unlock(&dev->ff_lock);
> +	return ret;
> +}

And here we have spin_unlock_irqrestore() instead of spin_ff_cond_unlock().

It would be best to convert this function to have a single return point. 
That tends to prevent problems like this from happening, and from creeping
in later on.

> +int input_ff_allocate(struct input_dev *dev)
> +{
> +	debug("allocating device");
> +	mutex_lock(&dev->ff_lock);
> +	if (dev->ff)
> +		printk(KERN_ERR "ff-effects: allocating to non-NULL pointer\n");
> +	dev->ff = kzalloc(sizeof(*dev->ff), GFP_KERNEL);
> +	if (!dev->ff) {
> +		mutex_unlock(&dev->ff_lock);
> +		return -ENOMEM;
> +	}
> +	spin_lock_init(&dev->ff->atomiclock);
> +	init_timer(&dev->ff->timer);
> +	dev->ff->timer.function = input_ff_timer;
> +	dev->ff->timer.data = (unsigned long) dev;
> +	dev->ff->event = input_ff_event;

setup_timer()

> +	mutex_unlock(&dev->ff_lock);
> +	debug("ff allocated");
> +	return 0;
> +}
> +
>
> ...
>
> Index: linux-2.6.17-rc4-git1/drivers/input/Kconfig
> ===================================================================
> --- linux-2.6.17-rc4-git1.orig/drivers/input/Kconfig	2006-03-20 07:53:29.000000000 +0200
> +++ linux-2.6.17-rc4-git1/drivers/input/Kconfig	2006-05-14 02:28:42.000000000 +0300
> @@ -24,6 +24,14 @@ config INPUT
>  
>  if INPUT
>  
> +config INPUT_FF_EFFECTS
> +	tristate "Force feedback effects"
> +	help
> +	  Say Y here if you want to be able to play force feedback effects.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ff-effects.

hm.  I'd have expected more dependencies than this.

>  comment "Userland interfaces"
>  
>  config INPUT_MOUSEDEV
> @@ -110,6 +118,7 @@ config INPUT_TSDEV_SCREEN_Y
>  
>  config INPUT_EVDEV
>  	tristate "Event interface"
> +	depends on INPUT_FF_EFFECTS || INPUT_FF_EFFECTS=n

Isn't that always true?

> +
> +struct ff_effect_status {
> +	pid_t owner;

This code is almost devoid of comments.  Those which it does have tend to
cover little low-level implementation details.  But it's the *big* things
which a reader is not able to learn from the implementation, and which
should be commented.  Like: why on earth does this code need to know about
pids?

> +#if defined(CONFIG_INPUT_FF_EFFECTS_MODULE) || defined(CONFIG_INPUT_FF_EFFECTS)

No, we shouldn't use CONFIG_FOO_MODULE.  We just don't know at compile-time
whether the user will later compile and insert a particular module.

> +		mutex_lock(&dev->ff_lock);
> +		del_timer_sync(&ff->timer);
> +		dev->flush = NULL;
> +		dev->ff = NULL;
> +		kfree(ff);
> +		mutex_unlock(&dev->ff_lock);

The kfree can be moved outside the lock.

Re: [patch 03/11] input: new force feedback interface

[Anssi Hannula]

Andrew Morton wrote:
> Anssi Hannula <anssi.hannula@gmail.com> wrote:
> 
>>Implement a new force feedback interface, in which all non-driver-specific
>>operations are separated to a common module. This module handles effect type
>>validations, effect timers, locking, etc.
>>
>>As a result, support is added for gain and envelope for memoryless devices,
>>periodic => rumble conversion for memoryless devices and rumble => periodic
>>conversion for devices with periodic support instead of rumble support. Also
>>the effect memory of devices is not emptied if the root user opens and closes
>>the device while another user is using effects. This module also obsoletes
>>some flawed locking and timer code in few ff drivers.
>>
>>The module is named ff-effects. If INPUT_FF_EFFECTS is enabled, the force
>>feedback drivers and interfaces (evdev) will be depending on it.
>>
>>Userspace interface is left unaltered.
>>
> 
> 
> Nice-looking patches.
> 

Thanks for looking and providing helpful comments :)

>>+#define spin_ff_cond_lock(_ff, _flags)					  \
>>+	do {								  \
>>+		if (!_ff->driver->playback)				  \
>>+			spin_lock_irqsave(&_ff->atomiclock, _flags);	  \
>>+	} while (0);
>>+
>>+#define spin_ff_cond_unlock(_ff, _flags)					  \
>>+	do {								  \
>>+		if (!_ff->driver->playback)				  \
>>+			spin_unlock_irqrestore(&_ff->atomiclock, _flags); \
>>+	} while (0);
> 
> 
> Making these static inline functions would deuglify them a bit.
> 

I put them like that because spin_lock_irqsave is a macro that uses the
local variable "flags".
But yes, I can probably pass that parameter to an inline function as a
pointer. I'll change that.


>>+static int input_ff_effect_access(struct input_dev *dev, int id, int override)
>>+{
>>+	struct ff_device *ff = dev->ff;
>>+	if (id < dev->ff_effects_max && id >= 0 && test_bit(FF_EFFECT_USED, ff->effects[id].flags))
> 
> 
> Kernel does have an 80-columns rule, but input seems to have always spurned it.
> 
> 
>>+static int input_ff_envelope_time(struct ff_effect_status *effect, struct ff_envelope *envelope, unsigned long *event_time)
>>+{
>>+	unsigned long fade_start;
>>+	if (!envelope)
>>+		return 0;
>>+
>>+	if (envelope->attack_length && time_after(effect->play_at + msecs_to_jiffies(envelope->attack_length), effect->adj_at)) {
> 
> 
> Try using an 80-column wondow for a while ;)

Okay, I'll make the lines shorter.

> 
> 
>>+		return value;
>>+	}
>>+
>>+	difference = abs(value) - envelope_level;
>>+
>>+	debug("difference = %d", difference);
>>+	debug("time_from_level = 0x%x", time_from_level);
>>+	debug("time_of_envelope = 0x%x", time_of_envelope);
>>+	if (difference < 0)
>>+		difference = -((-difference) * time_from_level / time_of_envelope);
>>+	else
>>+		difference = difference * time_from_level / time_of_envelope;
> 
> 
> You've checked there's no possibility of divide-by-zero here?
> 

Yes, the "time_of_envelope" is set a few lines above from
"envelope->attack_length" or "envelope->fade_length", and there is an if
block that checks they're non-zero.

>>+
>>+static int input_ff_safe_sum(int a, int b, int limit) {
> 
> 
> The opening brace goes in column zero, please.
> 
> 
>>+	int c;
>>+	if (!a)
>>+		return b;
>>+	c = a + b;
>>+	if (c > limit)
>>+		return limit;
>>+	return c;
>>+}
>>+
>>+static s8 input_ff_s8_sum(int a, int b) {
> 
> 
> dittoes.

Okay, will fix.

> 
>>+	int c;
>>+	c = input_ff_safe_sum(a, b, 0x7f);
>>+	if (c < -0x80)
>>+		return -0x80;
>>+	return c;
>>+}
>>
>>...
>>
>>+static void input_ff_timer(unsigned long timer_data)
>>+{
>>+	struct input_dev *dev = (struct input_dev *) timer_data;
>>+	struct ff_device *ff = dev->ff;
>>+	struct ff_effect effect;
>>+	int i;
>>+	unsigned long flags;
>>+	int effects_pending;
>>+	unsigned long effect_handled[NBITS(FF_EFFECTS_MAX)];
> 
> 
> DECLARE_BITMAP would be more usual.  (Yes, it should have been called
> DEFINE_BITMAP).
> 

Ok.

>>+	int effect_type;
>>+	int safety;
>>+
>>+	debug("timer: updating effects");
>>+
>>+	spin_lock_irqsave(&ff->atomiclock, flags);
>>+
>>+	memset(effect_handled, 0, sizeof(effect_handled));
> 
> 
> You could take the lock after the memset.
> 

Ok.

>>+int input_ff_erase(struct input_dev *dev, int id)
>>+{
>>+	struct ff_device *ff;
>>+	unsigned long flags = 0;
>>+	int ret;
>>+	if (!test_bit(EV_FF, dev->evbit))
>>+		return -EINVAL;
>>+	mutex_lock(&dev->ff_lock);
>>+	ff = dev->ff;
>>+	if (!ff) {
>>+		mutex_unlock(&dev->ff_lock);
>>+		return -ENODEV;
>>+	}
>>+	spin_ff_cond_lock(ff, flags);
>>+	ret = _input_ff_erase(dev, id, current->pid == 0);
>>+	spin_ff_cond_unlock(ff, flags);
>>+
>>+	mutex_unlock(&dev->ff_lock);
>>+	return ret;
>>+}
> 
> 
> Perhaps you meant `current->uid == 0' here.  There's no way in which pid
> 0 will call this code.

Right, a silly mistake.

> What's happening here anyway?  Why does this code need to know about pids?
> 
> Checking for uid==0 woud be a fishy thing to do as well.

User ID 0 is allowed to delete effects of other users. Pids are used to
keep a track of what process owns what effects. This is the same
behaviour as before.

There is a problem with this, though:
When a process closes any fd to this device, all pid-matching effects
are deleted whether the process has another fd using the device or not.

One solution would probably be to add some handle parameter to
input_ff_upload() and input_ff_erase(), and then in
evdev_ioctl_handler() pass an id unique to this fd. Then effects would
be fd-specific, not pid-specific. I think the uid == 0 thing can also be
dropped... I don't think the root user needs ability to override user
effects (it can delete them anyway, just kill the user process owning
the effects).

WDYT?

> 
>>+static int input_ff_flush(struct input_dev *dev, struct file *file)
>>+{
>>+	struct ff_device *ff;
>>+	unsigned long flags = 0;
>>+	int i;
>>+	debug("flushing now");
>>+	mutex_lock(&dev->ff_lock);
>>+	ff = dev->ff;
>>+	if (!ff) {
>>+		mutex_unlock(&dev->ff_lock);
>>+		return -ENODEV;
>>+	}
>>+	spin_ff_cond_lock(ff, flags);
>>+	for (i = 0; i < dev->ff_effects_max; i++) {
>>+		_input_ff_erase(dev, i, 0);
>>+	}
> 
> 
> Unneeded braces.
> 

Will remove.

>>+	spin_ff_cond_unlock(ff, flags);
>>+	mutex_unlock(&dev->ff_lock);
>>+	return 0;
>>+}
>>+
>>+
>>+		ff->effects[id].flags[0] = 0;
>>+		ff->effects[id].effect = *effect;
>>+
>>+		if (ff->driver->playback) {
>>+			if (!test_bit(effect->type, ff->flags))
>>+				input_ff_convert_effect(dev, effect);
>>+			ret = ff->driver->upload(dev, effect, NULL);
>>+			if (!ret)
>>+				set_bit(FF_EFFECT_USED, ff->effects[id].flags);
>>+			mutex_unlock(&dev->ff_lock);
>>+			return ret;
>>+		}
>>+		set_bit(FF_EFFECT_USED, ff->effects[id].flags);
>>+
>>+	} else {
>>+		id = effect->id;
>>+
>>+		ret = input_ff_effect_access(dev, id, 1);
>>+		if (ret) {
>>+			spin_ff_cond_unlock(ff, flags);
>>+			mutex_unlock(&dev->ff_lock);
>>+			return ret;
>>+		}
>>+
>>+		if (effect->type != ff->effects[id].effect.type ||
>>+				  (effect->type == FF_PERIODIC && effect->u.periodic.waveform !=
>>+				  ff->effects[id].effect.u.periodic.waveform)) {
>>+			spin_ff_cond_unlock(ff, flags);
>>+			mutex_unlock(&dev->ff_lock);
>>+			return -EINVAL;
>>+		}
>>+
>>+		if (ff->driver->playback) {
>>+			if (!test_bit(effect->type, ff->flags))
>>+				input_ff_convert_effect(dev, effect);
>>+			ret = ff->driver->upload(dev, effect, &ff->effects[id].effect);
>>+			ff->effects[id].effect = *effect;
>>+			mutex_unlock(&dev->ff_lock);
>>+			return ret;
> 
> 
> I think we're missing a spin_ff_cond_unlock() here?

Well, spin_ff_cond_unlock() checks for ff->driver->playback and it is
already tested in the if block above.

> 
>>+		}
>>+		ff->effects[id].effect = *effect;
>>+		clear_bit(FF_EFFECT_PLAYING, ff->effects[id].flags);
>>+
>>+	}
>>+
>>+	spin_unlock_irqrestore(&ff->atomiclock, flags);
>>+	mutex_unlock(&dev->ff_lock);
>>+	return ret;
>>+}
> 
> 
> And here we have spin_unlock_irqrestore() instead of spin_ff_cond_unlock().

If there is no need to unlock, one of the above "if
(ff->driver->playback)" would be true and the function would've already
returned before this point.

> It would be best to convert this function to have a single return point. 
> That tends to prevent problems like this from happening, and from creeping
> in later on.

I agree that the locking is too confusing in input_ff_event() and
input_ff_upload(), even if it is correct.

I'll modify the function to have a single return point, or maybe split
the function for the two different locking paths, which then call common
functions without the need to cond_lock. I guess that could be done for
all cond_locking functions.

> 
>>+int input_ff_allocate(struct input_dev *dev)
>>+{
>>+	debug("allocating device");
>>+	mutex_lock(&dev->ff_lock);
>>+	if (dev->ff)
>>+		printk(KERN_ERR "ff-effects: allocating to non-NULL pointer\n");
>>+	dev->ff = kzalloc(sizeof(*dev->ff), GFP_KERNEL);
>>+	if (!dev->ff) {
>>+		mutex_unlock(&dev->ff_lock);
>>+		return -ENOMEM;
>>+	}
>>+	spin_lock_init(&dev->ff->atomiclock);
>>+	init_timer(&dev->ff->timer);
>>+	dev->ff->timer.function = input_ff_timer;
>>+	dev->ff->timer.data = (unsigned long) dev;
>>+	dev->ff->event = input_ff_event;
> 
> 
> setup_timer()
> 

Will change.

>>+	mutex_unlock(&dev->ff_lock);
>>+	debug("ff allocated");
>>+	return 0;
>>+}
>>+
>>
>>...
>>
>>Index: linux-2.6.17-rc4-git1/drivers/input/Kconfig
>>===================================================================
>>--- linux-2.6.17-rc4-git1.orig/drivers/input/Kconfig	2006-03-20 07:53:29.000000000 +0200
>>+++ linux-2.6.17-rc4-git1/drivers/input/Kconfig	2006-05-14 02:28:42.000000000 +0300
>>@@ -24,6 +24,14 @@ config INPUT
>> 
>> if INPUT
>> 
>>+config INPUT_FF_EFFECTS
>>+	tristate "Force feedback effects"
>>+	help
>>+	  Say Y here if you want to be able to play force feedback effects.
>>+
>>+	  To compile this driver as a module, choose M here: the
>>+	  module will be called ff-effects.
> 
> 
> hm.  I'd have expected more dependencies than this.
> 

Only INPUT.

>> comment "Userland interfaces"
>> 
>> config INPUT_MOUSEDEV
>>@@ -110,6 +118,7 @@ config INPUT_TSDEV_SCREEN_Y
>> 
>> config INPUT_EVDEV
>> 	tristate "Event interface"
>>+	depends on INPUT_FF_EFFECTS || INPUT_FF_EFFECTS=n
> 
> 
> Isn't that always true?
> 

This disallows building evdev as builtin and ff-effects as module.

>>+
>>+struct ff_effect_status {
>>+	pid_t owner;
> 
> 
> This code is almost devoid of comments.  Those which it does have tend to
> cover little low-level implementation details.  But it's the *big* things
> which a reader is not able to learn from the implementation, and which
> should be commented.  Like: why on earth does this code need to know about
> pids?

Okay, I'll try to add some.

> 
>>+#if defined(CONFIG_INPUT_FF_EFFECTS_MODULE) || defined(CONFIG_INPUT_FF_EFFECTS)
> 
> 
> No, we shouldn't use CONFIG_FOO_MODULE.  We just don't know at compile-time
> whether the user will later compile and insert a particular module.
> 

Right... so maybe we should just make ff-effects a bool instead of
tristate or put it in the input module?

>>+		mutex_lock(&dev->ff_lock);
>>+		del_timer_sync(&ff->timer);
>>+		dev->flush = NULL;
>>+		dev->ff = NULL;
>>+		kfree(ff);
>>+		mutex_unlock(&dev->ff_lock);
> 
> 
> The kfree can be moved outside the lock.
> 

Indeed.

BTW, what is the best way to send corrected patches for this patchset?
Probably as a reply to the individual patches?


-- 
Anssi Hannula

Re: [patch 03/11] input: new force feedback interface

[Anssi Hannula]

Anssi Hannula wrote:
> Andrew Morton wrote:
> 
>>Anssi Hannula <anssi.hannula@gmail.com> wrote:
>>
>>
>>>Implement a new force feedback interface, in which all non-driver-specific
>>>operations are separated to a common module. This module handles effect type
>>>validations, effect timers, locking, etc.
>>>
>>>As a result, support is added for gain and envelope for memoryless devices,
>>>periodic => rumble conversion for memoryless devices and rumble => periodic
>>>conversion for devices with periodic support instead of rumble support. Also
>>>the effect memory of devices is not emptied if the root user opens and closes
>>>the device while another user is using effects. This module also obsoletes
>>>some flawed locking and timer code in few ff drivers.
>>>
>>>The module is named ff-effects. If INPUT_FF_EFFECTS is enabled, the force
>>>feedback drivers and interfaces (evdev) will be depending on it.
>>>
>>>Userspace interface is left unaltered.
>>>
>>
>>
>>Nice-looking patches.
>>
> 
> 
> Thanks for looking and providing helpful comments :)
> 

Andrew, did you get my response? I received Delivery Status Notification
for your address:
<copy>
- These recipients of your message have been processed by the mail server:
akpm@osdl.org; Failed; 5.3.0 (other or undefined mail system status)

    Remote MTA smtp.osdl.org: network error


 - SMTP protocol diagnostic: 554 5.7.1 gmail.com suggested we reject
your email: 81.228.11.120 is neither permitted nor denied by domain of
anssi.hannula@gmail.com
</paste>


> 
> BTW, what is the best way to send corrected patches for this patchset?
> Probably as a reply to the individual patches?
> 

Hmm, I think it is easier to just send the whole updated set...

I'm going to do all the changes discussed and then send the set probably
tomorrow or in the weekend.


-- 
Anssi Hannula

Re: [patch 03/11] input: new force feedback interface

[Andrew Morton]

Anssi Hannula <anssi.hannula@gmail.com> wrote:
>
> 
> Andrew, did you get my response?

Nope.

> I received Delivery Status Notification
> for your address:
> <copy>
> - These recipients of your message have been processed by the mail server:
> akpm@osdl.org; Failed; 5.3.0 (other or undefined mail system status)
> 
>     Remote MTA smtp.osdl.org: network error
> 
> 
>  - SMTP protocol diagnostic: 554 5.7.1 gmail.com suggested we reject
> your email: 81.228.11.120 is neither permitted nor denied by domain of
> anssi.hannula@gmail.com
> </paste>
> 

Appropriate people have been informed ;)

> 
> > 
> > BTW, what is the best way to send corrected patches for this patchset?
> > Probably as a reply to the individual patches?
> > 
> 
> Hmm, I think it is easier to just send the whole updated set...
> 
> I'm going to do all the changes discussed and then send the set probably
> tomorrow or in the weekend.
> 

Yes, that's fine.  Once patches have matured a bit, incremental (and
fine-grained) updates are preferred.  And I'll often turn
wholesale-replacement-attempts into incremental updates, so we can see what
changed.

But at this stage, rip-it-out-and-redo is fine.  Although it does help if
you can tell us which of the review comments were and were not implemented,
so we don't have to re-read the whole thing with the same level of
attention.

Re: [patch 03/11] input: new force feedback interface

[Anssi Hannula]

Andrew Morton wrote:
> Anssi Hannula <anssi.hannula@gmail.com> wrote:
> 
>>>BTW, what is the best way to send corrected patches for this patchset?
>>>Probably as a reply to the individual patches?
>>>
>>
>>Hmm, I think it is easier to just send the whole updated set...
>>
>>I'm going to do all the changes discussed and then send the set probably
>>tomorrow or in the weekend.
>>
> 
> Yes, that's fine.  Once patches have matured a bit, incremental (and
> fine-grained) updates are preferred.  And I'll often turn
> wholesale-replacement-attempts into incremental updates, so we can see what
> changed.
> 
> But at this stage, rip-it-out-and-redo is fine.  Although it does help if
> you can tell us which of the review comments were and were not implemented,
> so we don't have to re-read the whole thing with the same level of
> attention.

Okay, I sent the whole set again.
I hope you have time to take a look :)


(didn't CC the patches for you because quilt sends them through my ISP
with @gmail.com => smpt.osdl.org would've not accepted them)

-- 
Anssi Hannula

Re: [patch 03/11] input: new force feedback interface

[Anssi Hannula]

Here's this reply again with different From email address, so that
Andrew Morton gets it too:

Andrew Morton wrote:
> Anssi Hannula <anssi.hannula@gmail.com> wrote:
> 
>>Implement a new force feedback interface, in which all non-driver-specific
>>operations are separated to a common module. This module handles effect type
>>validations, effect timers, locking, etc.
>>
>>As a result, support is added for gain and envelope for memoryless devices,
>>periodic => rumble conversion for memoryless devices and rumble => periodic
>>conversion for devices with periodic support instead of rumble support. Also
>>the effect memory of devices is not emptied if the root user opens and closes
>>the device while another user is using effects. This module also obsoletes
>>some flawed locking and timer code in few ff drivers.
>>
>>The module is named ff-effects. If INPUT_FF_EFFECTS is enabled, the force
>>feedback drivers and interfaces (evdev) will be depending on it.
>>
>>Userspace interface is left unaltered.
>>
> 
> 
> Nice-looking patches.
> 

Thanks for looking and providing helpful comments :)

>>+#define spin_ff_cond_lock(_ff, _flags)					  \
>>+	do {								  \
>>+		if (!_ff->driver->playback)				  \
>>+			spin_lock_irqsave(&_ff->atomiclock, _flags);	  \
>>+	} while (0);
>>+
>>+#define spin_ff_cond_unlock(_ff, _flags)					  \
>>+	do {								  \
>>+		if (!_ff->driver->playback)				  \
>>+			spin_unlock_irqrestore(&_ff->atomiclock, _flags); \
>>+	} while (0);
> 
> 
> Making these static inline functions would deuglify them a bit.
> 

I put them like that because spin_lock_irqsave is a macro that uses the
local variable "flags".
But yes, I can probably pass that parameter to an inline function as a
pointer. I'll change that.


>>+static int input_ff_effect_access(struct input_dev *dev, int id, int override)
>>+{
>>+	struct ff_device *ff = dev->ff;
>>+	if (id < dev->ff_effects_max && id >= 0 && test_bit(FF_EFFECT_USED, ff->effects[id].flags))
> 
> 
> Kernel does have an 80-columns rule, but input seems to have always spurned it.
> 
> 
>>+static int input_ff_envelope_time(struct ff_effect_status *effect, struct ff_envelope *envelope, unsigned long *event_time)
>>+{
>>+	unsigned long fade_start;
>>+	if (!envelope)
>>+		return 0;
>>+
>>+	if (envelope->attack_length && time_after(effect->play_at + msecs_to_jiffies(envelope->attack_length), effect->adj_at)) {
> 
> 
> Try using an 80-column wondow for a while ;)

Okay, I'll make the lines shorter.

> 
> 
>>+		return value;
>>+	}
>>+
>>+	difference = abs(value) - envelope_level;
>>+
>>+	debug("difference = %d", difference);
>>+	debug("time_from_level = 0x%x", time_from_level);
>>+	debug("time_of_envelope = 0x%x", time_of_envelope);
>>+	if (difference < 0)
>>+		difference = -((-difference) * time_from_level / time_of_envelope);
>>+	else
>>+		difference = difference * time_from_level / time_of_envelope;
> 
> 
> You've checked there's no possibility of divide-by-zero here?
> 

Yes, the "time_of_envelope" is set a few lines above from
"envelope->attack_length" or "envelope->fade_length", and there is an if
block that checks they're non-zero.

>>+
>>+static int input_ff_safe_sum(int a, int b, int limit) {
> 
> 
> The opening brace goes in column zero, please.
> 
> 
>>+	int c;
>>+	if (!a)
>>+		return b;
>>+	c = a + b;
>>+	if (c > limit)
>>+		return limit;
>>+	return c;
>>+}
>>+
>>+static s8 input_ff_s8_sum(int a, int b) {
> 
> 
> dittoes.

Okay, will fix.

> 
>>+	int c;
>>+	c = input_ff_safe_sum(a, b, 0x7f);
>>+	if (c < -0x80)
>>+		return -0x80;
>>+	return c;
>>+}
>>
>>...
>>
>>+static void input_ff_timer(unsigned long timer_data)
>>+{
>>+	struct input_dev *dev = (struct input_dev *) timer_data;
>>+	struct ff_device *ff = dev->ff;
>>+	struct ff_effect effect;
>>+	int i;
>>+	unsigned long flags;
>>+	int effects_pending;
>>+	unsigned long effect_handled[NBITS(FF_EFFECTS_MAX)];
> 
> 
> DECLARE_BITMAP would be more usual.  (Yes, it should have been called
> DEFINE_BITMAP).
> 

Ok.

>>+	int effect_type;
>>+	int safety;
>>+
>>+	debug("timer: updating effects");
>>+
>>+	spin_lock_irqsave(&ff->atomiclock, flags);
>>+
>>+	memset(effect_handled, 0, sizeof(effect_handled));
> 
> 
> You could take the lock after the memset.
> 

Ok.

>>+int input_ff_erase(struct input_dev *dev, int id)
>>+{
>>+	struct ff_device *ff;
>>+	unsigned long flags = 0;
>>+	int ret;
>>+	if (!test_bit(EV_FF, dev->evbit))
>>+		return -EINVAL;
>>+	mutex_lock(&dev->ff_lock);
>>+	ff = dev->ff;
>>+	if (!ff) {
>>+		mutex_unlock(&dev->ff_lock);
>>+		return -ENODEV;
>>+	}
>>+	spin_ff_cond_lock(ff, flags);
>>+	ret = _input_ff_erase(dev, id, current->pid == 0);
>>+	spin_ff_cond_unlock(ff, flags);
>>+
>>+	mutex_unlock(&dev->ff_lock);
>>+	return ret;
>>+}
> 
> 
> Perhaps you meant `current->uid == 0' here.  There's no way in which pid
> 0 will call this code.

Right, a silly mistake.

> What's happening here anyway?  Why does this code need to know about pids?
> 
> Checking for uid==0 woud be a fishy thing to do as well.

User ID 0 is allowed to delete effects of other users. Pids are used to
keep a track of what process owns what effects. This is the same
behaviour as before.

There is a problem with this, though:
When a process closes any fd to this device, all pid-matching effects
are deleted whether the process has another fd using the device or not.

One solution would probably be to add some handle parameter to
input_ff_upload() and input_ff_erase(), and then in
evdev_ioctl_handler() pass an id unique to this fd. Then effects would
be fd-specific, not pid-specific. I think the uid == 0 thing can also be
dropped... I don't think the root user needs ability to override user
effects (it can delete them anyway, just kill the user process owning
the effects).

WDYT?

> 
>>+static int input_ff_flush(struct input_dev *dev, struct file *file)
>>+{
>>+	struct ff_device *ff;
>>+	unsigned long flags = 0;
>>+	int i;
>>+	debug("flushing now");
>>+	mutex_lock(&dev->ff_lock);
>>+	ff = dev->ff;
>>+	if (!ff) {
>>+		mutex_unlock(&dev->ff_lock);
>>+		return -ENODEV;
>>+	}
>>+	spin_ff_cond_lock(ff, flags);
>>+	for (i = 0; i < dev->ff_effects_max; i++) {
>>+		_input_ff_erase(dev, i, 0);
>>+	}
> 
> 
> Unneeded braces.
> 

Will remove.

>>+	spin_ff_cond_unlock(ff, flags);
>>+	mutex_unlock(&dev->ff_lock);
>>+	return 0;
>>+}
>>+
>>+
>>+		ff->effects[id].flags[0] = 0;
>>+		ff->effects[id].effect = *effect;
>>+
>>+		if (ff->driver->playback) {
>>+			if (!test_bit(effect->type, ff->flags))
>>+				input_ff_convert_effect(dev, effect);
>>+			ret = ff->driver->upload(dev, effect, NULL);
>>+			if (!ret)
>>+				set_bit(FF_EFFECT_USED, ff->effects[id].flags);
>>+			mutex_unlock(&dev->ff_lock);
>>+			return ret;
>>+		}
>>+		set_bit(FF_EFFECT_USED, ff->effects[id].flags);
>>+
>>+	} else {
>>+		id = effect->id;
>>+
>>+		ret = input_ff_effect_access(dev, id, 1);
>>+		if (ret) {
>>+			spin_ff_cond_unlock(ff, flags);
>>+			mutex_unlock(&dev->ff_lock);
>>+			return ret;
>>+		}
>>+
>>+		if (effect->type != ff->effects[id].effect.type ||
>>+				  (effect->type == FF_PERIODIC && effect->u.periodic.waveform !=
>>+				  ff->effects[id].effect.u.periodic.waveform)) {
>>+			spin_ff_cond_unlock(ff, flags);
>>+			mutex_unlock(&dev->ff_lock);
>>+			return -EINVAL;
>>+		}
>>+
>>+		if (ff->driver->playback) {
>>+			if (!test_bit(effect->type, ff->flags))
>>+				input_ff_convert_effect(dev, effect);
>>+			ret = ff->driver->upload(dev, effect, &ff->effects[id].effect);
>>+			ff->effects[id].effect = *effect;
>>+			mutex_unlock(&dev->ff_lock);
>>+			return ret;
> 
> 
> I think we're missing a spin_ff_cond_unlock() here?

Well, spin_ff_cond_unlock() checks for ff->driver->playback and it is
already tested in the if block above.

> 
>>+		}
>>+		ff->effects[id].effect = *effect;
>>+		clear_bit(FF_EFFECT_PLAYING, ff->effects[id].flags);
>>+
>>+	}
>>+
>>+	spin_unlock_irqrestore(&ff->atomiclock, flags);
>>+	mutex_unlock(&dev->ff_lock);
>>+	return ret;
>>+}
> 
> 
> And here we have spin_unlock_irqrestore() instead of spin_ff_cond_unlock().

If there is no need to unlock, one of the above "if
(ff->driver->playback)" would be true and the function would've already
returned before this point.

> It would be best to convert this function to have a single return point. 
> That tends to prevent problems like this from happening, and from creeping
> in later on.

I agree that the locking is too confusing in input_ff_event() and
input_ff_upload(), even if it is correct.

I'll modify the function to have a single return point, or maybe split
the function for the two different locking paths, which then call common
functions without the need to cond_lock. I guess that could be done for
all cond_locking functions.

> 
>>+int input_ff_allocate(struct input_dev *dev)
>>+{
>>+	debug("allocating device");
>>+	mutex_lock(&dev->ff_lock);
>>+	if (dev->ff)
>>+		printk(KERN_ERR "ff-effects: allocating to non-NULL pointer\n");
>>+	dev->ff = kzalloc(sizeof(*dev->ff), GFP_KERNEL);
>>+	if (!dev->ff) {
>>+		mutex_unlock(&dev->ff_lock);
>>+		return -ENOMEM;
>>+	}
>>+	spin_lock_init(&dev->ff->atomiclock);
>>+	init_timer(&dev->ff->timer);
>>+	dev->ff->timer.function = input_ff_timer;
>>+	dev->ff->timer.data = (unsigned long) dev;
>>+	dev->ff->event = input_ff_event;
> 
> 
> setup_timer()
> 

Will change.

>>+	mutex_unlock(&dev->ff_lock);
>>+	debug("ff allocated");
>>+	return 0;
>>+}
>>+
>>
>>...
>>
>>Index: linux-2.6.17-rc4-git1/drivers/input/Kconfig
>>===================================================================
>>--- linux-2.6.17-rc4-git1.orig/drivers/input/Kconfig	2006-03-20 07:53:29.000000000 +0200
>>+++ linux-2.6.17-rc4-git1/drivers/input/Kconfig	2006-05-14 02:28:42.000000000 +0300
>>@@ -24,6 +24,14 @@ config INPUT
>> 
>> if INPUT
>> 
>>+config INPUT_FF_EFFECTS
>>+	tristate "Force feedback effects"
>>+	help
>>+	  Say Y here if you want to be able to play force feedback effects.
>>+
>>+	  To compile this driver as a module, choose M here: the
>>+	  module will be called ff-effects.
> 
> 
> hm.  I'd have expected more dependencies than this.
> 

Only INPUT.

>> comment "Userland interfaces"
>> 
>> config INPUT_MOUSEDEV
>>@@ -110,6 +118,7 @@ config INPUT_TSDEV_SCREEN_Y
>> 
>> config INPUT_EVDEV
>> 	tristate "Event interface"
>>+	depends on INPUT_FF_EFFECTS || INPUT_FF_EFFECTS=n
> 
> 
> Isn't that always true?
> 

This disallows building evdev as builtin and ff-effects as module.

>>+
>>+struct ff_effect_status {
>>+	pid_t owner;
> 
> 
> This code is almost devoid of comments.  Those which it does have tend to
> cover little low-level implementation details.  But it's the *big* things
> which a reader is not able to learn from the implementation, and which
> should be commented.  Like: why on earth does this code need to know about
> pids?

Okay, I'll try to add some.

> 
>>+#if defined(CONFIG_INPUT_FF_EFFECTS_MODULE) || defined(CONFIG_INPUT_FF_EFFECTS)
> 
> 
> No, we shouldn't use CONFIG_FOO_MODULE.  We just don't know at compile-time
> whether the user will later compile and insert a particular module.
> 

Right... so maybe we should just make ff-effects a bool instead of
tristate or put it in the input module?

>>+		mutex_lock(&dev->ff_lock);
>>+		del_timer_sync(&ff->timer);
>>+		dev->flush = NULL;
>>+		dev->ff = NULL;
>>+		kfree(ff);
>>+		mutex_unlock(&dev->ff_lock);
> 
> 
> The kfree can be moved outside the lock.
> 

Indeed.

BTW, what is the best way to send corrected patches for this patchset?
Probably as a reply to the individual patches?


-- 
Anssi Hannula

Re: [patch 03/11] input: new force feedback interface

[Andrew Morton]

Anssi Hannula <anssi@mandriva.org> wrote:
>
>  >>+int input_ff_erase(struct input_dev *dev, int id)
>  >>+{
>  >>+	struct ff_device *ff;
>  >>+	unsigned long flags = 0;
>  >>+	int ret;
>  >>+	if (!test_bit(EV_FF, dev->evbit))
>  >>+		return -EINVAL;
>  >>+	mutex_lock(&dev->ff_lock);
>  >>+	ff = dev->ff;
>  >>+	if (!ff) {
>  >>+		mutex_unlock(&dev->ff_lock);
>  >>+		return -ENODEV;
>  >>+	}
>  >>+	spin_ff_cond_lock(ff, flags);
>  >>+	ret = _input_ff_erase(dev, id, current->pid == 0);
>  >>+	spin_ff_cond_unlock(ff, flags);
>  >>+
>  >>+	mutex_unlock(&dev->ff_lock);
>  >>+	return ret;
>  >>+}
>  > 
>  > 
>  > Perhaps you meant `current->uid == 0' here.  There's no way in which pid
>  > 0 will call this code.
> 
>  Right, a silly mistake.
> 
>  > What's happening here anyway?  Why does this code need to know about pids?
>  > 
>  > Checking for uid==0 woud be a fishy thing to do as well.
> 
>  User ID 0 is allowed to delete effects of other users. Pids are used to
>  keep a track of what process owns what effects. This is the same
>  behaviour as before.

Oh dear.

Whatever we do here should remain 100%-compatible with "before".  Which
rather limits our options.

>  There is a problem with this, though:
>  When a process closes any fd to this device, all pid-matching effects
>  are deleted whether the process has another fd using the device or not.
> 
>  One solution would probably be to add some handle parameter to
>  input_ff_upload() and input_ff_erase(), and then in
>  evdev_ioctl_handler() pass an id unique to this fd. Then effects would
>  be fd-specific, not pid-specific. I think the uid == 0 thing can also be
>  dropped... I don't think the root user needs ability to override user
>  effects (it can delete them anyway, just kill the user process owning
>  the effects).
> 

Generally we use file descriptors (and driver-specific state at
file.f_private) to manage things like that.  But I'd imagine that we
couldn't retain the existing semantics with any such scheme.

A pragmatic approach would be to put a big fat comment in there explaining
how it all works and leave it at that.

Re: [patch 03/11] input: new force feedback interface

[Anssi Hannula]

Andrew Morton wrote:
> Anssi Hannula <anssi@mandriva.org> wrote:
> 
>> >>+int input_ff_erase(struct input_dev *dev, int id)
>> >>+{
>> >>+	struct ff_device *ff;
>> >>+	unsigned long flags = 0;
>> >>+	int ret;
>> >>+	if (!test_bit(EV_FF, dev->evbit))
>> >>+		return -EINVAL;
>> >>+	mutex_lock(&dev->ff_lock);
>> >>+	ff = dev->ff;
>> >>+	if (!ff) {
>> >>+		mutex_unlock(&dev->ff_lock);
>> >>+		return -ENODEV;
>> >>+	}
>> >>+	spin_ff_cond_lock(ff, flags);
>> >>+	ret = _input_ff_erase(dev, id, current->pid == 0);
>> >>+	spin_ff_cond_unlock(ff, flags);
>> >>+
>> >>+	mutex_unlock(&dev->ff_lock);
>> >>+	return ret;
>> >>+}
>> > 
>> > 
>> > Perhaps you meant `current->uid == 0' here.  There's no way in which pid
>> > 0 will call this code.
>>
>> Right, a silly mistake.
>>
>> > What's happening here anyway?  Why does this code need to know about pids?
>> > 
>> > Checking for uid==0 woud be a fishy thing to do as well.
>>
>> User ID 0 is allowed to delete effects of other users. Pids are used to
>> keep a track of what process owns what effects. This is the same
>> behaviour as before.
> 
> 
> Oh dear.
> 
> Whatever we do here should remain 100%-compatible with "before".  Which
> rather limits our options.

There are only _very_ few programs using FF on Linux ATM, and I don't
think any of them opens multiple fd:s to the same device and expects to
be able to delete effects created on the other fd. And that is the only
con of changing the behaviour.

Should the behaviour be changed so that the effect owners are file
descriptors, the effects of one fd would not be lost if the process
opens and closes another fd on the same device (currently all effects of
a process are deleted if the process closes any fd to the device).

>> There is a problem with this, though:
>> When a process closes any fd to this device, all pid-matching effects
>> are deleted whether the process has another fd using the device or not.
>>
>> One solution would probably be to add some handle parameter to
>> input_ff_upload() and input_ff_erase(), and then in
>> evdev_ioctl_handler() pass an id unique to this fd. Then effects would
>> be fd-specific, not pid-specific. I think the uid == 0 thing can also be
>> dropped... I don't think the root user needs ability to override user
>> effects (it can delete them anyway, just kill the user process owning
>> the effects).
>>
> 
> 
> Generally we use file descriptors (and driver-specific state at
> file.f_private) to manage things like that.  But I'd imagine that we
> couldn't retain the existing semantics with any such scheme.
> 
> A pragmatic approach would be to put a big fat comment in there explaining
> how it all works and leave it at that.

As I don't see this could break any existing applications, I would very
much like to change the behaviour so that the effects are file
descriptor specific. What should I use to differentiate the descriptors?
Can I just compare the "struct file*"? (it seems to work well, I just
modified the code so)

-- 
Anssi Hannula

Re: [patch 03/11] input: new force feedback interface

[Andrew Morton]

Anssi Hannula <anssi@mandriva.org> wrote:
>
> > Generally we use file descriptors (and driver-specific state at
>  > file.f_private) to manage things like that.  But I'd imagine that we
>  > couldn't retain the existing semantics with any such scheme.
>  > 
>  > A pragmatic approach would be to put a big fat comment in there explaining
>  > how it all works and leave it at that.
> 
>  As I don't see this could break any existing applications, I would very
>  much like to change the behaviour so that the effects are file
>  descriptor specific.

ooh, that's always risky - we just don't know what people are doing out
there.  They do the damnedest things.

Is it possible to implement the new behaviour while retaining the old
behaviour as well?  And to detect when an app is using the old behaviour
and to drop a printk("stop doing this")?  So we can kill the old behaviour
in a year or so?

> What should I use to differentiate the descriptors?
>  Can I just compare the "struct file*"? (it seems to work well, I just
>  modified the code so)

Depends what you're trying to do.  Different threads in the same process
can share the same file*'s.

Re: [patch 03/11] input: new force feedback interface

[Anssi Hannula]

Andrew Morton wrote:
> Anssi Hannula <anssi@mandriva.org> wrote:
> 
>>>Generally we use file descriptors (and driver-specific state at
>>
>> > file.f_private) to manage things like that.  But I'd imagine that we
>> > couldn't retain the existing semantics with any such scheme.
>> > 
>> > A pragmatic approach would be to put a big fat comment in there explaining
>> > how it all works and leave it at that.
>>
>> As I don't see this could break any existing applications, I would very
>> much like to change the behaviour so that the effects are file
>> descriptor specific.
> 
> 
> ooh, that's always risky - we just don't know what people are doing out
> there.  They do the damnedest things.
> 
> Is it possible to implement the new behaviour while retaining the old
> behaviour as well?  And to detect when an app is using the old behaviour
> and to drop a printk("stop doing this")?  So we can kill the old behaviour
> in a year or so?

I *really* don't think any app is trying to do one of the following:
- open another fd to the same device and delete effect created in the
first fd
- open and close another fd to the same device to delete all effects

The latter one doesn't make any sense (the process could just open and
close the first fd), but the former one is possible, though very unlikely.

I count only 6 programs using linux ff, of which 3 are test programs.
None of those messes up with fd's so that they would depend on the old
behaviour. I would change the behaviour, while we still can.

What we could do is allow deleting effects, whether they're owned by the
fd or not. If they aren't, printk() would be issued.

> 
>>What should I use to differentiate the descriptors?
>> Can I just compare the "struct file*"? (it seems to work well, I just
>> modified the code so)
> 
> Depends what you're trying to do.  Different threads in the same process
> can share the same file*'s.

I try to have a coherent behaviour:
- fd1 opened
- fd1: effects 0, 1 are created
- fd2 opened
- fd2: effects 2, 3 are created
- fd2 closed
=> effects 2, 3 get deleted
- fd1 uses effects
- fd1 closed
=> effects 0, 1 get deleted


I will once again give an example how things would go with the old
behaviour:
- fd1 opened
- fd1: effects 0, 1 are created
- fd2 opened
- fd2: effects 2, 3 are created
- fd2 closed
=> effects 0, 1, 2, 3 get deleted
- fd1 uses effects
=> failure
- fd1 closed


(fd1 and fd2 are of the same process and the same device)

-- 
Anssi Hannula

[patch 04/11] input: adapt hid force feedback drivers for the new interface

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-hid-to-new-interface.diff --]
[-- Type: text/plain, Size: 29802 bytes --]

Modify the HID force feedback drivers for the new interface. This removes a
lot of code duplication.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/usb/input/Kconfig     |    2 
 drivers/usb/input/hid-input.c |    3 
 drivers/usb/input/hid-lgff.c  |  457 ++++--------------------------------------
 drivers/usb/input/hid-tmff.c  |  353 ++------------------------------
 drivers/usb/input/hid.h       |    9 
 5 files changed, 81 insertions(+), 743 deletions(-)

Index: linux-2.6.17-rc4-git1/drivers/usb/input/hid.h
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/usb/input/hid.h	2006-05-13 19:58:57.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/usb/input/hid.h	2006-05-14 01:36:21.000000000 +0300
@@ -442,8 +442,6 @@ struct hid_device {							/* device repo
 
 	void *ff_private;                                               /* Private data for the force-feedback driver */
 	void (*ff_exit)(struct hid_device*);                            /* Called by hid_exit_ff(hid) */
-	int (*ff_event)(struct hid_device *hid, struct input_dev *input,
-			unsigned int type, unsigned int code, int value);
 
 #ifdef CONFIG_USB_HIDINPUT_POWERBOOK
 	unsigned long pb_pressed_fn[NBITS(KEY_MAX)];
@@ -526,13 +524,6 @@ static inline void hid_ff_exit(struct hi
 	if (hid->ff_exit)
 		hid->ff_exit(hid);
 }
-static inline int hid_ff_event(struct hid_device *hid, struct input_dev *input,
-			unsigned int type, unsigned int code, int value)
-{
-	if (hid->ff_event)
-		return hid->ff_event(hid, input, type, code, value);
-	return -ENOSYS;
-}
 
 int hid_lgff_init(struct hid_device* hid);
 int hid_tmff_init(struct hid_device* hid);
Index: linux-2.6.17-rc4-git1/drivers/usb/input/hid-input.c
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/usb/input/hid-input.c	2006-05-13 19:58:57.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/usb/input/hid-input.c	2006-05-14 01:36:21.000000000 +0300
@@ -736,9 +736,6 @@ static int hidinput_input_event(struct i
 	struct hid_field *field;
 	int offset;
 
-	if (type == EV_FF)
-		return hid_ff_event(hid, dev, type, code, value);
-
 	if (type != EV_LED)
 		return -1;
 
Index: linux-2.6.17-rc4-git1/drivers/usb/input/hid-lgff.c
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/usb/input/hid-lgff.c	2006-05-14 00:14:43.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/usb/input/hid-lgff.c	2006-05-14 01:41:39.000000000 +0300
@@ -7,6 +7,7 @@
  * - WingMan Force 3D
  *
  *  Copyright (c) 2002-2004 Johann Deneux
+ *  Copyright (c) 2006 Anssi Hannula <anssi.hannula@gmail.com>
  */
 
 /*
@@ -29,41 +30,8 @@
  */
 
 #include <linux/input.h>
-#include <linux/sched.h>
-
-//#define DEBUG
 #include <linux/usb.h>
-
-#include <linux/circ_buf.h>
-
 #include "hid.h"
-//#include "fixp-arith.h"
-
-
-/* Periodicity of the update */
-#define PERIOD (HZ/10)
-
-#define RUN_AT(t) (jiffies + (t))
-
-/* Effect status */
-#define EFFECT_STARTED 0     /* Effect is going to play after some time
-				(ff_replay.delay) */
-#define EFFECT_PLAYING 1     /* Effect is being played */
-#define EFFECT_USED    2
-
-// For lgff_device::flags
-#define DEVICE_CLOSING 0     /* The driver is being unitialised */
-
-/* Check that the current process can access an effect */
-#define CHECK_OWNERSHIP(effect) (current->pid == 0 \
-        || effect.owner == current->pid)
-
-#define LGFF_CHECK_OWNERSHIP(i, l) \
-        (i>=0 && i<LGFF_EFFECTS \
-        && test_bit(EFFECT_USED, l->effects[i].flags) \
-        && CHECK_OWNERSHIP(l->effects[i]))
-
-#define LGFF_EFFECTS 8
 
 struct device_type {
 	u16 idVendor;
@@ -71,48 +39,12 @@ struct device_type {
 	signed short *ff;
 };
 
-struct lgff_effect {
-	pid_t owner;
-
-	struct ff_effect effect;
-
-	unsigned long flags[1];
-	unsigned int count;          /* Number of times left to play */
-	unsigned long started_at;    /* When the effect started to play */
-};
-
-struct lgff_device {
-	struct hid_device* hid;
-
-	struct hid_report* constant;
-	struct hid_report* rumble;
-	struct hid_report* condition;
-
-	struct lgff_effect effects[LGFF_EFFECTS];
-	spinlock_t lock;             /* device-level lock. Having locks on
-					a per-effect basis could be nice, but
-					isn't really necessary */
-
-	unsigned long flags[1];      /* Contains various information about the
-				        state of the driver for this device */
-
-	struct timer_list timer;
-};
-
 /* Callbacks */
 static void hid_lgff_exit(struct hid_device* hid);
-static int hid_lgff_event(struct hid_device *hid, struct input_dev *input,
-			  unsigned int type, unsigned int code, int value);
-static int hid_lgff_flush(struct input_dev *input, struct file *file);
-static int hid_lgff_upload_effect(struct input_dev *input,
-				  struct ff_effect *effect);
-static int hid_lgff_erase(struct input_dev *input, int id);
+static int hid_lgff_play(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old);
 
-/* Local functions */
+/* Local function */
 static void hid_lgff_input_init(struct hid_device* hid);
-static void hid_lgff_timer(unsigned long timer_data);
-static struct hid_report* hid_lgff_duplicate_report(struct hid_report*);
-static void hid_lgff_delete_report(struct hid_report*);
 
 static signed short ff_rumble[] = {
 	FF_RUMBLE,
@@ -131,11 +63,17 @@ static struct device_type devices[] = {
 	{0x0000, 0x0000, ff_joystick}
 };
 
+static struct ff_driver lgff_driver = {
+	.upload = hid_lgff_play,
+};
+
 int hid_lgff_init(struct hid_device* hid)
 {
-	struct lgff_device *private;
 	struct hid_report* report;
 	struct hid_field* field;
+	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
+	struct input_dev *dev = hidinput->input;
+	int ret;
 
 	/* Find the report to use */
 	if (list_empty(&hid->report_enum[HID_OUTPUT_REPORT].report_list)) {
@@ -143,7 +81,7 @@ int hid_lgff_init(struct hid_device* hid
 		return -1;
 	}
 	/* Check that the report looks ok */
-	report = (struct hid_report*)hid->report_enum[HID_OUTPUT_REPORT].report_list.next;
+	report = list_entry(hid->report_enum[HID_OUTPUT_REPORT].report_list.next, struct hid_report, list);
 	if (!report) {
 		err("NULL output report");
 		return -1;
@@ -154,98 +92,22 @@ int hid_lgff_init(struct hid_device* hid
 		return -1;
 	}
 
-	private = kzalloc(sizeof(struct lgff_device), GFP_KERNEL);
-	if (!private)
-		return -1;
-	hid->ff_private = private;
+	ret = input_ff_allocate(dev);
+	if (ret)
+		return ret;
 
 	/* Input init */
 	hid_lgff_input_init(hid);
 
-
-	private->constant = hid_lgff_duplicate_report(report);
-	if (!private->constant) {
-		kfree(private);
-		return -1;
-	}
-	private->constant->field[0]->value[0] = 0x51;
-	private->constant->field[0]->value[1] = 0x08;
-	private->constant->field[0]->value[2] = 0x7f;
-	private->constant->field[0]->value[3] = 0x7f;
-
-	private->rumble = hid_lgff_duplicate_report(report);
-	if (!private->rumble) {
-		hid_lgff_delete_report(private->constant);
-		kfree(private);
-		return -1;
-	}
-	private->rumble->field[0]->value[0] = 0x42;
-
-
-	private->condition = hid_lgff_duplicate_report(report);
-	if (!private->condition) {
-		hid_lgff_delete_report(private->rumble);
-		hid_lgff_delete_report(private->constant);
-		kfree(private);
-		return -1;
-	}
-
-	private->hid = hid;
-
-	spin_lock_init(&private->lock);
-	init_timer(&private->timer);
-	private->timer.data = (unsigned long)private;
-	private->timer.function = hid_lgff_timer;
-
-	/* Event and exit callbacks */
-	hid->ff_exit = hid_lgff_exit;
-	hid->ff_event = hid_lgff_event;
-
-	/* Start the update task */
-	private->timer.expires = RUN_AT(PERIOD);
-	add_timer(&private->timer);  /*TODO: only run the timer when at least
-				       one effect is playing */
+	ret = input_ff_register(dev, &lgff_driver);
+	if (ret)
+		return ret;
 
 	printk(KERN_INFO "Force feedback for Logitech force feedback devices by Johann Deneux <johann.deneux@it.uu.se>\n");
 
 	return 0;
 }
 
-static struct hid_report* hid_lgff_duplicate_report(struct hid_report* report)
-{
-	struct hid_report* ret;
-
-	ret = kmalloc(sizeof(struct lgff_device), GFP_KERNEL);
-	if (!ret)
-		return NULL;
-	*ret = *report;
-
-	ret->field[0] = kmalloc(sizeof(struct hid_field), GFP_KERNEL);
-	if (!ret->field[0]) {
-		kfree(ret);
-		return NULL;
-	}
-	*ret->field[0] = *report->field[0];
-
-	ret->field[0]->value = kzalloc(sizeof(s32[8]), GFP_KERNEL);
-	if (!ret->field[0]->value) {
-		kfree(ret->field[0]);
-		kfree(ret);
-		return NULL;
-	}
-
-	return ret;
-}
-
-static void hid_lgff_delete_report(struct hid_report* report)
-{
-	if (report) {
-		kfree(report->field[0]->value);
-		kfree(report->field[0]);
-		kfree(report);
-	}
-}
-
 static void hid_lgff_input_init(struct hid_device* hid)
 {
 	struct device_type* dev = devices;
@@ -259,265 +121,46 @@ static void hid_lgff_input_init(struct h
 		dev++;
 
 	for (ff = dev->ff; *ff >= 0; ff++)
-		set_bit(*ff, input_dev->ffbit);
-
-	input_dev->upload_effect = hid_lgff_upload_effect;
-	input_dev->flush = hid_lgff_flush;
+		set_bit(*ff, input_dev->ff->flags);
 
-	set_bit(EV_FF, input_dev->evbit);
-	input_dev->ff_effects_max = LGFF_EFFECTS;
 }
 
-static void hid_lgff_exit(struct hid_device* hid)
-{
-	struct lgff_device *lgff = hid->ff_private;
-
-	set_bit(DEVICE_CLOSING, lgff->flags);
-	del_timer_sync(&lgff->timer);
-
-	hid_lgff_delete_report(lgff->condition);
-	hid_lgff_delete_report(lgff->rumble);
-	hid_lgff_delete_report(lgff->constant);
-
-	kfree(lgff);
-}
-
-static int hid_lgff_event(struct hid_device *hid, struct input_dev* input,
-			  unsigned int type, unsigned int code, int value)
-{
-	struct lgff_device *lgff = hid->ff_private;
-	struct lgff_effect *effect = lgff->effects + code;
-	unsigned long flags;
-
-	if (type != EV_FF)                     return -EINVAL;
-	if (!LGFF_CHECK_OWNERSHIP(code, lgff)) return -EACCES;
-	if (value < 0)                         return -EINVAL;
-
-	spin_lock_irqsave(&lgff->lock, flags);
-
-	if (value > 0) {
-		if (test_bit(EFFECT_STARTED, effect->flags)) {
-			spin_unlock_irqrestore(&lgff->lock, flags);
-			return -EBUSY;
-		}
-		if (test_bit(EFFECT_PLAYING, effect->flags)) {
-			spin_unlock_irqrestore(&lgff->lock, flags);
-			return -EBUSY;
-		}
-
-		effect->count = value;
-
-		if (effect->effect.replay.delay) {
-			set_bit(EFFECT_STARTED, effect->flags);
-		} else {
-			set_bit(EFFECT_PLAYING, effect->flags);
-		}
-		effect->started_at = jiffies;
-	}
-	else { /* value == 0 */
-		clear_bit(EFFECT_STARTED, effect->flags);
-		clear_bit(EFFECT_PLAYING, effect->flags);
-	}
-
-	spin_unlock_irqrestore(&lgff->lock, flags);
-
-	return 0;
-
-}
-
-/* Erase all effects this process owns */
-static int hid_lgff_flush(struct input_dev *dev, struct file *file)
-{
-	struct hid_device *hid = dev->private;
-	struct lgff_device *lgff = hid->ff_private;
-	int i;
-
-	for (i=0; i<dev->ff_effects_max; ++i) {
-
-		/*NOTE: no need to lock here. The only times EFFECT_USED is
-		  modified is when effects are uploaded or when an effect is
-		  erased. But a process cannot close its dev/input/eventX fd
-		  and perform ioctls on the same fd all at the same time */
-		if ( current->pid == lgff->effects[i].owner
-		     && test_bit(EFFECT_USED, lgff->effects[i].flags)) {
-
-			if (hid_lgff_erase(dev, i))
-				warn("erase effect %d failed", i);
-		}
-
-	}
-
-	return 0;
-}
-
-static int hid_lgff_erase(struct input_dev *dev, int id)
+static int hid_lgff_play(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old)
 {
 	struct hid_device *hid = dev->private;
-	struct lgff_device *lgff = hid->ff_private;
-	unsigned long flags;
-
-	if (!LGFF_CHECK_OWNERSHIP(id, lgff)) return -EACCES;
-
-	spin_lock_irqsave(&lgff->lock, flags);
-	lgff->effects[id].flags[0] = 0;
-	spin_unlock_irqrestore(&lgff->lock, flags);
-
-	return 0;
-}
-
-static int hid_lgff_upload_effect(struct input_dev* input,
-				  struct ff_effect* effect)
-{
-	struct hid_device *hid = input->private;
-	struct lgff_device *lgff = hid->ff_private;
-	struct lgff_effect new;
-	int id;
-	unsigned long flags;
-
-	dbg("ioctl rumble");
-
-	if (!test_bit(effect->type, input->ffbit)) return -EINVAL;
-
-	spin_lock_irqsave(&lgff->lock, flags);
-
-	if (effect->id == -1) {
-		int i;
-
-		for (i=0; i<LGFF_EFFECTS && test_bit(EFFECT_USED, lgff->effects[i].flags); ++i);
-		if (i >= LGFF_EFFECTS) {
-			spin_unlock_irqrestore(&lgff->lock, flags);
-			return -ENOSPC;
-		}
-
-		effect->id = i;
-		lgff->effects[i].owner = current->pid;
-		lgff->effects[i].flags[0] = 0;
-		set_bit(EFFECT_USED, lgff->effects[i].flags);
-	}
-	else if (!LGFF_CHECK_OWNERSHIP(effect->id, lgff)) {
-		spin_unlock_irqrestore(&lgff->lock, flags);
-		return -EACCES;
-	}
-
-	id = effect->id;
-	new = lgff->effects[id];
-
-	new.effect = *effect;
-
-	if (test_bit(EFFECT_STARTED, lgff->effects[id].flags)
-	    || test_bit(EFFECT_STARTED, lgff->effects[id].flags)) {
-
-		/* Changing replay parameters is not allowed (for the time
-		   being) */
-		if (new.effect.replay.delay != lgff->effects[id].effect.replay.delay
-		    || new.effect.replay.length != lgff->effects[id].effect.replay.length) {
-			spin_unlock_irqrestore(&lgff->lock, flags);
-			return -ENOSYS;
-		}
-
-		lgff->effects[id] = new;
-
-	} else {
-		lgff->effects[id] = new;
-	}
-
-	spin_unlock_irqrestore(&lgff->lock, flags);
-	return 0;
-}
-
-static void hid_lgff_timer(unsigned long timer_data)
-{
-	struct lgff_device *lgff = (struct lgff_device*)timer_data;
-	struct hid_device *hid = lgff->hid;
-	unsigned long flags;
-	int x = 0x7f, y = 0x7f;   // Coordinates of constant effects
-	unsigned int left = 0, right = 0;   // Rumbling
-	int i;
-
-	spin_lock_irqsave(&lgff->lock, flags);
-
-	for (i=0; i<LGFF_EFFECTS; ++i) {
-		struct lgff_effect* effect = lgff->effects +i;
-
-		if (test_bit(EFFECT_PLAYING, effect->flags)) {
-
-			switch (effect->effect.type) {
-			case FF_CONSTANT: {
-				//TODO: handle envelopes
-				int degrees = effect->effect.direction * 360 >> 16;
-	/*			x += fixp_mult(fixp_sin(degrees),
-					       fixp_new16(effect->effect.u.constant.level));
-				y += fixp_mult(-fixp_cos(degrees),
-					       fixp_new16(effect->effect.u.constant.level));*/
-			}       break;
-			case FF_RUMBLE:
-				right += effect->effect.u.rumble.strong_magnitude;
-				left += effect->effect.u.rumble.weak_magnitude;
-				break;
-			};
-
-			/* One run of the effect is finished playing */
-			if (time_after(jiffies,
-					effect->started_at
-					+ effect->effect.replay.delay*HZ/1000
-					+ effect->effect.replay.length*HZ/1000)) {
-				dbg("Finished playing once %d", i);
-				if (--effect->count <= 0) {
-					dbg("Stopped %d", i);
-					clear_bit(EFFECT_PLAYING, effect->flags);
-				}
-				else {
-					dbg("Start again %d", i);
-					if (effect->effect.replay.length != 0) {
-						clear_bit(EFFECT_PLAYING, effect->flags);
-						set_bit(EFFECT_STARTED, effect->flags);
-					}
-					effect->started_at = jiffies;
-				}
-			}
-
-		} else if (test_bit(EFFECT_STARTED, lgff->effects[i].flags)) {
-			/* Check if we should start playing the effect */
-			if (time_after(jiffies,
-					lgff->effects[i].started_at
-					+ lgff->effects[i].effect.replay.delay*HZ/1000)) {
-				dbg("Now playing %d", i);
-				clear_bit(EFFECT_STARTED, lgff->effects[i].flags);
-				set_bit(EFFECT_PLAYING, lgff->effects[i].flags);
-			}
-		}
-	}
-
+	int x, y;   // Coordinates of constant effects, 0x7f = center
+	unsigned int left, right;   // Rumbling
+	struct hid_report* report = list_entry(hid->report_enum[HID_OUTPUT_REPORT].report_list.next,
+					       struct hid_report, list);
+
 #define CLAMP(x) if (x < 0) x = 0; if (x > 0xff) x = 0xff
-
-	// Clamp values
-	CLAMP(x);
-	CLAMP(y);
-	CLAMP(left);
-	CLAMP(right);
-
-#undef CLAMP
-
-	if (x != lgff->constant->field[0]->value[2]
-	    || y != lgff->constant->field[0]->value[3]) {
-		lgff->constant->field[0]->value[2] = x;
-		lgff->constant->field[0]->value[3] = y;
+	switch (effect->type) {
+	case FF_CONSTANT:
+		x = effect->u.ramp.start_level + 0x7f;
+		y = effect->u.ramp.end_level + 0x7f;
+		CLAMP(x);
+		CLAMP(y);
+		report->field[0]->value[0] = 0x51;
+		report->field[0]->value[1] = 0x08;
+		report->field[0]->value[2] = x;
+		report->field[0]->value[3] = y;
 		dbg("(x,y)=(%04x, %04x)", x, y);
-		hid_submit_report(hid, lgff->constant, USB_DIR_OUT);
-	}
-
-	if (left != lgff->rumble->field[0]->value[2]
-	    || right != lgff->rumble->field[0]->value[3]) {
-		lgff->rumble->field[0]->value[2] = left;
-		lgff->rumble->field[0]->value[3] = right;
+		hid_submit_report(hid, report, USB_DIR_OUT);
+		break;
+	case FF_RUMBLE:
+		right = effect->u.rumble.strong_magnitude;
+		left = effect->u.rumble.weak_magnitude;
+		right = right * 0xff / 0xffff;
+		left = left * 0xff / 0xffff;
+		CLAMP(left);
+		CLAMP(right);
+		report->field[0]->value[0] = 0x42;
+		report->field[0]->value[1] = 0x00;
+		report->field[0]->value[2] = left;
+		report->field[0]->value[3] = right;
 		dbg("(left,right)=(%04x, %04x)", left, right);
-		hid_submit_report(hid, lgff->rumble, USB_DIR_OUT);
-	}
-
-	if (!test_bit(DEVICE_CLOSING, lgff->flags)) {
-		lgff->timer.expires = RUN_AT(PERIOD);
-		add_timer(&lgff->timer);
+		hid_submit_report(hid, report, USB_DIR_OUT);
+		break;
 	}
-
-	spin_unlock_irqrestore(&lgff->lock, flags);
+	return 0;
 }
Index: linux-2.6.17-rc4-git1/drivers/usb/input/hid-tmff.c
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/usb/input/hid-tmff.c	2006-05-13 19:58:57.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/usb/input/hid-tmff.c	2006-05-14 00:14:44.000000000 +0300
@@ -28,83 +28,28 @@
  */
 
 #include <linux/input.h>
-#include <linux/sched.h>
 
 #undef DEBUG
 #include <linux/usb.h>
 
-#include <linux/circ_buf.h>
-
 #include "hid.h"
-#include "fixp-arith.h"
 
 /* Usages for thrustmaster devices I know about */
 #define THRUSTMASTER_USAGE_RUMBLE_LR	(HID_UP_GENDESK | 0xbb)
-#define DELAY_CALC(t,delay)		((t) + (delay)*HZ/1000)
-
-/* Effect status */
-#define EFFECT_STARTED 0	/* Effect is going to play after some time */
-#define EFFECT_PLAYING 1	/* Effect is playing */
-#define EFFECT_USED    2
-
-/* For tmff_device::flags */
-#define DEVICE_CLOSING 0	/* The driver is being unitialised */
-
-/* Check that the current process can access an effect */
-#define CHECK_OWNERSHIP(effect) (current->pid == 0 \
-        || effect.owner == current->pid)
-
-#define TMFF_CHECK_ID(id)	((id) >= 0 && (id) < TMFF_EFFECTS)
-
-#define TMFF_CHECK_OWNERSHIP(i, l) \
-        (test_bit(EFFECT_USED, l->effects[i].flags) \
-        && CHECK_OWNERSHIP(l->effects[i]))
-
-#define TMFF_EFFECTS 8
-
-struct tmff_effect {
-	pid_t owner;
 
-	struct ff_effect effect;
-
-	unsigned long flags[1];
-	unsigned int count;             /* Number of times left to play */
-
-	unsigned long play_at;          /* When the effect starts to play */
-	unsigned long stop_at;		/* When the effect ends */
-};
 
 struct tmff_device {
-	struct hid_device *hid;
-
 	struct hid_report *report;
-
 	struct hid_field *rumble;
-
-	unsigned int effects_playing;
-	struct tmff_effect effects[TMFF_EFFECTS];
-	spinlock_t lock;             /* device-level lock. Having locks on
-					a per-effect basis could be nice, but
-					isn't really necessary */
-
-	unsigned long flags[1];      /* Contains various information about the
-					state of the driver for this device */
-
-	struct timer_list timer;
 };
 
 /* Callbacks */
 static void hid_tmff_exit(struct hid_device *hid);
-static int hid_tmff_event(struct hid_device *hid, struct input_dev *input,
-			  unsigned int type, unsigned int code, int value);
-static int hid_tmff_flush(struct input_dev *input, struct file *file);
-static int hid_tmff_upload_effect(struct input_dev *input,
-				  struct ff_effect *effect);
-static int hid_tmff_erase(struct input_dev *input, int id);
-
-/* Local functions */
-static void hid_tmff_recalculate_timer(struct tmff_device *tmff);
-static void hid_tmff_timer(unsigned long timer_data);
+static int hid_tmff_play(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old);
+
+static struct ff_driver tmff_driver = {
+	.upload = hid_tmff_play,
+};
 
 int hid_tmff_init(struct hid_device *hid)
 {
@@ -112,11 +57,18 @@ int hid_tmff_init(struct hid_device *hid
 	struct list_head *pos;
 	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
 	struct input_dev *input_dev = hidinput->input;
+	int ret;
 
 	private = kzalloc(sizeof(struct tmff_device), GFP_KERNEL);
 	if (!private)
 		return -ENOMEM;
 
+	ret = input_ff_allocate(input_dev);
+	if (ret) {
+		kfree(private);
+		return ret;
+	}
+
 	hid->ff_private = private;
 
 	/* Find the report to use */
@@ -155,33 +107,24 @@ int hid_tmff_init(struct hid_device *hid
 					private->report = report;
 					private->rumble = field;
 
-					set_bit(FF_RUMBLE, input_dev->ffbit);
+					set_bit(FF_RUMBLE, input_dev->ff->flags);
 					break;
 
 				default:
 					warn("ignoring unknown output usage %08x", field->usage[0].hid);
 					continue;
 			}
-
-			/* Fallthrough to here only when a valid usage is found */
-			input_dev->upload_effect = hid_tmff_upload_effect;
-			input_dev->flush = hid_tmff_flush;
-
-			set_bit(EV_FF, input_dev->evbit);
-			input_dev->ff_effects_max = TMFF_EFFECTS;
 		}
 	}
 
-	private->hid = hid;
-
-	spin_lock_init(&private->lock);
-	init_timer(&private->timer);
-	private->timer.data = (unsigned long)private;
-	private->timer.function = hid_tmff_timer;
+	ret = input_ff_register(input_dev, &tmff_driver);
+	if (ret) {
+		kfree(private);
+		return ret;
+	}
 
-	/* Event and exit callbacks */
+	/* Exit callback */
 	hid->ff_exit = hid_tmff_exit;
-	hid->ff_event = hid_tmff_event;
 
 	info("Force feedback for ThrustMaster rumble pad devices by Zinx Verituse <zinx@epicsol.org>");
 
@@ -191,196 +134,10 @@ int hid_tmff_init(struct hid_device *hid
 static void hid_tmff_exit(struct hid_device *hid)
 {
 	struct tmff_device *tmff = hid->ff_private;
-	unsigned long flags;
-
-	spin_lock_irqsave(&tmff->lock, flags);
-
-	set_bit(DEVICE_CLOSING, tmff->flags);
-	del_timer_sync(&tmff->timer);
-
-	spin_unlock_irqrestore(&tmff->lock, flags);
 
 	kfree(tmff);
 }
 
-static int hid_tmff_event(struct hid_device *hid, struct input_dev *input,
-			  unsigned int type, unsigned int code, int value)
-{
-	struct tmff_device *tmff = hid->ff_private;
-	struct tmff_effect *effect = &tmff->effects[code];
-	unsigned long flags;
-
-	if (type != EV_FF)
-		return -EINVAL;
-	if (!TMFF_CHECK_ID(code))
-		return -EINVAL;
-	if (!TMFF_CHECK_OWNERSHIP(code, tmff))
-		return -EACCES;
-	if (value < 0)
-		return -EINVAL;
-
-	spin_lock_irqsave(&tmff->lock, flags);
-
-	if (value > 0) {
-		set_bit(EFFECT_STARTED, effect->flags);
-		clear_bit(EFFECT_PLAYING, effect->flags);
-		effect->count = value;
-		effect->play_at = DELAY_CALC(jiffies, effect->effect.replay.delay);
-	} else {
-		clear_bit(EFFECT_STARTED, effect->flags);
-		clear_bit(EFFECT_PLAYING, effect->flags);
-	}
-
-	hid_tmff_recalculate_timer(tmff);
-
-	spin_unlock_irqrestore(&tmff->lock, flags);
-
-	return 0;
-
-}
-
-/* Erase all effects this process owns */
-
-static int hid_tmff_flush(struct input_dev *dev, struct file *file)
-{
-	struct hid_device *hid = dev->private;
-	struct tmff_device *tmff = hid->ff_private;
-	int i;
-
-	for (i=0; i<dev->ff_effects_max; ++i)
-
-	     /* NOTE: no need to lock here. The only times EFFECT_USED is
-		modified is when effects are uploaded or when an effect is
-		erased. But a process cannot close its dev/input/eventX fd
-		and perform ioctls on the same fd all at the same time */
-
-		if (current->pid == tmff->effects[i].owner
-		     && test_bit(EFFECT_USED, tmff->effects[i].flags))
-			if (hid_tmff_erase(dev, i))
-				warn("erase effect %d failed", i);
-
-
-	return 0;
-}
-
-static int hid_tmff_erase(struct input_dev *dev, int id)
-{
-	struct hid_device *hid = dev->private;
-	struct tmff_device *tmff = hid->ff_private;
-	unsigned long flags;
-
-	if (!TMFF_CHECK_ID(id))
-		return -EINVAL;
-	if (!TMFF_CHECK_OWNERSHIP(id, tmff))
-		return -EACCES;
-
-	spin_lock_irqsave(&tmff->lock, flags);
-
-	tmff->effects[id].flags[0] = 0;
-	hid_tmff_recalculate_timer(tmff);
-
-	spin_unlock_irqrestore(&tmff->lock, flags);
-
-	return 0;
-}
-
-static int hid_tmff_upload_effect(struct input_dev *input,
-				  struct ff_effect *effect)
-{
-	struct hid_device *hid = input->private;
-	struct tmff_device *tmff = hid->ff_private;
-	int id;
-	unsigned long flags;
-
-	if (!test_bit(effect->type, input->ffbit))
-		return -EINVAL;
-	if (effect->id != -1 && !TMFF_CHECK_ID(effect->id))
-		return -EINVAL;
-
-	spin_lock_irqsave(&tmff->lock, flags);
-
-	if (effect->id == -1) {
-		/* Find a free effect */
-		for (id = 0; id < TMFF_EFFECTS && test_bit(EFFECT_USED, tmff->effects[id].flags); ++id);
-
-		if (id >= TMFF_EFFECTS) {
-			spin_unlock_irqrestore(&tmff->lock, flags);
-			return -ENOSPC;
-		}
-
-		effect->id = id;
-		tmff->effects[id].owner = current->pid;
-		tmff->effects[id].flags[0] = 0;
-		set_bit(EFFECT_USED, tmff->effects[id].flags);
-
-	} else {
-		/* Re-uploading an owned effect, to change parameters */
-		id = effect->id;
-		clear_bit(EFFECT_PLAYING, tmff->effects[id].flags);
-	}
-
-	tmff->effects[id].effect = *effect;
-
-	hid_tmff_recalculate_timer(tmff);
-
-	spin_unlock_irqrestore(&tmff->lock, flags);
-	return 0;
-}
-
-/* Start the timer for the next start/stop/delay */
-/* Always call this while tmff->lock is locked */
-
-static void hid_tmff_recalculate_timer(struct tmff_device *tmff)
-{
-	int i;
-	int events = 0;
-	unsigned long next_time;
-
-	next_time = 0;	/* Shut up compiler's incorrect warning */
-
-	/* Find the next change in an effect's status */
-	for (i = 0; i < TMFF_EFFECTS; ++i) {
-		struct tmff_effect *effect = &tmff->effects[i];
-		unsigned long play_time;
-
-		if (!test_bit(EFFECT_STARTED, effect->flags))
-			continue;
-
-		effect->stop_at = DELAY_CALC(effect->play_at, effect->effect.replay.length);
-
-		if (!test_bit(EFFECT_PLAYING, effect->flags))
-			play_time = effect->play_at;
-		else
-			play_time = effect->stop_at;
-
-		events++;
-
-		if (time_after(jiffies, play_time))
-			play_time = jiffies;
-
-		if (events == 1)
-			next_time = play_time;
-		else {
-			if (time_after(next_time, play_time))
-				next_time = play_time;
-		}
-	}
-
-	if (!events && tmff->effects_playing) {
-		/* Treat all effects turning off as an event */
-		events = 1;
-		next_time = jiffies;
-	}
-
-	if (!events) {
-		/* No events, no time, no need for a timer. */
-		del_timer_sync(&tmff->timer);
-		return;
-	}
-
-	mod_timer(&tmff->timer, next_time);
-}
-
 /* Changes values from 0 to 0xffff into values from minimum to maximum */
 static inline int hid_tmff_scale(unsigned int in, int minimum, int maximum)
 {
@@ -394,70 +151,20 @@ static inline int hid_tmff_scale(unsigne
 	return ret;
 }
 
-static void hid_tmff_timer(unsigned long timer_data)
+static int hid_tmff_play(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old)
 {
-	struct tmff_device *tmff = (struct tmff_device *) timer_data;
-	struct hid_device *hid = tmff->hid;
-	unsigned long flags;
-	int left = 0, right = 0;	/* Rumbling */
-	int i;
-
-	spin_lock_irqsave(&tmff->lock, flags);
-
-	tmff->effects_playing = 0;
-
-	for (i = 0; i < TMFF_EFFECTS; ++i) {
-		struct tmff_effect *effect = &tmff->effects[i];
-
-		if (!test_bit(EFFECT_STARTED, effect->flags))
-			continue;
-
-		if (!time_after(jiffies, effect->play_at))
-			continue;
-
-		if (time_after(jiffies, effect->stop_at)) {
-
-			dbg("Finished playing once %d", i);
-			clear_bit(EFFECT_PLAYING, effect->flags);
-
-			if (--effect->count <= 0) {
-				dbg("Stopped %d", i);
-				clear_bit(EFFECT_STARTED, effect->flags);
-				continue;
-			} else {
-				dbg("Start again %d", i);
-				effect->play_at = DELAY_CALC(jiffies, effect->effect.replay.delay);
-				continue;
-			}
-		}
-
-		++tmff->effects_playing;
-
-		set_bit(EFFECT_PLAYING, effect->flags);
-
-		switch (effect->effect.type) {
-			case FF_RUMBLE:
-				right += effect->effect.u.rumble.strong_magnitude;
-				left += effect->effect.u.rumble.weak_magnitude;
-				break;
-			default:
-				BUG();
-				break;
-		}
-	}
+	struct hid_device *hid = dev->private;
+	struct tmff_device *tmff = hid->ff_private;
+	int left, right;	/* Rumbling */
+	right = effect->u.rumble.strong_magnitude;
+	left = effect->u.rumble.weak_magnitude;
 
 	left = hid_tmff_scale(left, tmff->rumble->logical_minimum, tmff->rumble->logical_maximum);
 	right = hid_tmff_scale(right, tmff->rumble->logical_minimum, tmff->rumble->logical_maximum);
 
-	if (left != tmff->rumble->value[0] || right != tmff->rumble->value[1]) {
-		tmff->rumble->value[0] = left;
-		tmff->rumble->value[1] = right;
-		dbg("(left,right)=(%08x, %08x)", left, right);
-		hid_submit_report(hid, tmff->report, USB_DIR_OUT);
-	}
-
-	if (!test_bit(DEVICE_CLOSING, tmff->flags))
-		hid_tmff_recalculate_timer(tmff);
-
-	spin_unlock_irqrestore(&tmff->lock, flags);
+	tmff->rumble->value[0] = left;
+	tmff->rumble->value[1] = right;
+	dbg("(left,right)=(%08x, %08x)", left, right);
+	hid_submit_report(hid, tmff->report, USB_DIR_OUT);
+	return 0;
 }
Index: linux-2.6.17-rc4-git1/drivers/usb/input/Kconfig
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/usb/input/Kconfig	2006-05-13 19:58:57.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/usb/input/Kconfig	2006-05-14 01:36:21.000000000 +0300
@@ -49,7 +49,7 @@ config USB_HIDINPUT_POWERBOOK
 
 config HID_FF
 	bool "Force feedback support (EXPERIMENTAL)"
-	depends on USB_HIDINPUT && EXPERIMENTAL
+	depends on USB_HIDINPUT && EXPERIMENTAL && INPUT_FF_EFFECTS
 	help
 	  Say Y here is you want force feedback support for a few HID devices.
 	  See below for a list of supported devices.

--
Anssi Hannula

[patch 05/11] input: adapt uinput for the new force feedback interface

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-uinput-to-new-interface.diff --]
[-- Type: text/plain, Size: 8311 bytes --]

Modify the uinput driver for the new force feedback interface. The userspace
interface of the force feedback part is changed and documentation in uinput.h
is updated accordingly. MODULE_VERSION is also added to reflect the revision.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/input/misc/uinput.c |   74 +++++++++++++++++++++++++++++++++++---------
 include/linux/uinput.h      |   17 ++++++----
 2 files changed, 71 insertions(+), 20 deletions(-)

Index: linux-2.6.16-git20/drivers/input/misc/uinput.c
===================================================================
--- linux-2.6.16-git20.orig/drivers/input/misc/uinput.c	2006-04-15 03:10:24.000000000 +0300
+++ linux-2.6.16-git20/drivers/input/misc/uinput.c	2006-04-15 03:23:09.000000000 +0300
@@ -20,6 +20,9 @@
  * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org>
  *
  * Changes/Revisions:
+ *	0.3	09/04/2006 (Anssi Hannula <anssi.hannula@gmail.com>)
+ *		- updated ff support for the changes in kernel interface
+ *		- added MODULE_VERSION
  *	0.2	16/10/2004 (Micah Dowty <micah@navi.cx>)
  *		- added force feedback support
  *              - added UI_SET_PHYS
@@ -107,18 +110,31 @@ static int uinput_request_submit(struct 
 	return request->retval;
 }
 
-static int uinput_dev_upload_effect(struct input_dev *dev, struct ff_effect *effect)
+static void uinput_dev_set_gain(struct input_dev *dev, u16 gain)
+{
+	uinput_dev_event(dev, EV_FF, FF_GAIN, gain);
+}
+
+static void uinput_dev_set_autocenter(struct input_dev *dev, u16 magnitude)
+{
+	uinput_dev_event(dev, EV_FF, FF_AUTOCENTER, magnitude);
+}
+
+static int uinput_dev_playback(struct input_dev *dev, int effect_id, int value)
+{
+	return uinput_dev_event(dev, EV_FF, effect_id, value);
+}
+
+static int uinput_dev_upload_effect(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old)
 {
 	struct uinput_request request;
 	int retval;
 
-	if (!test_bit(EV_FF, dev->evbit))
-		return -ENOSYS;
-
 	request.id = -1;
 	init_completion(&request.done);
 	request.code = UI_FF_UPLOAD;
-	request.u.effect = effect;
+	request.u.upload.effect = effect;
+	request.u.upload.old = old;
 
 	retval = uinput_request_reserve_slot(dev->private, &request);
 	if (!retval)
@@ -166,6 +182,14 @@ static void uinput_destroy_device(struct
 	udev->state = UIST_NEW_DEVICE;
 }
 
+static struct ff_driver uinput_ff_driver = {
+	.upload = uinput_dev_upload_effect,
+	.erase = uinput_dev_erase_effect,
+	.playback = uinput_dev_playback,
+	.set_gain = uinput_dev_set_gain,
+	.set_autocenter = uinput_dev_set_autocenter
+};
+
 static int uinput_create_device(struct uinput_device *udev)
 {
 	int error;
@@ -181,6 +205,14 @@ static int uinput_create_device(struct u
 		return error;
 	}
 
+	if (udev->dev->ff) {
+		error = input_ff_register(udev->dev, &uinput_ff_driver);
+		if (error) {
+			uinput_destroy_device(udev);
+			return error;
+		}
+	}
+
 	udev->state = UIST_CREATED;
 
 	return 0;
@@ -243,8 +275,6 @@ static int uinput_allocate_device(struct
 		return -ENOMEM;
 
 	udev->dev->event = uinput_dev_event;
-	udev->dev->upload_effect = uinput_dev_upload_effect;
-	udev->dev->erase_effect = uinput_dev_erase_effect;
 	udev->dev->private = udev;
 
 	return 0;
@@ -296,7 +326,12 @@ static int uinput_setup_device(struct ui
 	dev->id.vendor	= user_dev->id.vendor;
 	dev->id.product	= user_dev->id.product;
 	dev->id.version	= user_dev->id.version;
-	dev->ff_effects_max = user_dev->ff_effects_max;
+	if (user_dev->ff_effects_max) {
+		retval = input_ff_allocate(dev);
+		if (retval)
+			goto exit;
+		dev->ff_effects_max = user_dev->ff_effects_max;
+	}
 
 	size = sizeof(int) * (ABS_MAX + 1);
 	memcpy(dev->absmax, user_dev->absmax, size);
@@ -459,7 +494,10 @@ static long uinput_ioctl(struct file *fi
 			break;
 
 		case UI_SET_EVBIT:
-			retval = uinput_set_bit(arg, evbit, EV_MAX);
+			if (arg == EV_FF)
+				retval = -EINVAL;
+			else
+				retval = uinput_set_bit(arg, evbit, EV_MAX);
 			break;
 
 		case UI_SET_KEYBIT:
@@ -487,7 +525,10 @@ static long uinput_ioctl(struct file *fi
 			break;
 
 		case UI_SET_FFBIT:
-			retval = uinput_set_bit(arg, ffbit, FF_MAX);
+			if (!udev->dev->ff)
+				retval = -EINVAL;
+			else
+				retval = uinput_set_bit(arg, ff->flags, FF_MAX);
 			break;
 
 		case UI_SET_SWBIT:
@@ -525,12 +566,17 @@ static long uinput_ioctl(struct file *fi
 				break;
 			}
 			req = uinput_request_find(udev, ff_up.request_id);
-			if (!(req && req->code == UI_FF_UPLOAD && req->u.effect)) {
+			if (!(req && req->code == UI_FF_UPLOAD && req->u.upload.effect)) {
 				retval = -EINVAL;
 				break;
 			}
 			ff_up.retval = 0;
-			memcpy(&ff_up.effect, req->u.effect, sizeof(struct ff_effect));
+			memcpy(&ff_up.effect, req->u.upload.effect, sizeof(struct ff_effect));
+			if (req->u.upload.old)
+				memcpy(&ff_up.old, req->u.upload.old, sizeof(struct ff_effect));
+			else
+				memset(&ff_up.old, 0, sizeof(struct ff_effect));
+
 			if (copy_to_user(p, &ff_up, sizeof(ff_up))) {
 				retval = -EFAULT;
 				break;
@@ -561,12 +607,11 @@ static long uinput_ioctl(struct file *fi
 				break;
 			}
 			req = uinput_request_find(udev, ff_up.request_id);
-			if (!(req && req->code == UI_FF_UPLOAD && req->u.effect)) {
+			if (!(req && req->code == UI_FF_UPLOAD && req->u.upload.effect)) {
 				retval = -EINVAL;
 				break;
 			}
 			req->retval = ff_up.retval;
-			memcpy(req->u.effect, &ff_up.effect, sizeof(struct ff_effect));
 			uinput_request_done(udev, req);
 			break;
 
@@ -622,6 +667,7 @@ static void __exit uinput_exit(void)
 MODULE_AUTHOR("Aristeu Sergio Rozanski Filho");
 MODULE_DESCRIPTION("User level driver support for input subsystem");
 MODULE_LICENSE("GPL");
+MODULE_VERSION("0.3");
 
 module_init(uinput_init);
 module_exit(uinput_exit);
Index: linux-2.6.16-git20/include/linux/uinput.h
===================================================================
--- linux-2.6.16-git20.orig/include/linux/uinput.h	2006-04-15 03:10:24.000000000 +0300
+++ linux-2.6.16-git20/include/linux/uinput.h	2006-04-15 03:15:43.000000000 +0300
@@ -22,6 +22,8 @@
  * Author: Aristeu Sergio Rozanski Filho <aris@cathedrallabs.org>
  * 
  * Changes/Revisions:
+ *	0.3	09/04/2006 (Anssi Hannula <anssi.hannulagmail.com>)
+ *		- update ff support for the changes in kernel interface
  *	0.2	16/10/2004 (Micah Dowty <micah@navi.cx>)
  *		- added force feedback support
  *             - added UI_SET_PHYS
@@ -45,7 +47,10 @@ struct uinput_request {
 
 	union {
 		int		effect_id;
-		struct ff_effect* effect;
+		struct {
+			struct ff_effect* effect;
+			struct ff_effect* old;
+		} upload;
 	} u;
 };
 
@@ -69,6 +74,7 @@ struct uinput_ff_upload {
 	int			request_id;
 	int			retval;
 	struct ff_effect	effect;
+	struct ff_effect	old;
 };
 
 struct uinput_ff_erase {
@@ -105,7 +111,7 @@ struct uinput_ff_erase {
  * ioctls to retrieve additional parameters and send the return code.
  * The callback blocks until this return code is sent.
  *
- * The described callback mechanism is only used if EV_FF is set.
+ * The described callback mechanism is only used if ff_effects_max is set.
  * Otherwise, default implementations of upload_effect and erase_effect
  * are used.
  *
@@ -116,9 +122,9 @@ struct uinput_ff_erase {
  *      the 'value' from the EV_UINPUT event.
  *   3. Issue a UI_BEGIN_FF_UPLOAD ioctl, giving it the
  *      uinput_ff_upload struct. It will be filled in with the
- *      ff_effect passed to upload_effect().
- *   4. Perform the effect upload, and place the modified ff_effect
- *      and a return code back into the uinput_ff_upload struct.
+ *      ff_effects passed to upload_effect().
+ *   4. Perform the effect upload, and place a return code back into
+        the uinput_ff_upload struct.
  *   5. Issue a UI_END_FF_UPLOAD ioctl, also giving it the
  *      uinput_ff_upload_effect struct. This will complete execution
  *      of our upload_effect() handler.
@@ -133,7 +139,6 @@ struct uinput_ff_erase {
  *      effect ID passed to erase_effect().
  *   4. Perform the effect erasure, and place a return code back
  *      into the uinput_ff_erase struct.
- *      and a return code back into the uinput_ff_erase struct.
  *   5. Issue a UI_END_FF_ERASE ioctl, also giving it the
  *      uinput_ff_erase_effect struct. This will complete execution
  *      of our erase_effect() handler.

--
Anssi Hannula

[patch 06/11] input: adapt iforce driver for the new force feedback interface

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-iforce-to-new-interface.diff --]
[-- Type: text/plain, Size: 19916 bytes --]

Modify the iforce driver for the new force feedback interface.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/input/joystick/iforce/Kconfig       |    2 
 drivers/input/joystick/iforce/iforce-ff.c   |   61 +++------
 drivers/input/joystick/iforce/iforce-main.c |  188 ++++++++--------------------
 drivers/input/joystick/iforce/iforce.h      |   15 --
 4 files changed, 87 insertions(+), 179 deletions(-)

Index: linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/iforce-ff.c
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/joystick/iforce/iforce-ff.c	2006-05-14 01:36:21.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/iforce-ff.c	2006-05-14 01:42:00.000000000 +0300
@@ -198,10 +198,8 @@ static unsigned char find_button(struct 
  * Analyse the changes in an effect, and tell if we need to send an condition
  * parameter packet
  */
-static int need_condition_modifier(struct iforce* iforce, struct ff_effect* new)
+static int need_condition_modifier(struct ff_effect* old, struct ff_effect* new)
 {
-	int id = new->id;
-	struct ff_effect* old = &iforce->core_effects[id].effect;
 	int ret=0;
 	int i;
 
@@ -225,11 +223,8 @@ static int need_condition_modifier(struc
  * Analyse the changes in an effect, and tell if we need to send a magnitude
  * parameter packet
  */
-static int need_magnitude_modifier(struct iforce* iforce, struct ff_effect* effect)
+static int need_magnitude_modifier(struct ff_effect* old, struct ff_effect* effect)
 {
-	int id = effect->id;
-	struct ff_effect* old = &iforce->core_effects[id].effect;
-
 	if (effect->type != FF_CONSTANT) {
 		printk(KERN_WARNING "iforce.c: bad effect type in need_envelope_modifier\n");
 		return FALSE;
@@ -242,11 +237,8 @@ static int need_magnitude_modifier(struc
  * Analyse the changes in an effect, and tell if we need to send an envelope
  * parameter packet
  */
-static int need_envelope_modifier(struct iforce* iforce, struct ff_effect* effect)
+static int need_envelope_modifier(struct ff_effect* old, struct ff_effect* effect)
 {
-	int id = effect->id;
-	struct ff_effect* old = &iforce->core_effects[id].effect;
-
 	switch (effect->type) {
 	case FF_CONSTANT:
 		if (old->u.constant.envelope.attack_length != effect->u.constant.envelope.attack_length
@@ -275,16 +267,12 @@ static int need_envelope_modifier(struct
  * Analyse the changes in an effect, and tell if we need to send a periodic
  * parameter effect
  */
-static int need_period_modifier(struct iforce* iforce, struct ff_effect* new)
+static int need_period_modifier(struct ff_effect* old, struct ff_effect* new)
 {
-	int id = new->id;
-	struct ff_effect* old = &iforce->core_effects[id].effect;
-
 	if (new->type != FF_PERIODIC) {
-		printk(KERN_WARNING "iforce.c: bad effect type in need_periodic_modifier\n");
+		printk(KERN_WARNING "iforce.c: bad effect type in need_period_modifier\n");
 		return FALSE;
 	}
-
 	return (old->u.periodic.period != new->u.periodic.period
 		|| old->u.periodic.magnitude != new->u.periodic.magnitude
 		|| old->u.periodic.offset != new->u.periodic.offset
@@ -295,11 +283,8 @@ static int need_period_modifier(struct i
  * Analyse the changes in an effect, and tell if we need to send an effect
  * packet
  */
-static int need_core(struct iforce* iforce, struct ff_effect* new)
+static int need_core(struct ff_effect* old, struct ff_effect* new)
 {
-	int id = new->id;
-	struct ff_effect* old = &iforce->core_effects[id].effect;
-
 	if (old->direction != new->direction
 		|| old->trigger.button != new->trigger.button
 		|| old->trigger.interval != new->trigger.interval
@@ -360,7 +345,7 @@ static int make_core(struct iforce* ifor
  * Upload a periodic effect to the device
  * See also iforce_upload_constant.
  */
-int iforce_upload_periodic(struct iforce* iforce, struct ff_effect* effect, int is_update)
+int iforce_upload_periodic(struct iforce* iforce, struct ff_effect* effect, struct ff_effect *old)
 {
 	u8 wave_code;
 	int core_id = effect->id;
@@ -371,18 +356,18 @@ int iforce_upload_periodic(struct iforce
 	int param2_err = 1;
 	int core_err = 0;
 
-	if (!is_update || need_period_modifier(iforce, effect)) {
+	if (!old || need_period_modifier(old, effect)) {
 		param1_err = make_period_modifier(iforce, mod1_chunk,
-			is_update,
+			old ? 1 : 0,
 			effect->u.periodic.magnitude, effect->u.periodic.offset,
 			effect->u.periodic.period, effect->u.periodic.phase);
 		if (param1_err) return param1_err;
 		set_bit(FF_MOD1_IS_USED, core_effect->flags);
 	}
 
-	if (!is_update || need_envelope_modifier(iforce, effect)) {
+	if (!old || need_envelope_modifier(old, effect)) {
 		param2_err = make_envelope_modifier(iforce, mod2_chunk,
-			is_update,
+			old ? 1 : 0,
 			effect->u.periodic.envelope.attack_length,
 			effect->u.periodic.envelope.attack_level,
 			effect->u.periodic.envelope.fade_length,
@@ -400,7 +385,7 @@ int iforce_upload_periodic(struct iforce
 		default:		wave_code = 0x20; break;
 	}
 
-	if (!is_update || need_core(iforce, effect)) {
+	if (!old || need_core(old, effect)) {
 		core_err = make_core(iforce, effect->id,
 			mod1_chunk->start,
 			mod2_chunk->start,
@@ -429,7 +414,7 @@ int iforce_upload_periodic(struct iforce
  *  0 Ok, effect created or updated
  *  1 effect did not change since last upload, and no packet was therefore sent
  */
-int iforce_upload_constant(struct iforce* iforce, struct ff_effect* effect, int is_update)
+int iforce_upload_constant(struct iforce* iforce, struct ff_effect* effect, struct ff_effect* old)
 {
 	int core_id = effect->id;
 	struct iforce_core_effect* core_effect = iforce->core_effects + core_id;
@@ -439,17 +424,17 @@ int iforce_upload_constant(struct iforce
 	int param2_err = 1;
 	int core_err = 0;
 
-	if (!is_update || need_magnitude_modifier(iforce, effect)) {
+	if (!old || need_magnitude_modifier(old, effect)) {
 		param1_err = make_magnitude_modifier(iforce, mod1_chunk,
-			is_update,
+			old ? 1 : 0,
 			effect->u.constant.level);
 		if (param1_err) return param1_err;
 		set_bit(FF_MOD1_IS_USED, core_effect->flags);
 	}
 
-	if (!is_update || need_envelope_modifier(iforce, effect)) {
+	if (!old || need_envelope_modifier(old, effect)) {
 		param2_err = make_envelope_modifier(iforce, mod2_chunk,
-			is_update,
+			old ? 1 : 0,
 			effect->u.constant.envelope.attack_length,
 			effect->u.constant.envelope.attack_level,
 			effect->u.constant.envelope.fade_length,
@@ -458,7 +443,7 @@ int iforce_upload_constant(struct iforce
 		set_bit(FF_MOD2_IS_USED, core_effect->flags);
 	}
 
-	if (!is_update || need_core(iforce, effect)) {
+	if (!old || need_core(old, effect)) {
 		core_err = make_core(iforce, effect->id,
 			mod1_chunk->start,
 			mod2_chunk->start,
@@ -483,7 +468,7 @@ int iforce_upload_constant(struct iforce
 /*
  * Upload an condition effect. Those are for example friction, inertia, springs...
  */
-int iforce_upload_condition(struct iforce* iforce, struct ff_effect* effect, int is_update)
+int iforce_upload_condition(struct iforce* iforce, struct ff_effect* effect, struct ff_effect* old)
 {
 	int core_id = effect->id;
 	struct iforce_core_effect* core_effect = iforce->core_effects + core_id;
@@ -499,9 +484,9 @@ int iforce_upload_condition(struct iforc
 		default: return -1;
 	}
 
-	if (!is_update || need_condition_modifier(iforce, effect)) {
+	if (!old || need_condition_modifier(old, effect)) {
 		param_err = make_condition_modifier(iforce, mod1_chunk,
-			is_update,
+			old ? 1 : 0,
 			effect->u.condition[0].right_saturation,
 			effect->u.condition[0].left_saturation,
 			effect->u.condition[0].right_coeff,
@@ -512,7 +497,7 @@ int iforce_upload_condition(struct iforc
 		set_bit(FF_MOD1_IS_USED, core_effect->flags);
 
 		param_err = make_condition_modifier(iforce, mod2_chunk,
-			is_update,
+			old ? 1 : 0,
 			effect->u.condition[1].right_saturation,
 			effect->u.condition[1].left_saturation,
 			effect->u.condition[1].right_coeff,
@@ -524,7 +509,7 @@ int iforce_upload_condition(struct iforc
 
 	}
 
-	if (!is_update || need_core(iforce, effect)) {
+	if (!old || need_core(old, effect)) {
 		core_err = make_core(iforce, effect->id,
 			mod1_chunk->start, mod2_chunk->start,
 			type, 0xc0,
Index: linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/iforce.h
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/joystick/iforce/iforce.h	2006-05-14 01:36:21.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/iforce.h	2006-05-14 01:42:00.000000000 +0300
@@ -55,7 +55,7 @@
 #define FALSE 0
 #define TRUE 1
 
-#define FF_EFFECTS_MAX	32
+#define IFORCE_EFFECTS_MAX	32
 
 /* Each force feedback effect is made of one core effect, which can be
  * associated to at most to effect modifiers
@@ -79,13 +79,6 @@ struct iforce_core_effect {
 	struct resource mod1_chunk;
 	struct resource mod2_chunk;
 	unsigned long flags[NBITS(FF_MODCORE_MAX)];
-	pid_t owner;
-	/* Used to keep track of parameters of an effect. They are needed
-	 * to know what parts of an effect changed in an update operation.
-	 * We try to send only parameter packets if possible, as sending
-	 * effect parameter requires the effect to be stoped and restarted
-	 */
-	struct ff_effect effect;
 };
 
 #define FF_CMD_EFFECT		0x010e
@@ -183,9 +176,9 @@ void iforce_dump_packet(char *msg, u16 c
 int iforce_get_id_packet(struct iforce *iforce, char *packet);
 
 /* iforce-ff.c */
-int iforce_upload_periodic(struct iforce*, struct ff_effect*, int is_update);
-int iforce_upload_constant(struct iforce*, struct ff_effect*, int is_update);
-int iforce_upload_condition(struct iforce*, struct ff_effect*, int is_update);
+int iforce_upload_periodic(struct iforce*, struct ff_effect*, struct ff_effect* old);
+int iforce_upload_constant(struct iforce*, struct ff_effect*, struct ff_effect* old);
+int iforce_upload_condition(struct iforce*, struct ff_effect*, struct ff_effect* old);
 
 /* Public variables */
 extern struct serio_driver iforce_serio_drv;
Index: linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/iforce-main.c
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/joystick/iforce/iforce-main.c	2006-05-14 01:36:21.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/iforce-main.c	2006-05-14 01:42:00.000000000 +0300
@@ -82,103 +82,57 @@ static struct iforce_device iforce_devic
 	{ 0x0000, 0x0000, "Unknown I-Force Device [%04x:%04x]",		btn_joystick, abs_joystick, ff_iforce }
 };
 
+static int iforce_playback(struct input_dev *dev, int effect_id, int value)
+{
+	struct iforce* iforce = (struct iforce*)(dev->private);
+	if (value > 0) {
+		set_bit(FF_CORE_SHOULD_PLAY, iforce->core_effects[effect_id].flags);
+	}
+	else {
+		clear_bit(FF_CORE_SHOULD_PLAY, iforce->core_effects[effect_id].flags);
+	}
 
+	iforce_control_playback(iforce, effect_id, value);
+	return 0;
+}
 
-static int iforce_input_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
+static void iforce_set_gain(struct input_dev *dev, u16 gain)
 {
 	struct iforce* iforce = (struct iforce*)(dev->private);
 	unsigned char data[3];
+	data[0] = gain >> 9;
+	iforce_send_packet(iforce, FF_CMD_GAIN, data);
+}
 
-	if (type != EV_FF)
-		return -1;
-
-	switch (code) {
-
-		case FF_GAIN:
-
-			data[0] = value >> 9;
-			iforce_send_packet(iforce, FF_CMD_GAIN, data);
-
-			return 0;
-
-		case FF_AUTOCENTER:
-
-			data[0] = 0x03;
-			data[1] = value >> 9;
-			iforce_send_packet(iforce, FF_CMD_AUTOCENTER, data);
-
-			data[0] = 0x04;
-			data[1] = 0x01;
-			iforce_send_packet(iforce, FF_CMD_AUTOCENTER, data);
-
-			return 0;
-
-		default: /* Play or stop an effect */
-
-			if (!CHECK_OWNERSHIP(code, iforce)) {
-				return -1;
-			}
-			if (value > 0) {
-				set_bit(FF_CORE_SHOULD_PLAY, iforce->core_effects[code].flags);
-			}
-			else {
-				clear_bit(FF_CORE_SHOULD_PLAY, iforce->core_effects[code].flags);
-			}
-
-			iforce_control_playback(iforce, code, value);
-			return 0;
-	}
-
-	return -1;
+static void iforce_set_autocenter(struct input_dev *dev, u16 magnitude)
+{
+	struct iforce* iforce = (struct iforce*)(dev->private);
+	unsigned char data[3];
+	data[0] = 0x03;
+	data[1] = magnitude >> 9;
+	iforce_send_packet(iforce, FF_CMD_AUTOCENTER, data);
+
+	data[0] = 0x04;
+	data[1] = 0x01;
+	iforce_send_packet(iforce, FF_CMD_AUTOCENTER, data);
 }
 
 /*
  * Function called when an ioctl is performed on the event dev entry.
  * It uploads an effect to the device
  */
-static int iforce_upload_effect(struct input_dev *dev, struct ff_effect *effect)
+static int iforce_upload_effect(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old)
 {
 	struct iforce* iforce = (struct iforce*)(dev->private);
-	int id;
 	int ret;
-	int is_update;
-
-/* Check this effect type is supported by this device */
-	if (!test_bit(effect->type, iforce->dev->ffbit))
-		return -EINVAL;
-
-/*
- * If we want to create a new effect, get a free id
- */
-	if (effect->id == -1) {
-
-		for (id = 0; id < FF_EFFECTS_MAX; ++id)
-			if (!test_and_set_bit(FF_CORE_IS_USED, iforce->core_effects[id].flags))
-				break;
-
-		if (id == FF_EFFECTS_MAX || id >= iforce->dev->ff_effects_max)
-			return -ENOMEM;
 
-		effect->id = id;
-		iforce->core_effects[id].owner = current->pid;
-		iforce->core_effects[id].flags[0] = (1 << FF_CORE_IS_USED);	/* Only IS_USED bit must be set */
-
-		is_update = FALSE;
+	if (!old) {
+		iforce->core_effects[effect->id].flags[0] = (1 << FF_CORE_IS_USED);	/* Only IS_USED bit must be set */
 	}
 	else {
-		/* We want to update an effect */
-		if (!CHECK_OWNERSHIP(effect->id, iforce))
-			return -EACCES;
-
-		/* Parameter type cannot be updated */
-		if (effect->type != iforce->core_effects[effect->id].effect.type)
-			return -EINVAL;
-
 		/* Check the effect is not already being updated */
 		if (test_bit(FF_CORE_UPDATE, iforce->core_effects[effect->id].flags))
 			return -EAGAIN;
-
-		is_update = TRUE;
 	}
 
 /*
@@ -187,16 +141,16 @@ static int iforce_upload_effect(struct i
 	switch (effect->type) {
 
 		case FF_PERIODIC:
-			ret = iforce_upload_periodic(iforce, effect, is_update);
+			ret = iforce_upload_periodic(iforce, effect, old);
 			break;
 
 		case FF_CONSTANT:
-			ret = iforce_upload_constant(iforce, effect, is_update);
+			ret = iforce_upload_constant(iforce, effect, old);
 			break;
 
 		case FF_SPRING:
 		case FF_DAMPER:
-			ret = iforce_upload_condition(iforce, effect, is_update);
+			ret = iforce_upload_condition(iforce, effect, old);
 			break;
 
 		default:
@@ -208,7 +162,6 @@ static int iforce_upload_effect(struct i
 		 */
 		set_bit(FF_CORE_UPDATE, iforce->core_effects[effect->id].flags);
 	}
-	iforce->core_effects[effect->id].effect = *effect;
 	return ret;
 }
 
@@ -222,15 +175,6 @@ static int iforce_erase_effect(struct in
 	int err = 0;
 	struct iforce_core_effect* core_effect;
 
-	/* Check who is trying to erase this effect */
-	if (iforce->core_effects[effect_id].owner != current->pid) {
-		printk(KERN_WARNING "iforce-main.c: %d tried to erase an effect belonging to %d\n", current->pid, iforce->core_effects[effect_id].owner);
-		return -EACCES;
-	}
-
-	if (effect_id < 0 || effect_id >= FF_EFFECTS_MAX)
-		return -EINVAL;
-
 	core_effect = iforce->core_effects + effect_id;
 
 	if (test_bit(FF_MOD1_IS_USED, core_effect->flags))
@@ -265,30 +209,6 @@ static int iforce_open(struct input_dev 
 	return 0;
 }
 
-static int iforce_flush(struct input_dev *dev, struct file *file)
-{
-	struct iforce *iforce = dev->private;
-	int i;
-
-	/* Erase all effects this process owns */
-	for (i=0; i<dev->ff_effects_max; ++i) {
-
-		if (test_bit(FF_CORE_IS_USED, iforce->core_effects[i].flags) &&
-			current->pid == iforce->core_effects[i].owner) {
-
-			/* Stop effect */
-			input_report_ff(dev, i, 0);
-
-			/* Free ressources assigned to effect */
-			if (iforce_erase_effect(dev, i)) {
-				printk(KERN_WARNING "iforce_flush: erase effect %d failed\n", i);
-			}
-		}
-
-	}
-	return 0;
-}
-
 static void iforce_release(struct input_dev *dev)
 {
 	struct iforce *iforce = dev->private;
@@ -302,7 +222,6 @@ static void iforce_release(struct input_
 	if (i<dev->ff_effects_max) {
 		printk(KERN_WARNING "iforce_release: Device still owns effects\n");
 	}
-
 	/* Disable force feedback playback */
 	iforce_send_packet(iforce, FF_CMD_ENABLE, "\001");
 
@@ -338,11 +257,19 @@ void iforce_delete_device(struct iforce 
 	}
 }
 
+static struct ff_driver iforce_ff_driver = {
+	.upload = iforce_upload_effect,
+	.erase	= iforce_erase_effect,
+	.set_gain = iforce_set_gain,
+	.set_autocenter = iforce_set_autocenter,
+	.playback = iforce_playback,
+};
+
 int iforce_init_device(struct iforce *iforce)
 {
 	struct input_dev *input_dev;
 	unsigned char c[] = "CEOV";
-	int i;
+	int i, ff_err;
 
 	input_dev = input_allocate_device();
 	if (!input_dev)
@@ -354,6 +281,8 @@ int iforce_init_device(struct iforce *if
 	iforce->xmit.buf = iforce->xmit_data;
 	iforce->dev = input_dev;
 
+	ff_err = input_ff_allocate(input_dev);
+
 /*
  * Input device fields.
  */
@@ -377,11 +306,8 @@ int iforce_init_device(struct iforce *if
 	input_dev->name = "Unknown I-Force device";
 	input_dev->open = iforce_open;
 	input_dev->close = iforce_release;
-	input_dev->flush = iforce_flush;
-	input_dev->event = iforce_input_event;
-	input_dev->upload_effect = iforce_upload_effect;
-	input_dev->erase_effect = iforce_erase_effect;
-	input_dev->ff_effects_max = 10;
+	if (!ff_err)
+		input_dev->ff_effects_max = 10;
 
 /*
  * On-device memory allocation.
@@ -428,16 +354,17 @@ int iforce_init_device(struct iforce *if
 	else
 		printk(KERN_WARNING "iforce-main.c: Device does not respond to id packet B\n");
 
-	if (!iforce_get_id_packet(iforce, "N"))
-		iforce->dev->ff_effects_max = iforce->edata[1];
-	else
+	if (!iforce_get_id_packet(iforce, "N")) {
+		if (!ff_err)
+			iforce->dev->ff_effects_max = iforce->edata[1];
+	} else
 		printk(KERN_WARNING "iforce-main.c: Device does not respond to id packet N\n");
 
 	/* Check if the device can store more effects than the driver can really handle */
-	if (iforce->dev->ff_effects_max > FF_EFFECTS_MAX) {
+	if (!ff_err && iforce->dev->ff_effects_max > IFORCE_EFFECTS_MAX) {
 		printk(KERN_WARNING "input??: Device can handle %d effects, but N_EFFECTS_MAX is set to %d in iforce.h\n",
-			iforce->dev->ff_effects_max, FF_EFFECTS_MAX);
-		iforce->dev->ff_effects_max = FF_EFFECTS_MAX;
+		       iforce->dev->ff_effects_max, IFORCE_EFFECTS_MAX);
+		iforce->dev->ff_effects_max = IFORCE_EFFECTS_MAX;
 	}
 
 /*
@@ -471,7 +398,7 @@ int iforce_init_device(struct iforce *if
  * Set input device bitfields and ranges.
  */
 
-	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS) | BIT(EV_FF) | BIT(EV_FF_STATUS);
+	input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_ABS) | BIT(EV_FF_STATUS);
 
 	for (i = 0; iforce->type->btn[i] >= 0; i++) {
 		signed short t = iforce->type->btn[i];
@@ -515,8 +442,11 @@ int iforce_init_device(struct iforce *if
 		}
 	}
 
-	for (i = 0; iforce->type->ff[i] >= 0; i++)
-		set_bit(iforce->type->ff[i], input_dev->ffbit);
+	if (!ff_err) {
+		for (i = 0; iforce->type->ff[i] >= 0; i++)
+			set_bit(iforce->type->ff[i], input_dev->ff->flags);
+		input_ff_register(input_dev, &iforce_ff_driver);
+	}
 
 /*
  * Register input device.
Index: linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/Kconfig
===================================================================
--- linux-2.6.17-rc4-git1.orig/drivers/input/joystick/iforce/Kconfig	2006-05-15 22:16:40.000000000 +0300
+++ linux-2.6.17-rc4-git1/drivers/input/joystick/iforce/Kconfig	2006-05-15 22:16:56.000000000 +0300
@@ -3,7 +3,7 @@
 #
 config JOYSTICK_IFORCE
 	tristate "I-Force devices"
-	depends on INPUT && INPUT_JOYSTICK
+	depends on INPUT && INPUT_JOYSTICK && (INPUT_FF_EFFECTS || INPUT_FF_EFFECTS=n)
 	help
 	  Say Y here if you have an I-Force joystick or steering wheel
 

--
Anssi Hannula

[patch 07/11] input: force feedback driver for PID devices

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-new-driver-pidff.diff --]
[-- Type: text/plain, Size: 43245 bytes --]

Add force feedback driver for PID devices. This replaces the older PID driver
which was never completed.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/usb/input/Kconfig     |    4 
 drivers/usb/input/Makefile    |    2 
 drivers/usb/input/hid-ff.c    |   14 
 drivers/usb/input/hid-input.c |   21 
 drivers/usb/input/hid-pidff.c | 1105 ++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/input/hid.h       |    6 
 6 files changed, 1121 insertions(+), 31 deletions(-)

Index: linux-2.6.17-rc3-git12/drivers/usb/input/hid-ff.c
===================================================================
--- linux-2.6.17-rc3-git12.orig/drivers/usb/input/hid-ff.c	2006-05-13 23:37:59.000000000 +0300
+++ linux-2.6.17-rc3-git12/drivers/usb/input/hid-ff.c	2006-05-13 23:40:03.000000000 +0300
@@ -51,9 +51,6 @@ static struct hid_ff_initializer inits[]
 	{0x46d, 0xc295, hid_lgff_init},	// Logitech MOMO force wheel
 	{0x46d, 0xc219, hid_lgff_init}, // Logitech Cordless rumble pad 2
 #endif
-#ifdef CONFIG_HID_PID
-	{0x45e, 0x001b, hid_pid_init},
-#endif
 #ifdef CONFIG_THRUSTMASTER_FF
 	{0x44f, 0xb304, hid_tmff_init},
 #endif
@@ -80,9 +77,12 @@ int hid_ff_init(struct hid_device* hid)
 	init = hid_get_ff_init(le16_to_cpu(hid->dev->descriptor.idVendor),
 			       le16_to_cpu(hid->dev->descriptor.idProduct));
 
-	if (!init) {
-		dbg("hid_ff_init could not find initializer");
-		return -ENOSYS;
-	}
+	/* We try pidff when no other driver is found because PID is the
+	standards compliant way of implementing force feedback in HID.
+	pidff_init() will quickly abort if the device doesn't appear to
+	be a PID device */
+	if (!init)
+		return pidff_init(hid);
+
 	return init->init(hid);
 }
Index: linux-2.6.17-rc3-git12/drivers/usb/input/hid.h
===================================================================
--- linux-2.6.17-rc3-git12.orig/drivers/usb/input/hid.h	2006-05-13 23:40:02.000000000 +0300
+++ linux-2.6.17-rc3-git12/drivers/usb/input/hid.h	2006-05-13 23:40:03.000000000 +0300
@@ -527,5 +527,9 @@ static inline void hid_ff_exit(struct hi
 
 int hid_lgff_init(struct hid_device* hid);
 int hid_tmff_init(struct hid_device* hid);
-int hid_pid_init(struct hid_device* hid);
 
+#ifdef CONFIG_HID_PID
+int pidff_init(struct hid_device *hid);
+#else
+static inline int pidff_init(struct hid_device *hid) { return -1; }
+#endif
Index: linux-2.6.17-rc3-git12/drivers/usb/input/hid-input.c
===================================================================
--- linux-2.6.17-rc3-git12.orig/drivers/usb/input/hid-input.c	2006-05-13 23:40:02.000000000 +0300
+++ linux-2.6.17-rc3-git12/drivers/usb/input/hid-input.c	2006-05-13 23:40:03.000000000 +0300
@@ -67,11 +67,9 @@ static const struct {
 #define map_rel(c)	do { usage->code = c; usage->type = EV_REL; bit = input->relbit; max = REL_MAX; } while (0)
 #define map_key(c)	do { usage->code = c; usage->type = EV_KEY; bit = input->keybit; max = KEY_MAX; } while (0)
 #define map_led(c)	do { usage->code = c; usage->type = EV_LED; bit = input->ledbit; max = LED_MAX; } while (0)
-#define map_ff(c)	do { usage->code = c; usage->type = EV_FF;  bit = input->ffbit;  max =  FF_MAX; } while (0)
 
 #define map_abs_clear(c)	do { map_abs(c); clear_bit(c, bit); } while (0)
 #define map_key_clear(c)	do { map_key(c); clear_bit(c, bit); } while (0)
-#define map_ff_effect(c)	do { set_bit(c, input->ffbit); } while (0)
 
 #ifdef CONFIG_USB_HIDINPUT_POWERBOOK
 
@@ -527,23 +525,7 @@ static void hidinput_configure_usage(str
 
 		case HID_UP_PID:
 
-			set_bit(EV_FF, input->evbit);
 			switch(usage->hid & HID_USAGE) {
-				case 0x26: map_ff_effect(FF_CONSTANT);	goto ignore;
-				case 0x27: map_ff_effect(FF_RAMP);	goto ignore;
-				case 0x28: map_ff_effect(FF_CUSTOM);	goto ignore;
-				case 0x30: map_ff_effect(FF_SQUARE);	map_ff_effect(FF_PERIODIC); goto ignore;
-				case 0x31: map_ff_effect(FF_SINE);	map_ff_effect(FF_PERIODIC); goto ignore;
-				case 0x32: map_ff_effect(FF_TRIANGLE);	map_ff_effect(FF_PERIODIC); goto ignore;
-				case 0x33: map_ff_effect(FF_SAW_UP);	map_ff_effect(FF_PERIODIC); goto ignore;
-				case 0x34: map_ff_effect(FF_SAW_DOWN);	map_ff_effect(FF_PERIODIC); goto ignore;
-				case 0x40: map_ff_effect(FF_SPRING);	goto ignore;
-				case 0x41: map_ff_effect(FF_DAMPER);	goto ignore;
-				case 0x42: map_ff_effect(FF_INERTIA);	goto ignore;
-				case 0x43: map_ff_effect(FF_FRICTION);	goto ignore;
-				case 0x7e: map_ff(FF_GAIN);		break;
-				case 0x83: input->ff_effects_max = field->value[0]; goto ignore;
-				case 0x98: map_ff(FF_AUTOCENTER);	break;
 				case 0xa4: map_key_clear(BTN_DEAD);	break;
 				default: goto ignore;
 			}
@@ -687,8 +669,7 @@ void hidinput_hid_event(struct hid_devic
 	}
 
 	if (usage->hid == (HID_UP_PID | 0x83UL)) { /* Simultaneous Effects Max */
-		input->ff_effects_max = value;
-		dbg("Maximum Effects - %d",input->ff_effects_max);
+		dbg("Maximum Effects - %d",value);
 		return;
 	}
 
Index: linux-2.6.17-rc3-git12/drivers/usb/input/hid-pidff.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.17-rc3-git12/drivers/usb/input/hid-pidff.c	2006-05-13 23:43:22.000000000 +0300
@@ -0,0 +1,1105 @@
+/*
+ *  Force feedback driver for USB HID PID compliant devices
+ *
+ *  Copyright (c) 2005, 2006 Anssi Hannula <anssi.hannula@gmail.com>
+ */
+
+/*
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/sched.h>
+#include <linux/input.h>
+#include <linux/usb.h>
+
+#include "hid.h"
+
+/* #define DEBUG */
+
+#ifdef DEBUG
+#define debug(format, arg...) printk(KERN_DEBUG "hid-pidff: " format "\n" , ## arg)
+#else
+#define debug(format, arg...) do {} while (0)
+#endif
+
+/* Report usage table used to put reports into an array */
+
+#define PID_SET_EFFECT		0
+#define PID_EFFECT_OPERATION	1
+#define PID_DEVICE_GAIN		2
+#define PID_POOL		3
+#define PID_BLOCK_LOAD		4
+#define PID_BLOCK_FREE		5
+#define PID_DEVICE_CONTROL	6
+#define PID_CREATE_NEW_EFFECT	7
+
+#define PID_REQUIRED_REPORTS	7
+
+#define PID_SET_ENVELOPE	8
+#define PID_SET_CONDITION	9
+#define PID_SET_PERIODIC	10
+#define PID_SET_CONSTANT	11
+#define PID_SET_RAMP		12
+static u8 pidff_reports[] = { 0x21, 0x77, 0x7d, 0x7f, 0x89, 0x90, 0x96, 0xab,
+				 0x5a, 0x5f, 0x6e, 0x73, 0x74 };
+/* device_control is really 0x95, but 0x96 specified as it is the usage of
+the only field in that report */
+
+
+/* Value usage tables used to put fields and values into arrays */
+
+#define PID_EFFECT_BLOCK_INDEX	0
+
+#define PID_DURATION		1
+#define PID_GAIN		2
+#define PID_TRIGGER_BUTTON	3
+#define PID_TRIGGER_REPEAT_INT	4
+#define PID_DIRECTION_ENABLE	5
+#define PID_START_DELAY		6
+static u8 pidff_set_effect[] = { 0x22, 0x50, 0x52, 0x53, 0x54, 0x56, 0xa7 };
+
+#define PID_ATTACK_LEVEL	1
+#define PID_ATTACK_TIME		2
+#define PID_FADE_LEVEL		3
+#define PID_FADE_TIME		4
+static u8 pidff_set_envelope[] = { 0x22, 0x5b, 0x5c, 0x5d, 0x5e };
+
+#define PID_PARAM_BLOCK_OFFSET	1
+#define PID_CP_OFFSET		2
+#define PID_POS_COEFFICIENT	3
+#define PID_NEG_COEFFICIENT	4
+#define PID_POS_SATURATION	5
+#define PID_NEG_SATURATION	6
+#define PID_DEAD_BAND		7
+static u8 pidff_set_condition[] = { 0x22, 0x23, 0x60, 0x61, 0x62, 0x63, 0x64,
+				0x65 };
+
+#define PID_MAGNITUDE		1
+#define PID_OFFSET		2
+#define PID_PHASE		3
+#define PID_PERIOD		4
+static u8 pidff_set_periodic[] = { 0x22, 0x70, 0x6f, 0x71, 0x72 };
+static u8 pidff_set_constant[] = { 0x22, 0x70 };
+
+#define PID_RAMP_START		1
+#define PID_RAMP_END		2
+static u8 pidff_set_ramp[] = { 0x22, 0x75, 0x76 };
+
+#define PID_RAM_POOL_AVAILABLE	1
+static u8 pidff_block_load[] = { 0x22, 0xac };
+
+#define PID_LOOP_COUNT		1
+static u8 pidff_effect_operation[] = { 0x22, 0x7c };
+
+static u8 pidff_block_free[] = { 0x22 };
+
+#define PID_DEVICE_GAIN_FIELD	0
+static u8 pidff_device_gain[] = { 0x7e };
+
+#define PID_RAM_POOL_SIZE	0
+#define PID_SIMULTANEOUS_MAX	1
+#define PID_DEVICE_MANAGED_POOL	2
+static u8 pidff_pool[] = { 0x80, 0x83, 0xa9 };
+
+/* Special field key tables used to put special field keys into arrays */
+
+#define PID_ENABLE_ACTUATORS	0
+#define PID_RESET		1
+static u8 pidff_device_control[] = { 0x97, 0x9a };
+
+#define PID_CONSTANT	0
+#define PID_RAMP	1
+#define PID_SQUARE	2
+#define PID_SINE	3
+#define PID_TRIANGLE	4
+#define PID_SAW_UP	5
+#define PID_SAW_DOWN	6
+#define PID_SPRING	7
+#define PID_DAMPER	8
+#define PID_INERTIA	9
+#define PID_FRICTION	10
+static u8 pidff_effect_types[] = { 0x26, 0x27, 0x30, 0x31, 0x32, 0x33, 0x34,
+				0x40, 0x41, 0x42, 0x43 };
+
+#define PID_BLOCK_LOAD_SUCCESS	0
+#define PID_BLOCK_LOAD_FULL	1
+static u8 pidff_block_load_status[] = { 0x8c, 0x8d };
+
+#define PID_EFFECT_START	0
+#define PID_EFFECT_STOP		1
+static u8 pidff_effect_operation_status[] = { 0x79, 0x7b };
+
+struct pidff_usage {
+	struct hid_field *field;
+	s32 *value;
+};
+
+struct pidff_device {
+
+	struct hid_report *reports[sizeof(pidff_reports)];
+
+	struct pidff_usage set_effect[sizeof(pidff_set_effect)];
+	struct pidff_usage set_envelope[sizeof(pidff_set_envelope)];
+	struct pidff_usage set_condition[sizeof(pidff_set_condition)];
+	struct pidff_usage set_periodic[sizeof(pidff_set_periodic)];
+	struct pidff_usage set_constant[sizeof(pidff_set_constant)];
+	struct pidff_usage set_ramp[sizeof(pidff_set_ramp)];
+
+	struct pidff_usage device_gain[sizeof(pidff_device_gain)];
+	struct pidff_usage block_load[sizeof(pidff_block_load)];
+	struct pidff_usage pool[sizeof(pidff_pool)];
+	struct pidff_usage effect_operation[sizeof(pidff_effect_operation)];
+	struct pidff_usage block_free[sizeof(pidff_block_free)];
+
+	/* Special field is a field that is not composed of
+	usage<->value pairs that pidff_usage values are */
+
+	/* Special field in create_new_effect */
+	struct hid_field *create_new_effect_type;
+
+	/* Special fields in set_effect */
+	struct hid_field *set_effect_type;
+	struct hid_field *effect_direction;
+
+	/* Special field in device_control */
+	struct hid_field *device_control;
+
+	/* Special field in block_load */
+	struct hid_field *block_load_status;
+
+	/* Special field in effect_operation */
+	struct hid_field *effect_operation_status;
+
+	int control_id[sizeof(pidff_device_control)];
+	int type_id[sizeof(pidff_effect_types)];
+	int status_id[sizeof(pidff_block_load_status)];
+	int operation_id[sizeof(pidff_effect_operation_status)];
+
+	int pid_id[FF_EFFECTS_MAX];
+};
+
+static void pidff_exit(struct hid_device *hid)
+{
+	struct pidff_device *pidff = hid->ff_private;
+	debug("exiting");
+	hid->ff_private = NULL;
+	kfree(pidff);
+}
+
+static int pidff_find_fields(struct pidff_usage *usage, u8 *table, struct hid_report *report, int count, int strict)
+{
+	int i, j, k, found;
+
+	for (k = 0; k < count; k++) {
+		found = 0;
+		for (i = 0; i < report->maxfield; i++) {
+			if (report->field[i]->maxusage != report->field[i]->report_count) {
+				debug("maxusage and report_count do not match, skipping");
+				continue;
+			}
+			for (j = 0; j < report->field[i]->maxusage; j++) {
+				if (report->field[i]->usage[j].hid == (HID_UP_PID | table[k])) {
+					debug("found %d at %d->%d", k, i, j);
+					usage[k].field = report->field[i];
+					usage[k].value = &report->field[i]->value[j];
+					found = 1;
+					break;
+				}
+			}
+			if (found)
+				break;
+		}
+		if (!found && strict) {
+			debug("failed to locate %d", k);
+			return -1;
+		}
+	}
+	return 0;
+}
+
+static int pidff_rescale(int i, int max, struct hid_field *field)
+{
+	return i * (field->logical_maximum - field->logical_minimum) / max + field->logical_minimum;
+}
+
+static int pidff_rescale_signed(int i, struct hid_field *field)
+{
+	return i == 0 ? 0 : i > 0 ? i * field->logical_maximum / 0x7fff : i * field->logical_minimum / -0x8000;
+}
+
+
+static void pidff_set(struct pidff_usage *usage, u16 value)
+{
+	usage->value[0] = pidff_rescale(value, 0xffff, usage->field);
+	debug("calculated from %d to %d", value, usage->value[0]);
+}
+
+static void pidff_set_signed(struct pidff_usage *usage, s16 value)
+{
+	if (usage->field->logical_minimum < 0)
+		usage->value[0] = pidff_rescale_signed(value, usage->field);
+	else {
+		if (value < 0)
+			usage->value[0] = pidff_rescale(-value, 0x8000, usage->field);
+		else
+			usage->value[0] = pidff_rescale(value, 0x7fff, usage->field);
+	}
+	debug("calculated from %d to %d", value, usage->value[0]);
+}
+
+static void pidff_set_envelope_report(struct input_dev *dev, struct ff_envelope *envelope)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+
+	pidff->set_envelope[PID_EFFECT_BLOCK_INDEX].value[0] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+
+	pidff->set_envelope[PID_ATTACK_LEVEL].value[0] =
+			pidff_rescale(envelope->attack_level > 0x7fff ? 0x7fff : envelope->attack_level,
+				      0x7fff, pidff->set_envelope[PID_ATTACK_LEVEL].field);
+	pidff->set_envelope[PID_FADE_LEVEL].value[0] =
+			pidff_rescale(envelope->fade_level > 0x7fff ? 0x7fff : envelope->fade_level,
+				      0x7fff, pidff->set_envelope[PID_FADE_LEVEL].field);
+
+	pidff->set_envelope[PID_ATTACK_TIME].value[0] = envelope->attack_length;
+	pidff->set_envelope[PID_FADE_TIME].value[0] = envelope->fade_length;
+
+	debug("attack %u => %d", envelope->attack_level, pidff->set_envelope[PID_ATTACK_LEVEL].value[0]);
+
+	hid_submit_report(hid, pidff->reports[PID_SET_ENVELOPE], USB_DIR_OUT);
+}
+
+static int pidff_needs_set_envelope(struct ff_envelope *envelope, struct ff_envelope *old)
+{
+	return (envelope->attack_level != old->attack_level ||
+			envelope->fade_level != old->fade_level ||
+			envelope->attack_length != old->attack_length ||
+			envelope->fade_length != old->fade_length);
+}
+
+static void pidff_set_constant_force_report(struct input_dev *dev, struct ff_effect *effect)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	pidff->set_constant[PID_EFFECT_BLOCK_INDEX].value[0] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+	pidff_set_signed(&pidff->set_constant[PID_MAGNITUDE], effect->u.constant.level);
+
+	hid_submit_report(hid, pidff->reports[PID_SET_CONSTANT], USB_DIR_OUT);
+}
+
+static int pidff_needs_set_constant(struct ff_effect *effect, struct ff_effect *old)
+{
+	return (effect->u.constant.level != old->u.constant.level);
+}
+
+static void pidff_set_effect_report(struct input_dev *dev, struct ff_effect *effect)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	pidff->set_effect[PID_EFFECT_BLOCK_INDEX].value[0] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+	pidff->set_effect_type->value[0] = pidff->create_new_effect_type->value[0];
+	pidff->set_effect[PID_DURATION].value[0] = effect->replay.length;
+	pidff->set_effect[PID_TRIGGER_BUTTON].value[0] = effect->trigger.button;
+	pidff->set_effect[PID_TRIGGER_REPEAT_INT].value[0] = effect->trigger.interval;
+	pidff->set_effect[PID_GAIN].value[0] = pidff->set_effect[PID_GAIN].field->logical_maximum;
+	pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
+	pidff->effect_direction->value[0] = pidff_rescale(effect->direction, 0xffff, pidff->effect_direction);
+	pidff->set_effect[PID_START_DELAY].value[0] = effect->replay.delay;
+
+	hid_submit_report(hid, pidff->reports[PID_SET_EFFECT], USB_DIR_OUT);
+}
+
+static int pidff_needs_set_effect(struct ff_effect *effect, struct ff_effect *old)
+{
+	return (effect->replay.length != old->replay.length ||
+			effect->trigger.interval != old->trigger.interval ||
+			effect->trigger.button != old->trigger.button ||
+			effect->direction != old->direction ||
+			effect->replay.delay != old->replay.delay);
+}
+
+
+static void pidff_set_periodic_report(struct input_dev *dev, struct ff_effect *effect)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+
+	pidff->set_periodic[PID_EFFECT_BLOCK_INDEX].value[0] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+	pidff_set_signed(&pidff->set_periodic[PID_MAGNITUDE], effect->u.periodic.magnitude);
+	pidff_set_signed(&pidff->set_periodic[PID_OFFSET], effect->u.periodic.offset);
+	pidff_set(&pidff->set_periodic[PID_PHASE], effect->u.periodic.phase);
+	pidff->set_periodic[PID_PERIOD].value[0] = effect->u.periodic.period;
+
+	hid_submit_report(hid, pidff->reports[PID_SET_PERIODIC], USB_DIR_OUT);
+
+}
+
+static int pidff_needs_set_periodic(struct ff_effect *effect, struct ff_effect *old)
+{
+	return (effect->u.periodic.magnitude != old->u.periodic.magnitude ||
+			effect->u.periodic.offset != old->u.periodic.offset ||
+			effect->u.periodic.phase != old->u.periodic.phase ||
+			effect->u.periodic.period != old->u.periodic.period);
+}
+
+static void pidff_set_condition_report(struct input_dev *dev, struct ff_effect *effect)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	int i;
+
+	pidff->set_condition[PID_EFFECT_BLOCK_INDEX].value[0] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+
+	for (i = 0; i < 2; i++) {
+		pidff->set_condition[PID_PARAM_BLOCK_OFFSET].value[0] = i;
+		pidff_set_signed(&pidff->set_condition[PID_CP_OFFSET], effect->u.condition[i].center);
+		pidff_set_signed(&pidff->set_condition[PID_POS_COEFFICIENT], effect->u.condition[i].right_coeff);
+		pidff_set_signed(&pidff->set_condition[PID_NEG_COEFFICIENT], effect->u.condition[i].left_coeff);
+		pidff_set(&pidff->set_condition[PID_POS_SATURATION], effect->u.condition[i].right_saturation);
+		pidff_set(&pidff->set_condition[PID_NEG_SATURATION], effect->u.condition[i].left_saturation);
+		pidff_set(&pidff->set_condition[PID_DEAD_BAND], effect->u.condition[i].deadband);
+		hid_wait_io(hid);
+		hid_submit_report(hid, pidff->reports[PID_SET_CONDITION], USB_DIR_OUT);
+	}
+
+}
+
+static int pidff_needs_set_condition(struct ff_effect *effect, struct ff_effect *old)
+{
+	int i; int ret = 0;
+	for (i = 0; i < 2; i++)
+		ret |= effect->u.condition[i].center != old->u.condition[i].center ||
+			effect->u.condition[i].right_coeff != old->u.condition[i].right_coeff ||
+			effect->u.condition[i].left_coeff != old->u.condition[i].left_coeff ||
+			effect->u.condition[i].right_saturation != old->u.condition[i].right_saturation ||
+			effect->u.condition[i].left_saturation != old->u.condition[i].left_saturation ||
+			effect->u.condition[i].deadband != old->u.condition[i].deadband;
+	return ret;
+}
+
+
+static void pidff_set_ramp_force_report(struct input_dev *dev, struct ff_effect *effect)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	pidff->set_ramp[PID_EFFECT_BLOCK_INDEX].value[0] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+	pidff_set_signed(&pidff->set_ramp[PID_RAMP_START], effect->u.ramp.start_level);
+	pidff_set_signed(&pidff->set_ramp[PID_RAMP_END], effect->u.ramp.end_level);
+	hid_submit_report(hid, pidff->reports[PID_SET_RAMP], USB_DIR_OUT);
+}
+
+static int pidff_needs_set_ramp(struct ff_effect *effect, struct ff_effect *old)
+{
+	return (effect->u.ramp.start_level != old->u.ramp.start_level ||
+			effect->u.ramp.end_level != old->u.ramp.end_level);
+}
+
+
+static int pidff_request_effect_upload(struct input_dev *dev, int efnum)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	int j;
+
+	pidff->create_new_effect_type->value[0] = efnum;
+	hid_submit_report(hid, pidff->reports[PID_CREATE_NEW_EFFECT], USB_DIR_OUT);
+	debug("create_new_effect sent, type: %d", efnum);
+
+	pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0;
+	pidff->block_load_status->value[0] = 0;
+	hid_wait_io(hid);
+
+	for (j = 0; j < 60; j++) {
+		debug("pid_block_load requested");
+		hid_submit_report(hid, pidff->reports[PID_BLOCK_LOAD], USB_DIR_IN);
+		hid_wait_io(hid);
+		if (pidff->block_load_status->value[0] == pidff->status_id[PID_BLOCK_LOAD_SUCCESS]) {
+			debug("device reported free memory: %d bytes",
+			      pidff->block_load[PID_RAM_POOL_AVAILABLE].value ?
+					      pidff->block_load[PID_RAM_POOL_AVAILABLE].value[0] : -1);
+			return 0;
+		}
+		if (pidff->block_load_status->value[0] == pidff->status_id[PID_BLOCK_LOAD_FULL]) {
+			debug("not enough memory free: %d bytes",
+			      pidff->block_load[PID_RAM_POOL_AVAILABLE].value ?
+					      pidff->block_load[PID_RAM_POOL_AVAILABLE].value[0] : -1);
+			return -ENOSPC;
+		}
+	}
+	printk(KERN_ERR "hid-pidff: pid_block_load failed 60 times\n");
+	return -EIO;
+}
+
+
+static int pidff_playback_pid(struct input_dev *dev, int pid_id, int value)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+
+	pidff->effect_operation[PID_EFFECT_BLOCK_INDEX].value[0] = pid_id;
+
+	if (value == 0) {
+		pidff->effect_operation_status->value[0] = pidff->operation_id[PID_EFFECT_STOP];
+	}
+	else {
+		pidff->effect_operation_status->value[0] = pidff->operation_id[PID_EFFECT_START];
+		pidff->effect_operation[PID_LOOP_COUNT].value[0] = value;
+	}
+	hid_wait_io(hid);
+	hid_submit_report(hid, pidff->reports[PID_EFFECT_OPERATION], USB_DIR_OUT);
+	return 0;
+}
+
+static int pidff_playback(struct input_dev *dev, int effect_id, int value)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	return pidff_playback_pid(dev, pidff->pid_id[effect_id], value);
+}
+
+static void pidff_erase_effect_pid(struct input_dev *dev, int pid_id)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	pidff->block_free[PID_EFFECT_BLOCK_INDEX].value[0] = pid_id;
+	hid_submit_report(hid, pidff->reports[PID_BLOCK_FREE], USB_DIR_OUT);
+}
+
+static int pidff_erase_effect(struct input_dev *dev, int effect_id)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	debug("starting to erase %d/%d", effect_id, pidff->pid_id[effect_id]);
+	pidff_playback(dev, effect_id, 0);
+	pidff_erase_effect_pid(dev, pidff->pid_id[effect_id]);
+	return 0;
+}
+
+static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect, struct ff_effect *old)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	int ret = 0;
+
+	switch (effect->type) {
+	case FF_CONSTANT:
+		if (!old) {
+			ret = pidff_request_effect_upload(dev, pidff->type_id[PID_CONSTANT]);
+			if (ret)
+				return ret;
+		}
+		if (!old || pidff_needs_set_effect(effect, old))
+			pidff_set_effect_report(dev, effect);
+		if (!old || pidff_needs_set_constant(effect, old))
+			pidff_set_constant_force_report(dev, effect);
+		if (!old || pidff_needs_set_envelope(&effect->u.constant.envelope, &old->u.constant.envelope))
+			pidff_set_envelope_report(dev, &effect->u.constant.envelope);
+		break;
+	case FF_PERIODIC:
+		if (!old) {
+			switch (effect->u.periodic.waveform) {
+			case FF_SQUARE:
+				ret = pidff_request_effect_upload(dev, pidff->type_id[PID_SQUARE]);
+				break;
+			case FF_TRIANGLE:
+				ret = pidff_request_effect_upload(dev, pidff->type_id[PID_TRIANGLE]);
+				break;
+			case FF_SINE:
+				ret = pidff_request_effect_upload(dev, pidff->type_id[PID_SINE]);
+				break;
+			case FF_SAW_UP:
+				ret = pidff_request_effect_upload(dev, pidff->type_id[PID_SAW_UP]);
+				break;
+			case FF_SAW_DOWN:
+				ret = pidff_request_effect_upload(dev, pidff->type_id[PID_SAW_DOWN]);
+				break;
+			default:
+				printk(KERN_ERR "hid-pidff: invalid waveform\n");
+				ret = -EINVAL;
+			}
+		}
+		if (ret)
+			return ret;
+		if (!old || pidff_needs_set_effect(effect, old))
+			pidff_set_effect_report(dev, effect);
+		if (!old || pidff_needs_set_periodic(effect, old))
+			pidff_set_periodic_report(dev, effect);
+		if (!old || pidff_needs_set_envelope(&effect->u.periodic.envelope, &old->u.periodic.envelope))
+			pidff_set_envelope_report(dev, &(effect->u.periodic.envelope));
+		break;
+	case FF_RAMP:
+		if (!old) {
+			ret = pidff_request_effect_upload(dev, pidff->type_id[PID_RAMP]);
+			if (ret)
+				return ret;
+		}
+		if (!old || pidff_needs_set_effect(effect, old))
+			pidff_set_effect_report(dev, effect);
+		if (!old || pidff_needs_set_ramp(effect, old))
+			pidff_set_ramp_force_report(dev, effect);
+		if (!old || pidff_needs_set_envelope(&effect->u.ramp.envelope, &old->u.ramp.envelope))
+			pidff_set_envelope_report(dev, &(effect->u.ramp.envelope));
+		break;
+	case FF_SPRING:
+		if (!old) {
+			ret = pidff_request_effect_upload(dev, pidff->type_id[PID_SPRING]);
+			if (ret)
+				return ret;
+		}
+		if (!old || pidff_needs_set_effect(effect, old))
+			pidff_set_effect_report(dev, effect);
+		if (!old || pidff_needs_set_condition(effect, old))
+			pidff_set_condition_report(dev, effect);
+		break;
+	case FF_FRICTION:
+		if (!old) {
+			ret = pidff_request_effect_upload(dev, pidff->type_id[PID_FRICTION]);
+			if (ret)
+				return ret;
+		}
+		if (!old || pidff_needs_set_effect(effect, old))
+			pidff_set_effect_report(dev, effect);
+		if (!old || pidff_needs_set_condition(effect, old))
+			pidff_set_condition_report(dev, effect);
+		break;
+	case FF_DAMPER:
+		if (!old) {
+			ret = pidff_request_effect_upload(dev, pidff->type_id[PID_DAMPER]);
+			if (ret)
+				return ret;
+		}
+		if (!old || pidff_needs_set_effect(effect, old))
+			pidff_set_effect_report(dev, effect);
+		if (!old || pidff_needs_set_condition(effect, old))
+			pidff_set_condition_report(dev, effect);
+		break;
+	case FF_INERTIA:
+		if (!old) {
+			ret = pidff_request_effect_upload(dev, pidff->type_id[PID_INERTIA]);
+			if (ret)
+				return ret;
+		}
+		if (!old || pidff_needs_set_effect(effect, old))
+			pidff_set_effect_report(dev, effect);
+		if (!old || pidff_needs_set_condition(effect, old))
+			pidff_set_condition_report(dev, effect);
+		break;
+
+	default:
+		printk(KERN_ERR "hid-pidff: invalid type\n");
+		return -EINVAL;
+	}
+
+	if (!old)
+		pidff->pid_id[effect->id] = pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+	debug("uploaded");
+	return 0;
+}
+
+static void pidff_set_gain(struct input_dev *dev, u16 gain)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	pidff_set(&pidff->device_gain[PID_DEVICE_GAIN_FIELD], gain);
+	hid_submit_report(hid, pidff->reports[PID_DEVICE_GAIN], USB_DIR_OUT);
+	return;
+
+}
+
+static void pidff_set_autocenter(struct input_dev *dev, u16 magnitude)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+
+
+	if (!magnitude) {
+		pidff_playback_pid(dev, pidff->block_load[PID_EFFECT_BLOCK_INDEX].field->logical_minimum, 0);
+		return;
+	}
+
+	pidff_playback_pid(dev, pidff->block_load[PID_EFFECT_BLOCK_INDEX].field->logical_minimum, 1);
+
+	pidff->set_effect[PID_EFFECT_BLOCK_INDEX].value[0] =
+			pidff->block_load[PID_EFFECT_BLOCK_INDEX].field->logical_minimum;
+	pidff->set_effect_type->value[0] = pidff->type_id[PID_SPRING];
+	pidff->set_effect[PID_DURATION].value[0] = 0;
+	pidff->set_effect[PID_TRIGGER_BUTTON].value[0] = 0;
+	pidff->set_effect[PID_TRIGGER_REPEAT_INT].value[0] = 0;
+	pidff_set(&pidff->set_effect[PID_GAIN], magnitude);
+	pidff->set_effect[PID_START_DELAY].value[0] = 0;
+
+	hid_submit_report(hid, pidff->reports[PID_SET_EFFECT], USB_DIR_OUT);
+
+}
+
+static int pidff_check_usage(int usage)
+{
+	int i;
+
+	for (i = 0; i < sizeof(pidff_reports); i++) {
+		if (usage == (HID_UP_PID | pidff_reports[i]) ) {
+			return i;
+		}
+	}
+	return -1;
+}
+
+static void pidff_find_reports(struct input_dev *dev, int report_type)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+
+	struct hid_report *report;
+	int i, ret;
+
+	list_for_each_entry(report, &hid->report_enum[report_type].report_list, list) {
+		if (report->maxfield < 1)
+			continue;
+		ret = pidff_check_usage(report->field[0]->logical);
+		if (ret != -1) {
+			debug("found usage 0x%02x from field->logical", pidff_reports[ret]);
+			pidff->reports[ret] = report;
+			continue;
+		}
+
+		/* Sometimes logical collections are stacked to
+		indicate different usages for the report and the field,
+		in which case we want the usage of the parent */
+
+		/* However, Linux HID implementation hides that,
+		so we have to dig it up ourselves */
+		i = report->field[0]->usage[0].collection_index;
+		if (i <= 0 || hid->collection[i-1].type != HID_COLLECTION_LOGICAL)
+			continue;
+		ret = pidff_check_usage(hid->collection[i-1].usage);
+		if (ret != -1 && !pidff->reports[ret]) {
+			debug("found usage 0x%02x from collection array", pidff_reports[ret]);
+			pidff->reports[ret] = report;
+		}
+	}
+}
+
+static int pidff_reports_ok(struct input_dev *dev)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	int i;
+
+	for (i = 0; i <= PID_REQUIRED_REPORTS; i++) {
+		if (!pidff->reports[i]) {
+			debug("%d missing", i);
+			return 0;
+		}
+	}
+	return 1;
+}
+
+
+static struct hid_field *pidff_find_special_field(struct hid_report *report, int usage, int enforce_min)
+{
+	int i;
+	for (i = 0; i < report->maxfield; i++) {
+		if (report->field[i]->logical == (HID_UP_PID | usage) && report->field[i]->report_count > 0) {
+			if (!enforce_min || report->field[i]->logical_minimum == 1)
+				return report->field[i];
+			else {
+				printk(KERN_ERR "hid-pidff: logical_minimum not 1 when it should be\n");
+				return NULL;
+			}
+		}
+	}
+	return NULL;
+}
+
+static int pidff_find_special_keys(int *keys, struct hid_field *field, u8 *usagetable, int count)
+{
+
+	int i, j;
+	int found = 0;
+	for (i = 0; i < count; i++) {
+		for (j = 0; j < field->maxusage; j++) {
+			if (field->usage[j].hid == (HID_UP_PID | usagetable[i])) {
+				keys[i] = j+1;
+				found++;
+				break;
+			}
+		}
+	}
+	return found;
+}
+
+#define PIDFF_FIND_SPECIAL_KEYS(keys, field, name) \
+	pidff_find_special_keys(pidff->keys, pidff->field, pidff_ ## name, \
+		sizeof(pidff_ ## name))
+
+static int pidff_find_special_fields(struct pidff_device *pidff)
+{
+	debug("finding special fields");
+	pidff->create_new_effect_type = pidff_find_special_field(pidff->reports[PID_CREATE_NEW_EFFECT], 0x25, 1);
+	pidff->set_effect_type = pidff_find_special_field(pidff->reports[PID_SET_EFFECT], 0x25, 1);
+	pidff->effect_direction = pidff_find_special_field(pidff->reports[PID_SET_EFFECT], 0x57, 0);
+	pidff->device_control = pidff_find_special_field(pidff->reports[PID_DEVICE_CONTROL], 0x96, 1);
+	pidff->block_load_status = pidff_find_special_field(pidff->reports[PID_BLOCK_LOAD], 0x8b, 1);
+	pidff->effect_operation_status = pidff_find_special_field(pidff->reports[PID_EFFECT_OPERATION], 0x78, 1);
+	debug("search done");
+
+	if (!pidff->create_new_effect_type || !pidff->set_effect_type) {
+		printk(KERN_ERR "hid-pidff: effect lists not found\n");
+		return -1;
+	}
+	if (!pidff->effect_direction) {
+		printk(KERN_ERR "hid-pidff: direction field not found\n");
+		return -1;
+	}
+	if (!pidff->device_control) {
+		printk(KERN_ERR "hid-pidff: device control field not found\n");
+		return -1;
+	}
+	if (!pidff->block_load_status) {
+		printk(KERN_ERR "hid-pidff: block load status field not found\n");
+		return -1;
+	}
+	if (!pidff->effect_operation_status) {
+		printk(KERN_ERR "hid-pidff: effect operation field not found\n");
+		return -1;
+	}
+
+	pidff_find_special_keys(pidff->control_id, pidff->device_control, pidff_device_control,
+				sizeof(pidff_device_control));
+
+	PIDFF_FIND_SPECIAL_KEYS(control_id, device_control, device_control);
+
+	if (!PIDFF_FIND_SPECIAL_KEYS(type_id, create_new_effect_type, effect_types)) {
+		printk(KERN_ERR "hid-pidff: no effect types found\n");
+		return -1;
+	}
+
+	if (PIDFF_FIND_SPECIAL_KEYS(status_id, block_load_status, block_load_status) !=
+		   sizeof(pidff_block_load_status)) {
+		printk(KERN_ERR "hidpidff: block load status identifiers not found\n");
+		return -1;
+	}
+
+	if (PIDFF_FIND_SPECIAL_KEYS(operation_id, effect_operation_status, effect_operation_status) !=
+		   sizeof(pidff_effect_operation_status)) {
+		printk(KERN_ERR "hidpidff: effect operation identifiers not found\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int pidff_find_effects(struct input_dev *dev)
+{
+	struct hid_device *hid = dev->private;
+	struct ff_device *ff = dev->ff;
+	struct pidff_device *pidff = hid->ff_private;
+	int i;
+
+	for (i = 0; i < sizeof(pidff_effect_types); i++) {
+		if (pidff->set_effect_type->usage[pidff->type_id[i]].hid !=
+				  pidff->create_new_effect_type->usage[pidff->type_id[i]].hid) {
+			printk(KERN_ERR "hid-pidff: effect type number %d is invalid\n", i);
+			return -1;
+		}
+	}
+
+	if (pidff->type_id[PID_CONSTANT])
+		set_bit(FF_CONSTANT, ff->flags);
+	if (pidff->type_id[PID_RAMP])
+		set_bit(FF_RAMP, ff->flags);
+	if (pidff->type_id[PID_SQUARE]) {
+		set_bit(FF_SQUARE, ff->flags);
+		set_bit(FF_PERIODIC, ff->flags);
+	}
+	if (pidff->type_id[PID_SINE]) {
+		set_bit(FF_SINE, ff->flags);
+		set_bit(FF_PERIODIC, ff->flags);
+	}
+	if (pidff->type_id[PID_TRIANGLE]) {
+		set_bit(FF_TRIANGLE, ff->flags);
+		set_bit(FF_PERIODIC, ff->flags);
+	}
+	if (pidff->type_id[PID_SAW_UP]) {
+		set_bit(FF_SAW_UP, ff->flags);
+		set_bit(FF_PERIODIC, ff->flags);
+	}
+	if (pidff->type_id[PID_SAW_DOWN]) {
+		set_bit(FF_SAW_DOWN, ff->flags);
+		set_bit(FF_PERIODIC, ff->flags);
+	}
+	if (pidff->type_id[PID_SPRING])
+		set_bit(FF_SPRING, ff->flags);
+	if (pidff->type_id[PID_DAMPER])
+		set_bit(FF_DAMPER, ff->flags);
+	if (pidff->type_id[PID_INERTIA])
+		set_bit(FF_INERTIA, ff->flags);
+	if (pidff->type_id[PID_FRICTION])
+		set_bit(FF_FRICTION, ff->flags);
+
+	return 0;
+
+}
+
+#define PIDFF_FIND_FIELDS(name, report, strict) \
+	pidff_find_fields(pidff->name, pidff_ ## name, \
+		pidff->reports[report], \
+		sizeof(pidff_ ## name), strict)
+
+static int pidff_init_fields(struct input_dev *dev)
+{
+	struct hid_device *hid = dev->private;
+	struct ff_device *ff = dev->ff;
+	struct pidff_device *pidff = hid->ff_private;
+	int envelope_ok = 0;
+
+	if (PIDFF_FIND_FIELDS(set_effect, PID_SET_EFFECT, 1)) {
+		printk(KERN_ERR "hid-pidff: unknown set_effect report layout\n");
+		return -ENODEV;
+	}
+
+	PIDFF_FIND_FIELDS(block_load, PID_BLOCK_LOAD, 0);
+	if (!pidff->block_load[PID_EFFECT_BLOCK_INDEX].value) {
+		printk(KERN_ERR "hid-pidff: unknown pid_block_load report layout\n");
+		return -ENODEV;
+	}
+
+	if (PIDFF_FIND_FIELDS(effect_operation, PID_EFFECT_OPERATION, 1)) {
+		printk(KERN_ERR "hid-pidff: unknown effect_operation report layout\n");
+		return -ENODEV;
+	}
+
+	if (PIDFF_FIND_FIELDS(block_free, PID_BLOCK_FREE, 1)) {
+		printk(KERN_ERR "hid-pidff: unknown pid_block_free report layout\n");
+		return -ENODEV;
+	}
+	if (!PIDFF_FIND_FIELDS(set_envelope, PID_SET_ENVELOPE, 1))
+		envelope_ok = 1;
+
+	if (pidff_find_special_fields(pidff) || pidff_find_effects(dev)) {
+		return -ENODEV;
+	}
+
+	if (!envelope_ok) {
+		if (test_and_clear_bit(FF_CONSTANT, ff->flags))
+			printk(KERN_WARNING "hid-pidff: has constant effect but no envelope\n");
+		if (test_and_clear_bit(FF_RAMP, ff->flags))
+			printk(KERN_WARNING "hid-pidff: has ramp effect but no envelope\n");
+		if (test_and_clear_bit(FF_PERIODIC, ff->flags))
+			printk(KERN_WARNING "hid-pidff: has periodic effect but no envelope\n");
+	}
+
+	if (test_bit(FF_CONSTANT, ff->flags) && PIDFF_FIND_FIELDS(set_constant, PID_SET_CONSTANT, 1)) {
+		printk(KERN_WARNING "hid-pidff: unknown constant effect layout\n");
+		clear_bit(FF_CONSTANT, ff->flags);
+	}
+
+	if (test_bit(FF_RAMP, ff->flags) && PIDFF_FIND_FIELDS(set_ramp, PID_SET_RAMP, 1)) {
+		printk(KERN_WARNING "hid-pidff: unknown ramp effect layout\n");
+		clear_bit(FF_RAMP, ff->flags);
+	}
+
+	if ((test_bit(FF_SPRING, ff->flags) || test_bit(FF_DAMPER, ff->flags) ||
+		    test_bit(FF_FRICTION, ff->flags) || test_bit(FF_INERTIA, ff->flags)) &&
+		    PIDFF_FIND_FIELDS(set_condition, PID_SET_CONDITION, 1)) {
+		printk(KERN_WARNING "hid-pidff: unknown condition effect layout\n");
+		clear_bit(FF_SPRING, ff->flags);
+		clear_bit(FF_DAMPER, ff->flags);
+		clear_bit(FF_FRICTION, ff->flags);
+		clear_bit(FF_INERTIA, ff->flags);
+	}
+
+	if (test_bit(FF_PERIODIC, ff->flags) && PIDFF_FIND_FIELDS(set_periodic, PID_SET_PERIODIC, 1)) {
+		printk(KERN_WARNING "hid-pidff: unknown periodic effect layout\n");
+		clear_bit(FF_PERIODIC, ff->flags);
+	}
+
+	PIDFF_FIND_FIELDS(pool, PID_POOL, 0);
+
+	if (!PIDFF_FIND_FIELDS(device_gain, PID_DEVICE_GAIN, 1)) {
+		set_bit(FF_GAIN, ff->flags);
+	}
+
+	return 0;
+}
+
+static void pidff_reset(struct input_dev *dev)
+{
+	struct hid_device *hid = dev->private;
+	struct pidff_device *pidff = hid->ff_private;
+	int i = 0;;
+
+	pidff->device_control->value[0] = pidff->control_id[PID_RESET];
+	/* We reset twice as sometimes hid_wait_io isn't waiting long enough */
+	hid_submit_report(hid, pidff->reports[PID_DEVICE_CONTROL], USB_DIR_OUT);
+	hid_wait_io(hid);
+	hid_submit_report(hid, pidff->reports[PID_DEVICE_CONTROL], USB_DIR_OUT);
+	hid_wait_io(hid);
+
+	pidff->device_control->value[0] = pidff->control_id[PID_ENABLE_ACTUATORS];
+	hid_submit_report(hid, pidff->reports[PID_DEVICE_CONTROL], USB_DIR_OUT);
+	hid_wait_io(hid);
+
+	/* pool report is sometimes messed up, refetch it */
+	hid_submit_report(hid, pidff->reports[PID_POOL], USB_DIR_IN);
+	hid_wait_io(hid);
+
+	if (pidff->pool[PID_SIMULTANEOUS_MAX].value) {
+		while (pidff->pool[PID_SIMULTANEOUS_MAX].value[0] < 2) {
+			if (i++ > 20) {
+				printk(KERN_WARNING "hid-pidff: device reports %d simultaneous effects\n",
+				       pidff->pool[PID_SIMULTANEOUS_MAX].value[0]);
+				break;
+			}
+			debug("pid_pool requested again");
+			hid_submit_report(hid, pidff->reports[PID_POOL], USB_DIR_IN);
+			hid_wait_io(hid);
+		}
+	}
+}
+
+static int pidff_check_autocenter(struct input_dev *dev)
+{
+	struct hid_device *hid = dev->private;
+	struct ff_device *ff = dev->ff;
+	struct pidff_device *pidff = hid->ff_private;
+	int ret;
+
+	/* Let's find out if autocenter modification is supported */
+	/* Specification doesn't specify anything, so we request an
+	effect upload and cancel it immediately. If the approved
+	effect id was one above the minimum, then we assume the first
+	effect id is a built-in spring type effect used for autocenter */
+
+	ret = pidff_request_effect_upload(dev, 1);
+	if (ret) {
+		printk(KERN_ERR "hid-pidff: upload request failed\n");
+		return ret;
+	}
+	if (pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] ==
+		   pidff->block_load[PID_EFFECT_BLOCK_INDEX].field->logical_minimum + 1) {
+		set_bit(FF_AUTOCENTER, ff->flags);
+		pidff_set_autocenter(dev, 0xffff);
+	} else {
+		printk(KERN_NOTICE "hid-pidff: device has unknown autocenter control method\n");
+	}
+	debug("status 11");
+	pidff_erase_effect_pid(dev, pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0]);
+	debug("status 12");
+
+	return 0;
+
+}
+
+static struct ff_driver pidff_driver = {
+	.upload = pidff_upload_effect,
+	.erase	= pidff_erase_effect,
+	.set_gain = pidff_set_gain,
+	.set_autocenter = pidff_set_autocenter,
+	.playback = pidff_playback,
+};
+
+
+int pidff_init(struct hid_device *hid)
+{
+	int ret;
+	struct pidff_device *pidff;
+	struct hid_report *report;
+	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
+	struct input_dev *dev = hidinput->input;
+	struct ff_device *ff;
+	debug("starting pid init");
+
+	if (list_empty(&hid->report_enum[HID_OUTPUT_REPORT].report_list)) {
+		debug("not a PID device, no output report");
+		return -ENODEV;
+	}
+	report = list_entry(hid->report_enum[HID_OUTPUT_REPORT].report_list.next, struct hid_report, list);
+	if (report->maxfield < 1 || (report->field[0]->logical & HID_USAGE_PAGE) != HID_UP_PID) {
+		debug("not a PID device, first report not PID");
+		return -ENODEV;
+	}
+
+	pidff = kzalloc(sizeof(*pidff), GFP_KERNEL);
+	if (!pidff)
+		return -ENOMEM;
+
+     	ret = input_ff_allocate(dev);
+        if (ret) {
+		kfree(pidff);
+		return ret;
+	}
+
+	ff = dev->ff;
+	hid->ff_private = pidff;
+
+	debug("debug mode enabled");
+
+	pidff_find_reports(dev, HID_OUTPUT_REPORT);
+	pidff_find_reports(dev, HID_FEATURE_REPORT);
+
+	if (!pidff_reports_ok(dev)) {
+		debug("reports not ok, aborting");
+		pidff_exit(hid);
+		return -ENODEV;
+	}
+
+	printk(KERN_INFO "Force feedback for USB HID PID devices by Anssi Hannula <anssi.hannula@gmail.com>\n");
+
+	ret = pidff_init_fields(dev);
+	if (ret) {
+		pidff_exit(hid);
+		return ret;
+	}
+
+	pidff_reset(dev);
+
+	if (test_bit(FF_GAIN, ff->flags))
+		pidff_set_gain(dev, 0xffff);
+
+	ret = pidff_check_autocenter(dev);
+	if (ret) {
+		pidff_exit(hid);
+		return ret;
+	}
+
+	dev->ff_effects_max = pidff->block_load[PID_EFFECT_BLOCK_INDEX].field->logical_maximum -
+			pidff->block_load[PID_EFFECT_BLOCK_INDEX].field->logical_minimum + 1;
+
+	debug("effects max is %d", dev->ff_effects_max);
+	if (pidff->pool[PID_SIMULTANEOUS_MAX].value)
+		debug("simultaneous effects max is %d", pidff->pool[PID_SIMULTANEOUS_MAX].value[0]);
+
+	if (dev->ff_effects_max > FF_EFFECTS_MAX)
+		dev->ff_effects_max = FF_EFFECTS_MAX;
+
+	if (pidff->pool[PID_RAM_POOL_SIZE].value)
+		debug("device memory size is %d bytes", pidff->pool[PID_RAM_POOL_SIZE].value[0]);
+
+	if (pidff->pool[PID_DEVICE_MANAGED_POOL].value && pidff->pool[PID_DEVICE_MANAGED_POOL].value[0] == 0) {
+		printk(KERN_NOTICE "hid-pidff: device advertises to not support device managed pool\n");
+	}
+
+	hid->ff_exit = pidff_exit;
+
+	ret = input_ff_register(dev, &pidff_driver);
+	if (ret) {
+		kfree(pidff);
+		return ret;
+	}
+
+	return 0;
+}
+
Index: linux-2.6.17-rc3-git12/drivers/usb/input/Kconfig
===================================================================
--- linux-2.6.17-rc3-git12.orig/drivers/usb/input/Kconfig	2006-05-13 23:40:02.000000000 +0300
+++ linux-2.6.17-rc3-git12/drivers/usb/input/Kconfig	2006-05-13 23:40:03.000000000 +0300
@@ -60,10 +60,10 @@ config HID_FF
 	  If unsure, say N.
 
 config HID_PID
-	bool "PID Devices (Microsoft Sidewinder Force Feedback 2)"
+	bool "PID device support"
 	depends on HID_FF
 	help
-	  Say Y here if you have a PID-compliant joystick and wish to enable force
+	  Say Y here if you have a PID-compliant device and wish to enable force
 	  feedback for it. The Microsoft Sidewinder Force Feedback 2 is one such
 	  device.
 
Index: linux-2.6.17-rc3-git12/drivers/usb/input/Makefile
===================================================================
--- linux-2.6.17-rc3-git12.orig/drivers/usb/input/Makefile	2006-05-13 23:37:59.000000000 +0300
+++ linux-2.6.17-rc3-git12/drivers/usb/input/Makefile	2006-05-13 23:40:03.000000000 +0300
@@ -14,7 +14,7 @@ ifeq ($(CONFIG_USB_HIDINPUT),y)
 	usbhid-objs	+= hid-input.o
 endif
 ifeq ($(CONFIG_HID_PID),y)
-	usbhid-objs	+= pid.o
+	usbhid-objs	+= hid-pidff.o
 endif
 ifeq ($(CONFIG_LOGITECH_FF),y)
 	usbhid-objs	+= hid-lgff.o

--
Anssi Hannula

[patch 08/11] input: force feedback driver for Zeroplus devices

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-new-driver-zpff.diff --]
[-- Type: text/plain, Size: 6709 bytes --]

Add force feedback driver for some PSX-style Zeroplus devices.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/usb/input/Kconfig    |    7 ++
 drivers/usb/input/Makefile   |    3 +
 drivers/usb/input/hid-ff.c   |    4 +
 drivers/usb/input/hid-zpff.c |  128 +++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/input/hid.h      |    1 
 5 files changed, 143 insertions(+)

Index: linux-2.6.16-rc1-git11/drivers/usb/input/hid-ff.c
===================================================================
--- linux-2.6.16-rc1-git11.orig/drivers/usb/input/hid-ff.c	2006-04-15 16:22:52.000000000 +0300
+++ linux-2.6.16-rc1-git11/drivers/usb/input/hid-ff.c	2006-04-15 16:23:50.000000000 +0300
@@ -54,6 +54,10 @@ static struct hid_ff_initializer inits[]
 #ifdef CONFIG_THRUSTMASTER_FF
 	{0x44f, 0xb304, hid_tmff_init},
 #endif
+#ifdef CONFIG_ZEROPLUS_FF
+	{0xc12, 0x0005, zpff_init},
+	{0xc12, 0x0030, zpff_init},
+#endif
 	{0, 0, NULL} /* Terminating entry */
 };
 
Index: linux-2.6.16-rc1-git11/drivers/usb/input/hid-zpff.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.16-rc1-git11/drivers/usb/input/hid-zpff.c	2006-04-15 16:26:58.000000000 +0300
@@ -0,0 +1,128 @@
+/*
+ *  Force feedback support for Zeroplus based devices
+ *
+ *  Copyright (c) 2005, 2006 Anssi Hannula <anssi.hannula@gmail.com>
+ */
+
+/*
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/input.h>
+#include <linux/usb.h>
+#include "hid.h"
+
+/* #define DEBUG */
+
+#ifdef DEBUG
+#define debug(format, arg...) printk(KERN_DEBUG "hid-zpff: " format "\n" , ## arg)
+#else
+#define debug(format, arg...) do {} while (0)
+#endif
+
+struct zpff_device {
+	struct hid_report *report;
+};
+
+static int zpff_play(struct input_dev *dev, struct ff_effect *effect,
+		     struct ff_effect *old)
+{
+	struct hid_device *hid = dev->private;
+	struct zpff_device *zpff = hid->ff_private;
+	int left, right;
+
+	/* The following is specified the other way around in the Zeroplus
+	   datasheet, but the order below is correct for the XFX Executioner */
+	/* However it is possible that the XFX Executioner is an exception */
+
+	left = effect->u.rumble.strong_magnitude;
+	right = effect->u.rumble.weak_magnitude;
+	debug("called with 0x%04x 0x%04x", left, right);
+
+	left = left * 0x7f / 0xffff;
+	right = right * 0x7f / 0xffff;
+
+	zpff->report->field[2]->value[0] = left;
+	zpff->report->field[3]->value[0] = right;
+	debug("running with 0x%02x 0x%02x", left, right);
+	hid_submit_report(hid, zpff->report, USB_DIR_OUT);
+	return 0;
+}
+
+static void zpff_exit(struct hid_device *hid)
+{
+	struct zpff_device *zpff = hid->ff_private;
+	hid->ff_private = NULL;
+	kfree(zpff);
+}
+
+static struct ff_driver zpff_driver = {
+	.upload = zpff_play,
+};
+
+int zpff_init(struct hid_device *hid)
+{
+	struct zpff_device *zpff;
+	struct hid_report *report;
+	struct hid_input *hidinput =
+	    list_entry(hid->inputs.next, struct hid_input, list);
+	struct input_dev *dev = hidinput->input;
+	int ret;
+
+	if (list_empty(&hid->report_enum[HID_OUTPUT_REPORT].report_list)) {
+		printk(KERN_ERR "hid-zpff: no output report found\n");
+		return -1;
+	}
+	report =
+	    list_entry(hid->report_enum[HID_OUTPUT_REPORT].report_list.next,
+		       struct hid_report, list);
+
+	if (report->maxfield < 4) {
+		printk(KERN_ERR "hid-zpff: not enough fields in report\n");
+		return -1;
+	}
+
+	ret = input_ff_allocate(dev);
+	if (ret)
+		return ret;
+
+	zpff = kzalloc(sizeof(*zpff), GFP_KERNEL);
+	if (!zpff) {
+		return -ENOMEM;
+	}
+
+	hid->ff_private = zpff;
+	hid->ff_exit = zpff_exit;
+
+	set_bit(FF_RUMBLE, dev->ff->flags);
+
+	zpff->report = report;
+	zpff->report->field[0]->value[0] = 0x00;
+	zpff->report->field[1]->value[0] = 0x02;
+	zpff->report->field[2]->value[0] = 0x00;
+	zpff->report->field[3]->value[0] = 0x00;
+	hid_submit_report(hid, zpff->report, USB_DIR_OUT);
+
+	ret = input_ff_register(dev, &zpff_driver);
+	if (ret) {
+		kfree(zpff);
+		return ret;
+	}
+
+	printk(KERN_INFO "Force feedback for Zeroplus based devices by "
+	       "Anssi Hannula <anssi.hannula@gmail.com>\n");
+
+	return 0;
+}
Index: linux-2.6.16-rc1-git11/drivers/usb/input/Kconfig
===================================================================
--- linux-2.6.16-rc1-git11.orig/drivers/usb/input/Kconfig	2006-04-15 16:22:52.000000000 +0300
+++ linux-2.6.16-rc1-git11/drivers/usb/input/Kconfig	2006-04-15 16:23:50.000000000 +0300
@@ -87,6 +87,13 @@ config THRUSTMASTER_FF
 	  Note: if you say N here, this device will still be supported, but without
 	  force feedback.
 
+config ZEROPLUS_FF
+	bool "Zeroplus based game controller support"
+	depends on HID_FF
+	help
+	  Say Y here if you have a Zeroplus based game controller and want to
+	  enable force feedback for it.
+
 config USB_HIDDEV
 	bool "/dev/hiddev raw HID device support"
 	depends on USB_HID
Index: linux-2.6.16-rc1-git11/drivers/usb/input/Makefile
===================================================================
--- linux-2.6.16-rc1-git11.orig/drivers/usb/input/Makefile	2006-04-15 16:22:52.000000000 +0300
+++ linux-2.6.16-rc1-git11/drivers/usb/input/Makefile	2006-04-15 16:23:50.000000000 +0300
@@ -22,6 +22,9 @@ endif
 ifeq ($(CONFIG_THRUSTMASTER_FF),y)
 	usbhid-objs	+= hid-tmff.o
 endif
+ifeq ($(CONFIG_ZEROPLUS_FF),y)
+	usbhid-objs	+= hid-zpff.o
+endif
 ifeq ($(CONFIG_HID_FF),y)
 	usbhid-objs	+= hid-ff.o
 endif
Index: linux-2.6.16-rc1-git11/drivers/usb/input/hid.h
===================================================================
--- linux-2.6.16-rc1-git11.orig/drivers/usb/input/hid.h	2006-04-15 16:22:52.000000000 +0300
+++ linux-2.6.16-rc1-git11/drivers/usb/input/hid.h	2006-04-15 16:23:50.000000000 +0300
@@ -527,6 +527,7 @@ static inline void hid_ff_exit(struct hi
 
 int hid_lgff_init(struct hid_device* hid);
 int hid_tmff_init(struct hid_device* hid);
+int zpff_init(struct hid_device* hid);
 
 #ifdef CONFIG_HID_PID
 int pidff_init(struct hid_device *hid);

--
Anssi Hannula

[patch 09/11] input: update documentation of force feedback

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-documentation.diff --]
[-- Type: text/plain, Size: 8292 bytes --]

Update the force feedback documentation in ff.txt and input.h.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 Documentation/input/ff.txt |   93 +++++++++++++++++++--------------------------
 include/linux/input.h      |   12 +++++
 2 files changed, 51 insertions(+), 54 deletions(-)

Index: linux-2.6.16-rc1-git11/Documentation/input/ff.txt
===================================================================
--- linux-2.6.16-rc1-git11.orig/Documentation/input/ff.txt	2006-04-15 18:09:39.000000000 +0300
+++ linux-2.6.16-rc1-git11/Documentation/input/ff.txt	2006-04-15 18:10:57.000000000 +0300
@@ -1,27 +1,27 @@
 Force feedback for Linux.
 By Johann Deneux <deneux@ifrance.com> on 2001/04/22.
+Updated by Anssi Hannula <anssi.hannula@gmail.com> on 2006/04/09.
 You may redistribute this file. Please remember to include shape.fig and
 interactive.fig as well.
 ----------------------------------------------------------------------------
 
-0. Introduction
+1. Introduction
 ~~~~~~~~~~~~~~~
 This document describes how to use force feedback devices under Linux. The
 goal is not to support these devices as if they were simple input-only devices
 (as it is already the case), but to really enable the rendering of force
 effects.
-At the moment, only I-Force devices are supported, and not officially. That
-means I had to find out how the protocol works on my own. Of course, the
-information I managed to grasp is far from being complete, and I can not
-guarranty that this driver will work for you.
-This document only describes the force feedback part of the driver for I-Force
-devices. Please read joystick.txt before reading further this document.
+This document only describes the force feedback part of the Linux input
+interface. Please read joystick.txt and input.txt before reading further this
+document.
 
 2. Instructions to the user
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
-Here are instructions on how to compile and use the driver. In fact, this
-driver is the normal iforce, input and evdev drivers written by Vojtech
-Pavlik, plus additions to support force feedback.
+To enable force feedback, you have to
+1. have your kernel configured with evdev and force feedback effects enabled
+   with a driver that supports your device.
+2. make sure evdev module is loaded and /dev/input/event* device files are
+   created.
 
 Before you start, let me WARN you that some devices shake violently during the
 initialisation phase. This happens for example with my "AVB Top Shot Pegasus".
@@ -29,39 +29,8 @@ To stop this annoying behaviour, move yo
 should keep a hand on your device, in order to avoid it to brake down if
 something goes wrong.
 
-At the kernel's compilation:
-	- Enable IForce/Serial
-	- Enable Event interface
-
-Compile the modules, install them.
-
-You also need inputattach.
-
-You then need to insert the modules into the following order:
-% modprobe joydev
-% modprobe serport		# Only for serial
-% modprobe iforce
-% modprobe evdev
-% ./inputattach -ifor $2 &	# Only for serial
-If you are using USB, you don't need the inputattach step.
-
-Please check that you have all the /dev/input entries needed:
-cd /dev
-rm js*
-mkdir input
-mknod input/js0 c 13 0
-mknod input/js1 c 13 1
-mknod input/js2 c 13 2
-mknod input/js3 c 13 3
-ln -s input/js0 js0
-ln -s input/js1 js1
-ln -s input/js2 js2
-ln -s input/js3 js3
-
-mknod input/event0 c 13 64
-mknod input/event1 c 13 65
-mknod input/event2 c 13 66
-mknod input/event3 c 13 67
+If you have a serial iforce device, you need to start inputattach. See
+joystick.txt for details.
 
 2.1 Does it work ?
 ~~~~~~~~~~~~~~~~~~
@@ -86,18 +55,29 @@ int ioctl(int file_descriptor, int reque
 
 Returns the features supported by the device. features is a bitfield with the
 following bits:
-- FF_X		has an X axis (usually joysticks)
-- FF_Y		has an Y axis (usually joysticks)
-- FF_WHEEL	has a wheel (usually sterring wheels)
 - FF_CONSTANT	can render constant force effects
-- FF_PERIODIC	can render periodic effects (sine, triangle, square...)
+- FF_PERIODIC	can render periodic effects with the following waveforms:
+  - FF_SQUARE	  square waveform
+  - FF_TRIANGLE	  triangle waveform
+  - FF_SINE	  sine waveform
+  - FF_SAW_UP	  sawtooth up waveform
+  - FF_SAW_DOWN	  sawtooth down waveform
+  - FF_CUSTOM	  custom waveform
 - FF_RAMP       can render ramp effects
 - FF_SPRING	can simulate the presence of a spring
 - FF_FRICTION	can simulate friction 
 - FF_DAMPER	can simulate damper effects
-- FF_RUMBLE	rumble effects (normally the only effect supported by rumble
-		pads)
+- FF_RUMBLE	rumble effects
 - FF_INERTIA    can simulate inertia
+- FF_GAIN	gain is adjustable
+- FF_AUTOCENTER	autocenter is adjustable
+
+Note: In most cases you should use FF_PERIODIC instead of FF_RUMBLE. All
+      devices that support FF_RUMBLE support FF_PERIODIC (square, triangle,
+      sine) and the other way around.
+
+Note: The exact syntax FF_CUSTOM is undefined for the time being as no driver
+      supports it yet.
 
 
 int ioctl(int fd, EVIOCGEFFECTS, int *n);
@@ -128,8 +108,8 @@ You need xfig to visualize these files.
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 int ioctl(int fd, EVIOCRMFF, effect.id);
 
-This makes room for new effects in the device's memory. Please note this won't
-stop the effect if it was playing.
+This makes room for new effects in the device's memory. Note that this also
+stops the effect if it was playing.
 
 3.4 Controlling the playback of effects
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -163,8 +143,7 @@ Control of playing is done with write().
 ~~~~~~~~~~~~~~~~~~~~
 Not all devices have the same strength. Therefore, users should set a gain
 factor depending on how strong they want effects to be. This setting is
-persistent across access to the driver, so you should not care about it if
-you are writing games, as another utility probably already set this for you.
+persistent across access to the driver.
 
 /* Set the gain of the device
 int gain;		/* between 0 and 100 */
@@ -204,9 +183,17 @@ type of device, not all parameters can b
 the direction of an effect cannot be updated with iforce devices. In this
 case, the driver stops the effect, up-load it, and restart it.
 
+Therefore it is recommended to dynamically change direction while the effect
+is playing only when it is ok to restart the effect with a replay count of 1.
 
 3.8 Information about the status of effects
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+NOTE: This is deprecated and should not be used. It applies only to iforce
+      devices and is provided for backward-compatibility only. If you have a
+      really good reason to use this, please contact
+      linux-joystick@atrey.karlin.mff.cuni.cz or anssi.hannula@gmail.com
+      so that support for it can be added to all drivers.
+
 Every time the status of an effect is changed, an event is sent. The values
 and meanings of the fields of the event are as follows:
 struct input_event {
Index: linux-2.6.16-rc1-git11/include/linux/input.h
===================================================================
--- linux-2.6.16-rc1-git11.orig/include/linux/input.h	2006-04-15 18:09:39.000000000 +0300
+++ linux-2.6.16-rc1-git11/include/linux/input.h	2006-04-15 18:40:08.000000000 +0300
@@ -672,8 +672,14 @@ struct input_absinfo {
  * They are sub-structures of the actually sent structure (called ff_effect)
  */
 
+/*
+ * All time duration values are expressed in ms.
+ * Time values above 32767ms (0x7fff) should not be used and have unspecified
+ * results depending on device.
+ */
+
 struct ff_replay {
-	__u16 length; /* Duration of an effect in ms. All other times are also expressed in ms */
+	__u16 length; /* Duration of an effect in ms */
 	__u16 delay;  /* Time to wait before to start playing an effect */
 };
 
@@ -683,6 +689,10 @@ struct ff_trigger {
 };
 
 struct ff_envelope {
+	/* The attack_level and fade_level are absolute values of the level,
+	i.e. if the default magnitude is negative the envelope level is
+	also seen as an absolute value of a below-zero value */
+	/* Therefore the valid level range is from 0x0000 to 0x7fff */
 	__u16 attack_length;	/* Duration of attack (ms) */
 	__u16 attack_level;	/* Level at beginning of attack */
 	__u16 fade_length;	/* Duration of fade (ms) */

--
Anssi Hannula

[patch 10/11] input: drop the remains of the old ff interface

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-drop-old-interface.diff --]
[-- Type: text/plain, Size: 1290 bytes --]

Drop the now unused handlers and input_report_ff().

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 include/linux/input.h |    7 -------
 1 files changed, 7 deletions(-)

Index: linux-2.6.16-git20/include/linux/input.h
===================================================================
--- linux-2.6.16-git20.orig/include/linux/input.h	2006-04-15 00:05:00.000000000 +0300
+++ linux-2.6.16-git20/include/linux/input.h	2006-04-15 00:05:41.000000000 +0300
@@ -938,8 +938,6 @@ struct input_dev {
 	int (*accept)(struct input_dev *dev, struct file *file);
 	int (*flush)(struct input_dev *dev, struct file *file);
 	int (*event)(struct input_dev *dev, unsigned int type, unsigned int code, int value);
-	int (*upload_effect)(struct input_dev *dev, struct ff_effect *effect);
-	int (*erase_effect)(struct input_dev *dev, int effect_id);
 
 	struct input_handle *grab;
 
@@ -1118,11 +1116,6 @@ static inline void input_report_abs(stru
 	input_event(dev, EV_ABS, code, value);
 }
 
-static inline void input_report_ff(struct input_dev *dev, unsigned int code, int value)
-{
-	input_event(dev, EV_FF, code, value);
-}
-
 static inline void input_report_ff_status(struct input_dev *dev, unsigned int code, int value)
 {
 	input_event(dev, EV_FF_STATUS, code, value);

--
Anssi Hannula

[patch 11/11] input: drop the old PID driver

[Anssi Hannula]

[-- Attachment #1: ff-refactoring-drop-old-pid.diff --]
[-- Type: text/plain, Size: 11519 bytes --]

Drop the old PID driver that is now obsoleted by a new implementation in
hid-pidff.c. This older driver was never completed and is not able to upload
effects.

Signed-off-by: Anssi Hannula <anssi.hannula@gmail.com>

---
 drivers/usb/input/pid.c |  295 ------------------------------------------------
 drivers/usb/input/pid.h |   62 ----------
 2 files changed, 357 deletions(-)

Index: linux-2.6.16-git20/drivers/usb/input/pid.c
===================================================================
--- linux-2.6.16-git20.orig/drivers/usb/input/pid.c	2006-04-10 21:02:44.000000000 +0300
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,295 +0,0 @@
-/*
- *  PID Force feedback support for hid devices.
- *
- *  Copyright (c) 2002 Rodrigo Damazio.
- *  Portions by Johann Deneux and Bjorn Augustson
- */
-
-/*
- * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- * Should you need to contact me, the author, you can do so by
- * e-mail - mail your message to <rdamazio@lsi.usp.br>
- */
-
-#include <linux/config.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/mm.h>
-#include <linux/smp_lock.h>
-#include <linux/spinlock.h>
-#include <linux/input.h>
-#include <linux/usb.h>
-#include "hid.h"
-#include "pid.h"
-
-#define CHECK_OWNERSHIP(i, hid_pid)	\
-	((i) < FF_EFFECTS_MAX && i >= 0 && \
-	test_bit(FF_PID_FLAGS_USED, &hid_pid->effects[(i)].flags) && \
-	(current->pid == 0 || \
-	(hid_pid)->effects[(i)].owner == current->pid))
-
-/* Called when a transfer is completed */
-static void hid_pid_ctrl_out(struct urb *u, struct pt_regs *regs)
-{
-	dev_dbg(&u->dev->dev, "hid_pid_ctrl_out - Transfer Completed\n");
-}
-
-static void hid_pid_exit(struct hid_device *hid)
-{
-	struct hid_ff_pid *private = hid->ff_private;
-
-	if (private->urbffout) {
-		usb_kill_urb(private->urbffout);
-		usb_free_urb(private->urbffout);
-	}
-}
-
-static int pid_upload_periodic(struct hid_ff_pid *pid, struct ff_effect *effect, int is_update)
-{
-	dev_info(&pid->hid->dev->dev, "requested periodic force upload\n");
-	return 0;
-}
-
-static int pid_upload_constant(struct hid_ff_pid *pid, struct ff_effect *effect, int is_update)
-{
-	dev_info(&pid->hid->dev->dev, "requested constant force upload\n");
-	return 0;
-}
-
-static int pid_upload_condition(struct hid_ff_pid *pid, struct ff_effect *effect, int is_update)
-{
-	dev_info(&pid->hid->dev->dev, "requested Condition force upload\n");
-	return 0;
-}
-
-static int pid_upload_ramp(struct hid_ff_pid *pid, struct ff_effect *effect, int is_update)
-{
-	dev_info(&pid->hid->dev->dev, "request ramp force upload\n");
-	return 0;
-}
-
-static int hid_pid_event(struct hid_device *hid, struct input_dev *input,
-			 unsigned int type, unsigned int code, int value)
-{
-	dev_dbg(&hid->dev->dev, "PID event received: type=%d,code=%d,value=%d.\n", type, code, value);
-
-	if (type != EV_FF)
-		return -1;
-
-	return 0;
-}
-
-/* Lock must be held by caller */
-static void hid_pid_ctrl_playback(struct hid_device *hid, struct hid_pid_effect *effect, int play)
-{
-	if (play)
-		set_bit(FF_PID_FLAGS_PLAYING, &effect->flags);
-	else
-		clear_bit(FF_PID_FLAGS_PLAYING, &effect->flags);
-}
-
-static int hid_pid_erase(struct input_dev *dev, int id)
-{
-	struct hid_device *hid = dev->private;
-	struct hid_ff_pid *pid = hid->ff_private;
-	struct hid_field *field;
-	unsigned long flags;
-	int ret;
-
-	if (!CHECK_OWNERSHIP(id, pid))
-		return -EACCES;
-
-	/* Find report */
-	field = hid_find_field_by_usage(hid, HID_UP_PID | FF_PID_USAGE_BLOCK_FREE,
-					HID_OUTPUT_REPORT);
-	if (!field) {
-		dev_err(&hid->dev->dev, "couldn't find report\n");
-		return -EIO;
-	}
-
-	ret = hid_set_field(field, 0, pid->effects[id].device_id);
-	if (ret) {
-		dev_err(&hid->dev->dev, "couldn't set field\n");
-		return ret;
-	}
-
-	hid_submit_report(hid, field->report, USB_DIR_OUT);
-
-	spin_lock_irqsave(&pid->lock, flags);
-	hid_pid_ctrl_playback(hid, pid->effects + id, 0);
-	pid->effects[id].flags = 0;
-	spin_unlock_irqrestore(&pid->lock, flags);
-
-	return 0;
-}
-
-/* Erase all effects this process owns */
-static int hid_pid_flush(struct input_dev *dev, struct file *file)
-{
-	struct hid_device *hid = dev->private;
-	struct hid_ff_pid *pid = hid->ff_private;
-	int i;
-
-	/*NOTE: no need to lock here. The only times EFFECT_USED is
-	   modified is when effects are uploaded or when an effect is
-	   erased. But a process cannot close its dev/input/eventX fd
-	   and perform ioctls on the same fd all at the same time */
-	/*FIXME: multiple threads, anyone? */
-	for (i = 0; i < dev->ff_effects_max; ++i)
-		if (current->pid == pid->effects[i].owner
-		    && test_bit(FF_PID_FLAGS_USED, &pid->effects[i].flags))
-			if (hid_pid_erase(dev, i))
-				dev_warn(&hid->dev->dev, "erase effect %d failed", i);
-
-	return 0;
-}
-
-static int hid_pid_upload_effect(struct input_dev *dev,
-				 struct ff_effect *effect)
-{
-	struct hid_ff_pid *pid_private = (struct hid_ff_pid *)(dev->private);
-	int ret;
-	int is_update;
-	unsigned long flags;
-
-	dev_dbg(&pid_private->hid->dev->dev, "upload effect called: effect_type=%x\n", effect->type);
-	/* Check this effect type is supported by this device */
-	if (!test_bit(effect->type, dev->ffbit)) {
-		dev_dbg(&pid_private->hid->dev->dev,
-			"invalid kind of effect requested.\n");
-		return -EINVAL;
-	}
-
-	/*
-	 * If we want to create a new effect, get a free id
-	 */
-	if (effect->id == -1) {
-		int id = 0;
-
-		// Spinlock so we don`t get a race condition when choosing IDs
-		spin_lock_irqsave(&pid_private->lock, flags);
-
-		while (id < FF_EFFECTS_MAX)
-			if (!test_and_set_bit(FF_PID_FLAGS_USED, &pid_private->effects[id++].flags))
-				break;
-
-		if (id == FF_EFFECTS_MAX) {
-			spin_unlock_irqrestore(&pid_private->lock, flags);
-// TEMP - We need to get ff_effects_max correctly first:  || id >= dev->ff_effects_max) {
-			dev_dbg(&pid_private->hid->dev->dev, "Not enough device memory\n");
-			return -ENOMEM;
-		}
-
-		effect->id = id;
-		dev_dbg(&pid_private->hid->dev->dev, "effect ID is %d.\n", id);
-		pid_private->effects[id].owner = current->pid;
-		pid_private->effects[id].flags = (1 << FF_PID_FLAGS_USED);
-		spin_unlock_irqrestore(&pid_private->lock, flags);
-
-		is_update = FF_PID_FALSE;
-	} else {
-		/* We want to update an effect */
-		if (!CHECK_OWNERSHIP(effect->id, pid_private))
-			return -EACCES;
-
-		/* Parameter type cannot be updated */
-		if (effect->type != pid_private->effects[effect->id].effect.type)
-			return -EINVAL;
-
-		/* Check the effect is not already being updated */
-		if (test_bit(FF_PID_FLAGS_UPDATING, &pid_private->effects[effect->id].flags))
-			return -EAGAIN;
-
-		is_update = FF_PID_TRUE;
-	}
-
-	/*
-	 * Upload the effect
-	 */
-	switch (effect->type) {
-	case FF_PERIODIC:
-		ret = pid_upload_periodic(pid_private, effect, is_update);
-		break;
-
-	case FF_CONSTANT:
-		ret = pid_upload_constant(pid_private, effect, is_update);
-		break;
-
-	case FF_SPRING:
-	case FF_FRICTION:
-	case FF_DAMPER:
-	case FF_INERTIA:
-		ret = pid_upload_condition(pid_private, effect, is_update);
-		break;
-
-	case FF_RAMP:
-		ret = pid_upload_ramp(pid_private, effect, is_update);
-		break;
-
-	default:
-		dev_dbg(&pid_private->hid->dev->dev,
-			"invalid type of effect requested - %x.\n",
-			effect->type);
-		return -EINVAL;
-	}
-	/* If a packet was sent, forbid new updates until we are notified
-	 * that the packet was updated
-	 */
-	if (ret == 0)
-		set_bit(FF_PID_FLAGS_UPDATING, &pid_private->effects[effect->id].flags);
-	pid_private->effects[effect->id].effect = *effect;
-	return ret;
-}
-
-int hid_pid_init(struct hid_device *hid)
-{
-	struct hid_ff_pid *private;
-	struct hid_input *hidinput = list_entry(hid->inputs.next, struct hid_input, list);
-	struct input_dev *input_dev = hidinput->input;
-
-	private = hid->ff_private = kzalloc(sizeof(struct hid_ff_pid), GFP_KERNEL);
-	if (!private)
-		return -ENOMEM;
-
-	private->hid = hid;
-
-	hid->ff_exit = hid_pid_exit;
-	hid->ff_event = hid_pid_event;
-
-	/* Open output URB */
-	if (!(private->urbffout = usb_alloc_urb(0, GFP_KERNEL))) {
-		kfree(private);
-		return -1;
-	}
-
-	usb_fill_control_urb(private->urbffout, hid->dev, 0,
-			     (void *)&private->ffcr, private->ctrl_buffer, 8,
-			     hid_pid_ctrl_out, hid);
-
-	input_dev->upload_effect = hid_pid_upload_effect;
-	input_dev->flush = hid_pid_flush;
-	input_dev->ff_effects_max = 8;	// A random default
-	set_bit(EV_FF, input_dev->evbit);
-	set_bit(EV_FF_STATUS, input_dev->evbit);
-
-	spin_lock_init(&private->lock);
-
-	printk(KERN_INFO "Force feedback driver for PID devices by Rodrigo Damazio <rdamazio@lsi.usp.br>.\n");
-
-	return 0;
-}
Index: linux-2.6.16-git20/drivers/usb/input/pid.h
===================================================================
--- linux-2.6.16-git20.orig/drivers/usb/input/pid.h	2006-04-10 21:02:44.000000000 +0300
+++ /dev/null	1970-01-01 00:00:00.000000000 +0000
@@ -1,62 +0,0 @@
-/*
- *  PID Force feedback support for hid devices.
- *
- *  Copyright (c) 2002 Rodrigo Damazio.
- */
-
-/*
- * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- * Should you need to contact me, the author, you can do so by
- * e-mail - mail your message to <rdamazio@lsi.usp.br>
- */
-
-#define FF_EFFECTS_MAX 64
-
-#define FF_PID_FLAGS_USED	1	/* If the effect exists */
-#define FF_PID_FLAGS_UPDATING	2	/* If the effect is being updated */
-#define FF_PID_FLAGS_PLAYING	3	/* If the effect is currently being played */
-
-#define FF_PID_FALSE	0
-#define FF_PID_TRUE	1
-
-struct hid_pid_effect {
-	unsigned long flags;
-	pid_t owner;
-	unsigned int device_id;	/* The device-assigned ID */
-	struct ff_effect effect;
-};
-
-struct hid_ff_pid {
-	struct hid_device *hid;
-	unsigned long gain;
-
-	struct urb *urbffout;
-	struct usb_ctrlrequest ffcr;
-	spinlock_t lock;
-
-	unsigned char ctrl_buffer[8];
-
-	struct hid_pid_effect effects[FF_EFFECTS_MAX];
-};
-
-/*
- * Constants from the PID usage table (still far from complete)
- */
-
-#define FF_PID_USAGE_BLOCK_LOAD 	0x89UL
-#define FF_PID_USAGE_BLOCK_FREE		0x90UL
-#define FF_PID_USAGE_NEW_EFFECT 	0xABUL
-#define FF_PID_USAGE_POOL_REPORT	0x7FUL

--
Anssi Hannula

Re: [patch 00/11] input: force feedback updates

[Dmitry Torokhov]

--- Anssi Hannula <anssi.hannula@gmail.com> wrote:

> Major update for the force feedback support, including a new force feedback
> driver interface and two new HID ff drivers. 
> 

Hi Anssi,

Thank you for the patches. I should finish settling down and setting up my
connection by the end of this week and then I should be able to review the
patches.

--
Dmitry