[Qemu-devel] [PATCH v2 0/4] usb/hid: bugfixes, more on usb and hid split

[Qemu-devel] [PATCH v2 0/4] usb/hid: bugfixes, more on usb and hid split

[Michael Walle]

This USB patchset moves the HID VM state stuff from usb-hid.c to hid.c, so
it can be reused by other devices.

changes v1->v2:
  - don't reorder vmstate field to be backward compatible

Michael Walle (4):
  hid: register kbd hander in init()
  hid: introduce hid vmstate macros
  usb-hid: use hid vmstate macro
  milkymist-softusb: use hid code directly

 hw/hid.c               |   63 ++++++++++++++++++++++++-
 hw/hw.h                |   20 ++++++++
 hw/milkymist-softusb.c |  122 +++++++++++++++---------------------------------
 hw/usb-hid.c           |   41 +---------------
 4 files changed, 121 insertions(+), 125 deletions(-)

-- 
1.7.2.5

[Qemu-devel] [PATCH 1/4] hid: register kbd hander in init()

[Michael Walle]

Register the keyboard event handler in hid's init() instead of its reset()
function.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 hw/hid.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/hid.c b/hw/hid.c
index 7b5ef5f..6934f05 100644
--- a/hw/hid.c
+++ b/hw/hid.c
@@ -359,7 +359,6 @@ void hid_reset(HIDState *hs)
 {
     switch (hs->kind) {
     case HID_KEYBOARD:
-        qemu_add_kbd_event_handler(hid_keyboard_event, hs);
         memset(hs->kbd.keycodes, 0, sizeof(hs->kbd.keycodes));
         memset(hs->kbd.key, 0, sizeof(hs->kbd.key));
         hs->kbd.keys = 0;
@@ -393,7 +392,9 @@ void hid_init(HIDState *hs, int kind, HIDEventFunc event)
     hs->kind = kind;
     hs->event = event;
 
-    if (hs->kind == HID_MOUSE) {
+    if (hs->kind == HID_KEYBOARD) {
+        qemu_add_kbd_event_handler(hid_keyboard_event, hs);
+    } else if (hs->kind == HID_MOUSE) {
         hs->ptr.eh_entry = qemu_add_mouse_event_handler(hid_pointer_event, hs,
                                                         0, "QEMU HID Mouse");
     } else if (hs->kind == HID_TABLET) {
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH 1/4] hid: register kbd hander in init()

[Peter Maydell]

On 9 August 2011 22:54, Michael Walle <michael@walle.cc> wrote:
> Register the keyboard event handler in hid's init() instead of its reset()
> function.
>
> Signed-off-by: Michael Walle <michael@walle.cc>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

There's actually an equivalent patch to this lurking in the
qemu-linaro patch stack (inherited from the meego tree).

If we fix qemu_add_kbd_event_handler() so that it actually
lets you register more than one keyboard at once (as you
can with mice) then this is definitely required, otherwise
the USB keyboard reregisters itself every time there's a
USB reset, and you get key duplication.
(There's a meego patch to add the multiple-keyboard support:
http://git.linaro.org/gitweb?p=qemu/qemu-linaro.git;a=commitdiff;h=41fc7075cfd74524acc2db73a48534e6bfbea8c4
but that's still in my "to clean up" list so it contains some
changes which should be in a separate patch.)

-- PMM

[Qemu-devel] [PATCH 2/4] hid: introduce hid vmstate macros

[Michael Walle]

Add VMSTATE macros to describe a HIDState. Based on usb-hid.c descriptions.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 hw/hid.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/hw.h  |   20 ++++++++++++++++++++
 2 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/hw/hid.c b/hw/hid.c
index 6934f05..4049809 100644
--- a/hw/hid.c
+++ b/hw/hid.c
@@ -402,3 +402,61 @@ void hid_init(HIDState *hs, int kind, HIDEventFunc event)
                                                         1, "QEMU HID Tablet");
     }
 }
