[PATCH 0/2] input: atakbd.c bug fixes

[PATCH 0/2] input: atakbd.c bug fixes

[Michael Schmitz]

Two bug fixes for the Atari keyboard input driver:

- keymap fixes: multiple keymap errors that had gone unnoticed
  since migration of the m68k keyboard driver code to the
  input framework. Correct the wrong keycodes - keymap is stil
  US but keypad, help and undo keys all generate the correct keycodes now.

- capslock fix: the Atari driver incorrectly assumed that capslock
  were a toggle key (i.e. sending no break scancodes), and synthesized
  a make/break keycode pair on each capslock scancode. This resulted in
  duplicate make/break codes on key release, and capslock functioning as
  a shift key. Remove code sending make/break pairs to get functional
  capslock behaviour.

Tested on my own Atari Falcon 030.

TIA for your review. Please CC linux-m68k in replies.

Cheers,

	Michael

CC to debian-68k, for consideration at the upcoming m68k hackers meeting.

[PATCH 1/2] input: atakbd.c - fix Atari keymap

[Michael Schmitz]

From: Andreas Schwab <schwab@linux-m68k.org>

Fix errors in Atari keymap (mostly in keypad, help and undo keys).

Patch provided on debian-68k ML by Andreas Schwab <schwab@linux-m68k.org>,
keymap array size and unhandled scancode limit adjusted to 0x73 by me.

Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>

---
 drivers/input/keyboard/atakbd.c |   64 ++++++++++++++++-----------------------
 1 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/drivers/input/keyboard/atakbd.c b/drivers/input/keyboard/atakbd.c
index 6f62da2..e989574 100644
--- a/drivers/input/keyboard/atakbd.c
+++ b/drivers/input/keyboard/atakbd.c
@@ -75,8 +75,7 @@
  */
 
 
