From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41903) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S9cRR-0000gG-Dr for qemu-devel@nongnu.org; Mon, 19 Mar 2012 09:16:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S9cRK-0002Sz-Dv for qemu-devel@nongnu.org; Mon, 19 Mar 2012 09:16:04 -0400 Message-ID: <4F67318C.8050302@suse.de> Date: Mon, 19 Mar 2012 14:15:56 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1331995186-18507-1-git-send-email-hpoussin@reactos.org> <1331995186-18507-6-git-send-email-hpoussin@reactos.org> In-Reply-To: <1331995186-18507-6-git-send-email-hpoussin@reactos.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 5/6] prep: add pc87312 Super I/O emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org Am 17.03.2012 15:39, schrieb Herv=C3=A9 Poussineau: > This provides floppy and IDE controllers as well as serial and parallel= ports. > However, dynamic configuration of devices is not yet supported. >=20 > Cc: Andreas F=C3=A4rber > Signed-off-by: Herv=C3=A9 Poussineau > --- > Makefile.objs | 1 + > hw/pc87312.c | 425 +++++++++++++++++++++++++++++++++++++++++++++++++= ++++++++ > 2 files changed, 426 insertions(+), 0 deletions(-) > create mode 100644 hw/pc87312.c > diff --git a/hw/pc87312.c b/hw/pc87312.c > new file mode 100644 > index 0000000..1e28dbd > --- /dev/null > +++ b/hw/pc87312.c > @@ -0,0 +1,425 @@ > +/* > + * QEMU National Semiconductor PC87312 (Super I/O) > + * > + * Copyright (c) 2010-2012 Herve Poussineau FWIW mind to add Copyright (c) 2011 Andreas F=C3=A4rber ? > + * > + * Permission is hereby granted, free of charge, to any person obtaini= ng a copy > + * of this software and associated documentation files (the "Software"= ), to deal > + * in the Software without restriction, including without limitation t= he rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/o= r sell > + * copies of the Software, and to permit persons to whom the Software = is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be incl= uded in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXP= RESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABI= LITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT S= HALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES O= R OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARI= SING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALI= NGS IN > + * THE SOFTWARE. > + */ > +//#define DEBUG_PC87312 > + > +#ifdef DEBUG_PC87312 > +#define DPRINTF(fmt, ...) \ > +do { fprintf(stderr, "pc87312: " fmt , ## __VA_ARGS__); } while (0) > +#else > +#define DPRINTF(fmt, ...) \ > +do {} while (0) > +#endif Mid-term we should replace this through proper tracing. > +static int pc87312_init(ISADevice *dev) > +{ > + PC87312State *s; > + ISADevice *isa; > + ISABus *bus; > + CharDriverState *chr; > + BlockDriverState *bs; > + int i; > + > + s =3D DO_UPCAST(PC87312State, dev, dev); > + bus =3D isa_bus_from_device(dev); > + pc87312_hard_reset(s); > + > + chr =3D s->parallel.chr; > + if (s->parallel.chr !=3D NULL && is_parallel_enabled(s)) { This logic still seems to be flawed: it should not depend on s->parallel.chr. If that is NULL we need to create a null char device to match the device that's present in hardware according to is_parallel_enabled(s). > + qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HACK */ HACK alarm in [PATCH]: What for? > + isa =3D isa_create(bus, "isa-parallel"); > + qdev_prop_set_uint32(&isa->qdev, "index", 0); > + qdev_prop_set_uint32(&isa->qdev, "iobase", get_parallel_iobase= (s)); > + qdev_prop_set_uint32(&isa->qdev, "irq", get_parallel_irq(s)); > + qdev_prop_set_chr(&isa->qdev, "chardev", chr); > + qdev_init_nofail(&isa->qdev); > + s->parallel.dev =3D &isa->qdev; > + DPRINTF("parallel: base 0x%x, irq %u\n", > + get_parallel_iobase(s), get_parallel_irq(s)); > + } > + > + for (i =3D 0; i < 2; i++) { > + chr =3D s->uart[i].chr; > + if (chr !=3D NULL && is_uart_enabled(s, i)) { > + qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL); /* HAC= K */ 2x ditto. > + isa =3D isa_create(bus, "isa-serial"); > + qdev_prop_set_uint32(&isa->qdev, "index", i); > + qdev_prop_set_uint32(&isa->qdev, "iobase", get_uart_iobase= (s, i)); > + qdev_prop_set_uint32(&isa->qdev, "irq", get_uart_irq(s, i)= ); > + qdev_prop_set_chr(&isa->qdev, "chardev", chr); > + qdev_init_nofail(&isa->qdev); > + s->uart[i].dev =3D &isa->qdev; > + DPRINTF("uart%d: base 0x%x, irq %u\n", i, > + get_uart_iobase(s, i), > + get_uart_irq(s, i)); > + } > + } > + > + if (is_fdc_enabled(s)) { > + isa =3D isa_create(bus, "isa-fdc"); > + qdev_prop_set_uint32(&isa->qdev, "iobase", get_fdc_iobase(s)); > + qdev_prop_set_uint32(&isa->qdev, "irq", 6); > + bs =3D s->fdc.drive[0]; > + if (bs !=3D NULL) { > + bdrv_detach_dev(bs, bdrv_get_attached_dev(bs)); /* HACK */ > + qdev_prop_set_drive_nofail(&isa->qdev, "driveA", bs); > + } > + bs =3D s->fdc.drive[1]; > + if (bs !=3D NULL) { > + bdrv_detach_dev(bs, bdrv_get_attached_dev(bs)); /* HACK */ > + qdev_prop_set_drive_nofail(&isa->qdev, "driveB", bs); > + } > + qdev_init_nofail(&isa->qdev); > + s->fdc.dev =3D &isa->qdev; > + DPRINTF("fdc: base 0x%x\n", get_fdc_iobase(s)); > + } > + > + if (is_ide_enabled(s)) { > + isa =3D isa_create(bus, "isa-ide"); > + qdev_prop_set_uint32(&isa->qdev, "iobase", get_ide_iobase(s)); > + qdev_prop_set_uint32(&isa->qdev, "iobase2", get_ide_iobase(s) = + 0x206); > + qdev_prop_set_uint32(&isa->qdev, "irq", 14); > + qdev_init_nofail(&isa->qdev); > + s->ide.dev =3D &isa->qdev; > + DPRINTF("ide: base 0x%x\n", get_ide_iobase(s)); > + } > + > + register_ioport_write(s->iobase, 2, 1, pc87312_ioport_write, s); > + register_ioport_read(s->iobase, 2, 1, pc87312_ioport_read, s); > + return 0; > +} > + > +static const VMStateDescription vmstate_pc87312 =3D { > + .name =3D "pc87312", > + .version_id =3D 1, > + .minimum_version_id =3D 1, > + .post_load =3D pc87312_post_load, > + .fields =3D (VMStateField[]) { > + VMSTATE_UINT8(read_id_step, PC87312State), > + VMSTATE_UINT8(selected_index, PC87312State), > + VMSTATE_UINT8_ARRAY(regs, PC87312State, 3), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static Property pc87312_properties[] =3D { > + DEFINE_PROP_HEX32("iobase", PC87312State, iobase, 0x398), > + DEFINE_PROP_UINT8("config", PC87312State, config, 1), > + DEFINE_PROP_CHR("parallel", PC87312State, parallel.chr), > + DEFINE_PROP_CHR("uart1", PC87312State, uart[0].chr), > + DEFINE_PROP_CHR("uart2", PC87312State, uart[1].chr), > + DEFINE_PROP_DRIVE("floppyA", PC87312State, fdc.drive[0]), > + DEFINE_PROP_DRIVE("floppyB", PC87312State, fdc.drive[1]), > + DEFINE_PROP_END_OF_LIST() > +}; > + > +static void pc87312_class_initfn(ObjectClass *klass, void *data) I always thought initfn was used for instances... > +{ > + DeviceClass *dc =3D DEVICE_CLASS(klass); > + ISADeviceClass *ic =3D ISA_DEVICE_CLASS(klass); > + > + ic->init =3D pc87312_init; > + dc->reset =3D pc87312_reset; > + dc->vmsd =3D &vmstate_pc87312; > + dc->props =3D pc87312_properties; > +} > + > +static TypeInfo pc87312_info =3D { > + .name =3D "pc87312", > + .parent =3D TYPE_ISA_DEVICE, > + .instance_size =3D sizeof(PC87312State), > + .class_init =3D pc87312_class_initfn, > +}; > + > +static void pc87312_register_types(void) > +{ > + type_register_static(&pc87312_info); > +} > + > +type_init(pc87312_register_types) > + Trailing empty line. So what about the ugly ISA hot-plug issue that originally stalled our two series? I'm missing a Change Log about that. You changed the initial configuration to the one used by 40P firmware IIRC and stopped supporting the chipset's reconfiguration? Either way any limitation of this implementation should be prominently documented please. Thanks for your work on this, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg