public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drivers/input/joystick/interact.c ; Linux 2.6.13-1
@ 2005-09-13 15:36 roy wood
  2005-09-13 18:41 ` Dmitry Torokhov
  0 siblings, 1 reply; 2+ messages in thread
From: roy wood @ 2005-09-13 15:36 UTC (permalink / raw)
  To: linux-kernel

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

This patch to the Interact joystick driver adds support for the
"RaiderPro Digital" model of joystick from Interact.  The patch is
made against kernel version 2.6.13-1.

I've tested this using a RaiderPro and it works perfectly.  I do not
have versions of the other supported Interact controllers that this
driver supports, so I cannot attest to whether I've broken things or
not (hopefully not).

This is my first patch submission, so please don't jump on my head too
hard if I missed something in the LKML FAQ (I did read it!).

Assuming anyone's read this far, I'd appreciate direct feedback via
email on this patch, since I'm not a regular LKML subscriber.  I'll
poll the archive for a week or so to keep an eye on the thread, but
direct email would guarantee I don't miss anything.

Also, apparently I need to send this directly to Linus to get this
into the tree.  Anyone care to tell me the best email address to use
to do that?  I promise not to foreward it to recruiters at MS.  :-)



--- linux-2.6.13.1/drivers/input/joystick/interact.c	2005-09-09
22:42:58.000000000 -0400
+++ linux-2.6.13.1-Interact/drivers/input/joystick/interact.c	2005-09-12
16:07:50.000000000 -0400
@@ -5,10 +5,16 @@
  *
  *  Based on the work of:
  *	Toby Deshane
+ *
+ *  History:
+ *  --------
+ *  2005-09-12: rrwood - Add support for RaiderPro
  */
 
 /*
  * InterAct digital gamepad/joystick driver for Linux
+ *
+ * Note that the "new" Interact web site is http://www.speed-link.com/
  */
 
 /*
@@ -45,45 +51,66 @@
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-#define INTERACT_MAX_START	600	/* 400 us */
-#define INTERACT_MAX_STROBE	60	/* 40 us */
+#define INTERACT_MAX_START	600	/* 600 us */
+#define INTERACT_MAX_STROBE	60	/* 60 us */
 #define INTERACT_MAX_LENGTH	32	/* 32 bits */
 
 #define INTERACT_TYPE_HHFX	0	/* HammerHead/FX */
 #define INTERACT_TYPE_PP8D	1	/* ProPad 8 */
+#define INTERACT_TYPE_RAIDERPRO	2	/* RaiderPro */
 
 struct interact {
-	struct gameport *gameport;
-	struct input_dev dev;
-	int bads;
-	int reads;
-	unsigned char type;
-	unsigned char length;
-	char phys[32];
+	struct gameport *gameport; /* Kernel gameport struct ptr */
+	struct input_dev dev;      /* Kernel input_dev struct ptr */
+	int bads;                  /* Count of bad reads from joystick */
+	int reads;                 /* Count of total reads from joystick */
+	unsigned char type;        /* Joystick model index */
+	unsigned char length;      /* Number of bits in input packets */
+	char phys[32];             /* Physical device name */
 };
 
-static short interact_abs_hhfx[] =
+
+/* I think the original purpose of setting up lists of controller
+ * axes/buttons was to provide a single location to maintain such
+ * information.  Although the table-based approach certainly makes 
+ * the interact_connect() code below MUCH simpler and cleaner, the
+ * interact_poll() code ends up being very hard to read, unfortunately.
+ *
+ * I was tempted to either rewrite interact_poll() in a clearer fashion,
+ * or to implement a more comprehensive table-driven decoding approach
+ * (with values for offset, masking, shifting of each value).  I'm a
+ * bit leery of making such massive change though, since I don't have the
+ * controllers to test the result.  Instead, I'll just add support 
+ * for the RaiderPro as clearly as I can.....
+ */
+
+static short interact_abs_hhfx[] = 
 	{ ABS_RX, ABS_RY, ABS_X, ABS_Y, ABS_HAT0X, ABS_HAT0Y, -1 };
 static short interact_abs_pp8d[] =
 	{ ABS_X, ABS_Y, -1 };
+static short interact_abs_raiderpro[] = 
+	{ ABS_X, ABS_Y, ABS_THROTTLE, ABS_HAT0X, ABS_HAT0Y, -1 };
 
 static short interact_btn_hhfx[] =
 	{ BTN_TR, BTN_X, BTN_Y, BTN_Z, BTN_A, BTN_B, BTN_C, BTN_TL, BTN_TL2,
BTN_TR2, BTN_MODE, BTN_SELECT, -1 };
 static short interact_btn_pp8d[] =
 	{ BTN_C, BTN_TL, BTN_TR, BTN_A, BTN_B, BTN_Y, BTN_Z, BTN_X, -1 };
+static short interact_btn_raiderpro[] =
+	{ BTN_TRIGGER, BTN_THUMB, BTN_BASE, BTN_BASE2, BTN_BASE3, BTN_BASE4, -1 };
 
 struct interact_type {
-	int id;
-	short *abs;
-	short *btn;
-	char *name;
-	unsigned char length;
-	unsigned char b8;
+	int id;               /* Numeric device ID read during configuration */
+	short *abs;           /* Pointer to list of axes identifiers */
+	short *btn;           /* Pointer to list of button identifiers */
+	char *name;           /* Pointer to device model descriptor string */
+	unsigned char length; /* Number of bits in input data packet */
+	unsigned char b8;     /* Count of analog (non-hat) axes in *abs list */
 };
 
 static struct interact_type interact_type[] = {
 	{ 0x6202, interact_abs_hhfx, interact_btn_hhfx, "InterAct
HammerHead/FX",    32, 4 },
 	{ 0x53f8, interact_abs_pp8d, interact_btn_pp8d, "InterAct ProPad 8
Digital", 16, 0 },
+	{ 0x51F8, interact_abs_raiderpro, interact_btn_raiderpro, "InterAct
RaiderPro Digital",    32, 3 },
 	{ 0 }};
 
 /*
@@ -137,44 +164,70 @@
 	interact->reads++;
 
 	if (interact_read_packet(interact->gameport, interact->length, data)
< interact->length) {
+		/* Couldn't read a full packet, so update the bad-count, 
+		 * queue another read, and get out */
 		interact->bads++;
-	} else {
+		input_sync(dev);
+		return;
+	}
+
+/* During development and debugging, it's nice to see all the read data */
+/* printk("d0:%08X d1:%08X d2:%08X\n", data[0], data[1], data[2]); */
 
+	
+	if (INTERACT_MAX_LENGTH - interact->length > 0) {
+		/* If data packets are less than max length, shift them
+		 * for easier processing below (ProPad goofiness) */
 		for (i = 0; i < 3; i++)
 			data[i] <<= INTERACT_MAX_LENGTH - interact->length;
+	}
 
-		switch (interact->type) {
-
-			case INTERACT_TYPE_HHFX:
-
-				for (i = 0; i < 4; i++)
-					input_report_abs(dev, interact_abs_hhfx[i], (data[i & 1] >> ((i
>> 1) << 3)) & 0xff);
-
-				for (i = 0; i < 2; i++)
-					input_report_abs(dev, ABS_HAT0Y - i,
-						((data[1] >> ((i << 1) + 17)) & 1)  - ((data[1] >> ((i << 1) +
16)) & 1));
-
-				for (i = 0; i < 8; i++)
-					input_report_key(dev, interact_btn_hhfx[i], (data[0] >> (i + 16)) & 1);
-
-				for (i = 0; i < 4; i++)
-					input_report_key(dev, interact_btn_hhfx[i + 8], (data[1] >> (i +
20)) & 1);
-
-				break;
-
-			case INTERACT_TYPE_PP8D:
+	
+	/* Unpack the data we read and pass along the info to the input layer */
+	
+	if (interact->type == INTERACT_TYPE_HHFX) {
+		for (i = 0; i < 4; i++)
+			input_report_abs(dev, interact_abs_hhfx[i], (data[i & 1] >> ((i >>
1) << 3)) & 0xff);
 
-				for (i = 0; i < 2; i++)
-					input_report_abs(dev, interact_abs_pp8d[i],
-						((data[0] >> ((i << 1) + 20)) & 1)  - ((data[0] >> ((i << 1) +
21)) & 1));
+		for (i = 0; i < 2; i++)
+			input_report_abs(dev, ABS_HAT0Y - i, ((data[1] >> ((i << 1) + 17))
& 1)  - ((data[1] >> ((i << 1) + 16)) & 1));
 
-				for (i = 0; i < 8; i++)
-					input_report_key(dev, interact_btn_pp8d[i], (data[1] >> (i + 16)) & 1);
+		for (i = 0; i < 8; i++)
+			input_report_key(dev, interact_btn_hhfx[i], (data[0] >> (i + 16)) & 1);
 
-				break;
-		}
+		for (i = 0; i < 4; i++)
+			input_report_key(dev, interact_btn_hhfx[i + 8], (data[1] >> (i + 20)) & 1);
+	}
+	else if (interact->type == INTERACT_TYPE_PP8D) {
+		for (i = 0; i < 2; i++)
+			input_report_abs(dev, interact_abs_pp8d[i], ((data[0] >> ((i << 1)
+ 20)) & 1)  - ((data[0] >> ((i << 1) + 21)) & 1));
+
+		for (i = 0; i < 8; i++)
+			input_report_key(dev, interact_btn_pp8d[i], (data[1] >> (i + 16)) & 1);
+
+	}	
+	else if (interact->type == INTERACT_TYPE_RAIDERPRO) {
+		int hatRight = (data[0] >> 20) & 0x01;
+		int hatLeft =  (data[0] >> 21) & 0x01;
+		int hatDown =  (data[0] >> 22) & 0x01;
+		int hatUp =    (data[0] >> 23) & 0x01;
+		
+		input_report_abs(dev, ABS_HAT0X,    hatRight - hatLeft);
+		input_report_abs(dev, ABS_HAT0Y,    hatUp - hatDown);
+		
+		input_report_abs(dev, ABS_X,        (data[0] >> 8) & 0xff);
+		input_report_abs(dev, ABS_Y,        (data[1] >> 8) & 0xff);
+		input_report_abs(dev, ABS_THROTTLE, data[1] & 0xff);
+
+		input_report_key(dev, BTN_BASE4,    (data[1] >> 16) & 1);
+		input_report_key(dev, BTN_BASE2,    (data[1] >> 19) & 1);
+		input_report_key(dev, BTN_BASE3,    (data[1] >> 20) & 1);
+		input_report_key(dev, BTN_THUMB,    (data[1] >> 21) & 1);
+		input_report_key(dev, BTN_BASE,     (data[1] >> 22) & 1);
+		input_report_key(dev, BTN_TRIGGER,  (data[1] >> 23) & 1);
 	}
 
+	/* Queue another read */
 	input_sync(dev);
 }
 
@@ -235,7 +288,7 @@
 			break;
 
 	if (!interact_type[i].length) {
-		printk(KERN_WARNING "interact.c: Unknown joystick on %s. [len %d d0
%08x d1 %08x i2 %08x]\n",
+		printk(KERN_WARNING "interact.c: Unknown joystick on %s. [len %d d0
%08x d1 %08x d2 %08x]\n",
 			gameport->phys, i, data[0], data[1], data[2]);
 		err = -ENODEV;
 		goto fail2;
@@ -262,6 +315,10 @@
 
 	interact->dev.evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
 
+	/* This is the one place it's nice to have tables of the axes/buttons,
+	 * since it makes it so easy to report the controller characteristics
+	 * to the kernel's input layer */
+
 	for (i = 0; (t = interact_type[interact->type].abs[i]) >= 0; i++) {
 		set_bit(t, interact->dev.absbit);
 		if (i < interact_type[interact->type].b8) {

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: interact.diff --]
[-- Type: text/x-patch; name="interact.diff", Size: 8117 bytes --]

--- linux-2.6.13.1/drivers/input/joystick/interact.c	2005-09-09 22:42:58.000000000 -0400
+++ linux-2.6.13.1-Interact/drivers/input/joystick/interact.c	2005-09-12 16:07:50.000000000 -0400
@@ -5,10 +5,16 @@
  *
  *  Based on the work of:
  *	Toby Deshane
+ *
+ *  History:
+ *  --------
+ *  2005-09-12: rrwood - Add support for RaiderPro
  */
 
 /*
  * InterAct digital gamepad/joystick driver for Linux
+ *
+ * Note that the "new" Interact web site is http://www.speed-link.com/
  */
 
 /*
@@ -45,45 +51,66 @@
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
 
-#define INTERACT_MAX_START	600	/* 400 us */
-#define INTERACT_MAX_STROBE	60	/* 40 us */
+#define INTERACT_MAX_START	600	/* 600 us */
+#define INTERACT_MAX_STROBE	60	/* 60 us */
 #define INTERACT_MAX_LENGTH	32	/* 32 bits */
 
 #define INTERACT_TYPE_HHFX	0	/* HammerHead/FX */
 #define INTERACT_TYPE_PP8D	1	/* ProPad 8 */
+#define INTERACT_TYPE_RAIDERPRO	2	/* RaiderPro */
 
 struct interact {
-	struct gameport *gameport;
-	struct input_dev dev;
-	int bads;
-	int reads;
-	unsigned char type;
-	unsigned char length;
-	char phys[32];
+	struct gameport *gameport; /* Kernel gameport struct ptr */
+	struct input_dev dev;      /* Kernel input_dev struct ptr */
+	int bads;                  /* Count of bad reads from joystick */
+	int reads;                 /* Count of total reads from joystick */
+	unsigned char type;        /* Joystick model index */
+	unsigned char length;      /* Number of bits in input packets */
+	char phys[32];             /* Physical device name */
 };
 
-static short interact_abs_hhfx[] =
+
+/* I think the original purpose of setting up lists of controller
+ * axes/buttons was to provide a single location to maintain such
+ * information.  Although the table-based approach certainly makes 
+ * the interact_connect() code below MUCH simpler and cleaner, the
+ * interact_poll() code ends up being very hard to read, unfortunately.
+ *
+ * I was tempted to either rewrite interact_poll() in a clearer fashion,
+ * or to implement a more comprehensive table-driven decoding approach
+ * (with values for offset, masking, shifting of each value).  I'm a
+ * bit leery of making such massive change though, since I don't have the
+ * controllers to test the result.  Instead, I'll just add support 
+ * for the RaiderPro as clearly as I can.....
+ */
+
+static short interact_abs_hhfx[] = 
 	{ ABS_RX, ABS_RY, ABS_X, ABS_Y, ABS_HAT0X, ABS_HAT0Y, -1 };
 static short interact_abs_pp8d[] =
 	{ ABS_X, ABS_Y, -1 };
+static short interact_abs_raiderpro[] = 
+	{ ABS_X, ABS_Y, ABS_THROTTLE, ABS_HAT0X, ABS_HAT0Y, -1 };
 
 static short interact_btn_hhfx[] =
 	{ BTN_TR, BTN_X, BTN_Y, BTN_Z, BTN_A, BTN_B, BTN_C, BTN_TL, BTN_TL2, BTN_TR2, BTN_MODE, BTN_SELECT, -1 };
 static short interact_btn_pp8d[] =
 	{ BTN_C, BTN_TL, BTN_TR, BTN_A, BTN_B, BTN_Y, BTN_Z, BTN_X, -1 };
+static short interact_btn_raiderpro[] =
+	{ BTN_TRIGGER, BTN_THUMB, BTN_BASE, BTN_BASE2, BTN_BASE3, BTN_BASE4, -1 };
 
 struct interact_type {
-	int id;
-	short *abs;
-	short *btn;
-	char *name;
-	unsigned char length;
-	unsigned char b8;
+	int id;               /* Numeric device ID read during configuration */
+	short *abs;           /* Pointer to list of axes identifiers */
+	short *btn;           /* Pointer to list of button identifiers */
+	char *name;           /* Pointer to device model descriptor string */
+	unsigned char length; /* Number of bits in input data packet */
+	unsigned char b8;     /* Count of analog (non-hat) axes in *abs list */
 };
 
 static struct interact_type interact_type[] = {
 	{ 0x6202, interact_abs_hhfx, interact_btn_hhfx, "InterAct HammerHead/FX",    32, 4 },
 	{ 0x53f8, interact_abs_pp8d, interact_btn_pp8d, "InterAct ProPad 8 Digital", 16, 0 },
+	{ 0x51F8, interact_abs_raiderpro, interact_btn_raiderpro, "InterAct RaiderPro Digital",    32, 3 },
 	{ 0 }};
 
 /*
@@ -137,44 +164,70 @@
 	interact->reads++;
 
 	if (interact_read_packet(interact->gameport, interact->length, data) < interact->length) {
+		/* Couldn't read a full packet, so update the bad-count, 
+		 * queue another read, and get out */
 		interact->bads++;
-	} else {
+		input_sync(dev);
+		return;
+	}
+
+/* During development and debugging, it's nice to see all the read data */
+/* printk("d0:%08X d1:%08X d2:%08X\n", data[0], data[1], data[2]); */
 
+	
+	if (INTERACT_MAX_LENGTH - interact->length > 0) {
+		/* If data packets are less than max length, shift them
+		 * for easier processing below (ProPad goofiness) */
 		for (i = 0; i < 3; i++)
 			data[i] <<= INTERACT_MAX_LENGTH - interact->length;
+	}
 
-		switch (interact->type) {
-
-			case INTERACT_TYPE_HHFX:
-
-				for (i = 0; i < 4; i++)
-					input_report_abs(dev, interact_abs_hhfx[i], (data[i & 1] >> ((i >> 1) << 3)) & 0xff);
-
-				for (i = 0; i < 2; i++)
-					input_report_abs(dev, ABS_HAT0Y - i,
-						((data[1] >> ((i << 1) + 17)) & 1)  - ((data[1] >> ((i << 1) + 16)) & 1));
-
-				for (i = 0; i < 8; i++)
-					input_report_key(dev, interact_btn_hhfx[i], (data[0] >> (i + 16)) & 1);
-
-				for (i = 0; i < 4; i++)
-					input_report_key(dev, interact_btn_hhfx[i + 8], (data[1] >> (i + 20)) & 1);
-
-				break;
-
-			case INTERACT_TYPE_PP8D:
+	
+	/* Unpack the data we read and pass along the info to the input layer */
+	
+	if (interact->type == INTERACT_TYPE_HHFX) {
+		for (i = 0; i < 4; i++)
+			input_report_abs(dev, interact_abs_hhfx[i], (data[i & 1] >> ((i >> 1) << 3)) & 0xff);
 
-				for (i = 0; i < 2; i++)
-					input_report_abs(dev, interact_abs_pp8d[i],
-						((data[0] >> ((i << 1) + 20)) & 1)  - ((data[0] >> ((i << 1) + 21)) & 1));
+		for (i = 0; i < 2; i++)
+			input_report_abs(dev, ABS_HAT0Y - i, ((data[1] >> ((i << 1) + 17)) & 1)  - ((data[1] >> ((i << 1) + 16)) & 1));
 
-				for (i = 0; i < 8; i++)
-					input_report_key(dev, interact_btn_pp8d[i], (data[1] >> (i + 16)) & 1);
+		for (i = 0; i < 8; i++)
+			input_report_key(dev, interact_btn_hhfx[i], (data[0] >> (i + 16)) & 1);
 
-				break;
-		}
+		for (i = 0; i < 4; i++)
+			input_report_key(dev, interact_btn_hhfx[i + 8], (data[1] >> (i + 20)) & 1);
+	}
+	else if (interact->type == INTERACT_TYPE_PP8D) {
+		for (i = 0; i < 2; i++)
+			input_report_abs(dev, interact_abs_pp8d[i], ((data[0] >> ((i << 1) + 20)) & 1)  - ((data[0] >> ((i << 1) + 21)) & 1));
+
+		for (i = 0; i < 8; i++)
+			input_report_key(dev, interact_btn_pp8d[i], (data[1] >> (i + 16)) & 1);
+
+	}	
+	else if (interact->type == INTERACT_TYPE_RAIDERPRO) {
+		int hatRight = (data[0] >> 20) & 0x01;
+		int hatLeft =  (data[0] >> 21) & 0x01;
+		int hatDown =  (data[0] >> 22) & 0x01;
+		int hatUp =    (data[0] >> 23) & 0x01;
+		
+		input_report_abs(dev, ABS_HAT0X,    hatRight - hatLeft);
+		input_report_abs(dev, ABS_HAT0Y,    hatUp - hatDown);
+		
+		input_report_abs(dev, ABS_X,        (data[0] >> 8) & 0xff);
+		input_report_abs(dev, ABS_Y,        (data[1] >> 8) & 0xff);
+		input_report_abs(dev, ABS_THROTTLE, data[1] & 0xff);
+
+		input_report_key(dev, BTN_BASE4,    (data[1] >> 16) & 1);
+		input_report_key(dev, BTN_BASE2,    (data[1] >> 19) & 1);
+		input_report_key(dev, BTN_BASE3,    (data[1] >> 20) & 1);
+		input_report_key(dev, BTN_THUMB,    (data[1] >> 21) & 1);
+		input_report_key(dev, BTN_BASE,     (data[1] >> 22) & 1);
+		input_report_key(dev, BTN_TRIGGER,  (data[1] >> 23) & 1);
 	}
 
+	/* Queue another read */
 	input_sync(dev);
 }
 
@@ -235,7 +288,7 @@
 			break;
 
 	if (!interact_type[i].length) {
-		printk(KERN_WARNING "interact.c: Unknown joystick on %s. [len %d d0 %08x d1 %08x i2 %08x]\n",
+		printk(KERN_WARNING "interact.c: Unknown joystick on %s. [len %d d0 %08x d1 %08x d2 %08x]\n",
 			gameport->phys, i, data[0], data[1], data[2]);
 		err = -ENODEV;
 		goto fail2;
@@ -262,6 +315,10 @@
 
 	interact->dev.evbit[0] = BIT(EV_KEY) | BIT(EV_ABS);
 
+	/* This is the one place it's nice to have tables of the axes/buttons,
+	 * since it makes it so easy to report the controller characteristics
+	 * to the kernel's input layer */
+
 	for (i = 0; (t = interact_type[interact->type].abs[i]) >= 0; i++) {
 		set_bit(t, interact->dev.absbit);
 		if (i < interact_type[interact->type].b8) {

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

* Re: [PATCH] drivers/input/joystick/interact.c ; Linux 2.6.13-1
  2005-09-13 15:36 [PATCH] drivers/input/joystick/interact.c ; Linux 2.6.13-1 roy wood
@ 2005-09-13 18:41 ` Dmitry Torokhov
  0 siblings, 0 replies; 2+ messages in thread