+
+static int hid_post_load(void *opaque, int version_id)
+{
+    HIDState *s = opaque;
+
+    if (s->idle) {
+        hid_set_next_idle(s, qemu_get_clock_ns(vm_clock));
+    }
+    return 0;
+}
+
+static const VMStateDescription vmstate_hid_ptr_queue = {
+    .name = "HIDPointerEventQueue",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT32(xdx, HIDPointerEvent),
+        VMSTATE_INT32(ydy, HIDPointerEvent),
+        VMSTATE_INT32(dz, HIDPointerEvent),
+        VMSTATE_INT32(buttons_state, HIDPointerEvent),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+const VMStateDescription vmstate_hid_ptr_device = {
+    .name = "HIDPointerDevice",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = hid_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT_ARRAY(ptr.queue, HIDState, QUEUE_LENGTH, 0,
+                             vmstate_hid_ptr_queue, HIDPointerEvent),
+        VMSTATE_UINT32(head, HIDState),
+        VMSTATE_UINT32(n, HIDState),
+        VMSTATE_INT32(protocol, HIDState),
+        VMSTATE_UINT8(idle, HIDState),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+const VMStateDescription vmstate_hid_keyboard_device = {
+    .name = "HIDKeyboardDevice",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = hid_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(kbd.keycodes, HIDState, QUEUE_LENGTH),
+        VMSTATE_UINT32(head, HIDState),
+        VMSTATE_UINT32(n, HIDState),
+        VMSTATE_UINT16(kbd.modifiers, HIDState),
+        VMSTATE_UINT8(kbd.leds, HIDState),
+        VMSTATE_UINT8_ARRAY(kbd.key, HIDState, 16),
+        VMSTATE_INT32(kbd.keys, HIDState),
+        VMSTATE_INT32(protocol, HIDState),
+        VMSTATE_UINT8(idle, HIDState),
+        VMSTATE_END_OF_LIST(),
+    }
+};
diff --git a/hw/hw.h b/hw/hw.h
index df6ca65..a124da9 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -701,6 +701,26 @@ extern const VMStateDescription vmstate_ptimer;
     .offset     = vmstate_offset_pointer(_state, _field, ptimer_state), \
 }
 
