From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=57689 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Poe5f-0001xx-V3 for qemu-devel@nongnu.org; Sun, 13 Feb 2011 10:42:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Poe5S-0000Oh-M7 for qemu-devel@nongnu.org; Sun, 13 Feb 2011 10:42:13 -0500 Received: from mail-yw0-f45.google.com ([209.85.213.45]:37468) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Poe5S-0000Od-Gj for qemu-devel@nongnu.org; Sun, 13 Feb 2011 10:42:10 -0500 Received: by ywa8 with SMTP id 8so1908295ywa.4 for ; Sun, 13 Feb 2011 07:42:10 -0800 (PST) Message-ID: <4D57FBCD.5060209@codemonkey.ws> Date: Sun, 13 Feb 2011 09:42:05 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 06/10] vmmouse: convert to qdev References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Blue Swirl , qemu-devel On 02/12/2011 11:03 AM, Markus Armbruster wrote: > Blue Swirl writes: > > >> Convert to qdev, also add a proper reset function. >> >> Signed-off-by: Blue Swirl >> --- >> hw/pc.c | 5 +++-- >> hw/pc.h | 3 --- >> hw/vmmouse.c | 37 +++++++++++++++++++++++++++++-------- >> 3 files changed, 32 insertions(+), 13 deletions(-) >> >> diff --git a/hw/pc.c b/hw/pc.c >> index fcee09a..f66ac5d 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -1096,7 +1096,7 @@ void pc_basic_device_init(qemu_irq *isa_irq, >> PITState *pit; >> qemu_irq rtc_irq = NULL; >> qemu_irq *a20_line; >> - ISADevice *i8042, *port92; >> + ISADevice *i8042, *port92, *vmmouse; >> qemu_irq *cpu_exit_irq; >> >> register_ioport_write(0x80, 1, 1, ioport80_write, NULL); >> @@ -1133,7 +1133,8 @@ void pc_basic_device_init(qemu_irq *isa_irq, >> a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2); >> i8042 = isa_create_simple("i8042"); >> i8042_setup_a20_line(i8042,&a20_line[0]); >> - vmmouse_init(i8042); >> + vmmouse = isa_create("vmmouse"); >> + qdev_prop_set_ptr(&vmmouse->qdev, "ps2_mouse", i8042); >> port92 = isa_create_simple("port92"); >> port92_init(port92,&a20_line[1]); >> >> diff --git a/hw/pc.h b/hw/pc.h >> index 603a2a3..ae83934 100644 >> --- a/hw/pc.h >> +++ b/hw/pc.h >> @@ -67,9 +67,6 @@ void hpet_pit_enable(void); >> /* vmport.c */ >> void vmport_register(unsigned char command, IOPortReadFunc *func, >> void *opaque); >> >> -/* vmmouse.c */ >> -void *vmmouse_init(void *m); >> - >> /* pckbd.c */ >> >> void i8042_init(qemu_irq kbd_irq, qemu_irq mouse_irq, uint32_t io_base); >> diff --git a/hw/vmmouse.c b/hw/vmmouse.c >> index 2097119..3b39144 100644 >> --- a/hw/vmmouse.c >> +++ b/hw/vmmouse.c >> @@ -25,6 +25,7 @@ >> #include "console.h" >> #include "ps2.h" >> #include "pc.h" >> +#include "qdev.h" >> >> /* debug only vmmouse */ >> //#define DEBUG_VMMOUSE >> @@ -52,6 +53,7 @@ >> >> typedef struct _VMMouseState >> { >> + ISADevice dev; >> uint32_t queue[VMMOUSE_QUEUE_SIZE]; >> int32_t queue_size; >> uint16_t nb_queue; >> @@ -270,22 +272,41 @@ static const VMStateDescription vmstate_vmmouse = { >> } >> }; >> >> -void *vmmouse_init(void *m) >> +static void vmmouse_reset(DeviceState *d) >> { >> - VMMouseState *s = NULL; >> + VMMouseState *s = container_of(d, VMMouseState, dev.qdev); >> >> - DPRINTF("vmmouse_init\n"); >> + s->status = 0xffff; >> +} >> >> - s = qemu_mallocz(sizeof(VMMouseState)); >> +static int vmmouse_initfn(ISADevice *dev) >> +{ >> + VMMouseState *s = DO_UPCAST(VMMouseState, dev, dev); >> >> - s->status = 0xffff; >> - s->ps2_mouse = m; >> - s->queue_size = VMMOUSE_QUEUE_SIZE; >> > Where is member queue_size initialized in your new code? > > >> + DPRINTF("vmmouse_init\n"); >> >> vmport_register(VMMOUSE_STATUS, vmmouse_ioport_read, s); >> vmport_register(VMMOUSE_COMMAND, vmmouse_ioport_read, s); >> vmport_register(VMMOUSE_DATA, vmmouse_ioport_read, s); >> vmstate_register(NULL, 0,&vmstate_vmmouse, s); >> >> - return s; >> + return 0; >> +} >> + >> +static ISADeviceInfo vmmouse_info = { >> + .init = vmmouse_initfn, >> + .qdev.name = "vmmouse", >> + .qdev.size = sizeof(VMMouseState), >> + .qdev.no_user = 1, >> + .qdev.reset = vmmouse_reset, >> + .qdev.props = (Property[]) { >> + DEFINE_PROP_PTR("ps2_mouse", VMMouseState, ps2_mouse), >> > Pointer properties are for dirty hacks only. Is there really no better > solution? Why does it have to be a property? > vmmouse is really just an extension to the PS2 Mouse. It's definitely not an ISA device. In terms of qdev enablement, I would just make it a boolean option to the PS2Mouse and not expose it as a top level device at all. It cannot exist without a PS2Mouse. Regards, Anthony Liguori >> + DEFINE_PROP_END_OF_LIST(), >> + } >> +}; >> + >> +static void vmmouse_dev_register(void) >> +{ >> + isa_qdev_register(&vmmouse_info); >> } >> +device_init(vmmouse_dev_register) >> >