Re: [PATCH v2] atkbd: Fix multi-char scancode handling on reconnect.

2013-01-10 Thread Shawn Nematbakhsh
Hi Dmitry,

On Sun, Jan 6, 2013 at 1:10 AM, Dmitry Torokhov
 wrote:
> Hi Shawn,
>
> On Thu, Dec 20, 2012 at 06:33:11PM -0800, Shawn Nematbakhsh wrote:
>> On resume from suspend there is a possibility for multi-byte scancodes
>> to be handled incorrectly. atkbd_reconnect disables the processing of
>> scancodes in software by calling atkbd_disable, but the keyboard may
>> still be active because no disconnect command was sent. Later, software
>> handling is re-enabled. If a multi-byte scancode sent from the keyboard
>> straddles the re-enable, only the latter byte(s) will be handled.
>>
>> In practice, this leads to cases where multi-byte break codes (ex. "e0
>> 4d" - break code for right-arrow) are misread as make codes ("4d" - make
>> code for numeric 6), leading to one or more unwanted, untyped characters
>> being interpreted.
>>
>> The solution implemented here involves sending command f5 (reset
>> disable) to the keyboard prior to disabling software handling of codes.
>> Later, the command to re-enable the keyboard is sent only after we are
>> prepared to handle scancodes.
>
> The core tries to avoid disturbing devices that are not keyboards, so I
> believe we should check the device ID first and if it is keyboard, then
> do the reset. We should also reset the internal state (emul and xl_bit)
> when re-enabling the device.
>
> Does the version of the patch below work for you?
>
> Thanks.
>
> --
> Dmitry

Thanks for the revision. This version of the patch tests good. I agree
with your changes, it makes sense to deactivate the keyboard from both
connect and reconnect, just in case the keyboard is able to send
scancodes at this point.

--
Shawn

