i8042 access timings

[Dmitry Torokhov]

Hi,

Recently there was a patch from Alan regarding access timing violations
in i8042. It made me curious as we only wait between accesses to status
register but not data register. I peeked into FreeBSD code and they use
delays to access both registers and I wonder if that's the piece that
makes i8042 mysteriously fail on some boards.

Anyway, regardless of whether access to data register should be done with
delay as well there seem to be another timing access violation - in
i8042_command we do i8042_wait_read which reads STR and then immediately
do i8042_read_status to check AUXDATA bit.

Does the patch below makes any sense?

-- 
Dmitry

===== drivers/input/serio/i8042.c 1.79 vs edited =====
--- 1.79/drivers/input/serio/i8042.c	2005-01-25 00:54:50 -05:00
+++ edited/drivers/input/serio/i8042.c	2005-01-25 02:04:56 -05:00
@@ -137,8 +137,8 @@
 static int i8042_wait_read(void)
 {
 	int i = 0;
-	while ((~i8042_read_status() & I8042_STR_OBF) && (i < I8042_CTL_TIMEOUT)) {
-		udelay(50);
+	while ((~i8042_read_status() & I8042_STR_OBF) && i < I8042_CTL_TIMEOUT) {
+		udelay(I8042_STR_DELAY);
 		i++;
 	}
 	return -(i == I8042_CTL_TIMEOUT);
@@ -147,8 +147,8 @@
 static int i8042_wait_write(void)
 {
 	int i = 0;
-	while ((i8042_read_status() & I8042_STR_IBF) && (i < I8042_CTL_TIMEOUT)) {
-		udelay(50);
+	while ((i8042_read_status() & I8042_STR_IBF) && i < I8042_CTL_TIMEOUT) {
+		udelay(I8042_STR_DELAY);
 		i++;
 	}
 	return -(i == I8042_CTL_TIMEOUT);
@@ -162,17 +162,20 @@
 static int i8042_flush(void)
 {
 	unsigned long flags;
-	unsigned char data;
+	unsigned char str, data;
 	int i = 0;
 
 	spin_lock_irqsave(&i8042_lock, flags);
 
-	while ((i8042_read_status() & I8042_STR_OBF) && (i++ < I8042_BUFFER_SIZE)) {
-		udelay(50);
+	do {
+		str = i8042_read_status();
+		if (~str & I8042_STR_OBF)
+			break;
+		udelay(I8042_DATA_DELAY);
 		data = i8042_read_data();
-		dbg("%02x <- i8042 (flush, %s)", data,
-			i8042_read_status() & I8042_STR_AUXDATA ? "aux" : "kbd");
-	}
+		dbg("%02x <- i8042 (flush, %s)", data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
+		udelay(I8042_STR_DELAY);
+	} while (i++ < I8042_BUFFER_SIZE);
 
 	spin_unlock_irqrestore(&i8042_lock, flags);
 
@@ -190,6 +193,7 @@
 static int i8042_command(unsigned char *param, int command)
 {
 	unsigned long flags;
+	unsigned char str;
 	int retval = 0, i = 0;
 
 	if (i8042_noloop && command == I8042_CMD_AUX_LOOP)
@@ -213,7 +217,10 @@
 	if (!retval)
 		for (i = 0; i < ((command >> 8) & 0xf); i++) {
 			if ((retval = i8042_wait_read())) break;
-			if (i8042_read_status() & I8042_STR_AUXDATA)
+			udelay(I8042_STR_DELAY);
+			str = i8042_read_status();
+			udelay(I8042_DATA_DELAY);
+			if (str & I8042_STR_AUXDATA)
 				param[i] = ~i8042_read_data();
 			else
 				param[i] = i8042_read_data();
@@ -424,6 +431,7 @@
 		ret = 0;
 		goto out;
 	}
+	udelay(I8042_DATA_DELAY);
 	data = i8042_read_data();
 	spin_unlock_irqrestore(&i8042_lock, flags);
 
===== drivers/input/serio/i8042.h 1.12 vs edited =====
--- 1.12/drivers/input/serio/i8042.h	2005-01-22 02:22:51 -05:00
+++ edited/drivers/input/serio/i8042.h	2005-01-25 01:44:36 -05:00
@@ -30,12 +30,19 @@
 #endif
 
 /*
- * This is in 50us units, the time we wait for the i8042 to react. This
+ * i8042 I/O recovery times
+ */
+#define I8042_STR_DELAY		20
+#define I8042_DATA_DELAY	7
+
+
+/*
+ * This is in 20us units, the time we wait for the i8042 to react. This
  * has to be long enough for the i8042 itself to timeout on sending a byte
  * to a non-existent mouse.
  */
 
-#define I8042_CTL_TIMEOUT	10000
+#define I8042_CTL_TIMEOUT	25000
 
 /*
  * When the device isn't opened and it's interrupts aren't used, we poll it at
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/