+extern const VMStateDescription vmstate_hid_keyboard_device;
+
+#define VMSTATE_HID_KEYBOARD_DEVICE(_field, _state) {                \
+    .name       = (stringify(_field)),                               \
+    .size       = sizeof(HIDState),                                  \
+    .vmsd       = &vmstate_hid_keyboard_device,                      \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, HIDState),    \
+}
+
+extern const VMStateDescription vmstate_hid_ptr_device;
+
+#define VMSTATE_HID_POINTER_DEVICE(_field, _state) {                 \
+    .name       = (stringify(_field)),                               \
+    .size       = sizeof(HIDState),                                  \
+    .vmsd       = &vmstate_hid_ptr_device,                           \
+    .flags      = VMS_STRUCT,                                        \
+    .offset     = vmstate_offset_value(_state, _field, HIDState),    \
+}
+
 /* _f : field name
    _f_n : num of elements field_name
    _n : num of elements
-- 
1.7.2.5

[Qemu-devel] [PATCH 3/4] usb-hid: use hid vmstate macro

[Michael Walle]

Use new hid vmstate macro. Version stays the same, because there is no
reordering of the fields.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 hw/usb-hid.c |   41 ++---------------------------------------
 1 files changed, 2 insertions(+), 39 deletions(-)

diff --git a/hw/usb-hid.c b/hw/usb-hid.c
index e5d57de..d9e2a6a 100644
--- a/hw/usb-hid.c
+++ b/hw/usb-hid.c
@@ -538,41 +538,13 @@ void usb_hid_datain_cb(USBDevice *dev, void *opaque, void (*datain)(void *))
     s->datain = datain;
 }
 
-static int usb_hid_post_load(void *opaque, int version_id)
-{
-    USBHIDState *s = opaque;
-
-    if (s->hid.idle) {
-        hid_set_next_idle(&s->hid, qemu_get_clock_ns(vm_clock));
-    }
-    return 0;
-}
-
-static const VMStateDescription vmstate_usb_ptr_queue = {
-    .name = "usb-ptr-queue",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .fields = (VMStateField []) {
-        VMSTATE_INT32(xdx, HIDPointerEvent),
-        VMSTATE_INT32(ydy, HIDPointerEvent),
-        VMSTATE_INT32(dz, HIDPointerEvent),
-        VMSTATE_INT32(buttons_state, HIDPointerEvent),
-        VMSTATE_END_OF_LIST()
-    }
-};
 static const VMStateDescription vmstate_usb_ptr = {
     .name = "usb-ptr",
     .version_id = 1,
     .minimum_version_id = 1,
-    .post_load = usb_hid_post_load,
     .fields = (VMStateField []) {
         VMSTATE_USB_DEVICE(dev, USBHIDState),
-        VMSTATE_STRUCT_ARRAY(hid.ptr.queue, USBHIDState, QUEUE_LENGTH, 0,
-                             vmstate_usb_ptr_queue, HIDPointerEvent),
-        VMSTATE_UINT32(hid.head, USBHIDState),
-        VMSTATE_UINT32(hid.n, USBHIDState),
-        VMSTATE_INT32(hid.protocol, USBHIDState),
-        VMSTATE_UINT8(hid.idle, USBHIDState),
+        VMSTATE_HID_POINTER_DEVICE(hid, USBHIDState),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -581,18 +553,9 @@ static const VMStateDescription vmstate_usb_kbd = {
     .name = "usb-kbd",
     .version_id = 1,
     .minimum_version_id = 1,
-    .post_load = usb_hid_post_load,
     .fields = (VMStateField []) {
         VMSTATE_USB_DEVICE(dev, USBHIDState),
-        VMSTATE_UINT32_ARRAY(hid.kbd.keycodes, USBHIDState, QUEUE_LENGTH),
-        VMSTATE_UINT32(hid.head, USBHIDState),
-        VMSTATE_UINT32(hid.n, USBHIDState),
-        VMSTATE_UINT16(hid.kbd.modifiers, USBHIDState),
-        VMSTATE_UINT8(hid.kbd.leds, USBHIDState),
-        VMSTATE_UINT8_ARRAY(hid.kbd.key, USBHIDState, 16),
-        VMSTATE_INT32(hid.kbd.keys, USBHIDState),
-        VMSTATE_INT32(hid.protocol, USBHIDState),
-        VMSTATE_UINT8(hid.idle, USBHIDState),
+        VMSTATE_HID_KEYBOARD_DEVICE(hid, USBHIDState),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
1.7.2.5

[Qemu-devel] [PATCH 4/4] milkymist-softusb: use hid code directly

[Michael Walle]

Remove the dummy USB device and use the HID code directly. Use the HID code
for the mouse support, too.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 hw/milkymist-softusb.c |  122 +++++++++++++++---------------------------------
 1 files changed, 38 insertions(+), 84 deletions(-)

diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c
index 75c85ae..fe4eedb 100644
--- a/hw/milkymist-softusb.c
+++ b/hw/milkymist-softusb.c
@@ -25,7 +25,7 @@
 #include "sysbus.h"
 #include "trace.h"
 #include "console.h"
-#include "usb.h"
+#include "hid.h"
 #include "qemu-error.h"
 
 enum {
@@ -46,9 +46,8 @@ enum {
 
 struct MilkymistSoftUsbState {
     SysBusDevice busdev;
-    USBBus usbbus;
-    USBPort usbport[2];
-    USBDevice *usbdev;
+    HIDState hid_kbd;
+    HIDState hid_mouse;
 
     qemu_irq irq;
 
@@ -62,13 +61,10 @@ struct MilkymistSoftUsbState {
     uint32_t regs[R_MAX];
 
     /* mouse state */
-    int mouse_dx;
-    int mouse_dy;
-    int mouse_dz;
-    uint8_t mouse_buttons_state;
+    uint8_t mouse_hid_buffer[4];
 
     /* keyboard state */
-    uint8_t kbd_usb_buffer[8];
+    uint8_t kbd_hid_buffer[8];
 };
 typedef struct MilkymistSoftUsbState MilkymistSoftUsbState;
 