>
> Input: atkbd - fix multi-byte scancode handling on reconnect
>
> From: Shawn Nematbakhsh 
>
> On resume from suspend there is a possibility for multi-byte scancodes
> to be handled incorrectly. atkbd_reconnect disables the processing of
> scancodes in software by calling atkbd_disable, but the keyboard may
> still be active because no disconnect command was sent. Later, software
> handling is re-enabled. If a multi-byte scancode sent from the keyboard
> straddles the re-enable, only the latter byte(s) will be handled.
>
> In practice, this leads to cases where multi-byte break codes (ex. "e0
> 4d" - break code for right-arrow) are misread as make codes ("4d" - make
> code for numeric 6), leading to one or more unwanted, untyped characters
> being interpreted.
>
> The solution implemented here involves sending command f5 (reset
> disable) to the keyboard prior to disabling software handling of codes.
> Later, the command to re-enable the keyboard is sent only after we are
> prepared to handle scancodes.
>
> Signed-off-by: Shawn Nematbakhsh 
> Signed-off-by: Dmitry Torokhov 
> ---
>  drivers/input/keyboard/atkbd.c |   72 
> 
>  1 file changed, 51 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> index 33d0fcd..2626773 100644
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -676,6 +676,39 @@ static inline void atkbd_disable(struct atkbd *atkbd)
> serio_continue_rx(atkbd->ps2dev.serio);
>  }
>
> +static int atkbd_activate(struct atkbd *atkbd)
> +{
> +   struct ps2dev *ps2dev = >ps2dev;
> +
> +/*
> + * Enable the keyboard to receive keystrokes.
> + */
> +
> +   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
> +   dev_err(>serio->dev,
> +   "Failed to enable keyboard on %s\n",
> +   ps2dev->serio->phys);
> +   return -1;
> +   }
> +
> +   return 0;
> +}
> +
> +/*
> + * atkbd_deactivate() resets and disables the keyboard from sending
> + * keystrokes.
> + */
> +
> +static void atkbd_deactivate(struct atkbd *atkbd)
> +{
> +   struct ps2dev *ps2dev = >ps2dev;
> +
> +   if (ps2_command(ps2dev, NULL, ATKBD_CMD_RESET_DIS))
> +   dev_err(>serio->dev,
> +   "Failed to deactivate keyboard on %s\n",
> +   ps2dev->serio->phys);
> +}
> +
>  /*
>   * atkbd_probe() probes for an AT keyboard on a serio port.
>   */
> @@ -731,6 +764,12 @@ static int atkbd_probe(struct atkbd *atkbd)
> return -1;
> }
>
> +/*
> + * Make sure nothing is coming from the keyboard and disturbs our
> + * internal state.
> + */
> +   atkbd_deactivate(atkbd);
> +
> return 0;
>  }
>
> @@ -825,24 +864,6 @@ static int atkbd_reset_state(struct atkbd *atkbd)
> return 0;
>  }
>
> -static int atkbd_activate(struct atkbd *atkbd)
> -{
> -   struct ps2dev *ps2dev = >ps2dev;
> -
> -/*
> - * Enable the keyboard to receive keystrokes.
> - */
> -
> -   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
> -   dev_err(>serio->dev,
> -   "Failed to enable keyboard on %s\n",
> -   ps2dev->serio->phys);
> -   return -1;
> -  

Re: [PATCH v2] atkbd: Fix multi-char scancode handling on reconnect.

2013-01-10 Thread Shawn Nematbakhsh
Hi Dmitry,

On Sun, Jan 6, 2013 at 1:10 AM, Dmitry Torokhov
dmitry.torok...@gmail.com wrote:
 Hi Shawn,

 On Thu, Dec 20, 2012 at 06:33:11PM -0800, Shawn Nematbakhsh wrote:
 On resume from suspend there is a possibility for multi-byte scancodes
 to be handled incorrectly. atkbd_reconnect disables the processing of
 scancodes in software by calling atkbd_disable, but the keyboard may
 still be active because no disconnect command was sent. Later, software
 handling is re-enabled. If a multi-byte scancode sent from the keyboard
 straddles the re-enable, only the latter byte(s) will be handled.

 In practice, this leads to cases where multi-byte break codes (ex. e0
 4d - break code for right-arrow) are misread as make codes (4d - make
 code for numeric 6), leading to one or more unwanted, untyped characters
 being interpreted.

 The solution implemented here involves sending command f5 (reset
 disable) to the keyboard prior to disabling software handling of codes.
 Later, the command to re-enable the keyboard is sent only after we are
 prepared to handle scancodes.

 The core tries to avoid disturbing devices that are not keyboards, so I
 believe we should check the device ID first and if it is keyboard, then
 do the reset. We should also reset the internal state (emul and xl_bit)
 when re-enabling the device.

 Does the version of the patch below work for you?

 Thanks.

 --
 Dmitry

Thanks for the revision. This version of the patch tests good. I agree
with your changes, it makes sense to deactivate the keyboard from both
connect and reconnect, just in case the keyboard is able to send
scancodes at this point.

--
Shawn


 Input: atkbd - fix multi-byte scancode handling on reconnect

 From: Shawn Nematbakhsh sha...@chromium.org

 On resume from suspend there is a possibility for multi-byte scancodes
 to be handled incorrectly. atkbd_reconnect disables the processing of
 scancodes in software by calling atkbd_disable, but the keyboard may
 still be active because no disconnect command was sent. Later, software
 handling is re-enabled. If a multi-byte scancode sent from the keyboard
 straddles the re-enable, only the latter byte(s) will be handled.

 In practice, this leads to cases where multi-byte break codes (ex. e0
 4d - break code for right-arrow) are misread as make codes (4d - make
 code for numeric 6), leading to one or more unwanted, untyped characters
 being interpreted.

 The solution implemented here involves sending command f5 (reset
 disable) to the keyboard prior to disabling software handling of codes.
 Later, the command to re-enable the keyboard is sent only after we are
 prepared to handle scancodes.

 Signed-off-by: Shawn Nematbakhsh sha...@chromium.org
 Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com
 ---
  drivers/input/keyboard/atkbd.c |   72 
 
  1 file changed, 51 insertions(+), 21 deletions(-)

 diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
 index 33d0fcd..2626773 100644
 --- a/drivers/input/keyboard/atkbd.c
 +++ b/drivers/input/keyboard/atkbd.c
 @@ -676,6 +676,39 @@ static inline void atkbd_disable(struct atkbd *atkbd)
 serio_continue_rx(atkbd-ps2dev.serio);
  }

 +static int atkbd_activate(struct atkbd *atkbd)
 +{
 +   struct ps2dev *ps2dev = atkbd-ps2dev;
 +
 +/*
 + * Enable the keyboard to receive keystrokes.
 + */
 +
 +   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
 +   dev_err(ps2dev-serio-dev,
 +   Failed to enable keyboard on %s\n,
 +   ps2dev-serio-phys);
 +   return -1;
 +   }
 +
 +   return 0;
 +}
 +
 +/*
 + * atkbd_deactivate() resets and disables the keyboard from sending
 + * keystrokes.
 + */
 +
 +static void atkbd_deactivate(struct atkbd *atkbd)
 +{
 +   struct ps2dev *ps2dev = atkbd-ps2dev;
 +
 +   if (ps2_command(ps2dev, NULL, ATKBD_CMD_RESET_DIS))
 +   dev_err(ps2dev-serio-dev,
 +   Failed to deactivate keyboard on %s\n,
 +   ps2dev-serio-phys);
 +}
 +
  /*
   * atkbd_probe() probes for an AT keyboard on a serio port.
   */
 @@ -731,6 +764,12 @@ static int atkbd_probe(struct atkbd *atkbd)
 return -1;
 }

 +/*
 + * Make sure nothing is coming from the keyboard and disturbs our
 + * internal state.
 + */
 +   atkbd_deactivate(atkbd);
 +
 return 0;
  }

 @@ -825,24 +864,6 @@ static int atkbd_reset_state(struct atkbd *atkbd)
 return 0;
  }

 -static int atkbd_activate(struct atkbd *atkbd)
 -{
 -   struct ps2dev *ps2dev = atkbd-ps2dev;
 -
 -/*
 - * Enable the keyboard to receive keystrokes.
 - */
 -
 -   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
 -   dev_err(ps2dev-serio-dev,
 -   Failed to enable keyboard on %s\n,
 -   ps2dev-serio-phys);
 -   return -1;
 -   }
 -
 -   return 0;
 -}
 -
  /*
   * 

Re: [PATCH v2] atkbd: Fix multi-char scancode handling on reconnect.

2013-01-06 Thread Dmitry Torokhov
Hi Shawn,

On Thu, Dec 20, 2012 at 06:33:11PM -0800, Shawn Nematbakhsh wrote:
> On resume from suspend there is a possibility for multi-byte scancodes
> to be handled incorrectly. atkbd_reconnect disables the processing of
> scancodes in software by calling atkbd_disable, but the keyboard may
> still be active because no disconnect command was sent. Later, software
> handling is re-enabled. If a multi-byte scancode sent from the keyboard
> straddles the re-enable, only the latter byte(s) will be handled.
> 
> In practice, this leads to cases where multi-byte break codes (ex. "e0
> 4d" - break code for right-arrow) are misread as make codes ("4d" - make
> code for numeric 6), leading to one or more unwanted, untyped characters
> being interpreted.
> 
> The solution implemented here involves sending command f5 (reset
> disable) to the keyboard prior to disabling software handling of codes.
> Later, the command to re-enable the keyboard is sent only after we are
> prepared to handle scancodes.

The core tries to avoid disturbing devices that are not keyboards, so I
believe we should check the device ID first and if it is keyboard, then
do the reset. We should also reset the internal state (emul and xl_bit)
when re-enabling the device.

Does the version of the patch below work for you?

Thanks.

-- 
Dmitry

Input: atkbd - fix multi-byte scancode handling on reconnect

From: Shawn Nematbakhsh 

On resume from suspend there is a possibility for multi-byte scancodes
to be handled incorrectly. atkbd_reconnect disables the processing of
scancodes in software by calling atkbd_disable, but the keyboard may
still be active because no disconnect command was sent. Later, software
handling is re-enabled. If a multi-byte scancode sent from the keyboard
straddles the re-enable, only the latter byte(s) will be handled.

In practice, this leads to cases where multi-byte break codes (ex. "e0
4d" - break code for right-arrow) are misread as make codes ("4d" - make
code for numeric 6), leading to one or more unwanted, untyped characters
being interpreted.

The solution implemented here involves sending command f5 (reset
disable) to the keyboard prior to disabling software handling of codes.
Later, the command to re-enable the keyboard is sent only after we are
prepared to handle scancodes.

Signed-off-by: Shawn Nematbakhsh 
Signed-off-by: Dmitry Torokhov 
---
 drivers/input/keyboard/atkbd.c |   72 
 1 file changed, 51 insertions(+), 21 deletions(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 33d0fcd..2626773 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -676,6 +676,39 @@ static inline void atkbd_disable(struct atkbd *atkbd)
serio_continue_rx(atkbd->ps2dev.serio);
 }
 
+static int atkbd_activate(struct atkbd *atkbd)
+{
+   struct ps2dev *ps2dev = >ps2dev;
+
+/*
+ * Enable the keyboard to receive keystrokes.
+ */
+
+   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
+   dev_err(>serio->dev,
+   "Failed to enable keyboard on %s\n",
+   ps2dev->serio->phys);
+   return -1;
+   }
+
+   return 0;
+}
+
+/*
+ * atkbd_deactivate() resets and disables the keyboard from sending
+ * keystrokes.
+ */
+
+static void atkbd_deactivate(struct atkbd *atkbd)
+{
+   struct ps2dev *ps2dev = >ps2dev;
+
+   if (ps2_command(ps2dev, NULL, ATKBD_CMD_RESET_DIS))
+   dev_err(>serio->dev,
+   "Failed to deactivate keyboard on %s\n",
+   ps2dev->serio->phys);
+}
+
 /*
  * atkbd_probe() probes for an AT keyboard on a serio port.
  */
