| Message ID | 20220522181836.864-47-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | PS2 device QOMification - part 1 | expand |
On Sun, 22 May 2022 at 19:20, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> 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> > --- > hw/input/lasips2.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/hw/input/lasips2.c b/hw/input/lasips2.c > index 644cf70955..12ff95a05f 100644 > --- a/hw/input/lasips2.c > +++ b/hw/input/lasips2.c > @@ -117,6 +117,9 @@ static const char *lasips2_write_reg_name(uint64_t addr) > } > } > > +#define LASIPS2_KBD_INPUT_IRQ 0 > +#define LASIPS2_MOUSE_INPUT_IRQ 1 > + > static void lasips2_update_irq(LASIPS2State *s) > { > trace_lasips2_intr(s->kbd.irq | s->mouse.irq); > @@ -237,9 +240,10 @@ static const MemoryRegionOps lasips2_reg_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > -static void lasips2_port_set_irq(void *opaque, int level) > +static void lasips2_set_irq(void *opaque, int n, int level) > { > - LASIPS2Port *port = opaque; > + LASIPS2State *s = LASIPS2(opaque); > + LASIPS2Port *port = (n ? &s->mouse : &s->kbd); > > port->irq = level; > lasips2_update_irq(port->parent); > @@ -263,8 +267,12 @@ 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(dev, LASIPS2_KBD_INPUT_IRQ)); > + s->mouse.dev = ps2_mouse_init(NULL, NULL); > + qdev_connect_gpio_out(DEVICE(s->mouse.dev), PS2_DEVICE_IRQ, > + qdev_get_gpio_in(dev, LASIPS2_MOUSE_INPUT_IRQ)); > } > > static void lasips2_init(Object *obj) > @@ -285,6 +293,7 @@ static void lasips2_init(Object *obj) > sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mouse.reg); > > qdev_init_gpio_out(DEVICE(obj), &s->irq, 1); > + qdev_init_gpio_in(DEVICE(obj), lasips2_set_irq, 2); > } These definitely should be named GPIO inputs, as with the pl050. Aside, if you felt like adding "QEMU interface" comments to these devices that summarize what the various sysbus IRQs, MMIOs, qdev GPIOs, etc do, that would be nice, and then gives you a place to document that these GPIO lines are part of the internal implementation rather than externally-facing. include/hw/intc/ppc-uic.h is one example but you can find lots by grepping for "QEMU interface" in include/hw/. thanks -- PMM
On 09/06/2022 12:21, Peter Maydell wrote: > On Sun, 22 May 2022 at 19:20, Mark Cave-Ayland > <mark.cave-ayland@ilande.co.uk> 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> >> --- >> hw/input/lasips2.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/hw/input/lasips2.c b/hw/input/lasips2.c >> index 644cf70955..12ff95a05f 100644 >> --- a/hw/input/lasips2.c >> +++ b/hw/input/lasips2.c >> @@ -117,6 +117,9 @@ static const char *lasips2_write_reg_name(uint64_t addr) >> } >> } >> >> +#define LASIPS2_KBD_INPUT_IRQ 0 >> +#define LASIPS2_MOUSE_INPUT_IRQ 1 >> + >> static void lasips2_update_irq(LASIPS2State *s) >> { >> trace_lasips2_intr(s->kbd.irq | s->mouse.irq); >> @@ -237,9 +240,10 @@ static const MemoryRegionOps lasips2_reg_ops = { >> .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> -static void lasips2_port_set_irq(void *opaque, int level) >> +static void lasips2_set_irq(void *opaque, int n, int level) >> { >> - LASIPS2Port *port = opaque; >> + LASIPS2State *s = LASIPS2(opaque); >> + LASIPS2Port *port = (n ? &s->mouse : &s->kbd); >> >> port->irq = level; >> lasips2_update_irq(port->parent); >> @@ -263,8 +267,12 @@ 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(dev, LASIPS2_KBD_INPUT_IRQ)); >> + s->mouse.dev = ps2_mouse_init(NULL, NULL); >> + qdev_connect_gpio_out(DEVICE(s->mouse.dev), PS2_DEVICE_IRQ, >> + qdev_get_gpio_in(dev, LASIPS2_MOUSE_INPUT_IRQ)); >> } >> >> static void lasips2_init(Object *obj) >> @@ -285,6 +293,7 @@ static void lasips2_init(Object *obj) >> sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mouse.reg); >> >> qdev_init_gpio_out(DEVICE(obj), &s->irq, 1); >> + qdev_init_gpio_in(DEVICE(obj), lasips2_set_irq, 2); >> } > > These definitely should be named GPIO inputs, as with the pl050. > > Aside, if you felt like adding "QEMU interface" comments to these > devices that summarize what the various sysbus IRQs, MMIOs, > qdev GPIOs, etc do, that would be nice, and then gives you a > place to document that these GPIO lines are part of the internal > implementation rather than externally-facing. include/hw/intc/ppc-uic.h > is one example but you can find lots by grepping for "QEMU interface" > in include/hw/. Oh interesting, I wasn't aware of this. I'll see if I can add a summary of the modelling in v2. ATB, Mark.
diff --git a/hw/input/lasips2.c b/hw/input/lasips2.c index 644cf70955..12ff95a05f 100644 --- a/hw/input/lasips2.c +++ b/hw/input/lasips2.c @@ -117,6 +117,9 @@ static const char *lasips2_write_reg_name(uint64_t addr) } } +#define LASIPS2_KBD_INPUT_IRQ 0 +#define LASIPS2_MOUSE_INPUT_IRQ 1 + static void lasips2_update_irq(LASIPS2State *s) { trace_lasips2_intr(s->kbd.irq | s->mouse.irq); @@ -237,9 +240,10 @@ static const MemoryRegionOps lasips2_reg_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static void lasips2_port_set_irq(void *opaque, int level) +static void lasips2_set_irq(void *opaque, int n, int level) { - LASIPS2Port *port = opaque; + LASIPS2State *s = LASIPS2(opaque); + LASIPS2Port *port = (n ? &s->mouse : &s->kbd); port->irq = level; lasips2_update_irq(port->parent); @@ -263,8 +267,12 @@ 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(dev, LASIPS2_KBD_INPUT_IRQ)); + s->mouse.dev = ps2_mouse_init(NULL, NULL); + qdev_connect_gpio_out(DEVICE(s->mouse.dev), PS2_DEVICE_IRQ, + qdev_get_gpio_in(dev, LASIPS2_MOUSE_INPUT_IRQ)); } static void lasips2_init(Object *obj) @@ -285,6 +293,7 @@ static void lasips2_init(Object *obj) sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mouse.reg); qdev_init_gpio_out(DEVICE(obj), &s->irq, 1); + qdev_init_gpio_in(DEVICE(obj), lasips2_set_irq, 2); } static Property lasips2_properties[] = {
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> --- hw/input/lasips2.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)