Re: [PATCH v2 11/34] next-cube: move ESCC to be QOM child of next-pc device

[Mark Cave-Ayland]

On 14/12/2024 06:10, Thomas Huth wrote:

> Am Thu, 12 Dec 2024 11:45:57 +0000
> schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>
> > Since the ESCC is part of the next-pc device, move the ESCC to be a QOM child
> > of the next-pc device.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/m68k/next-cube.c | 54 ++++++++++++++++++++++-----------------------
> >   1 file changed, 26 insertions(+), 28 deletions(-)
> >
> > diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> > index efbb11e74b..20a0b073e1 100644
> > --- a/hw/m68k/next-cube.c
> > +++ b/hw/m68k/next-cube.c
> > @@ -124,6 +124,8 @@ struct NeXTPC {
> >       qemu_irq scsi_reset;
> >       qemu_irq scsi_dma;
> >   
> > +    ESCCState escc;
> > +
> >       NextRtc rtc;
> >   };
> >   
> > @@ -978,31 +980,6 @@ static const MemoryRegionOps next_floppy_ops = {
> >       .endianness = DEVICE_BIG_ENDIAN,
> >   };
> >   
> > -static void next_escc_init(DeviceState *pcdev)
> > -{
> > -    NeXTPC *next_pc = NEXT_PC(pcdev);
> > -    DeviceState *dev;
> > -    SysBusDevice *s;
> > -
> > -    dev = qdev_new(TYPE_ESCC);
> > -    qdev_prop_set_uint32(dev, "disabled", 0);
> > -    qdev_prop_set_uint32(dev, "frequency", 9600 * 384);
> > -    qdev_prop_set_uint32(dev, "it_shift", 0);
> > -    qdev_prop_set_bit(dev, "bit_swap", true);
> > -    qdev_prop_set_chr(dev, "chrB", serial_hd(1));
> > -    qdev_prop_set_chr(dev, "chrA", serial_hd(0));
> > -    qdev_prop_set_uint32(dev, "chnBtype", escc_serial);
> > -    qdev_prop_set_uint32(dev, "chnAtype", escc_serial);
> > -
> > -    s = SYS_BUS_DEVICE(dev);
> > -    sysbus_realize_and_unref(s, &error_fatal);
> > -    sysbus_connect_irq(s, 0, qdev_get_gpio_in(pcdev, NEXT_SCC_I));
> > -    sysbus_connect_irq(s, 1, qdev_get_gpio_in(pcdev, NEXT_SCC_DMA_I));
> > -
> > -    memory_region_add_subregion(&next_pc->scrmem, 0x18000,
> > -                                sysbus_mmio_get_region(s, 0));
> > -}
> > -
> >   static void next_pc_reset(DeviceState *dev)
> >   {
> >       NeXTPC *s = NEXT_PC(dev);
> > @@ -1043,6 +1020,28 @@ static void next_pc_realize(DeviceState *dev, Error 
> > **errp)
> >       /* Floppy */
> >       memory_region_add_subregion(&s->scrmem, 0x14108,
> >                                   &s->floppy_mem);
> > +
> > +    /* ESCC */
> > +    d = DEVICE(object_resolve_path_component(OBJECT(dev), "escc"));
>
> Can't you use s->escc directly here instead of taking the detour via
> object_resolve_path_component() ?

Indeed, its only there for the same reason as my previous email. I'll update it 
for v3.

> > +    qdev_prop_set_uint32(d, "disabled", 0);
> > +    qdev_prop_set_uint32(d, "frequency", 9600 * 384);
> > +    qdev_prop_set_uint32(d, "it_shift", 0);
> > +    qdev_prop_set_bit(d, "bit_swap", true);
> > +    qdev_prop_set_chr(d, "chrB", serial_hd(1));
> > +    qdev_prop_set_chr(d, "chrA", serial_hd(0));
> > +    qdev_prop_set_uint32(d, "chnBtype", escc_serial);
> > +    qdev_prop_set_uint32(d, "chnAtype", escc_serial);
> > +
> > +    sbd = SYS_BUS_DEVICE(d);
> > +    if (!sysbus_realize(sbd, errp)) {
> > +        return;
> > +    }
> > +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(dev, NEXT_SCC_I));
> > +    sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(dev, NEXT_SCC_DMA_I));
> > +
> > +    memory_region_add_subregion(&s->scrmem, 0x18000,
> > +                                sysbus_mmio_get_region(sbd, 0));
> > +
> >   }

ATB,

Mark.