@@ -731,6 +764,12 @@ static int atkbd_probe(struct atkbd *atkbd)
return -1;
}
 
+/*
+ * Make sure nothing is coming from the keyboard and disturbs our
+ * internal state.
+ */
+   atkbd_deactivate(atkbd);
+
return 0;
 }
 
@@ -825,24 +864,6 @@ static int atkbd_reset_state(struct atkbd *atkbd)
return 0;
 }
 
-static int atkbd_activate(struct atkbd *atkbd)
-{
-   struct ps2dev *ps2dev = >ps2dev;
-
-/*
- * Enable the keyboard to receive keystrokes.
- */
-
-   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
-   dev_err(>serio->dev,
-   "Failed to enable keyboard on %s\n",
-   ps2dev->serio->phys);
-   return -1;
-   }
-
-   return 0;
-}
-
 /*
  * atkbd_cleanup() restores the keyboard state so that BIOS is happy after a
  * reboot.
@@ -1150,7 +1171,6 @@ static int atkbd_connect(struct serio *serio, struct 
serio_driver *drv)
 
atkbd->set = atkbd_select_set(atkbd, atkbd_set, atkbd_extra);
atkbd_reset_state(atkbd);
-   atkbd_activate(atkbd);
 
} else {
atkbd->set = 2;
@@ -1165,6 +1185,8 @@ static int atkbd_connect(struct serio *serio, struct 
serio_driver *drv)

Re: [PATCH v2] atkbd: Fix multi-char scancode handling on reconnect.

2013-01-06 Thread Dmitry Torokhov
Hi Shawn,

On Thu, Dec 20, 2012 at 06:33:11PM -0800, Shawn Nematbakhsh wrote:
 On resume from suspend there is a possibility for multi-byte scancodes
 to be handled incorrectly. atkbd_reconnect disables the processing of
 scancodes in software by calling atkbd_disable, but the keyboard may
 still be active because no disconnect command was sent. Later, software
 handling is re-enabled. If a multi-byte scancode sent from the keyboard
 straddles the re-enable, only the latter byte(s) will be handled.
 
 In practice, this leads to cases where multi-byte break codes (ex. e0
 4d - break code for right-arrow) are misread as make codes (4d - make
 code for numeric 6), leading to one or more unwanted, untyped characters
 being interpreted.
 
 The solution implemented here involves sending command f5 (reset
 disable) to the keyboard prior to disabling software handling of codes.
 Later, the command to re-enable the keyboard is sent only after we are
 prepared to handle scancodes.

The core tries to avoid disturbing devices that are not keyboards, so I
believe we should check the device ID first and if it is keyboard, then
do the reset. We should also reset the internal state (emul and xl_bit)
when re-enabling the device.

Does the version of the patch below work for you?

Thanks.

-- 
Dmitry

Input: atkbd - fix multi-byte scancode handling on reconnect

From: Shawn Nematbakhsh sha...@chromium.org

On resume from suspend there is a possibility for multi-byte scancodes
to be handled incorrectly. atkbd_reconnect disables the processing of
scancodes in software by calling atkbd_disable, but the keyboard may
still be active because no disconnect command was sent. Later, software
handling is re-enabled. If a multi-byte scancode sent from the keyboard
straddles the re-enable, only the latter byte(s) will be handled.

In practice, this leads to cases where multi-byte break codes (ex. e0
4d - break code for right-arrow) are misread as make codes (4d - make
code for numeric 6), leading to one or more unwanted, untyped characters
being interpreted.

The solution implemented here involves sending command f5 (reset
disable) to the keyboard prior to disabling software handling of codes.
Later, the command to re-enable the keyboard is sent only after we are
prepared to handle scancodes.

Signed-off-by: Shawn Nematbakhsh sha...@chromium.org
Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com
---
 drivers/input/keyboard/atkbd.c |   72 
 1 file changed, 51 insertions(+), 21 deletions(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 33d0fcd..2626773 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -676,6 +676,39 @@ static inline void atkbd_disable(struct atkbd *atkbd)
serio_continue_rx(atkbd-ps2dev.serio);
 }
 
+static int atkbd_activate(struct atkbd *atkbd)
+{
+   struct ps2dev *ps2dev = atkbd-ps2dev;
+
+/*
+ * Enable the keyboard to receive keystrokes.
+ */
+
+   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
+   dev_err(ps2dev-serio-dev,
+   Failed to enable keyboard on %s\n,
+   ps2dev-serio-phys);
+   return -1;
+   }
+
+   return 0;
+}
+
+/*
+ * atkbd_deactivate() resets and disables the keyboard from sending
+ * keystrokes.
+ */
+
+static void atkbd_deactivate(struct atkbd *atkbd)
+{
+   struct ps2dev *ps2dev = atkbd-ps2dev;
+
+   if (ps2_command(ps2dev, NULL, ATKBD_CMD_RESET_DIS))
+   dev_err(ps2dev-serio-dev,
+   Failed to deactivate keyboard on %s\n,
+   ps2dev-serio-phys);
+}
+
 /*
  * atkbd_probe() probes for an AT keyboard on a serio port.
  */
