* [PATCH] Input: i8042 - i8042_flush fix for a full 8042 buffer
@ 2013-09-18 12:35 Andrey Moiseev
2013-09-18 14:44 ` Dmitry Torokhov
0 siblings, 1 reply; 4+ messages in thread
From: Andrey Moiseev @ 2013-09-18 12:35 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, 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.
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 <o2g.org.ru@gmail.com>
---
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 78e4de4..cdda786 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -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;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Input: i8042 - i8042_flush fix for a full 8042 buffer
2013-09-18 12:35 [PATCH] Input: i8042 - i8042_flush fix for a full 8042 buffer Andrey Moiseev
@ 2013-09-18 14:44 ` Dmitry Torokhov
2013-09-18 15:20 ` Andrey Moiseev
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2013-09-18 14:44 UTC (permalink / raw)
To: Andrey Moiseev; +Cc: linux-input, linux-kernel
Hi Andrey,
On Wed, Sep 18, 2013 at 04:35:56PM +0400, Andrey Moiseev wrote:
> 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 <o2g.org.ru@gmail.com>
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 <o2g.org.ru@gmail.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/serio/i8042.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 78e4de4..52c9ebf 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -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;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Input: i8042 - i8042_flush fix for a full 8042 buffer
2013-09-18 14:44 ` Dmitry Torokhov
@ 2013-09-18 15:20 ` Andrey Moiseev
0 siblings, 0 replies; 4+ messages in thread
From: Andrey Moiseev @ 2013-09-18 15:20 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel
Nice. That should be of no interest for the outside world how much
junk 8042 contained, so that's better.
On Wed, Sep 18, 2013 at 6:44 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Andrey,
>
> On Wed, Sep 18, 2013 at 04:35:56PM +0400, Andrey Moiseev wrote:
>> 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 <o2g.org.ru@gmail.com>
>
> 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 <o2g.org.ru@gmail.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/serio/i8042.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index 78e4de4..52c9ebf 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -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;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] Input: i8042 - i8042_flush fix for a full 8042 buffer
@ 2013-09-07 7:45 Andrey Moiseev
0 siblings, 0 replies; 4+ messages in thread
From: Andrey Moiseev @ 2013-09-07 7:45 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Andrey Moiseev
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.
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 <o2g.org.ru@gmail.com>
---
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 78e4de4..cdda786 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -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;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-18 15:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-18 12:35 [PATCH] Input: i8042 - i8042_flush fix for a full 8042 buffer Andrey Moiseev
2013-09-18 14:44 ` Dmitry Torokhov
2013-09-18 15:20 ` Andrey Moiseev
-- strict thread matches above, loose matches on Subject: below --
2013-09-07 7:45 Andrey Moiseev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).