@@ -177,16 +173,10 @@ static inline void softusb_write_pmem(MilkymistSoftUsbState *s,
 static void softusb_mouse_changed(MilkymistSoftUsbState *s)
 {
     uint8_t m;
-    uint8_t buf[4];
-
-    buf[0] = s->mouse_buttons_state;
-    buf[1] = s->mouse_dx;
-    buf[2] = s->mouse_dy;
-    buf[3] = s->mouse_dz;
 
     softusb_read_dmem(s, COMLOC_MEVT_PRODUCE, &m, 1);
     trace_milkymist_softusb_mevt(m);
-    softusb_write_dmem(s, COMLOC_MEVT_BASE + 4 * m, buf, 4);
+    softusb_write_dmem(s, COMLOC_MEVT_BASE + 4 * m, s->mouse_hid_buffer, 4);
     m = (m + 1) & 0xf;
     softusb_write_dmem(s, COMLOC_MEVT_PRODUCE, &m, 1);
 
@@ -200,7 +190,7 @@ static void softusb_kbd_changed(MilkymistSoftUsbState *s)
 
     softusb_read_dmem(s, COMLOC_KEVT_PRODUCE, &m, 1);
     trace_milkymist_softusb_kevt(m);
-    softusb_write_dmem(s, COMLOC_KEVT_BASE + 8 * m, s->kbd_usb_buffer, 8);
+    softusb_write_dmem(s, COMLOC_KEVT_BASE + 8 * m, s->kbd_hid_buffer, 8);
     m = (m + 1) & 0x7;
     softusb_write_dmem(s, COMLOC_KEVT_PRODUCE, &m, 1);
 
@@ -208,62 +198,42 @@ static void softusb_kbd_changed(MilkymistSoftUsbState *s)
     qemu_irq_pulse(s->irq);
 }
 
-static void softusb_mouse_event(void *opaque,
-       int dx, int dy, int dz, int buttons_state)
+static void softusb_kbd_hid_datain(HIDState *hs)
 {
-    MilkymistSoftUsbState *s = opaque;
+    MilkymistSoftUsbState *s = container_of(hs, MilkymistSoftUsbState, hid_kbd);
+    int len;
 
     /* if device is in reset, do nothing */
     if (s->regs[R_CTRL] & CTRL_RESET) {
         return;
     }
 
-    trace_milkymist_softusb_mouse_event(dx, dy, dz, buttons_state);
+    len = hid_keyboard_poll(hs, s->kbd_hid_buffer, sizeof(s->kbd_hid_buffer));
 
-    s->mouse_dx = dx;
-    s->mouse_dy = dy;
-    s->mouse_dz = dz;
-    s->mouse_buttons_state = buttons_state;
-
-    softusb_mouse_changed(s);
+    if (len == 8) {
+        softusb_kbd_changed(s);
+    }
 }
 
-static void softusb_usbdev_datain(void *opaque)
+static void softusb_mouse_hid_datain(HIDState *hs)
 {
-    MilkymistSoftUsbState *s = opaque;
-
-    USBPacket p;
-
-    usb_packet_init(&p);
-    usb_packet_setup(&p, USB_TOKEN_IN, 0, 1);
-    usb_packet_addbuf(&p, s->kbd_usb_buffer, sizeof(s->kbd_usb_buffer));
-    s->usbdev->info->handle_data(s->usbdev, &p);
-    usb_packet_cleanup(&p);
-
-    softusb_kbd_changed(s);
-}
+    MilkymistSoftUsbState *s =
+            container_of(hs, MilkymistSoftUsbState, hid_mouse);
+    int len;
 
-static void softusb_attach(USBPort *port)
-{
-}
+    /* if device is in reset, do nothing */
+    if (s->regs[R_CTRL] & CTRL_RESET) {
+        return;
+    }
 
-static void softusb_detach(USBPort *port)
-{
-}
+    len = hid_pointer_poll(hs, s->mouse_hid_buffer,
+            sizeof(s->mouse_hid_buffer));
 
-static void softusb_child_detach(USBPort *port, USBDevice *child)
-{
+    if (len == 4) {
+        softusb_mouse_changed(s);
+    }
 }
 
-static USBPortOps softusb_ops = {
-    .attach = softusb_attach,
-    .detach = softusb_detach,
-    .child_detach = softusb_child_detach,
-};
-
-static USBBusOps softusb_bus_ops = {
-};
-
 static void milkymist_softusb_reset(DeviceState *d)
 {
     MilkymistSoftUsbState *s =
@@ -273,11 +243,11 @@ static void milkymist_softusb_reset(DeviceState *d)
     for (i = 0; i < R_MAX; i++) {
         s->regs[i] = 0;
     }
-    s->mouse_dx = 0;
-    s->mouse_dy = 0;
-    s->mouse_dz = 0;
-    s->mouse_buttons_state = 0;
-    memset(s->kbd_usb_buffer, 0, sizeof(s->kbd_usb_buffer));
+    memset(s->kbd_hid_buffer, 0, sizeof(s->kbd_hid_buffer));
+    memset(s->mouse_hid_buffer, 0, sizeof(s->mouse_hid_buffer));
+
+    hid_reset(&s->hid_kbd);
+    hid_reset(&s->hid_mouse);
 
     /* defaults */
     s->regs[R_CTRL] = CTRL_RESET;
@@ -304,23 +274,8 @@ static int milkymist_softusb_init(SysBusDevice *dev)
     cpu_register_physical_memory(s->dmem_base, s->dmem_size,
             dmem_ram | IO_MEM_RAM);
 
-    qemu_add_mouse_event_handler(softusb_mouse_event, s, 0, "Milkymist Mouse");
-
-    /* create our usb bus */
-    usb_bus_new(&s->usbbus, &softusb_bus_ops, NULL);
-
-    /* our two ports */
-    /* FIXME: claim to support full speed devices. qemu mouse and keyboard
-     * report themselves as full speed devices. */
-    usb_register_port(&s->usbbus, &s->usbport[0], NULL, 0, &softusb_ops,
-            USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL);
-    usb_register_port(&s->usbbus, &s->usbport[1], NULL, 1, &softusb_ops,
-            USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL);
-
-    /* and finally create an usb keyboard */
-    s->usbdev = usb_create_simple(&s->usbbus, "usb-kbd");
-    usb_hid_datain_cb(s->usbdev, s, softusb_usbdev_datain);
-    s->usbdev->info->handle_reset(s->usbdev);
+    hid_init(&s->hid_kbd, HID_KEYBOARD, softusb_kbd_hid_datain);
+    hid_init(&s->hid_mouse, HID_MOUSE, softusb_mouse_hid_datain);
 
     return 0;
 }
@@ -332,11 +287,10 @@ static const VMStateDescription vmstate_milkymist_softusb = {
     .minimum_version_id_old = 1,
     .fields      = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, MilkymistSoftUsbState, R_MAX),
-        VMSTATE_INT32(mouse_dx, MilkymistSoftUsbState),
-        VMSTATE_INT32(mouse_dy, MilkymistSoftUsbState),
-        VMSTATE_INT32(mouse_dz, MilkymistSoftUsbState),
-        VMSTATE_UINT8(mouse_buttons_state, MilkymistSoftUsbState),
-        VMSTATE_BUFFER(kbd_usb_buffer, MilkymistSoftUsbState),
+        VMSTATE_HID_KEYBOARD_DEVICE(hid_kbd, MilkymistSoftUsbState),
+        VMSTATE_HID_POINTER_DEVICE(hid_mouse, MilkymistSoftUsbState),
+        VMSTATE_BUFFER(kbd_hid_buffer, MilkymistSoftUsbState),
+        VMSTATE_BUFFER(mouse_hid_buffer, MilkymistSoftUsbState),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH v2 0/4] usb/hid: bugfixes, more on usb and hid split

[Gerd Hoffmann]

On 08/09/11 23:54, Michael Walle wrote:
> This USB patchset moves the HID VM state stuff from usb-hid.c to hid.c, so
> it can be reused by other devices.
>
> changes v1->v2:
>    - don't reorder vmstate field to be backward compatible
>
> Michael Walle (4):
>    hid: register kbd hander in init()
>    hid: introduce hid vmstate macros
>    usb-hid: use hid vmstate macro
>    milkymist-softusb: use hid code directly
>
>   hw/hid.c               |   63 ++++++++++++++++++++++++-
>   hw/hw.h                |   20 ++++++++
>   hw/milkymist-softusb.c |  122 +++++++++++++++---------------------------------
>   hw/usb-hid.c           |   41 +---------------
>   4 files changed, 121 insertions(+), 125 deletions(-)
>

Patches added to the usb patch queue.

thanks,
   Gerd