@@ -731,6 +764,12 @@ static int atkbd_probe(struct atkbd *atkbd)
return -1;
}
 
+/*
+ * Make sure nothing is coming from the keyboard and disturbs our
+ * internal state.
+ */
+   atkbd_deactivate(atkbd);
+
return 0;
 }
 
@@ -825,24 +864,6 @@ static int atkbd_reset_state(struct atkbd *atkbd)
return 0;
 }
 
-static int atkbd_activate(struct atkbd *atkbd)
-{
-   struct ps2dev *ps2dev = atkbd-ps2dev;
-
-/*
- * Enable the keyboard to receive keystrokes.
- */
-
-   if (ps2_command(ps2dev, NULL, ATKBD_CMD_ENABLE)) {
-   dev_err(ps2dev-serio-dev,
-   Failed to enable keyboard on %s\n,
-   ps2dev-serio-phys);
-   return -1;
-   }
-
-   return 0;
-}
-
 /*
  * atkbd_cleanup() restores the keyboard state so that BIOS is happy after a
  * reboot.
@@ -1150,7 +1171,6 @@ static int atkbd_connect(struct serio *serio, struct 
serio_driver *drv)
 
atkbd-set = atkbd_select_set(atkbd, atkbd_set, atkbd_extra);
atkbd_reset_state(atkbd);
-   atkbd_activate(atkbd);
 
} else {
atkbd-set = 2;
@@ -1165,6 +1185,8 @@ static int atkbd_connect(struct serio 

[PATCH v2] atkbd: Fix multi-char scancode handling on reconnect.

2012-12-20 Thread Shawn Nematbakhsh
On resume from suspend there is a possibility for multi-byte scancodes
to be handled incorrectly. atkbd_reconnect disables the processing of
scancodes in software by calling atkbd_disable, but the keyboard may
still be active because no disconnect command was sent. Later, software
handling is re-enabled. If a multi-byte scancode sent from the keyboard
straddles the re-enable, only the latter byte(s) will be handled.

In practice, this leads to cases where multi-byte break codes (ex. "e0
4d" - break code for right-arrow) are misread as make codes ("4d" - make
code for numeric 6), leading to one or more unwanted, untyped characters
being interpreted.

The solution implemented here involves sending command f5 (reset
disable) to the keyboard prior to disabling software handling of codes.
Later, the command to re-enable the keyboard is sent only after we are
prepared to handle scancodes.

Signed-off-by: Shawn Nematbakhsh 
---
Changes since v1:
- Removed unused return values from atkbd_deactivate.

 drivers/input/keyboard/atkbd.c |   22 --
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index add5ffd..44dcf83 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -844,6 +844,20 @@ static int atkbd_activate(struct atkbd *atkbd)
 }
 
 /*
+ * atkbd_deactivate() resets and disables the keyboard from sending
+ * keystrokes.
+ */
+static void atkbd_deactivate(struct atkbd *atkbd)
+{
+   struct ps2dev *ps2dev = >ps2dev;
+
+   if (ps2_command(ps2dev, NULL, ATKBD_CMD_RESET_DIS))
+   dev_err(>serio->dev,
+   "Failed to deactivate keyboard on %s\n",
+   ps2dev->serio->phys);
+}
+
+/*
  * atkbd_cleanup() restores the keyboard state so that BIOS is happy after a
  * reboot.
  */
