Re: [PATCH v3 19/22] ppc/ppc405: QOM'ify FPGA

[Cédric Le Goater]

On 8/8/22 16:55, BALATON Zoltan wrote:
> On Mon, 8 Aug 2022, Cédric Le Goater wrote:
> > Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> > hw/ppc/ppc405_boards.c | 55 +++++++++++++++++++++++++++++-------------
> > 1 file changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> > index 5ba12d60bc00..195aa58c35ad 100644
> > --- a/hw/ppc/ppc405_boards.c
> > +++ b/hw/ppc/ppc405_boards.c
> > @@ -71,18 +71,23 @@ struct Ppc405MachineState {
> >  * - NVRAM (0xF0000000)
> >  * - FPGA  (0xF0300000)
> >  */
> > -typedef struct ref405ep_fpga_t ref405ep_fpga_t;
> > -struct ref405ep_fpga_t {
> > +
> > +#define TYPE_PPC405_FPGA "ppc405-fpga"
>
> If this is specific to ref405ep maybe it should be named accordingly.

done.

Thanks,

C.

> Regards,
> BALATON Zoltan
>
> > +OBJECT_DECLARE_SIMPLE_TYPE(Ppc405FpgaState, PPC405_FPGA);
> > +struct Ppc405FpgaState {
> > +    SysBusDevice parent_obj;
> > +
> > +    MemoryRegion iomem;
> > +
> >     uint8_t reg0;
> >     uint8_t reg1;
> > };
> >
> > static uint64_t ref405ep_fpga_readb(void *opaque, hwaddr addr, unsigned size)
> > {
> > -    ref405ep_fpga_t *fpga;
> > +    Ppc405FpgaState *fpga = PPC405_FPGA(opaque);
> >     uint32_t ret;
> >
> > -    fpga = opaque;
> >     switch (addr) {
> >     case 0x0:
> >         ret = fpga->reg0;
> > @@ -101,9 +106,8 @@ static uint64_t ref405ep_fpga_readb(void *opaque, hwaddr 
> > addr, unsigned size)
> > static void ref405ep_fpga_writeb(void *opaque, hwaddr addr, uint64_t value,
> >                                  unsigned size)
> > {
> > -    ref405ep_fpga_t *fpga;
> > +    Ppc405FpgaState *fpga = PPC405_FPGA(opaque);
> >
> > -    fpga = opaque;
> >     switch (addr) {
> >     case 0x0:
> >         /* Read only */
> > @@ -126,27 +130,39 @@ static const MemoryRegionOps ref405ep_fpga_ops = {
> >     .endianness = DEVICE_BIG_ENDIAN,
> > };
> >
> > -static void ref405ep_fpga_reset (void *opaque)
> > +static void ref405ep_fpga_reset(DeviceState *dev)
> > {
> > -    ref405ep_fpga_t *fpga;
> > +    Ppc405FpgaState *fpga = PPC405_FPGA(dev);
> >
> > -    fpga = opaque;
> >     fpga->reg0 = 0x00;
> >     fpga->reg1 = 0x0F;
> > }
> >
> > -static void ref405ep_fpga_init(MemoryRegion *sysmem, uint32_t base)
> > +static void ref405ep_fpga_realize(DeviceState *dev, Error **errp)
> > {
> > -    ref405ep_fpga_t *fpga;
> > -    MemoryRegion *fpga_memory = g_new(MemoryRegion, 1);
> > +    Ppc405FpgaState *s = PPC405_FPGA(dev);
> >
> > -    fpga = g_new0(ref405ep_fpga_t, 1);
> > -    memory_region_init_io(fpga_memory, NULL, &ref405ep_fpga_ops, fpga,
> > +    memory_region_init_io(&s->iomem, OBJECT(s), &ref405ep_fpga_ops, s,
> >                           "fpga", 0x00000100);
> > -    memory_region_add_subregion(sysmem, base, fpga_memory);
> > -    qemu_register_reset(&ref405ep_fpga_reset, fpga);
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
> > +}
> > +
> > +static void ref405ep_fpga_class_init(ObjectClass *oc, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(oc);
> > +
> > +    dc->realize = ref405ep_fpga_realize;
> > +    dc->user_creatable = false;
> > +    dc->reset = ref405ep_fpga_reset;
> > }
> >
> > +static const TypeInfo ref405ep_fpga_type = {
> > +    .name = TYPE_PPC405_FPGA,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(Ppc405FpgaState),
> > +    .class_init = ref405ep_fpga_class_init,
> > +};
> > +
> > /*
> >  * CPU reset handler when booting directly from a loaded kernel
> >  */
> > @@ -332,7 +348,11 @@ static void ref405ep_init(MachineState *machine)
> >     memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
> >
> >     /* Register FPGA */
> > -    ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
> > +    dev = qdev_new(TYPE_PPC405_FPGA);
> > +    object_property_add_child(OBJECT(machine), "fpga", OBJECT(dev));
> > +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> > +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, PPC405EP_FPGA_BASE);
> > +
> >     /* Register NVRAM */
> >     dev = qdev_new("sysbus-m48t08");
> >     qdev_prop_set_int32(dev, "base-year", 1968);
> > @@ -377,6 +397,7 @@ static void ppc405_machine_init(void)
> > {
> >     type_register_static(&ppc405_machine_type);
> >     type_register_static(&ref405ep_type);
> > +    type_register_static(&ref405ep_fpga_type);
> > }
> >
> > type_init(ppc405_machine_init)