Re: [PATCH v2 47/54] lasips2: switch over from update_irq() function to PS2 device gpio

[Mark Cave-Ayland]

On 24/06/2022 14:41, Mark Cave-Ayland wrote:

> Add a qdev gpio input in lasips2_init() by taking the existing 
> lasips2_port_set_irq()
> function, updating it accordingly and then renaming to lasips2_set_irq(). Use 
> these
> new qdev gpio inputs to wire up the PS2 keyboard and mouse devices.
>
> At the same time set update_irq() and update_arg to NULL in ps2_kbd_init() and
> ps2_mouse_init() to ensure that any accidental attempt to use the legacy 
> update_irq()
> function will cause a NULL pointer dereference.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Acked-by: Helge Deller <deller@gmx.de>
> ---
>   hw/input/lasips2.c | 30 ++++++++++++++++++++++++++----
>   1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/hw/input/lasips2.c b/hw/input/lasips2.c
> index bd72505411..e1a8a7e34b 100644
> --- a/hw/input/lasips2.c
> +++ b/hw/input/lasips2.c
> @@ -237,9 +237,19 @@ static const MemoryRegionOps lasips2_reg_ops = {
>       .endianness = DEVICE_NATIVE_ENDIAN,
>   };
>   
> -static void lasips2_port_set_irq(void *opaque, int level)
> +static void lasips2_set_kbd_irq(void *opaque, int n, int level)
>   {
> -    LASIPS2Port *port = opaque;
> +    LASIPS2State *s = LASIPS2(opaque);
> +    LASIPS2Port *port = &s->kbd;
> +
> +    port->irq = level;
> +    lasips2_update_irq(port->parent);
> +}
> +
> +static void lasips2_set_mouse_irq(void *opaque, int n, int level)
> +{
> +    LASIPS2State *s = LASIPS2(opaque);
> +    LASIPS2Port *port = &s->mouse;
>   
>       port->irq = level;
>       lasips2_update_irq(port->parent);
> @@ -264,8 +274,14 @@ static void lasips2_realize(DeviceState *dev, Error **errp)
>   
>       vmstate_register(NULL, s->base, &vmstate_lasips2, s);
>   
> -    s->kbd.dev = ps2_kbd_init(lasips2_port_set_irq, &s->kbd);
> -    s->mouse.dev = ps2_mouse_init(lasips2_port_set_irq, &s->mouse);
> +    s->kbd.dev = ps2_kbd_init(NULL, NULL);
> +    qdev_connect_gpio_out(DEVICE(s->kbd.dev), PS2_DEVICE_IRQ,
> +                          qdev_get_gpio_in_named(dev, "ps2-kbd-input-irq",
> +                                                 0));
> +    s->mouse.dev = ps2_mouse_init(NULL, NULL);
> +    qdev_connect_gpio_out(DEVICE(s->mouse.dev), PS2_DEVICE_IRQ,
> +                          qdev_get_gpio_in_named(dev, "ps2-mouse-input-irq",
> +                                                 0));
>   }
>   
>   static void lasips2_init(Object *obj)
> @@ -286,6 +302,12 @@ static void lasips2_init(Object *obj)
>       sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mouse.reg);
>   
>       sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
> +
> +    qdev_init_gpio_out(DEVICE(obj), &s->irq, 1);

I was just rebasing and testing part 2 of this series which flagged up that the 
rebase on part 1 had inadvertently pulled the old qdev_init_gpio_out() back in again 
on this patch. This line is no longer needed and should simply be removed.

I'll remove it locally and if everything still looks good after testing (including 
gitlab CI) then I shall queue this series to my qemu-sparc branch and send an MR.

> +    qdev_init_gpio_in_named(DEVICE(obj), lasips2_set_kbd_irq,
> +                            "ps2-kbd-input-irq", 1);
> +    qdev_init_gpio_in_named(DEVICE(obj), lasips2_set_mouse_irq,
> +                            "ps2-mouse-input-irq", 1);
>   }
>   
>   static Property lasips2_properties[] = {


ATB,

Mark.