-static unsigned char atakbd_keycode[0x72] = {	/* American layout */
-	[0]	 = KEY_GRAVE,
+static unsigned char atakbd_keycode[0x73] = {	/* American layout */
 	[1]	 = KEY_ESC,
 	[2]	 = KEY_1,
 	[3]	 = KEY_2,
@@ -117,9 +116,9 @@
 	[38]	 = KEY_L,
 	[39]	 = KEY_SEMICOLON,
 	[40]	 = KEY_APOSTROPHE,
-	[41]	 = KEY_BACKSLASH,	/* FIXME, '#' */
+	[41]	 = KEY_GRAVE,
 	[42]	 = KEY_LEFTSHIFT,
-	[43]	 = KEY_GRAVE,		/* FIXME: '~' */
+	[43]	 = KEY_BACKSLASH,
 	[44]	 = KEY_Z,
 	[45]	 = KEY_X,
 	[46]	 = KEY_C,
@@ -145,45 +144,34 @@
 	[66]	 = KEY_F8,
 	[67]	 = KEY_F9,
 	[68]	 = KEY_F10,
-	[69]	 = KEY_ESC,
-	[70]	 = KEY_DELETE,
-	[71]	 = KEY_KP7,
-	[72]	 = KEY_KP8,
-	[73]	 = KEY_KP9,
+	[71]	 = KEY_HOME,
+	[72]	 = KEY_UP,
 	[74]	 = KEY_KPMINUS,
-	[75]	 = KEY_KP4,
-	[76]	 = KEY_KP5,
-	[77]	 = KEY_KP6,
+	[75]	 = KEY_LEFT,
+	[77]	 = KEY_RIGHT,
 	[78]	 = KEY_KPPLUS,
-	[79]	 = KEY_KP1,
-	[80]	 = KEY_KP2,
-	[81]	 = KEY_KP3,
-	[82]	 = KEY_KP0,
-	[83]	 = KEY_KPDOT,
-	[90]	 = KEY_KPLEFTPAREN,
-	[91]	 = KEY_KPRIGHTPAREN,
-	[92]	 = KEY_KPASTERISK,	/* FIXME */
-	[93]	 = KEY_KPASTERISK,
-	[94]	 = KEY_KPPLUS,
-	[95]	 = KEY_HELP,
+	[80]	 = KEY_DOWN,
+	[82]	 = KEY_INSERT,
+	[83]	 = KEY_DELETE,
 	[96]	 = KEY_102ND,
-	[97]	 = KEY_KPASTERISK,	/* FIXME */
-	[98]	 = KEY_KPSLASH,
+	[97]	 = KEY_UNDO,
+	[98]	 = KEY_HELP,
 	[99]	 = KEY_KPLEFTPAREN,
 	[100]	 = KEY_KPRIGHTPAREN,
 	[101]	 = KEY_KPSLASH,
 	[102]	 = KEY_KPASTERISK,
-	[103]	 = KEY_UP,
-	[104]	 = KEY_KPASTERISK,	/* FIXME */
-	[105]	 = KEY_LEFT,
-	[106]	 = KEY_RIGHT,
-	[107]	 = KEY_KPASTERISK,	/* FIXME */
-	[108]	 = KEY_DOWN,
-	[109]	 = KEY_KPASTERISK,	/* FIXME */
-	[110]	 = KEY_KPASTERISK,	/* FIXME */
-	[111]	 = KEY_KPASTERISK,	/* FIXME */
-	[112]	 = KEY_KPASTERISK,	/* FIXME */
-	[113]	 = KEY_KPASTERISK	/* FIXME */
+	[103]	 = KEY_KP7,
+	[104]	 = KEY_KP8,
+	[105]	 = KEY_KP9,
+	[106]	 = KEY_KP4,
+	[107]	 = KEY_KP5,
+	[108]	 = KEY_KP6,
+	[109]	 = KEY_KP1,
+	[110]	 = KEY_KP2,
+	[111]	 = KEY_KP3,
+	[112]	 = KEY_KP0,
+	[113]	 = KEY_KPDOT,
+	[114]	 = KEY_KPENTER,
 };
 
 static struct input_dev *atakbd_dev;
@@ -191,7 +179,7 @@
 static void atakbd_interrupt(unsigned char scancode, char down)
 {
 
-	if (scancode < 0x72) {		/* scancodes < 0xf2 are keys */
+	if (scancode < 0x73) {		/* scancodes < 0xf3 are keys */
 
 		// report raw events here?
 
@@ -205,7 +193,7 @@ static void atakbd_interrupt(unsigned char scancode, char down)
 			input_report_key(atakbd_dev, scancode, down);
 			input_sync(atakbd_dev);
 		}
-	} else				/* scancodes >= 0xf2 are mouse data, most likely */
+	} else				/* scancodes >= 0xf3 are mouse data, most likely */
 		printk(KERN_INFO "atakbd: unhandled scancode %x\n", scancode);
 
 	return;

[PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour

[Michael Schmitz]

The CapsLock key on Atari keyboards is not a toggle, it does send the
normal make and break scancodes.

Drop the CapsLock toggle handling code, which did cause the CapsLock
key to merely act as a Shift key.

Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>

---
 drivers/input/keyboard/atakbd.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/atakbd.c b/drivers/input/keyboard/atakbd.c
index e989574..6caee80 100644
--- a/drivers/input/keyboard/atakbd.c
+++ b/drivers/input/keyboard/atakbd.c
@@ -185,14 +185,8 @@ static void atakbd_interrupt(unsigned char scancode, char down)
 
 		scancode = atakbd_keycode[scancode];
 
-		if (scancode == KEY_CAPSLOCK) {	/* CapsLock is a toggle switch key on Amiga */
-			input_report_key(atakbd_dev, scancode, 1);
-			input_report_key(atakbd_dev, scancode, 0);
-			input_sync(atakbd_dev);
-		} else {
-			input_report_key(atakbd_dev, scancode, down);
-			input_sync(atakbd_dev);
-		}
+		input_report_key(atakbd_dev, scancode, down);
+		input_sync(atakbd_dev);
 	} else				/* scancodes >= 0xf3 are mouse data, most likely */
 		printk(KERN_INFO "atakbd: unhandled scancode %x\n", scancode);
 

Re: [PATCH 1/2] input: atakbd.c - fix Atari keymap

[Finn Thain]

On Fri, 7 Sep 2018, Michael Schmitz wrote:

> From: Andreas Schwab <schwab@linux-m68k.org>
> 

Andreas, because the authorship credit is yours, this patch will also need 
a 'Signed-off-by' tag from you before it can be merged.

Re: [PATCH 1/2] input: atakbd.c - fix Atari keymap

[Andreas Schwab]

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>

Andreas.

Re: [PATCH 1/2] input: atakbd.c - fix Atari keymap

[Michael Schmitz]

Thanks Andreas!

Cheers,

	Michael

Am 07.09.2018 um 21:22 schrieb Andreas Schwab:
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
>
> Andreas.
>

Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour

[Dmitry Torokhov]

On Fri, Sep 07, 2018 at 11:40:43AM +1200, Michael Schmitz wrote:
> The CapsLock key on Atari keyboards is not a toggle, it does send the
> normal make and break scancodes.
> 
> Drop the CapsLock toggle handling code, which did cause the CapsLock
> key to merely act as a Shift key.
> 
> Tested-by: Michael Schmitz <schmitzmic@gmail.com>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>

Andreas, I have no idea how Atari keyboards work, can you add your
signed-off-by (or NAK) to this one as well please?

> 
> ---
>  drivers/input/keyboard/atakbd.c |   10 ++--------
>  1 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/keyboard/atakbd.c b/drivers/input/keyboard/atakbd.c
> index e989574..6caee80 100644
> --- a/drivers/input/keyboard/atakbd.c
> +++ b/drivers/input/keyboard/atakbd.c
> @@ -185,14 +185,8 @@ static void atakbd_interrupt(unsigned char scancode, char down)
>  
>  		scancode = atakbd_keycode[scancode];
>  
> -		if (scancode == KEY_CAPSLOCK) {	/* CapsLock is a toggle switch key on Amiga */
> -			input_report_key(atakbd_dev, scancode, 1);
> -			input_report_key(atakbd_dev, scancode, 0);
> -			input_sync(atakbd_dev);
> -		} else {
> -			input_report_key(atakbd_dev, scancode, down);
> -			input_sync(atakbd_dev);
> -		}
> +		input_report_key(atakbd_dev, scancode, down);
> +		input_sync(atakbd_dev);
>  	} else				/* scancodes >= 0xf3 are mouse data, most likely */
>  		printk(KERN_INFO "atakbd: unhandled scancode %x\n", scancode);
>  
> -- 
> 1.7.0.4
> 

Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour

[Andreas Schwab]

On Sep 17 2018, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> On Fri, Sep 07, 2018 at 11:40:43AM +1200, Michael Schmitz wrote:
>> The CapsLock key on Atari keyboards is not a toggle, it does send the
>> normal make and break scancodes.
>> 
>> Drop the CapsLock toggle handling code, which did cause the CapsLock
>> key to merely act as a Shift key.
>> 
>> Tested-by: Michael Schmitz <schmitzmic@gmail.com>
>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>
> Andreas, I have no idea how Atari keyboards work, can you add your
> signed-off-by (or NAK) to this one as well please?

The old atakbd driver never had that special handling.

Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>

Andreas.

Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour

[Dmitry Torokhov]

On Mon, Sep 17, 2018 at 10:29:08PM +0200, Andreas Schwab wrote:
> On Sep 17 2018, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > On Fri, Sep 07, 2018 at 11:40:43AM +1200, Michael Schmitz wrote:
> >> The CapsLock key on Atari keyboards is not a toggle, it does send the
> >> normal make and break scancodes.
> >> 
> >> Drop the CapsLock toggle handling code, which did cause the CapsLock
> >> key to merely act as a Shift key.
> >> 
> >> Tested-by: Michael Schmitz <schmitzmic@gmail.com>
> >> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> >
> > Andreas, I have no idea how Atari keyboards work, can you add your
> > signed-off-by (or NAK) to this one as well please?
> 
> The old atakbd driver never had that special handling.
> 
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>

Applied, thank you.

Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour

[Michael Schmitz]

Thanks Dmitry!

I had asked Andreas' signoff for his keymap fixes only - the rewrite of 
the Atari keyboard driver to the current input framework (and hence the 
CapsLock toggle bug) was entirely mine, no blame attaches to Andreas there.

Cheers,

     Michael



On 18/09/18 10:36, Dmitry Torokhov wrote:
> On Mon, Sep 17, 2018 at 10:29:08PM +0200, Andreas Schwab wrote:
>> On Sep 17 2018, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>>
>>> On Fri, Sep 07, 2018 at 11:40:43AM +1200, Michael Schmitz wrote:
>>>> The CapsLock key on Atari keyboards is not a toggle, it does send the
>>>> normal make and break scancodes.
>>>>
>>>> Drop the CapsLock toggle handling code, which did cause the CapsLock
>>>> key to merely act as a Shift key.
>>>>
>>>> Tested-by: Michael Schmitz <schmitzmic@gmail.com>
>>>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>>> Andreas, I have no idea how Atari keyboards work, can you add your
>>> signed-off-by (or NAK) to this one as well please?
>> The old atakbd driver never had that special handling.
>>
>> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
> Applied, thank you.
>

Re: [PATCH 2/2] input: atakbd.c - fix Atari CapsLock behaviour

[Dmitry Torokhov]

Hi Michael,

On Tue, Sep 18, 2018 at 02:03:29PM +1200, Michael Schmitz wrote:
> Thanks Dmitry!
> 
> I had asked Andreas' signoff for his keymap fixes only - the rewrite of the
> Atari keyboard driver to the current input framework (and hence the CapsLock
> toggle bug) was entirely mine, no blame attaches to Andreas there.

It was not about blame, it was about helping me to check the sanity of
the patch given I have no idea about the hardware ;)

Thanks.