@@ -1199,6 +1213,9 @@ static int atkbd_reconnect(struct serio *serio)
 
mutex_lock(>mutex);
 
+   if (atkbd->write)
+   atkbd_deactivate(atkbd);
+
atkbd_disable(atkbd);
 
if (atkbd->write) {
@@ -1208,8 +1225,6 @@ static int atkbd_reconnect(struct serio *serio)
if (atkbd->set != atkbd_select_set(atkbd, atkbd->set, 
atkbd->extra))
goto out;
 
-   atkbd_activate(atkbd);
-
/*
 * Restore LED state and repeat rate. While input core
 * will do this for us at resume time reconnect may happen
@@ -1224,6 +1239,9 @@ static int atkbd_reconnect(struct serio *serio)
}
 
atkbd_enable(atkbd);
+   if (atkbd->write)
+   atkbd_activate(atkbd);
+
retval = 0;
 
  out:
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v2] atkbd: Fix multi-char scancode handling on reconnect.

2012-12-20 Thread Shawn Nematbakhsh
On resume from suspend there is a possibility for multi-byte scancodes
to be handled incorrectly. atkbd_reconnect disables the processing of
scancodes in software by calling atkbd_disable, but the keyboard may
still be active because no disconnect command was sent. Later, software
handling is re-enabled. If a multi-byte scancode sent from the keyboard
straddles the re-enable, only the latter byte(s) will be handled.

In practice, this leads to cases where multi-byte break codes (ex. e0
4d - break code for right-arrow) are misread as make codes (4d - make
code for numeric 6), leading to one or more unwanted, untyped characters
being interpreted.

The solution implemented here involves sending command f5 (reset
disable) to the keyboard prior to disabling software handling of codes.
Later, the command to re-enable the keyboard is sent only after we are
prepared to handle scancodes.

Signed-off-by: Shawn Nematbakhsh sha...@chromium.org
---
Changes since v1:
- Removed unused return values from atkbd_deactivate.

 drivers/input/keyboard/atkbd.c |   22 --
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index add5ffd..44dcf83 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -844,6 +844,20 @@ static int atkbd_activate(struct atkbd *atkbd)
 }
 
 /*
+ * atkbd_deactivate() resets and disables the keyboard from sending
+ * keystrokes.
+ */
+static void atkbd_deactivate(struct atkbd *atkbd)
+{
+   struct ps2dev *ps2dev = atkbd-ps2dev;
+
+   if (ps2_command(ps2dev, NULL, ATKBD_CMD_RESET_DIS))
+   dev_err(ps2dev-serio-dev,
+   Failed to deactivate keyboard on %s\n,
+   ps2dev-serio-phys);
+}
+
+/*
  * atkbd_cleanup() restores the keyboard state so that BIOS is happy after a
  * reboot.
  */
@@ -1199,6 +1213,9 @@ static int atkbd_reconnect(struct serio *serio)
 
mutex_lock(atkbd-mutex);
 
+   if (atkbd-write)
+   atkbd_deactivate(atkbd);
+
atkbd_disable(atkbd);
 
if (atkbd-write) {
@@ -1208,8 +1225,6 @@ static int atkbd_reconnect(struct serio *serio)
if (atkbd-set != atkbd_select_set(atkbd, atkbd-set, 
atkbd-extra))
goto out;
 
-   atkbd_activate(atkbd);
-
/*
 * Restore LED state and repeat rate. While input core
 * will do this for us at resume time reconnect may happen
@@ -1224,6 +1239,9 @@ static int atkbd_reconnect(struct serio *serio)
}
 
atkbd_enable(atkbd);
+   if (atkbd-write)
+   atkbd_activate(atkbd);
+
retval = 0;
 
  out:
-- 
1.7.7.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/