Andreas Färber <afaer...@suse.de> writes:
> Move the parallel_hds[] code to the PReP machine.
Could use a brief explanation *why* we move it.
> Signed-off-by: Andreas Färber <afaer...@suse.de>
> ---
> hw/isa/pc87312.c | 25 +++++++++++++++----------
> hw/ppc/prep.c | 9 +++++++++
> include/hw/isa/pc87312.h | 5 ++---
> 3 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
> index 40a1106..ded6c01 100644
> --- a/hw/isa/pc87312.c
> +++ b/hw/isa/pc87312.c
> @@ -268,6 +268,7 @@ static void pc87312_realize(DeviceState *dev, Error
> **errp)
> ISABus *bus;
> CharDriverState *chr;
> DriveInfo *drive;
> + Error *local_err = NULL;
> char name[5];
> int i;
>
> @@ -278,20 +279,19 @@ static void pc87312_realize(DeviceState *dev, Error
> **errp)
> pc87312_hard_reset(s);
>
> if (is_parallel_enabled(s)) {
> - chr = parallel_hds[0];
> - if (chr == NULL) {
> - chr = qemu_chr_new("par0", "null", NULL);
> - }
> - isa = isa_create(bus, "isa-parallel");
> - d = DEVICE(isa);
> - qdev_prop_set_uint32(d, "index", 0);
> + d = DEVICE(&s->parallel);
> + qdev_set_parent_bus(d, BUS(bus));
> qdev_prop_set_uint32(d, "iobase", get_parallel_iobase(s));
> qdev_prop_set_uint32(d, "irq", get_parallel_irq(s));
> - qdev_prop_set_chr(d, "chardev", chr);
> - qdev_init_nofail(d);
> - s->parallel.dev = isa;
> trace_pc87312_info_parallel(get_parallel_iobase(s),
> get_parallel_irq(s));
> + object_property_set_bool(OBJECT(&s->parallel), true, "realized",
> + &local_err);
> + object_unref(OBJECT(&s->parallel));
Ignorant question: why unref here?
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> }
>
> for (i = 0; i < 2; i++) {
> @@ -351,6 +351,11 @@ static void pc87312_initfn(Object *obj)
> PC87312State *s = PC87312(obj);
>
> memory_region_init_io(&s->io, obj, &pc87312_io_ops, s, "pc87312", 2);
> +
> + object_initialize(&s->parallel, sizeof(s->parallel), TYPE_ISA_PARALLEL);
> + object_property_add_alias(obj, "par0-chardev",
> + OBJECT(&s->parallel), "chardev", &error_abort);
> + qdev_prop_set_uint32(DEVICE(&s->parallel), "index", 0);
> }
>
> static const VMStateDescription vmstate_pc87312 = {
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 7f52662..1dfa051 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -40,6 +40,7 @@
> #include "hw/isa/pc87312.h"
> #include "sysemu/block-backend.h"
> #include "sysemu/arch_init.h"
> +#include "sysemu/char.h"
> #include "sysemu/qtest.h"
> #include "exec/address-spaces.h"
> #include "elf.h"
> @@ -528,6 +529,7 @@ static void ppc_prep_init(MachineState *machine)
> PCIDevice *pci;
> ISABus *isa_bus;
> ISADevice *isa;
> + CharDriverState *chr;
> qemu_irq *cpu_exit_irq;
> int ppc_boot_device;
> DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
> @@ -639,6 +641,13 @@ static void ppc_prep_init(MachineState *machine)
> /* Super I/O (parallel + serial ports) */
> isa = isa_create(isa_bus, TYPE_PC87312);
> dev = DEVICE(isa);
> + chr = parallel_hds[0];
> + if (chr == NULL) {
> + chr = qemu_chr_new("par0", "null", NULL);
> + }
> + if (chr != NULL) {
> + qdev_prop_set_chr(dev, "par0-chardev", chr);
> + }
> qdev_prop_set_uint8(dev, "config", 13); /* fdc, ser0, ser1, par0 */
> qdev_init_nofail(dev);
>
Unlike the old code, this checks for qemu_chr_new() failure. Can this
happen?
If it does, property "par0-chardev" remains null.
parallel_isa_realizefn() will then fail. Worth a comment, I think.
I don't like boards creating backends with IDs on their own as long as
we don't have a ID namespace reserved for the system. However, this is
preexisting, so let's not worry about it now.
> diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
> index bf74470..b473b3b 100644
> --- a/include/hw/isa/pc87312.h
> +++ b/include/hw/isa/pc87312.h
> @@ -26,6 +26,7 @@
> #define QEMU_PC87312_H
>
> #include "hw/isa/isa.h"
> +#include "hw/char/parallel.h"
>
>
> #define TYPE_PC87312 "pc87312"
> @@ -37,9 +38,7 @@ typedef struct PC87312State {
> uint32_t iobase;
> uint8_t config; /* initial configuration */
>
> - struct {
> - ISADevice *dev;
> - } parallel;
> + ISAParallelState parallel;
>
> struct {
> ISADevice *dev;
