| Message ID | 20240923093016.66437-7-shentey@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | E500 Cleanup | expand |
On Mon, 23 Sep 2024, Bernhard Beschow wrote: > Rather than accessing the attributes of TYPE_CCSR directly, use the SysBusDevice > API which exists exactly for that purpose. Furthermore, registering the memory > region with the SysBusDevice API makes it show up in QMP's `info qom-tree` > command. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/pci-host/ppce500.c | 10 +++++----- > hw/ppc/e500.c | 8 ++++---- > 2 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c > index 95b983b2b3..97e5d47cec 100644 > --- a/hw/pci-host/ppce500.c > +++ b/hw/pci-host/ppce500.c > @@ -16,7 +16,6 @@ > > #include "qemu/osdep.h" > #include "hw/irq.h" > -#include "hw/ppc/e500-ccsr.h" > #include "hw/qdev-properties.h" > #include "migration/vmstate.h" > #include "hw/pci/pci_device.h" > @@ -419,11 +418,12 @@ static const VMStateDescription vmstate_ppce500_pci = { > static void e500_pcihost_bridge_realize(PCIDevice *d, Error **errp) > { > PPCE500PCIBridgeState *b = PPC_E500_PCI_BRIDGE(d); > - PPCE500CCSRState *ccsr = CCSR(container_get(qdev_get_machine(), > - "/e500-ccsr")); > + SysBusDevice *ccsr = SYS_BUS_DEVICE(container_get(qdev_get_machine(), > + "/e500-ccsr")); > + MemoryRegion *ccsr_space = sysbus_mmio_get_region(ccsr, 0); > > - memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0", &ccsr->ccsr_space, > - 0, int128_get64(ccsr->ccsr_space.size)); > + memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0", > + ccsr_space, 0, int128_get64(ccsr_space->size)); I wonder if this really needs an alias or could the original memory region be registered as the PCI BAR region? Otherwise: Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> Regards, BALATON Zoltan > pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0); > } > > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index e2a4f265a5..2225533e33 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -924,7 +924,6 @@ void ppce500_init(MachineState *machine) > DriveInfo *dinfo; > MemoryRegion *ccsr_addr_space; > SysBusDevice *s; > - PPCE500CCSRState *ccsr; > I2CBus *i2c; > > irqs = g_new0(IrqLines, smp_cpus); > @@ -980,10 +979,10 @@ void ppce500_init(MachineState *machine) > memory_region_add_subregion(address_space_mem, 0, machine->ram); > > dev = qdev_new("e500-ccsr"); > + s = SYS_BUS_DEVICE(dev); > object_property_add_child(OBJECT(machine), "e500-ccsr", OBJECT(dev)); > - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > - ccsr = CCSR(dev); > - ccsr_addr_space = &ccsr->ccsr_space; > + sysbus_realize_and_unref(s, &error_fatal); > + ccsr_addr_space = sysbus_mmio_get_region(s, 0); > memory_region_add_subregion(address_space_mem, pmc->ccsrbar_base, > ccsr_addr_space); > > @@ -1270,6 +1269,7 @@ static void e500_ccsr_initfn(Object *obj) > PPCE500CCSRState *ccsr = CCSR(obj); > memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr", > MPC8544_CCSRBAR_SIZE); > + sysbus_init_mmio(SYS_BUS_DEVICE(ccsr), &ccsr->ccsr_space); > } > > static const TypeInfo e500_ccsr_info = { >
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c index 95b983b2b3..97e5d47cec 100644 --- a/hw/pci-host/ppce500.c +++ b/hw/pci-host/ppce500.c @@ -16,7 +16,6 @@ #include "qemu/osdep.h" #include "hw/irq.h" -#include "hw/ppc/e500-ccsr.h" #include "hw/qdev-properties.h" #include "migration/vmstate.h" #include "hw/pci/pci_device.h" @@ -419,11 +418,12 @@ static const VMStateDescription vmstate_ppce500_pci = { static void e500_pcihost_bridge_realize(PCIDevice *d, Error **errp) { PPCE500PCIBridgeState *b = PPC_E500_PCI_BRIDGE(d); - PPCE500CCSRState *ccsr = CCSR(container_get(qdev_get_machine(), - "/e500-ccsr")); + SysBusDevice *ccsr = SYS_BUS_DEVICE(container_get(qdev_get_machine(), + "/e500-ccsr")); + MemoryRegion *ccsr_space = sysbus_mmio_get_region(ccsr, 0); - memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0", &ccsr->ccsr_space, - 0, int128_get64(ccsr->ccsr_space.size)); + memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0", + ccsr_space, 0, int128_get64(ccsr_space->size)); pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0); } diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index e2a4f265a5..2225533e33 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -924,7 +924,6 @@ void ppce500_init(MachineState *machine) DriveInfo *dinfo; MemoryRegion *ccsr_addr_space; SysBusDevice *s; - PPCE500CCSRState *ccsr; I2CBus *i2c; irqs = g_new0(IrqLines, smp_cpus); @@ -980,10 +979,10 @@ void ppce500_init(MachineState *machine) memory_region_add_subregion(address_space_mem, 0, machine->ram); dev = qdev_new("e500-ccsr"); + s = SYS_BUS_DEVICE(dev); object_property_add_child(OBJECT(machine), "e500-ccsr", OBJECT(dev)); - sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); - ccsr = CCSR(dev); - ccsr_addr_space = &ccsr->ccsr_space; + sysbus_realize_and_unref(s, &error_fatal); + ccsr_addr_space = sysbus_mmio_get_region(s, 0); memory_region_add_subregion(address_space_mem, pmc->ccsrbar_base, ccsr_addr_space); @@ -1270,6 +1269,7 @@ static void e500_ccsr_initfn(Object *obj) PPCE500CCSRState *ccsr = CCSR(obj); memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr", MPC8544_CCSRBAR_SIZE); + sysbus_init_mmio(SYS_BUS_DEVICE(ccsr), &ccsr->ccsr_space); } static const TypeInfo e500_ccsr_info = {
Rather than accessing the attributes of TYPE_CCSR directly, use the SysBusDevice API which exists exactly for that purpose. Furthermore, registering the memory region with the SysBusDevice API makes it show up in QMP's `info qom-tree` command. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/pci-host/ppce500.c | 10 +++++----- hw/ppc/e500.c | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-)