diff mbox series

[v4,19/24] ppc/ppc405: QOM'ify FPGA

Message ID 20220809153904.485018-20-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series ppc: QOM'ify 405 board | expand

Commit Message

Cédric Le Goater Aug. 9, 2022, 3:38 p.m. UTC 
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(-)

Comments

BALATON Zoltan Aug. 9, 2022, 5:37 p.m. UTC | #1 
On Tue, 9 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 3677793adc75..4ff6715f3533 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_REF405EP_FPGA "ref405ep-fpga"
> +OBJECT_DECLARE_SIMPLE_TYPE(Ref405epFpgaState, REF405EP_FPGA);
> +struct Ref405epFpgaState {
> +    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;
> +    Ref405epFpgaState *fpga = REF405EP_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;
> +    Ref405epFpgaState *fpga = REF405EP_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;
> +    Ref405epFpgaState *fpga = REF405EP_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);
> +    Ref405epFpgaState *s = REF405EP_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;

Comment missing (and I'd drop unnecessary QOM casts) but otherwise:

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Regards,
BALATON Zoltan

> +    dc->reset = ref405ep_fpga_reset;
> }
>
> +static const TypeInfo ref405ep_fpga_type = {
> +    .name = TYPE_REF405EP_FPGA,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(Ref405epFpgaState),
> +    .class_init = ref405ep_fpga_class_init,
> +};
> +
> /*
>  * CPU reset handler when booting directly from a loaded kernel
>  */
> @@ -331,7 +347,11 @@ static void ref405ep_init(MachineState *machine)
>     memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, sram);
>
>     /* Register FPGA */
> -    ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
> +    dev = qdev_new(TYPE_REF405EP_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);
> @@ -376,6 +396,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)
>
Cédric Le Goater Aug. 10, 2022, 6:22 a.m. UTC | #2 
On 8/9/22 19:37, BALATON Zoltan wrote:
> On Tue, 9 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 3677793adc75..4ff6715f3533 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_REF405EP_FPGA "ref405ep-fpga"
>> +OBJECT_DECLARE_SIMPLE_TYPE(Ref405epFpgaState, REF405EP_FPGA);
>> +struct Ref405epFpgaState {
>> +    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;
>> +    Ref405epFpgaState *fpga = REF405EP_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;
>> +    Ref405epFpgaState *fpga = REF405EP_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;
>> +    Ref405epFpgaState *fpga = REF405EP_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);
>> +    Ref405epFpgaState *s = REF405EP_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;
> 
> Comment missing (and I'd drop unnecessary QOM casts) but otherwise:

Ah yes. I don't think it is worth a v5 for that. I will send a patch
fixing the missing comments. There are a few: SoC, MAL, FPGA, SDRAM.
Unless Daniel is willing to amend the patches.

Thanks,

C.

> 
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
> 
> Regards,
> BALATON Zoltan
> 
>> +    dc->reset = ref405ep_fpga_reset;
>> }
>>
>> +static const TypeInfo ref405ep_fpga_type = {
>> +    .name = TYPE_REF405EP_FPGA,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(Ref405epFpgaState),
>> +    .class_init = ref405ep_fpga_class_init,
>> +};
>> +
>> /*
>>  * CPU reset handler when booting directly from a loaded kernel
>>  */
>> @@ -331,7 +347,11 @@ static void ref405ep_init(MachineState *machine)
>>     memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, sram);
>>
>>     /* Register FPGA */
>> -    ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
>> +    dev = qdev_new(TYPE_REF405EP_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);
>> @@ -376,6 +396,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)
>>
BALATON Zoltan Aug. 10, 2022, 11:40 a.m. UTC | #3 
On Wed, 10 Aug 2022, Cédric Le Goater wrote:
> On 8/9/22 19:37, BALATON Zoltan wrote:
>> On Tue, 9 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 3677793adc75..4ff6715f3533 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_REF405EP_FPGA "ref405ep-fpga"
>>> +OBJECT_DECLARE_SIMPLE_TYPE(Ref405epFpgaState, REF405EP_FPGA);
>>> +struct Ref405epFpgaState {
>>> +    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;
>>> +    Ref405epFpgaState *fpga = REF405EP_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;
>>> +    Ref405epFpgaState *fpga = REF405EP_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;
>>> +    Ref405epFpgaState *fpga = REF405EP_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);
>>> +    Ref405epFpgaState *s = REF405EP_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;
>> 
>> Comment missing (and I'd drop unnecessary QOM casts) but otherwise:
>
> Ah yes. I don't think it is worth a v5 for that. I will send a patch
> fixing the missing comments. There are a few: SoC, MAL, FPGA, SDRAM.
> Unless Daniel is willing to amend the patches.

I think you'll need a v5 for other changes anyway so it's easier if you 
update it too.

Regards,
BALATON Zoltan

> Thanks,
>
> C.
>
>> 
>> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>> 
>> Regards,
>> BALATON Zoltan
>> 
>>> +    dc->reset = ref405ep_fpga_reset;
>>> }
>>> 
>>> +static const TypeInfo ref405ep_fpga_type = {
>>> +    .name = TYPE_REF405EP_FPGA,
>>> +    .parent = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(Ref405epFpgaState),
>>> +    .class_init = ref405ep_fpga_class_init,
>>> +};
>>> +
>>> /*
>>>  * CPU reset handler when booting directly from a loaded kernel
>>>  */
>>> @@ -331,7 +347,11 @@ static void ref405ep_init(MachineState *machine)
>>>     memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, 
>>> sram);
>>> 
>>>     /* Register FPGA */
>>> -    ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
>>> +    dev = qdev_new(TYPE_REF405EP_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);
>>> @@ -376,6 +396,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)
>>> 
>
>
>
Daniel Henrique Barboza Aug. 10, 2022, 5:22 p.m. UTC | #4 
On 8/9/22 14:37, BALATON Zoltan wrote:
> On Tue, 9 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 3677793adc75..4ff6715f3533 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_REF405EP_FPGA "ref405ep-fpga"
>> +OBJECT_DECLARE_SIMPLE_TYPE(Ref405epFpgaState, REF405EP_FPGA);
>> +struct Ref405epFpgaState {
>> +    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;
>> +    Ref405epFpgaState *fpga = REF405EP_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;
>> +    Ref405epFpgaState *fpga = REF405EP_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;
>> +    Ref405epFpgaState *fpga = REF405EP_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);
>> +    Ref405epFpgaState *s = REF405EP_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;
> 
> Comment missing (and I'd drop unnecessary QOM casts) but otherwise:

Which QOM casts are you referring to?


Daniel

> 
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
> 
> Regards,
> BALATON Zoltan
> 
>> +    dc->reset = ref405ep_fpga_reset;
>> }
>>
>> +static const TypeInfo ref405ep_fpga_type = {
>> +    .name = TYPE_REF405EP_FPGA,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(Ref405epFpgaState),
>> +    .class_init = ref405ep_fpga_class_init,
>> +};
>> +
>> /*
>>  * CPU reset handler when booting directly from a loaded kernel
>>  */
>> @@ -331,7 +347,11 @@ static void ref405ep_init(MachineState *machine)
>>     memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, sram);
>>
>>     /* Register FPGA */
>> -    ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
>> +    dev = qdev_new(TYPE_REF405EP_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);
>> @@ -376,6 +396,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)
>>
BALATON Zoltan Aug. 10, 2022, 5:32 p.m. UTC | #5 
On Wed, 10 Aug 2022, Daniel Henrique Barboza wrote:
> On 8/9/22 14:37, BALATON Zoltan wrote:
>> On Tue, 9 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 3677793adc75..4ff6715f3533 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_REF405EP_FPGA "ref405ep-fpga"
>>> +OBJECT_DECLARE_SIMPLE_TYPE(Ref405epFpgaState, REF405EP_FPGA);
>>> +struct Ref405epFpgaState {
>>> +    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;
>>> +    Ref405epFpgaState *fpga = REF405EP_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;
>>> +    Ref405epFpgaState *fpga = REF405EP_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;
>>> +    Ref405epFpgaState *fpga = REF405EP_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);
>>> +    Ref405epFpgaState *s = REF405EP_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;
>> 
>> Comment missing (and I'd drop unnecessary QOM casts) but otherwise:
>
> Which QOM casts are you referring to?

Those that I've discussed in other patches, it's all QOMify patches not 
only this one. Cédric knows what I mean. But there are other small chnages 
elsewhere that probably need another version anyway so I think you don't 
need to do anything with this patch.

Regards,
BALATON Zoltan

>
> Daniel
>
>> 
>> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>> 
>> Regards,
>> BALATON Zoltan
>> 
>>> +    dc->reset = ref405ep_fpga_reset;
>>> }
>>> 
>>> +static const TypeInfo ref405ep_fpga_type = {
>>> +    .name = TYPE_REF405EP_FPGA,
>>> +    .parent = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(Ref405epFpgaState),
>>> +    .class_init = ref405ep_fpga_class_init,
>>> +};
>>> +
>>> /*
>>>  * CPU reset handler when booting directly from a loaded kernel
>>>  */
>>> @@ -331,7 +347,11 @@ static void ref405ep_init(MachineState *machine)
>>>     memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, 
>>> sram);
>>> 
>>>     /* Register FPGA */
>>> -    ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
>>> +    dev = qdev_new(TYPE_REF405EP_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);
>>> @@ -376,6 +396,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)
>>> 
>
>
diff mbox series

Patch

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 3677793adc75..4ff6715f3533 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_REF405EP_FPGA "ref405ep-fpga"
+OBJECT_DECLARE_SIMPLE_TYPE(Ref405epFpgaState, REF405EP_FPGA);
+struct Ref405epFpgaState {
+    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;
+    Ref405epFpgaState *fpga = REF405EP_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;
+    Ref405epFpgaState *fpga = REF405EP_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;
+    Ref405epFpgaState *fpga = REF405EP_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);
+    Ref405epFpgaState *s = REF405EP_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_REF405EP_FPGA,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(Ref405epFpgaState),
+    .class_init = ref405ep_fpga_class_init,
+};
+
 /*
  * CPU reset handler when booting directly from a loaded kernel
  */
@@ -331,7 +347,11 @@  static void ref405ep_init(MachineState *machine)
     memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE, sram);
 
     /* Register FPGA */
-    ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
+    dev = qdev_new(TYPE_REF405EP_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);
@@ -376,6 +396,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)