| Message ID | 1328342612-25826-4-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hello Paolo, What will be fixed/enhanced by this patch? Scenario? Can you also add a more detailed commit message. Thnx. Ciao, Gerhard -- http://www.wiesinger.com/ On Sat, 4 Feb 2012, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/pc.c | 7 +++---- > hw/pc.h | 1 - > hw/pckbd.c | 7 +++++-- > hw/vmmouse.c | 11 +++-------- > 4 files changed, 11 insertions(+), 15 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index 7f3aa65..76787c7 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1181,13 +1181,12 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > if (!no_vmport) { > vmport_init(isa_bus); > vmmouse = isa_try_create(isa_bus, "vmmouse"); > + qdev_init_nofail(&vmmouse->qdev); > + qdev_connect_gpio_out(DEVICE(vmmouse), 0, > + qdev_get_gpio_in(DEVICE(i8042), 0)); > } else { > vmmouse = NULL; > } > - if (vmmouse) { > - qdev_prop_set_ptr(&vmmouse->qdev, "ps2_mouse", i8042); > - qdev_init_nofail(&vmmouse->qdev); > - } > port92 = isa_create_simple(isa_bus, "port92"); > port92_init(port92, &a20_line[1]); > > diff --git a/hw/pc.h b/hw/pc.h > index c666ec9..5602549 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -121,7 +121,6 @@ void i8042_init(qemu_irq kbd_irq, qemu_irq mouse_irq, uint32_t io_base); > void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq, > MemoryRegion *region, ram_addr_t size, > target_phys_addr_t mask); > -void i8042_isa_mouse_fake_event(void *opaque); > void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out); > > /* pc.c */ > diff --git a/hw/pckbd.c b/hw/pckbd.c > index b4c53be..ea8ff73 100644 > --- a/hw/pckbd.c > +++ b/hw/pckbd.c > @@ -433,12 +433,14 @@ typedef struct ISAKBDState { > MemoryRegion io[2]; > } ISAKBDState; > > -void i8042_isa_mouse_fake_event(void *opaque) > +static void i8042_isa_mouse_fake_event(void *opaque, int n, int level) > { > ISADevice *dev = opaque; > KBDState *s = &(DO_UPCAST(ISAKBDState, dev, dev)->kbd); > > - ps2_mouse_fake_event(s->mouse); > + if (level) { > + ps2_mouse_fake_event(s->mouse); > + } > } > > void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out) > @@ -482,6 +484,7 @@ static int i8042_initfn(ISADevice *dev) > ISAKBDState *isa_s = DO_UPCAST(ISAKBDState, dev, dev); > KBDState *s = &isa_s->kbd; > > + qdev_init_gpio_in(DEVICE(dev), i8042_isa_mouse_fake_event, 1); > isa_init_irq(dev, &s->irq_kbd, 1); > isa_init_irq(dev, &s->irq_mouse, 12); > > diff --git a/hw/vmmouse.c b/hw/vmmouse.c > index fda4f89..0272e7e 100644 > --- a/hw/vmmouse.c > +++ b/hw/vmmouse.c > @@ -60,7 +60,7 @@ typedef struct _VMMouseState > uint16_t status; > uint8_t absolute; > QEMUPutMouseEntry *entry; > - void *ps2_mouse; > + qemu_irq ps2_mouse_event; > } VMMouseState; > > static uint32_t vmmouse_get_status(VMMouseState *s) > @@ -99,7 +99,7 @@ static void vmmouse_mouse_event(void *opaque, int x, int y, int dz, int buttons_ > > /* need to still generate PS2 events to notify driver to > read from queue */ > - i8042_isa_mouse_fake_event(s->ps2_mouse); > + qemu_set_irq(s->ps2_mouse_event, 1); > } > > static void vmmouse_remove_handler(VMMouseState *s) > @@ -264,6 +264,7 @@ static int vmmouse_initfn(ISADevice *dev) > > DPRINTF("vmmouse_init\n"); > > + qdev_init_gpio_out(DEVICE(dev), &s->ps2_mouse_event, 1); > vmport_register(VMMOUSE_STATUS, vmmouse_ioport_read, s); > vmport_register(VMMOUSE_COMMAND, vmmouse_ioport_read, s); > vmport_register(VMMOUSE_DATA, vmmouse_ioport_read, s); > @@ -271,11 +272,6 @@ static int vmmouse_initfn(ISADevice *dev) > return 0; > } > > -static Property vmmouse_properties[] = { > - DEFINE_PROP_PTR("ps2_mouse", VMMouseState, ps2_mouse), > - DEFINE_PROP_END_OF_LIST(), > -}; > - > static void vmmouse_class_initfn(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -284,7 +280,6 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data) > dc->no_user = 1; > dc->reset = vmmouse_reset; > dc->vmsd = &vmstate_vmmouse; > - dc->props = vmmouse_properties; > } > > static TypeInfo vmmouse_info = { > -- > 1.7.7.6 > > >
On 02/04/2012 02:03 PM, Gerhard Wiesinger wrote: > Hello Paolo, > > What will be fixed/enhanced by this patch? Scenario? Nothing, it's just a cleanup. It's a step towards removing PROP_PTR. Paolo
Paolo Bonzini <pbonzini@redhat.com> wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/pc.c | 7 +++---- > hw/pc.h | 1 - > hw/pckbd.c | 7 +++++-- > hw/vmmouse.c | 11 +++-------- > 4 files changed, 11 insertions(+), 15 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index 7f3aa65..76787c7 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1181,13 +1181,12 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > if (!no_vmport) { > vmport_init(isa_bus); > vmmouse = isa_try_create(isa_bus, "vmmouse"); > + qdev_init_nofail(&vmmouse->qdev); > + qdev_connect_gpio_out(DEVICE(vmmouse), 0, > + qdev_get_gpio_in(DEVICE(i8042), 0)); Are you sue that vmmouse is always non-NULL? My understanding is that all callers check if is_try_create() return NULL. Could you clarify? Thanks, Juan.
On 02/07/2012 01:39 PM, Juan Quintela wrote: >> > vmmouse = isa_try_create(isa_bus, "vmmouse"); >> > + qdev_init_nofail(&vmmouse->qdev); >> > + qdev_connect_gpio_out(DEVICE(vmmouse), 0, >> > + qdev_get_gpio_in(DEVICE(i8042), 0)); > Are you sue that vmmouse is always non-NULL? My understanding is that > all callers check if is_try_create() return NULL. > > Could you clarify? No, I couldn't. :) You're right. Paolo
> +static void i8042_isa_mouse_fake_event(void *opaque, int n, int level) > { > ISADevice *dev = opaque; > KBDState *s = &(DO_UPCAST(ISAKBDState, dev, dev)->kbd); > > - ps2_mouse_fake_event(s->mouse); > + if (level) { > + ps2_mouse_fake_event(s->mouse); > + } > } >... > @@ -99,7 +99,7 @@ static void vmmouse_mouse_event(void *opaque, int x, int > /* need to still generate PS2 events to notify driver to > read from queue */ > - i8042_isa_mouse_fake_event(s->ps2_mouse); > + qemu_set_irq(s->ps2_mouse_event, 1); > } Both of these are wrong. qemu_irq represents a state. It is not an messge passing API. Currently we don't discard duplicate sets, but that may change in the future. If you want to convey an event via an IRQ line you need to actually cause and detect state changes. i.e. i8042_isa_mouse_fake_event needs to remember the previous state, and only act on a low-high transition. vmmouse_mouse_event should use qemu_irq_pulse. Paul
diff --git a/hw/pc.c b/hw/pc.c index 7f3aa65..76787c7 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -1181,13 +1181,12 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, if (!no_vmport) { vmport_init(isa_bus); vmmouse = isa_try_create(isa_bus, "vmmouse"); + qdev_init_nofail(&vmmouse->qdev); + qdev_connect_gpio_out(DEVICE(vmmouse), 0, + qdev_get_gpio_in(DEVICE(i8042), 0)); } else { vmmouse = NULL; } - if (vmmouse) { - qdev_prop_set_ptr(&vmmouse->qdev, "ps2_mouse", i8042); - qdev_init_nofail(&vmmouse->qdev); - } port92 = isa_create_simple(isa_bus, "port92"); port92_init(port92, &a20_line[1]); diff --git a/hw/pc.h b/hw/pc.h index c666ec9..5602549 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -121,7 +121,6 @@ void i8042_init(qemu_irq kbd_irq, qemu_irq mouse_irq, uint32_t io_base); void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq, MemoryRegion *region, ram_addr_t size, target_phys_addr_t mask); -void i8042_isa_mouse_fake_event(void *opaque); void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out); /* pc.c */ diff --git a/hw/pckbd.c b/hw/pckbd.c index b4c53be..ea8ff73 100644 --- a/hw/pckbd.c +++ b/hw/pckbd.c @@ -433,12 +433,14 @@ typedef struct ISAKBDState { MemoryRegion io[2]; } ISAKBDState; -void i8042_isa_mouse_fake_event(void *opaque) +static void i8042_isa_mouse_fake_event(void *opaque, int n, int level) { ISADevice *dev = opaque; KBDState *s = &(DO_UPCAST(ISAKBDState, dev, dev)->kbd); - ps2_mouse_fake_event(s->mouse); + if (level) { + ps2_mouse_fake_event(s->mouse); + } } void i8042_setup_a20_line(ISADevice *dev, qemu_irq *a20_out) @@ -482,6 +484,7 @@ static int i8042_initfn(ISADevice *dev) ISAKBDState *isa_s = DO_UPCAST(ISAKBDState, dev, dev); KBDState *s = &isa_s->kbd; + qdev_init_gpio_in(DEVICE(dev), i8042_isa_mouse_fake_event, 1); isa_init_irq(dev, &s->irq_kbd, 1); isa_init_irq(dev, &s->irq_mouse, 12); diff --git a/hw/vmmouse.c b/hw/vmmouse.c index fda4f89..0272e7e 100644 --- a/hw/vmmouse.c +++ b/hw/vmmouse.c @@ -60,7 +60,7 @@ typedef struct _VMMouseState uint16_t status; uint8_t absolute; QEMUPutMouseEntry *entry; - void *ps2_mouse; + qemu_irq ps2_mouse_event; } VMMouseState; static uint32_t vmmouse_get_status(VMMouseState *s) @@ -99,7 +99,7 @@ static void vmmouse_mouse_event(void *opaque, int x, int y, int dz, int buttons_ /* need to still generate PS2 events to notify driver to read from queue */ - i8042_isa_mouse_fake_event(s->ps2_mouse); + qemu_set_irq(s->ps2_mouse_event, 1); } static void vmmouse_remove_handler(VMMouseState *s) @@ -264,6 +264,7 @@ static int vmmouse_initfn(ISADevice *dev) DPRINTF("vmmouse_init\n"); + qdev_init_gpio_out(DEVICE(dev), &s->ps2_mouse_event, 1); vmport_register(VMMOUSE_STATUS, vmmouse_ioport_read, s); vmport_register(VMMOUSE_COMMAND, vmmouse_ioport_read, s); vmport_register(VMMOUSE_DATA, vmmouse_ioport_read, s); @@ -271,11 +272,6 @@ static int vmmouse_initfn(ISADevice *dev) return 0; } -static Property vmmouse_properties[] = { - DEFINE_PROP_PTR("ps2_mouse", VMMouseState, ps2_mouse), - DEFINE_PROP_END_OF_LIST(), -}; - static void vmmouse_class_initfn(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -284,7 +280,6 @@ static void vmmouse_class_initfn(ObjectClass *klass, void *data) dc->no_user = 1; dc->reset = vmmouse_reset; dc->vmsd = &vmstate_vmmouse; - dc->props = vmmouse_properties; } static TypeInfo vmmouse_info = {
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/pc.c | 7 +++---- hw/pc.h | 1 - hw/pckbd.c | 7 +++++-- hw/vmmouse.c | 11 +++-------- 4 files changed, 11 insertions(+), 15 deletions(-)