Re: [PATCH] Hotplug support for several PSX controlers

[Peter Nelson <rufus-kernel@hackish.org>]

Eric Piel wrote:

> > This seems like a reasonable explination when ports float if
> > unconnected.  Your patch does almost the right thing.  First
> > gc_psx_command should take a data[5] argument, that was a logic 
> error on
> > my part.  Second, you compare the calculated length to PSX_LENGTH, 
> which
> > is just saying we read in bytes.  It should check <= 6, which is the
> > longest string of packets possible (buttons, buttons, right, right,
> > left, left, see
> > <http://www.gamesx.com/controldata/psxcont/psxcont.htm>).  Changing to
> > compare to 6 makes the patch look good to me.
> I didn't know about the official spec. So changing PSX_LENGTH to 6 
> makes everything even more correct, good.

Sorry I confused you, but the source didn't use the variables as it 
should.  GC_PSX_LENGTH should be 8 meaning we read down 8 bits (1 byte) 
at a time.  I'll create new variable called GC_PSX_BYTES set to 6 
meaning we can pull down up to 6 bytes in a row for a single read.

> >> I've heard that Linus wants 2.6.10 ready for Christmas, this patch
> >> should definitetly helps ;-)
> > I'm all for both my previous patch and this one making it into 
> 2.6.10 =)
> Is there anyone in particular to tell about those patches? Or is the 
> normal way that Vojtech inserts the patches in his tree and Linus 
> pulls it when he feels it's stable enough?

 From what I see Vojtech accepts them and then Linus eventually pulls 
from Vojtech's tree.  Here is my modified patch.  Please test it to 
double check it works.

-Peter

---
From: Eric Piel <Eric.Piel@tremplin-utc.net>

Fixes hotplug support for PSX controllers and some mis-sized arrays.

Signed-off-by: Peter Nelson <rufus-kernel@hackish.org>
---
===== gamecon.c 1.19 vs edited =====
--- 1.19/drivers/input/joystick/gamecon.c	2004-12-13 20:04:39 -05:00
+++ edited/gamecon.c	2004-12-13 20:18:02 -05:00
@@ -228,6 +228,7 @@
 
 #define GC_PSX_DELAY	25		/* 25 usec */
 #define GC_PSX_LENGTH	8		/* talk to the controller in bytes */
+#define GC_PSX_BYTES	6		/* the maximum number of bytes to read off the controller */
 
 #define GC_PSX_MOUSE	1		/* Mouse */
 #define GC_PSX_NEGCON	2		/* NegCon */
@@ -241,7 +242,7 @@
 #define GC_PSX_SELECT	0x02		/* Pin 3 */
 
 #define GC_PSX_ID(x)	((x) >> 4)	/* High nibble is device type */
-#define GC_PSX_LEN(x)	((x) & 0xf)	/* Low nibble is length in words */
+#define GC_PSX_LEN(x)	(((x) & 0xf) << 1)	/* Low nibble is length in bytes/2 */
 
 static int gc_psx_delay = GC_PSX_DELAY;
 module_param_named(psx_delay, gc_psx_delay, uint, 0);
@@ -259,13 +260,13 @@
  * the psx pad.
  */
 
-static void gc_psx_command(struct gc *gc, int b, unsigned char data[GC_PSX_LENGTH])
+static void gc_psx_command(struct gc *gc, int b, unsigned char data[5])
 {
 	int i, j, cmd, read;
 	for (i = 0; i < 5; i++)
 		data[i] = 0;
 
-	for (i = 0; i < 8; i++, b >>= 1) {
+	for (i = 0; i < GC_PSX_LENGTH; i++, b >>= 1) {
 		cmd = (b & 1) ? GC_PSX_COMMAND : 0;
 		parport_write_data(gc->pd->port, cmd | GC_PSX_POWER);
 		udelay(gc_psx_delay);
@@ -283,7 +284,7 @@
  * device identifier code.
  */
 
-static void gc_psx_read_packet(struct gc *gc, unsigned char data[5][GC_PSX_LENGTH], unsigned char id[5])
+static void gc_psx_read_packet(struct gc *gc, unsigned char data[5][GC_PSX_BYTES], unsigned char id[5])
 {
 	int i, j, max_len = 0;
 	unsigned long flags;
@@ -302,10 +303,11 @@
 
 	for (i =0; i < 5; i++)								/* Find the longest pad */
 		if((gc_status_bit[i] & (gc->pads[GC_PSX] | gc->pads[GC_DDR]))
-		   && (GC_PSX_LEN(id[i]) > max_len))
+		   && (GC_PSX_LEN(id[i]) > max_len)
+		   && (GC_PSX_LEN(id[i]) <= GC_PSX_BYTES))
 			max_len = GC_PSX_LEN(id[i]);
 
-	for (i = 0; i < max_len * 2; i++) {						/* Read in all the data */
+	for (i = 0; i < max_len; i++) {							/* Read in all the data */
 		gc_psx_command(gc, 0, data2);
 		for (j = 0; j < 5; j++)
 			data[j][i] = data2[j];
@@ -330,7 +332,7 @@
 	struct gc *gc = (void *) private;
 	struct input_dev *dev = gc->dev;
 	unsigned char data[GC_MAX_LENGTH];
-	unsigned char data_psx[5][GC_PSX_LENGTH];
+	unsigned char data_psx[5][GC_PSX_BYTES];
 	int i, j, s;
 
 /*