[Qemu-devel] [4476] musicpal: Improve button handling (Jan Kiszka).

[Qemu-devel] [4476] musicpal: Improve button handling (Jan Kiszka).

[Andrzej Zaborowski]

Revision: 4476
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=4476
Author:   balrog
Date:     2008-05-17 18:18:04 +0000 (Sat, 17 May 2008)

Log Message:
-----------
musicpal: Improve button handling (Jan Kiszka).

Looking at the hold-button-on-powerup thing, I came across some
improvable parts in the MusicPal's button handling. This patch allows
for repeated wheel events by keying the arrow keys pressed, corrects
some constant name, and introduces an explicitly maintained GPIO_ISR
state.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>

Modified Paths:
--------------
    trunk/hw/musicpal.c

Modified: trunk/hw/musicpal.c
===================================================================
--- trunk/hw/musicpal.c	2008-05-17 18:15:04 UTC (rev 4475)
+++ trunk/hw/musicpal.c	2008-05-17 18:18:04 UTC (rev 4476)
@@ -59,6 +59,7 @@
 #define MP_AUDIO_IRQ            30
 
 static uint32_t gpio_in_state = 0xffffffff;
+static uint32_t gpio_isr;
 static uint32_t gpio_out_state;
 static ram_addr_t sram_off;
 
@@ -1280,11 +1281,10 @@
                         (i2c_get_data(mixer_i2c) << MP_GPIO_I2C_DATA_BIT);
         return gpio_in_state >> 16;
 
-    /* This is a simplification of reality */
     case MP_GPIO_ISR_LO:
-        return ~gpio_in_state & 0xFFFF;
+        return gpio_isr & 0xFFFF;
     case MP_GPIO_ISR_HI:
-        return ~gpio_in_state >> 16;
+        return gpio_isr >> 16;
 
     /* Workaround to allow loading the binary-only wlandrv.ko crap
      * from the original Freecom firmware. */
@@ -1324,7 +1324,7 @@
 }
 
 /* Keyboard codes & masks */
-#define KEY_PRESSED             0x80
+#define KEY_RELEASED            0x80
 #define KEY_CODE                0x7f
 
 #define KEYCODE_TAB             0x0f
@@ -1367,7 +1367,7 @@
             event = MP_GPIO_WHEEL_VOL;
             break;
         }
-    else
+    else {
         switch (keycode & KEY_CODE) {
         case KEYCODE_F:
             event = MP_GPIO_BTN_FAVORITS;
@@ -1385,12 +1385,19 @@
             event = MP_GPIO_BTN_MENU;
             break;
         }
+        /* Do not repeat already pressed buttons */
+        if (!(keycode & KEY_RELEASED) && !(gpio_in_state & event))
+            event = 0;
+    }
 
-    if (keycode & KEY_PRESSED)
-        gpio_in_state |= event;
-    else if (gpio_in_state & event) {
-        gpio_in_state &= ~event;
-        qemu_irq_raise(irq);
+    if (event) {
+        if (keycode & KEY_RELEASED) {
+            gpio_in_state |= event;
+        } else {
+            gpio_in_state &= ~event;
+            gpio_isr = event;
+            qemu_irq_raise(irq);
+        }
     }
 
     kbd_extended = 0;

[Qemu-devel] Re: [4476] musicpal: Improve button handling (Jan Kiszka).

[consul]

Target arm-softmmu still does not compile on Windows. The fix is trivial, 
please apply it (or my previous patch, if you wish).

Alex.

Index: hw/musicpal.c
===================================================================
--- hw/musicpal.c       (revision 4504)
+++ hw/musicpal.c       (working copy)
@@ -1495,7 +1495,11 @@
      * Wait a bit to catch menu button during U-Boot start-up
      * (to trigger emergency update).
      */
+#ifdef _WIN32
+    Sleep(1000);
+#else
     sleep(1);
+#endif

Re: [Qemu-devel] Re: [4476] musicpal: Improve button handling (Jan Kiszka).

[Ian Jackson]

consul writes ("[Qemu-devel] Re: [4476] musicpal: Improve button handling (Jan Kiszka)."):
> Target arm-softmmu still does not compile on Windows. The fix is trivial, 
> please apply it (or my previous patch, if you wish).

> +#ifdef _WIN32
> +    Sleep(1000);
> +#else
>      sleep(1);
> +#endif

Surely this should be in osdep.h as a conditional definition of sleep.

Ian.

Re: [Qemu-devel] Re: [4476] musicpal: Improve button handling (Jan Kiszka).

[Paul Brook]

> > Target arm-softmmu still does not compile on Windows. The fix is trivial,
> > please apply it (or my previous patch, if you wish).
> >
> > +#ifdef _WIN32
> > +    Sleep(1000);
> > +#else
> >      sleep(1);
> > +#endif
>
> Surely this should be in osdep.h as a conditional definition of sleep.

The real bug is that we're using sleep() in the first place :-)

Paul

[Qemu-devel] Re: Re: [4476] musicpal: Improve button handling (JanKiszka).

[consul]

"Paul Brook" <paul@codesourcery.com> wrote in message 
news:200805211507.05887.paul@codesourcery.com...

>> Surely this should be in osdep.h as a conditional definition of sleep.
>
> The real bug is that we're using sleep() in the first place :-)
>

IMHO committed code should at least compile on all supported platforms. Even 
a temporary fix is better than no fix at all. Using the sleep is certainly 
not the best solution and Jan already proposed another patch without it, see 
[RFC][PATCH 2/2] MusicPal: Add specific -hold-buttonoption, without the 
sleep function, but as of today, the latest qemu svn (4511) does not compile 
on Windows, and that's a real bug.

Alex.