Re: [PATCH v2 5/6] hw/input/stellaris_input: Convert to qdev

[Peter Maydell]

On Mon, 30 Oct 2023 at 20:38, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 30/10/2023 11:48, Peter Maydell wrote:
>
> > Convert the hw/input/stellaris_input device to qdev.
> >
> > The interface uses an array property for the board to specify the
> > keycodes to use, so the s->keycodes memory is now allocated by the
> > array-property machinery.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > v1->v2: drop private/public comment lines
> > ---
> >   include/hw/input/stellaris_gamepad.h | 22 ++++++++-
> >   hw/arm/stellaris.c                   | 26 +++++++---
> >   hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
> >   3 files changed, 89 insertions(+), 32 deletions(-)
> >
> > diff --git a/include/hw/input/stellaris_gamepad.h 
> > b/include/hw/input/stellaris_gamepad.h
> > index 23cfd3c95f3..6140b889a28 100644
> > --- a/include/hw/input/stellaris_gamepad.h
> > +++ b/include/hw/input/stellaris_gamepad.h
> > @@ -11,8 +11,26 @@
> >   #ifndef HW_INPUT_STELLARIS_GAMEPAD_H
> >   #define HW_INPUT_STELLARIS_GAMEPAD_H
> >
> > +#include "hw/sysbus.h"
> > +#include "qom/object.h"
> >
> > -/* stellaris_gamepad.c */
> > -void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode);
> > +/*
> > + * QEMU interface:
> > + *  + QOM array property "keycodes": uint32_t QEMU keycodes to handle
> > + *  + unnamed GPIO outputs: one per keycode, in the same order as the
> > + *    "keycodes" array property entries; asserted when key is down
> > + */
> > +
> > +#define TYPE_STELLARIS_GAMEPAD "stellaris-gamepad"
> > +OBJECT_DECLARE_SIMPLE_TYPE(StellarisGamepad, STELLARIS_GAMEPAD)
> > +
> > +struct StellarisGamepad {
> > +    SysBusDevice parent_obj;
>
> Minor style comment: for QOM types there should be an empty line after 
> parent_obj to
> aid identification (as per
> https://qemu.readthedocs.io/en/master/devel/style.html#qemu-object-model-declarations).

Fixed.

> > +static const TypeInfo stellaris_gamepad_info = {
> > +    .name = TYPE_STELLARIS_GAMEPAD,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(StellarisGamepad),
> > +    .class_init = stellaris_gamepad_class_init,
> > +};
> > +
> > +static void stellaris_gamepad_register_types(void)
> > +{
> > +    type_register_static(&stellaris_gamepad_info);
> > +}
> > +
> > +type_init(stellaris_gamepad_register_types);
>
> Is it worth converting this to use DEFINE_TYPES() during the conversion? I 
> know Phil
> has considered some automation to remove the type_init() boilerplate for the 
> majority
> of cases.

I could, I guess. It seems a bit awkward that DEFINE_TYPES()
wants you to pass it an array even when you only have one type,
though, which is going to be a very common use case.

-- PMM