Skip to contentSkip to conversation

Input: i8042 - i8042_flush fix for a full 8042 buffer

PATCH In review Andrey Moiseev 2 participants 3 messages Sep 18, 2013

Subsystems (2)
HID Input drivers
lore
Share & tools
lore

Andrey Moiseev

author
#0 lore raw

PATCH 1/1 Input: i8042 - i8042_flush fix for a full 8042 buffer

When 8042 internal data buffer is full, the driver erroneously decides that the controller is not present.

I've already sent this 2 weeks ago, but that message received no comments.

i8042_flush returns the number of flushed bytes, which is in 0 - I8042_BUFFER_SIZE range inclusive. Therefore, i8042_flush has no way to indicate an error. Moreover i8042_controller_check takes initially full buffer (i8042_flush returned I8042_BUFFER_SIZE) as a sign of absence of the controller.

The fix:
i8042_flush should perform one more status check after I8042_BUFFER_SIZE bytes have been read, and, if it succeeds, indicate the error by returning -1. i8042_controller_check should be changed appropriately.

Signed-off-by Andrey Moiseev <[email protected]>

---

drivers/input/serio/i8042.c +5 -2
index 78e4de4..cdda786 100644
@@ -222,29 +222,32 @@ static int i8042_wait_write(void)
 static int i8042_flush(void)
 {
 	unsigned long flags;
 	unsigned char data, str;
 	int i = 0;
 
 	spin_lock_irqsave(&i8042_lock, flags);
 
-	while (((str = i8042_read_status()) & I8042_STR_OBF) && (i < I8042_BUFFER_SIZE)) {
+	while (((str = i8042_read_status()) & I8042_STR_OBF) && (i <= I8042_BUFFER_SIZE)) {
 		udelay(50);
 		data = i8042_read_data();
 		i++;
 		dbg("%02x <- i8042 (flush, %s)\n",
 		    data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
 	}
 
 	spin_unlock_irqrestore(&i8042_lock, flags);
 
+	if (i == I8042_BUFFER_SIZE + 1)
+		return -1;
+
 	return i;
 }
 
 /*
  * i8042_command() executes a command on the i8042. It also sends the input
  * parameter(s) of the commands to it, and receives the output value(s). The
  * parameters are to be stored in the param array, and the output is placed
  * into the same array. The number of the parameters and output values is
  * encoded in bits 8-11 of the command number.
  */
 

@@ -849,11 +852,11 @@ static int __init i8042_check_aux(void)
 
 static int i8042_controller_check(void)
 {
-	if (i8042_flush() == I8042_BUFFER_SIZE) {
+	if (i8042_flush() == -1) {
 		pr_err("No controller found\n");
 		return -ENODEV;
 	}
 
 	return 0;
 }
Signature · 1 line
1.8.4
maintainer
#1 lore raw

Hi Andrey,

Quoted 5 lines · Andrey Moiseev

When 8042 internal data buffer is full, the driver erroneously decides that the controller is not present.

I've already sent this 2 weeks ago, but that message received no comments.

Sorry about the delay. How about we rework it a bit and make flush return success/error instead of number of bytes read, like in the patch below?

Thanks.

Signature · 23 lines
Dmitry

Input: i8042 - i8042_flush fix for a full 8042 buffer

From: Andrey Moiseev <[email protected]>

When 8042 internal data buffer is full, the driver
erroneously decides that the controller is not present.

i8042_flush returns the number of flushed bytes, which is
in 0 - I8042_BUFFER_SIZE range inclusive. Therefore, i8042_flush
has no way to indicate an error. Moreover i8042_controller_check
takes initially full buffer (i8042_flush returned
I8042_BUFFER_SIZE) as a sign of absence of the controller.

Let's change i8042 to return success/error instead and make sure
we do not return error prematurely.

Signed-off-by: Andrey Moiseev <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
 drivers/input/serio/i8042.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/input/serio/i8042.c +14 -9
index 78e4de4..52c9ebf 100644
@@ -223,21 +223,26 @@ static int i8042_flush(void)
 {
 	unsigned long flags;
 	unsigned char data, str;
-	int i = 0;
+	int count = 0;
+	int retval = 0;
 
 	spin_lock_irqsave(&i8042_lock, flags);
 
-	while (((str = i8042_read_status()) & I8042_STR_OBF) && (i < I8042_BUFFER_SIZE)) {
-		udelay(50);
-		data = i8042_read_data();
-		i++;
-		dbg("%02x <- i8042 (flush, %s)\n",
-		    data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
+	while ((str = i8042_read_status()) & I8042_STR_OBF) {
+		if (count++ < I8042_BUFFER_SIZE) {
+			udelay(50);
+			data = i8042_read_data();
+			dbg("%02x <- i8042 (flush, %s)\n",
+			    data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
+		} else {
+			retval = -EIO;
+			break;
+		}
 	}
 
 	spin_unlock_irqrestore(&i8042_lock, flags);
 
-	return i;
+	return retval;
 }
 
 /*

@@ -849,7 +854,7 @@ static int __init i8042_check_aux(void)
 
 static int i8042_controller_check(void)
 {
-	if (i8042_flush() == I8042_BUFFER_SIZE) {
+	if (i8042_flush()) {
 		pr_err("No controller found\n");
 		return -ENODEV;
 	}
author
#2 lore raw

Nice. That should be of no interest for the outside world how much junk 8042 contained, so that's better.

Quoted 89 lines · Dmitry Torokhov

Hi Andrey,

Quoted 5 lines · Andrey Moiseev

When 8042 internal data buffer is full, the driver erroneously decides that the controller is not present.

I've already sent this 2 weeks ago, but that message received no comments.

Sorry about the delay. How about we rework it a bit and make flush return success/error instead of number of bytes read, like in the patch below?

Thanks.

--
Dmitry

Input: i8042 - i8042_flush fix for a full 8042 buffer

From: Andrey Moiseev <[email protected]>

When 8042 internal data buffer is full, the driver erroneously decides that the controller is not present.

i8042_flush returns the number of flushed bytes, which is in 0 - I8042_BUFFER_SIZE range inclusive. Therefore, i8042_flush has no way to indicate an error. Moreover i8042_controller_check takes initially full buffer (i8042_flush returned I8042_BUFFER_SIZE) as a sign of absence of the controller.

Let's change i8042 to return success/error instead and make sure we do not return error prematurely.

Signed-off-by Andrey Moiseev <[email protected]>
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/input/serio/i8042.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/input/serio/i8042.c +14 -9
index 78e4de4..52c9ebf 100644
@@ -223,21 +223,26 @@ static int i8042_flush(void)
 {
        unsigned long flags;
        unsigned char data, str;
-       int i = 0;
+       int count = 0;
+       int retval = 0;
 
        spin_lock_irqsave(&i8042_lock, flags);
 
-       while (((str = i8042_read_status()) & I8042_STR_OBF) && (i < I8042_BUFFER_SIZE)) {
-               udelay(50);
-               data = i8042_read_data();
-               i++;
-               dbg("%02x <- i8042 (flush, %s)\n",
-                   data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
+       while ((str = i8042_read_status()) & I8042_STR_OBF) {
+               if (count++ < I8042_BUFFER_SIZE) {
+                       udelay(50);
+                       data = i8042_read_data();
+                       dbg("%02x <- i8042 (flush, %s)\n",
+                           data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
+               } else {
+                       retval = -EIO;
+                       break;
+               }
        }
 
        spin_unlock_irqrestore(&i8042_lock, flags);
 
-       return i;
+       return retval;
 }
 
 /*

@@ -849,7 +854,7 @@ static int __init i8042_check_aux(void)
 
 static int i8042_controller_check(void)
 {
-       if (i8042_flush() == I8042_BUFFER_SIZE) {
+       if (i8042_flush()) {
                pr_err("No controller found\n");
                return -ENODEV;
        }