| Message ID | 20231017122302.1692902-5-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm/stellaris: convert gamepad input device to qdev | expand |
Hi Peter, On 17/10/23 14:23, Peter Maydell wrote: > Currently for each button on the device we have a > StellarisGamepadButton struct which has the irq, keycode and pressed > state for it. When we convert to qdev, the qdev property and GPIO > APIs are going to require that we have separate arrays for the irqs > and keycodes. Convert from array-of-structs to three separate arrays > in preparation. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/input/stellaris_gamepad.c | 43 ++++++++++++------------------------ > 1 file changed, 14 insertions(+), 29 deletions(-) > -static const VMStateDescription vmstate_stellaris_button = { > - .name = "stellaris_button", > - .version_id = 0, > - .minimum_version_id = 0, > - .fields = (VMStateField[]) { > - VMSTATE_UINT8(pressed, StellarisGamepadButton), > - VMSTATE_END_OF_LIST() > - } > -}; > - > static const VMStateDescription vmstate_stellaris_gamepad = { > .name = "stellaris_gamepad", > .version_id = 2, > .minimum_version_id = 2, > .fields = (VMStateField[]) { > VMSTATE_INT32(extension, StellarisGamepad), > - VMSTATE_STRUCT_VARRAY_POINTER_INT32(buttons, StellarisGamepad, > - num_buttons, > - vmstate_stellaris_button, > - StellarisGamepadButton), > + VMSTATE_VARRAY_UINT32(pressed, StellarisGamepad, num_buttons, > + 0, vmstate_info_uint8, uint8_t), Don't this break the migration stream? > VMSTATE_END_OF_LIST() > } > };
On Tue, 17 Oct 2023 at 13:44, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > Hi Peter, > > On 17/10/23 14:23, Peter Maydell wrote: > > Currently for each button on the device we have a > > StellarisGamepadButton struct which has the irq, keycode and pressed > > state for it. When we convert to qdev, the qdev property and GPIO > > APIs are going to require that we have separate arrays for the irqs > > and keycodes. Convert from array-of-structs to three separate arrays > > in preparation. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > hw/input/stellaris_gamepad.c | 43 ++++++++++++------------------------ > > 1 file changed, 14 insertions(+), 29 deletions(-) > > > > -static const VMStateDescription vmstate_stellaris_button = { > > - .name = "stellaris_button", > > - .version_id = 0, > > - .minimum_version_id = 0, > > - .fields = (VMStateField[]) { > > - VMSTATE_UINT8(pressed, StellarisGamepadButton), > > - VMSTATE_END_OF_LIST() > > - } > > -}; > > - > > static const VMStateDescription vmstate_stellaris_gamepad = { > > .name = "stellaris_gamepad", > > .version_id = 2, > > .minimum_version_id = 2, > > .fields = (VMStateField[]) { > > VMSTATE_INT32(extension, StellarisGamepad), > > - VMSTATE_STRUCT_VARRAY_POINTER_INT32(buttons, StellarisGamepad, > > - num_buttons, > > - vmstate_stellaris_button, > > - StellarisGamepadButton), > > + VMSTATE_VARRAY_UINT32(pressed, StellarisGamepad, num_buttons, > > + 0, vmstate_info_uint8, uint8_t), > > Don't this break the migration stream? Yes; this is OK because we don't care about migration compat for this board. But I forgot to mention it in the commit message, and we should bump the version_id fields too. thanks -- PMM
On 17/10/23 14:52, Peter Maydell wrote: > On Tue, 17 Oct 2023 at 13:44, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >> Hi Peter, >> >> On 17/10/23 14:23, Peter Maydell wrote: >>> Currently for each button on the device we have a >>> StellarisGamepadButton struct which has the irq, keycode and pressed >>> state for it. When we convert to qdev, the qdev property and GPIO >>> APIs are going to require that we have separate arrays for the irqs >>> and keycodes. Convert from array-of-structs to three separate arrays >>> in preparation. >>> >>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >>> --- >>> hw/input/stellaris_gamepad.c | 43 ++++++++++++------------------------ >>> 1 file changed, 14 insertions(+), 29 deletions(-) >> >> >>> -static const VMStateDescription vmstate_stellaris_button = { >>> - .name = "stellaris_button", >>> - .version_id = 0, >>> - .minimum_version_id = 0, >>> - .fields = (VMStateField[]) { >>> - VMSTATE_UINT8(pressed, StellarisGamepadButton), >>> - VMSTATE_END_OF_LIST() >>> - } >>> -}; >>> - >>> static const VMStateDescription vmstate_stellaris_gamepad = { >>> .name = "stellaris_gamepad", >>> .version_id = 2, >>> .minimum_version_id = 2, >>> .fields = (VMStateField[]) { >>> VMSTATE_INT32(extension, StellarisGamepad), >>> - VMSTATE_STRUCT_VARRAY_POINTER_INT32(buttons, StellarisGamepad, >>> - num_buttons, >>> - vmstate_stellaris_button, >>> - StellarisGamepadButton), >>> + VMSTATE_VARRAY_UINT32(pressed, StellarisGamepad, num_buttons, >>> + 0, vmstate_info_uint8, uint8_t), >> >> Don't this break the migration stream? > > Yes; this is OK because we don't care about migration compat > for this board. But I forgot to mention it in the commit > message, and we should bump the version_id fields too. OK, this is what I thought (I'm still trying to correctly understand that myself). With that updated: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c index 377101a4035..da974400b59 100644 --- a/hw/input/stellaris_gamepad.c +++ b/hw/input/stellaris_gamepad.c @@ -14,15 +14,11 @@ #include "ui/console.h" typedef struct { - qemu_irq irq; - int keycode; - uint8_t pressed; -} StellarisGamepadButton; - -typedef struct { - StellarisGamepadButton *buttons; - int num_buttons; + uint32_t num_buttons; int extension; + qemu_irq *irqs; + uint32_t *keycodes; + uint8_t *pressed; } StellarisGamepad; static void stellaris_gamepad_put_key(void * opaque, int keycode) @@ -40,36 +36,23 @@ static void stellaris_gamepad_put_key(void * opaque, int keycode) keycode = (keycode & 0x7f) | s->extension; for (i = 0; i < s->num_buttons; i++) { - if (s->buttons[i].keycode == keycode - && s->buttons[i].pressed != down) { - s->buttons[i].pressed = down; - qemu_set_irq(s->buttons[i].irq, down); + if (s->keycodes[i] == keycode && s->pressed[i] != down) { + s->pressed[i] = down; + qemu_set_irq(s->irqs[i], down); } } s->extension = 0; } -static const VMStateDescription vmstate_stellaris_button = { - .name = "stellaris_button", - .version_id = 0, - .minimum_version_id = 0, - .fields = (VMStateField[]) { - VMSTATE_UINT8(pressed, StellarisGamepadButton), - VMSTATE_END_OF_LIST() - } -}; - static const VMStateDescription vmstate_stellaris_gamepad = { .name = "stellaris_gamepad", .version_id = 2, .minimum_version_id = 2, .fields = (VMStateField[]) { VMSTATE_INT32(extension, StellarisGamepad), - VMSTATE_STRUCT_VARRAY_POINTER_INT32(buttons, StellarisGamepad, - num_buttons, - vmstate_stellaris_button, - StellarisGamepadButton), + VMSTATE_VARRAY_UINT32(pressed, StellarisGamepad, num_buttons, + 0, vmstate_info_uint8, uint8_t), VMSTATE_END_OF_LIST() } }; @@ -81,10 +64,12 @@ void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode) int i; s = g_new0(StellarisGamepad, 1); - s->buttons = g_new0(StellarisGamepadButton, n); + s->irqs = g_new0(qemu_irq, n); + s->keycodes = g_new0(uint32_t, n); + s->pressed = g_new0(uint8_t, n); for (i = 0; i < n; i++) { - s->buttons[i].irq = irq[i]; - s->buttons[i].keycode = keycode[i]; + s->irqs[i] = irq[i]; + s->keycodes[i] = keycode[i]; } s->num_buttons = n; qemu_add_kbd_event_handler(stellaris_gamepad_put_key, s);
Currently for each button on the device we have a StellarisGamepadButton struct which has the irq, keycode and pressed state for it. When we convert to qdev, the qdev property and GPIO APIs are going to require that we have separate arrays for the irqs and keycodes. Convert from array-of-structs to three separate arrays in preparation. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/input/stellaris_gamepad.c | 43 ++++++++++++------------------------ 1 file changed, 14 insertions(+), 29 deletions(-)