From: Dmitry Torokhov @ 2005-09-13 18:41 UTC (permalink / raw)
  To: roy.wood; +Cc: linux-kernel, Vojtech Pavlik

On 9/13/05, roy wood <roy.wood@gmail.com> wrote:
> This patch to the Interact joystick driver adds support for the
> "RaiderPro Digital" model of joystick from Interact.  The patch is
> made against kernel version 2.6.13-1.

Cool, looks pretty nice.

> Also, apparently I need to send this directly to Linus to get this
> into the tree.  Anyone care to tell me the best email address to use
> to do that?  I promise not to foreward it to recruiters at MS.  :-)
> 

Actually that would be Vojtech Pavlik <vojtech@suse.cz> - input system
maintainer.

> + *
> + *  History:
> + *  --------
> + *  2005-09-12: rrwood - Add support for RaiderPro
>  */
> 

We prefer keeping changelogs in SCM, when possible

>  struct interact {
> -       struct gameport *gameport;
> -       struct input_dev dev;
> -       int bads;
> -       int reads;
> -       unsigned char type;
> -       unsigned char length;
> -       char phys[32];
> +       struct gameport *gameport; /* Kernel gameport struct ptr */
> +       struct input_dev dev;      /* Kernel input_dev struct ptr */

And I don't think we should add comments like these - they don't tell
you anything new. Especially when they wrong (dev is not a pointer
[yet])

> 
> -static short interact_abs_hhfx[] =
> +
> +/* I think the original purpose of setting up lists of controller
> + * axes/buttons was to provide a single location to maintain such
> + * information.  Although the table-based approach certainly makes
> + * the interact_connect() code below MUCH simpler and cleaner, the
> + * interact_poll() code ends up being very hard to read, unfortunately.
> + *
> + * I was tempted to either rewrite interact_poll() in a clearer fashion,
> + * or to implement a more comprehensive table-driven decoding approach
> + * (with values for offset, masking, shifting of each value).  I'm a
> + * bit leery of making such massive change though, since I don't have the
> + * controllers to test the result.  Instead, I'll just add support
> + * for the RaiderPro as clearly as I can.....
> + */

This is not a book, what one could have done but decided not to is not
very interesting unless the solution is outlined as a future TODO
item.

> 
>        if (interact_read_packet(interact->gameport, interact->length, data)
> < interact->length) {
> +               /* Couldn't read a full packet, so update the bad-count,
> +                * queue another read, and get out */

Yes, that is what that "if" statement said. Why also comment it?

> +       if (INTERACT_MAX_LENGTH - interact->length > 0) {
> +               /* If data packets are less than max length, shift them
> +                * for easier processing below (ProPad goofiness) */

This one is a decent comment tough (IMHO).

> +       /* Queue another read */
>        input_sync(dev);

??? input_sync signals that the input event packet is complete. What
is it about queueing another read stuff?

-- 
Dmitry

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

end of thread, other threads:[~2005-09-13 18:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-13 15:36 [PATCH] drivers/input/joystick/interact.c ; Linux 2.6.13-1 roy wood
2005-09-13 18:41 ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox