[Qemu-devel] [PATCH v2] mainstone: Fix duplicate array values for key 'space'

[Qemu-devel] [PATCH v2] mainstone: Fix duplicate array values for key 'space'

[Stefan Weil]

cgcc reported a duplicate initialisation. Mainstone includes a matrix
keyboard where two different positions map to 'space'.

QEMU uses the reversed mapping and does not map 'space' to two different
matrix positions.

Some other keys are either missing or might be mapped wrongly (cf. Linux
kernel code). Don't fix these until someone can test them with real
hardware, but add TODO comments.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

v2: Address Peter Maydell's suggestions:
    * Use first alternative, so there is no change of behaviour
    * Don't use #if 0 ... #endif

As there won't be a follow up patch to fix the keyboard matrix,
this patch is no longer needed for qemu-stable.

Please note that checkpatch.pl reports a missing blank after
a comma. This was required to match the existing code.

Regards,
Stefan W.

 hw/arm/mainstone.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 9402c84..bfb2b78 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -76,8 +76,17 @@ static struct keymap map[0xE0] = {
     [0xc7] = {5,0}, /* Home */
     [0x2a] = {5,1}, /* shift */
     [0x39] = {5,2}, /* space */
-    [0x39] = {5,3}, /* space */
-    [0x1c] = {5,5}, /*  enter */
+    /*
+     * There are two matrix positions which map to space,
+     * but QEMU can only use one of them for the reverse
+     * mapping, so simply use the first one.
+     */
+    /* [0x39] = {5,3}, space */
+    /*
+     * Matrix position {5,4} and other keys are missing here.
+     * TODO: Compare with Linux code and test real hardware.
+     */
+    [0x1c] = {5,5}, /* enter (TODO: might be wrong) */
     [0xc8] = {6,0}, /* up */
     [0xd0] = {6,1}, /* down */
     [0xcb] = {6,2}, /* left */
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2] mainstone: Fix duplicate array values for key 'space'

[Peter Maydell]

On 22 December 2013 19:01, Stefan Weil <sw@weilnetz.de> wrote:
>
> v2: Address Peter Maydell's suggestions:
>     * Use first alternative, so there is no change of behaviour

No, I said that the compiler picked the last of the two, (5,3)

>      [0x39] = {5,2}, /* space */
> -    [0x39] = {5,3}, /* space */
> -    [0x1c] = {5,5}, /*  enter */
> +    /*
> +     * There are two matrix positions which map to space,
> +     * but QEMU can only use one of them for the reverse
> +     * mapping, so simply use the first one.
> +     */
> +    /* [0x39] = {5,3}, space */

So this is still wrong.

> +    /*
> +     * Matrix position {5,4} and other keys are missing here.
> +     * TODO: Compare with Linux code and test real hardware.
> +     */
> +    [0x1c] = {5,5}, /* enter (TODO: might be wrong) */
>      [0xc8] = {6,0}, /* up */
>      [0xd0] = {6,1}, /* down */
>      [0xcb] = {6,2}, /* left */

thanks
-- PMM