All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/4] ppc440_pcix: Stop using system io region for PCI bus
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-03 16:48 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Reduce the iomem region to 64K and use it for the PCI io space and map
it directly from the board without an intermediate alias that is not
really needed.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/ppc440_pcix.c | 9 ++++++---
 hw/ppc/sam460ex.c    | 6 +-----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 899558b055..54286cfef4 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -23,6 +23,7 @@
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qemu/units.h"
 #include "hw/irq.h"
 #include "hw/ppc/ppc.h"
 #include "hw/ppc/ppc4xx.h"
@@ -491,10 +492,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
     s = PPC440_PCIX_HOST_BRIDGE(dev);
 
     sysbus_init_irq(sbd, &s->irq);
-    memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
+    memory_region_init(&s->busmem, OBJECT(dev), "pci-mem", UINT64_MAX);
+    memory_region_init(&s->iomem, OBJECT(dev), "pci-io", 64 * KiB);
     h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
-                         ppc440_pcix_map_irq, &s->irq, &s->busmem,
-                         get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
+                         ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
+                         PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
 
     s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
 
@@ -514,6 +516,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem);
     memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->regs);
     sysbus_init_mmio(sbd, &s->container);
+    sysbus_init_mmio(sbd, &s->iomem);
 }
 
 static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index d446cfc37b..7da38bd58e 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -269,7 +269,6 @@ static void main_cpu_reset(void *opaque)
 
 static void sam460ex_init(MachineState *machine)
 {
-    MemoryRegion *isa = g_new(MemoryRegion, 1);
     MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
     DeviceState *uic[4];
     int i;
@@ -441,12 +440,9 @@ static void sam460ex_init(MachineState *machine)
     /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
     dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
                                qdev_get_gpio_in(uic[1], 0));
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, 0xc08000000);
     pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
 
-    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
-                             0, 0x10000);
-    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
-
     /* PCI devices */
     pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
     /* SoC has a single SATA port but we don't emulate that yet
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/4] ppc4xx_pci: Rename QOM type name define
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-03 21:09 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Rename the TYPE_PPC4xx_PCI_HOST_BRIDGE define and its string value to
match each other and other similar types and to avoid confusion with
"ppc4xx-host-bridge" type defined in same file.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_bamboo.c  | 3 +--
 hw/ppc/ppc4xx_pci.c     | 6 +++---
 include/hw/ppc/ppc4xx.h | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index f061b8cf3b..45f409c838 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -205,8 +205,7 @@ static void bamboo_init(MachineState *machine)
     ppc4xx_sdram_ddr_enable(PPC4xx_SDRAM_DDR(dev));
 
     /* PCI */
-    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST_BRIDGE,
-                                PPC440EP_PCI_CONFIG,
+    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST, PPC440EP_PCI_CONFIG,
                                 qdev_get_gpio_in(uicdev, pci_irq_nrs[0]),
                                 qdev_get_gpio_in(uicdev, pci_irq_nrs[1]),
                                 qdev_get_gpio_in(uicdev, pci_irq_nrs[2]),
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 1d4a50fa7c..fbdf8266d8 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -46,7 +46,7 @@ struct PCITargetMap {
     uint32_t la;
 };
 
-OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST_BRIDGE)
+OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST)
 
 #define PPC4xx_PCI_NR_PMMS 3
 #define PPC4xx_PCI_NR_PTMS 2
@@ -321,7 +321,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error **errp)
     int i;
 
     h = PCI_HOST_BRIDGE(dev);
-    s = PPC4xx_PCI_HOST_BRIDGE(dev);
+    s = PPC4xx_PCI_HOST(dev);
 
     for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
         sysbus_init_irq(sbd, &s->irq[i]);
@@ -386,7 +386,7 @@ static void ppc4xx_pcihost_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ppc4xx_pcihost_info = {
-    .name          = TYPE_PPC4xx_PCI_HOST_BRIDGE,
+    .name          = TYPE_PPC4xx_PCI_HOST,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(PPC4xxPCIState),
     .class_init    = ppc4xx_pcihost_class_init,
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 39ca602442..e053b9751b 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,7 +29,7 @@
 #include "exec/memory.h"
 #include "hw/sysbus.h"
 
-#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
+#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
 #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
 
 /*
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/4] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-03 21:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Add a QOM type name define for ppc4xx-host-bridge in the common header
and replace direct use of the string name with the constant.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_pcix.c    | 3 ++-
 hw/ppc/ppc4xx_pci.c     | 4 ++--
 include/hw/ppc/ppc4xx.h | 1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 54286cfef4..9321ca0abd 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -498,7 +498,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
                          ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
                          PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
 
-    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
+    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
+                               TYPE_PPC4xx_HOST_BRIDGE);
 
     memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
     memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index fbdf8266d8..6652119008 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -333,7 +333,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error **errp)
                               TYPE_PCI_BUS);
     h->bus = b;
 
-    pci_create_simple(b, 0, "ppc4xx-host-bridge");
+    pci_create_simple(b, 0, TYPE_PPC4xx_HOST_BRIDGE);
 
     /* XXX split into 2 memory regions, one for config space, one for regs */
     memory_region_init(&s->container, OBJECT(s), "pci-container", PCI_ALL_SIZE);
@@ -367,7 +367,7 @@ static void ppc4xx_host_bridge_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ppc4xx_host_bridge_info = {
-    .name          = "ppc4xx-host-bridge",
+    .name          = TYPE_PPC4xx_HOST_BRIDGE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCIDevice),
     .class_init    = ppc4xx_host_bridge_class_init,
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index e053b9751b..766d575e86 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
 #include "exec/memory.h"
 #include "hw/sysbus.h"
 
+#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
 #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
 #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
 
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 4/4] ppc440_pcix: Rename QOM type define abd move it to common header
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-03 21:21 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Rename TYPE_PPC440_PCIX_HOST_BRIDGE to better match its string value,
move it to common header and use it also in sam460ex to replace hard
coded type name.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_pcix.c    | 9 ++++-----
 hw/ppc/sam460ex.c       | 2 +-
 include/hw/ppc/ppc4xx.h | 1 +
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 9321ca0abd..672090de94 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -45,8 +45,7 @@ struct PLBInMap {
     MemoryRegion mr;
 };
 
-#define TYPE_PPC440_PCIX_HOST_BRIDGE "ppc440-pcix-host"
-OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST_BRIDGE)
+OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST)
 
 #define PPC440_PCIX_NR_POMS 3
 #define PPC440_PCIX_NR_PIMS 3
@@ -399,7 +398,7 @@ static const MemoryRegionOps pci_reg_ops = {
 
 static void ppc440_pcix_reset(DeviceState *dev)
 {
-    struct PPC440PCIXState *s = PPC440_PCIX_HOST_BRIDGE(dev);
+    struct PPC440PCIXState *s = PPC440_PCIX_HOST(dev);
     int i;
 
     for (i = 0; i < PPC440_PCIX_NR_POMS; i++) {
@@ -489,7 +488,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
     PCIHostState *h;
 
     h = PCI_HOST_BRIDGE(dev);
-    s = PPC440_PCIX_HOST_BRIDGE(dev);
+    s = PPC440_PCIX_HOST(dev);
 
     sysbus_init_irq(sbd, &s->irq);
     memory_region_init(&s->busmem, OBJECT(dev), "pci-mem", UINT64_MAX);
@@ -529,7 +528,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ppc440_pcix_info = {
-    .name          = TYPE_PPC440_PCIX_HOST_BRIDGE,
+    .name          = TYPE_PPC440_PCIX_HOST,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(PPC440PCIXState),
     .class_init    = ppc440_pcix_class_init,
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 7da38bd58e..1e615b8d35 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -438,7 +438,7 @@ static void sam460ex_init(MachineState *machine)
 
     /* PCI bus */
     /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
-    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
+    dev = sysbus_create_simple(TYPE_PPC440_PCIX_HOST, 0xc0ec00000,
                                qdev_get_gpio_in(uic[1], 0));
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, 0xc08000000);
     pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 766d575e86..ea7740239b 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -31,6 +31,7 @@
 
 #define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
 #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
+#define TYPE_PPC440_PCIX_HOST "ppc440-pcix-host"
 #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
 
 /*
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 0/4] PPC440 devices misc clean up
@ 2023-07-06 11:16 ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-06 11:07 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

These are some small misc clean ups to PPC440 related device models
which is all I have ready for now.

v3:
- rebased on ppc-next moving already reviewed patch to front

v2:
- Added R-b tags from Philippe
- Addressed review comments
- Added new patch to rename parent field of PPC460EXPCIEState to parent_obj

BALATON Zoltan (4):
  ppc440_pcix: Stop using system io region for PCI bus
  ppc4xx_pci: Rename QOM type name define
  ppc4xx_pci: Add define for ppc4xx-host-bridge type name
  ppc440_pcix: Rename QOM type define abd move it to common header

 hw/ppc/ppc440_bamboo.c  |  3 +--
 hw/ppc/ppc440_pcix.c    | 21 ++++++++++++---------
 hw/ppc/ppc4xx_pci.c     | 10 +++++-----
 hw/ppc/sam460ex.c       |  8 ++------
 include/hw/ppc/ppc4xx.h |  4 +++-
 5 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.30.9



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RESEND PATCH v3 0/4] PPC440 devices misc clean up
@ 2023-07-06 11:16 ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-06 11:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

These are some small misc clean ups to PPC440 related device models
which is all I have ready for now.

v3:
- rebased on ppc-next moving already reviewed patch to front

v2:
- Added R-b tags from Philippe
- Addressed review comments
- Added new patch to rename parent field of PPC460EXPCIEState to parent_obj

BALATON Zoltan (4):
  ppc440_pcix: Stop using system io region for PCI bus
  ppc4xx_pci: Rename QOM type name define
  ppc4xx_pci: Add define for ppc4xx-host-bridge type name
  ppc440_pcix: Rename QOM type define abd move it to common header

 hw/ppc/ppc440_bamboo.c  |  3 +--
 hw/ppc/ppc440_pcix.c    | 21 ++++++++++++---------
 hw/ppc/ppc4xx_pci.c     | 10 +++++-----
 hw/ppc/sam460ex.c       |  8 ++------
 include/hw/ppc/ppc4xx.h |  4 +++-
 5 files changed, 23 insertions(+), 23 deletions(-)

-- 
2.30.9



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [RESEND PATCH v3 1/4] ppc440_pcix: Stop using system io region for PCI bus
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-06 11:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Reduce the iomem region to 64K and use it for the PCI io space and map
it directly from the board without an intermediate alias that is not
really needed.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/ppc/ppc440_pcix.c | 9 ++++++---
 hw/ppc/sam460ex.c    | 6 +-----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 899558b055..54286cfef4 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -23,6 +23,7 @@
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qemu/units.h"
 #include "hw/irq.h"
 #include "hw/ppc/ppc.h"
 #include "hw/ppc/ppc4xx.h"
@@ -491,10 +492,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
     s = PPC440_PCIX_HOST_BRIDGE(dev);
 
     sysbus_init_irq(sbd, &s->irq);
-    memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
+    memory_region_init(&s->busmem, OBJECT(dev), "pci-mem", UINT64_MAX);
+    memory_region_init(&s->iomem, OBJECT(dev), "pci-io", 64 * KiB);
     h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
-                         ppc440_pcix_map_irq, &s->irq, &s->busmem,
-                         get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
+                         ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
+                         PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
 
     s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
 
@@ -514,6 +516,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem);
     memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->regs);
     sysbus_init_mmio(sbd, &s->container);
+    sysbus_init_mmio(sbd, &s->iomem);
 }
 
 static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index d446cfc37b..7da38bd58e 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -269,7 +269,6 @@ static void main_cpu_reset(void *opaque)
 
 static void sam460ex_init(MachineState *machine)
 {
-    MemoryRegion *isa = g_new(MemoryRegion, 1);
     MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
     DeviceState *uic[4];
     int i;
@@ -441,12 +440,9 @@ static void sam460ex_init(MachineState *machine)
     /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
     dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
                                qdev_get_gpio_in(uic[1], 0));
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, 0xc08000000);
     pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
 
-    memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
-                             0, 0x10000);
-    memory_region_add_subregion(get_system_memory(), 0xc08000000, isa);
-
     /* PCI devices */
     pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501");
     /* SoC has a single SATA port but we don't emulate that yet
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RESEND PATCH v3 2/4] ppc4xx_pci: Rename QOM type name define
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-06 11:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Rename the TYPE_PPC4xx_PCI_HOST_BRIDGE define and its string value to
match each other and other similar types and to avoid confusion with
"ppc4xx-host-bridge" type defined in same file.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_bamboo.c  | 3 +--
 hw/ppc/ppc4xx_pci.c     | 6 +++---
 include/hw/ppc/ppc4xx.h | 2 +-
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index f061b8cf3b..45f409c838 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -205,8 +205,7 @@ static void bamboo_init(MachineState *machine)
     ppc4xx_sdram_ddr_enable(PPC4xx_SDRAM_DDR(dev));
 
     /* PCI */
-    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST_BRIDGE,
-                                PPC440EP_PCI_CONFIG,
+    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST, PPC440EP_PCI_CONFIG,
                                 qdev_get_gpio_in(uicdev, pci_irq_nrs[0]),
                                 qdev_get_gpio_in(uicdev, pci_irq_nrs[1]),
                                 qdev_get_gpio_in(uicdev, pci_irq_nrs[2]),
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 1d4a50fa7c..fbdf8266d8 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -46,7 +46,7 @@ struct PCITargetMap {
     uint32_t la;
 };
 
-OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST_BRIDGE)
+OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST)
 
 #define PPC4xx_PCI_NR_PMMS 3
 #define PPC4xx_PCI_NR_PTMS 2
@@ -321,7 +321,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error **errp)
     int i;
 
     h = PCI_HOST_BRIDGE(dev);
-    s = PPC4xx_PCI_HOST_BRIDGE(dev);
+    s = PPC4xx_PCI_HOST(dev);
 
     for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
         sysbus_init_irq(sbd, &s->irq[i]);
@@ -386,7 +386,7 @@ static void ppc4xx_pcihost_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ppc4xx_pcihost_info = {
-    .name          = TYPE_PPC4xx_PCI_HOST_BRIDGE,
+    .name          = TYPE_PPC4xx_PCI_HOST,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(PPC4xxPCIState),
     .class_init    = ppc4xx_pcihost_class_init,
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 39ca602442..e053b9751b 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,7 +29,7 @@
 #include "exec/memory.h"
 #include "hw/sysbus.h"
 
-#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
+#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
 #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
 
 /*
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RESEND PATCH v3 3/4] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-06 11:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Add a QOM type name define for ppc4xx-host-bridge in the common header
and replace direct use of the string name with the constant.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_pcix.c    | 3 ++-
 hw/ppc/ppc4xx_pci.c     | 4 ++--
 include/hw/ppc/ppc4xx.h | 1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 54286cfef4..9321ca0abd 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -498,7 +498,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
                          ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
                          PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
 
-    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
+    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
+                               TYPE_PPC4xx_HOST_BRIDGE);
 
     memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
     memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index fbdf8266d8..6652119008 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -333,7 +333,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error **errp)
                               TYPE_PCI_BUS);
     h->bus = b;
 
-    pci_create_simple(b, 0, "ppc4xx-host-bridge");
+    pci_create_simple(b, 0, TYPE_PPC4xx_HOST_BRIDGE);
 
     /* XXX split into 2 memory regions, one for config space, one for regs */
     memory_region_init(&s->container, OBJECT(s), "pci-container", PCI_ALL_SIZE);
@@ -367,7 +367,7 @@ static void ppc4xx_host_bridge_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ppc4xx_host_bridge_info = {
-    .name          = "ppc4xx-host-bridge",
+    .name          = TYPE_PPC4xx_HOST_BRIDGE,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(PCIDevice),
     .class_init    = ppc4xx_host_bridge_class_init,
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index e053b9751b..766d575e86 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -29,6 +29,7 @@
 #include "exec/memory.h"
 #include "hw/sysbus.h"
 
+#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
 #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
 #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
 
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RESEND PATCH v3 4/4] ppc440_pcix: Rename QOM type define abd move it to common header
@ 2023-07-06 11:16   ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-06 11:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza, philmd

Rename TYPE_PPC440_PCIX_HOST_BRIDGE to better match its string value,
move it to common header and use it also in sam460ex to replace hard
coded type name.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_pcix.c    | 9 ++++-----
 hw/ppc/sam460ex.c       | 2 +-
 include/hw/ppc/ppc4xx.h | 1 +
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index 9321ca0abd..672090de94 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -45,8 +45,7 @@ struct PLBInMap {
     MemoryRegion mr;
 };
 
-#define TYPE_PPC440_PCIX_HOST_BRIDGE "ppc440-pcix-host"
-OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST_BRIDGE)
+OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST)
 
 #define PPC440_PCIX_NR_POMS 3
 #define PPC440_PCIX_NR_PIMS 3
@@ -399,7 +398,7 @@ static const MemoryRegionOps pci_reg_ops = {
 
 static void ppc440_pcix_reset(DeviceState *dev)
 {
-    struct PPC440PCIXState *s = PPC440_PCIX_HOST_BRIDGE(dev);
+    struct PPC440PCIXState *s = PPC440_PCIX_HOST(dev);
     int i;
 
     for (i = 0; i < PPC440_PCIX_NR_POMS; i++) {
@@ -489,7 +488,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
     PCIHostState *h;
 
     h = PCI_HOST_BRIDGE(dev);
-    s = PPC440_PCIX_HOST_BRIDGE(dev);
+    s = PPC440_PCIX_HOST(dev);
 
     sysbus_init_irq(sbd, &s->irq);
     memory_region_init(&s->busmem, OBJECT(dev), "pci-mem", UINT64_MAX);
@@ -529,7 +528,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo ppc440_pcix_info = {
-    .name          = TYPE_PPC440_PCIX_HOST_BRIDGE,
+    .name          = TYPE_PPC440_PCIX_HOST,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(PPC440PCIXState),
     .class_init    = ppc440_pcix_class_init,
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 7da38bd58e..1e615b8d35 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -438,7 +438,7 @@ static void sam460ex_init(MachineState *machine)
 
     /* PCI bus */
     /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
-    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
+    dev = sysbus_create_simple(TYPE_PPC440_PCIX_HOST, 0xc0ec00000,
                                qdev_get_gpio_in(uic[1], 0));
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, 0xc08000000);
     pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index 766d575e86..ea7740239b 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -31,6 +31,7 @@
 
 #define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
 #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
+#define TYPE_PPC440_PCIX_HOST "ppc440-pcix-host"
 #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
 
 /*
-- 
2.30.9



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [RESEND PATCH v3 2/4] ppc4xx_pci: Rename QOM type name define
  2023-07-06 11:16   ` [RESEND PATCH " BALATON Zoltan
  (?)
@ 2023-07-06 20:33   ` Daniel Henrique Barboza
  2023-07-06 22:42     ` BALATON Zoltan
  -1 siblings, 1 reply; 15+ messages in thread
From: Daniel Henrique Barboza @ 2023-07-06 20:33 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: philmd



On 7/6/23 08:16, BALATON Zoltan wrote:
> Rename the TYPE_PPC4xx_PCI_HOST_BRIDGE define and its string value to
> match each other and other similar types and to avoid confusion with
> "ppc4xx-host-bridge" type defined in same file.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---

I struggled a bit to understand what's to gain with this change, but it makes
more sense when we consider the changes made in the next patch (where a
TYPE_PPC4xx_HOST_BRIDGE macro is introduced to match the "ppc4xx-host-bridge"
name).

I also understand the comments Phil made in version 1. We have several QOM names
that are too similar ("ppc4xx-pci-host", "ppc4xx-host-bridge" in the next patch,
"ppc440-pcix-host" in patch 4), all of them being PCI host bridges. I am uncertain
whether renaming the QOM name of these devices to make them less similar is worth
it.

Matching the macro names with the actual QOM name is a step in the right direction
though.


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>




>   hw/ppc/ppc440_bamboo.c  | 3 +--
>   hw/ppc/ppc4xx_pci.c     | 6 +++---
>   include/hw/ppc/ppc4xx.h | 2 +-
>   3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index f061b8cf3b..45f409c838 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -205,8 +205,7 @@ static void bamboo_init(MachineState *machine)
>       ppc4xx_sdram_ddr_enable(PPC4xx_SDRAM_DDR(dev));
>   
>       /* PCI */
> -    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST_BRIDGE,
> -                                PPC440EP_PCI_CONFIG,
> +    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST, PPC440EP_PCI_CONFIG,
>                                   qdev_get_gpio_in(uicdev, pci_irq_nrs[0]),
>                                   qdev_get_gpio_in(uicdev, pci_irq_nrs[1]),
>                                   qdev_get_gpio_in(uicdev, pci_irq_nrs[2]),
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index 1d4a50fa7c..fbdf8266d8 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -46,7 +46,7 @@ struct PCITargetMap {
>       uint32_t la;
>   };
>   
> -OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST_BRIDGE)
> +OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST)
>   
>   #define PPC4xx_PCI_NR_PMMS 3
>   #define PPC4xx_PCI_NR_PTMS 2
> @@ -321,7 +321,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error **errp)
>       int i;
>   
>       h = PCI_HOST_BRIDGE(dev);
> -    s = PPC4xx_PCI_HOST_BRIDGE(dev);
> +    s = PPC4xx_PCI_HOST(dev);
>   
>       for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
>           sysbus_init_irq(sbd, &s->irq[i]);
> @@ -386,7 +386,7 @@ static void ppc4xx_pcihost_class_init(ObjectClass *klass, void *data)
>   }
>   
>   static const TypeInfo ppc4xx_pcihost_info = {
> -    .name          = TYPE_PPC4xx_PCI_HOST_BRIDGE,
> +    .name          = TYPE_PPC4xx_PCI_HOST,
>       .parent        = TYPE_PCI_HOST_BRIDGE,
>       .instance_size = sizeof(PPC4xxPCIState),
>       .class_init    = ppc4xx_pcihost_class_init,
> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
> index 39ca602442..e053b9751b 100644
> --- a/include/hw/ppc/ppc4xx.h
> +++ b/include/hw/ppc/ppc4xx.h
> @@ -29,7 +29,7 @@
>   #include "exec/memory.h"
>   #include "hw/sysbus.h"
>   
> -#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
> +#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
>   #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
>   
>   /*


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RESEND PATCH v3 3/4] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
  2023-07-06 11:16   ` [RESEND PATCH " BALATON Zoltan
  (?)
@ 2023-07-06 20:33   ` Daniel Henrique Barboza
  -1 siblings, 0 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2023-07-06 20:33 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: philmd



On 7/6/23 08:16, BALATON Zoltan wrote:
> Add a QOM type name define for ppc4xx-host-bridge in the common header
> and replace direct use of the string name with the constant.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/ppc/ppc440_pcix.c    | 3 ++-
>   hw/ppc/ppc4xx_pci.c     | 4 ++--
>   include/hw/ppc/ppc4xx.h | 1 +
>   3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index 54286cfef4..9321ca0abd 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -498,7 +498,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
>                            ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
>                            PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
>   
> -    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
> +    s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
> +                               TYPE_PPC4xx_HOST_BRIDGE);
>   
>       memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
>       memory_region_add_subregion(&s->bm, 0x0, &s->busmem);
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index fbdf8266d8..6652119008 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -333,7 +333,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, Error **errp)
>                                 TYPE_PCI_BUS);
>       h->bus = b;
>   
> -    pci_create_simple(b, 0, "ppc4xx-host-bridge");
> +    pci_create_simple(b, 0, TYPE_PPC4xx_HOST_BRIDGE);
>   
>       /* XXX split into 2 memory regions, one for config space, one for regs */
>       memory_region_init(&s->container, OBJECT(s), "pci-container", PCI_ALL_SIZE);
> @@ -367,7 +367,7 @@ static void ppc4xx_host_bridge_class_init(ObjectClass *klass, void *data)
>   }
>   
>   static const TypeInfo ppc4xx_host_bridge_info = {
> -    .name          = "ppc4xx-host-bridge",
> +    .name          = TYPE_PPC4xx_HOST_BRIDGE,
>       .parent        = TYPE_PCI_DEVICE,
>       .instance_size = sizeof(PCIDevice),
>       .class_init    = ppc4xx_host_bridge_class_init,
> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
> index e053b9751b..766d575e86 100644
> --- a/include/hw/ppc/ppc4xx.h
> +++ b/include/hw/ppc/ppc4xx.h
> @@ -29,6 +29,7 @@
>   #include "exec/memory.h"
>   #include "hw/sysbus.h"
>   
> +#define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
>   #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
>   #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
>   


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RESEND PATCH v3 4/4] ppc440_pcix: Rename QOM type define abd move it to common header
  2023-07-06 11:16   ` [RESEND PATCH " BALATON Zoltan
  (?)
@ 2023-07-06 20:33   ` Daniel Henrique Barboza
  -1 siblings, 0 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2023-07-06 20:33 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: philmd



On 7/6/23 08:16, BALATON Zoltan wrote:
> Rename TYPE_PPC440_PCIX_HOST_BRIDGE to better match its string value,
> move it to common header and use it also in sam460ex to replace hard
> coded type name.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/ppc/ppc440_pcix.c    | 9 ++++-----
>   hw/ppc/sam460ex.c       | 2 +-
>   include/hw/ppc/ppc4xx.h | 1 +
>   3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index 9321ca0abd..672090de94 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -45,8 +45,7 @@ struct PLBInMap {
>       MemoryRegion mr;
>   };
>   
> -#define TYPE_PPC440_PCIX_HOST_BRIDGE "ppc440-pcix-host"
> -OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST_BRIDGE)
> +OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST)
>   
>   #define PPC440_PCIX_NR_POMS 3
>   #define PPC440_PCIX_NR_PIMS 3
> @@ -399,7 +398,7 @@ static const MemoryRegionOps pci_reg_ops = {
>   
>   static void ppc440_pcix_reset(DeviceState *dev)
>   {
> -    struct PPC440PCIXState *s = PPC440_PCIX_HOST_BRIDGE(dev);
> +    struct PPC440PCIXState *s = PPC440_PCIX_HOST(dev);
>       int i;
>   
>       for (i = 0; i < PPC440_PCIX_NR_POMS; i++) {
> @@ -489,7 +488,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
>       PCIHostState *h;
>   
>       h = PCI_HOST_BRIDGE(dev);
> -    s = PPC440_PCIX_HOST_BRIDGE(dev);
> +    s = PPC440_PCIX_HOST(dev);
>   
>       sysbus_init_irq(sbd, &s->irq);
>       memory_region_init(&s->busmem, OBJECT(dev), "pci-mem", UINT64_MAX);
> @@ -529,7 +528,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
>   }
>   
>   static const TypeInfo ppc440_pcix_info = {
> -    .name          = TYPE_PPC440_PCIX_HOST_BRIDGE,
> +    .name          = TYPE_PPC440_PCIX_HOST,
>       .parent        = TYPE_PCI_HOST_BRIDGE,
>       .instance_size = sizeof(PPC440PCIXState),
>       .class_init    = ppc440_pcix_class_init,
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 7da38bd58e..1e615b8d35 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -438,7 +438,7 @@ static void sam460ex_init(MachineState *machine)
>   
>       /* PCI bus */
>       /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
> -    dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000,
> +    dev = sysbus_create_simple(TYPE_PPC440_PCIX_HOST, 0xc0ec00000,
>                                  qdev_get_gpio_in(uic[1], 0));
>       sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, 0xc08000000);
>       pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
> index 766d575e86..ea7740239b 100644
> --- a/include/hw/ppc/ppc4xx.h
> +++ b/include/hw/ppc/ppc4xx.h
> @@ -31,6 +31,7 @@
>   
>   #define TYPE_PPC4xx_HOST_BRIDGE "ppc4xx-host-bridge"
>   #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
> +#define TYPE_PPC440_PCIX_HOST "ppc440-pcix-host"
>   #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
>   
>   /*


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RESEND PATCH v3 2/4] ppc4xx_pci: Rename QOM type name define
  2023-07-06 20:33   ` Daniel Henrique Barboza
@ 2023-07-06 22:42     ` BALATON Zoltan
  0 siblings, 0 replies; 15+ messages in thread
From: BALATON Zoltan @ 2023-07-06 22:42 UTC (permalink / raw)
  To: Daniel Henrique Barboza; +Cc: qemu-devel, qemu-ppc, philmd

On Thu, 6 Jul 2023, Daniel Henrique Barboza wrote:
> On 7/6/23 08:16, BALATON Zoltan wrote:
>> Rename the TYPE_PPC4xx_PCI_HOST_BRIDGE define and its string value to
>> match each other and other similar types and to avoid confusion with
>> "ppc4xx-host-bridge" type defined in same file.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>
> I struggled a bit to understand what's to gain with this change, but it makes
> more sense when we consider the changes made in the next patch (where a
> TYPE_PPC4xx_HOST_BRIDGE macro is introduced to match the "ppc4xx-host-bridge"
> name).
>
> I also understand the comments Phil made in version 1. We have several 
> QOM names that are too similar ("ppc4xx-pci-host", "ppc4xx-host-bridge" 
> in the next patch, "ppc440-pcix-host" in patch 4), all of them being PCI 
> host bridges. I am uncertain whether renaming the QOM name of these 
> devices to make them less similar is worth it.

These SoCs have slighlty different PCI hosts in them. I think it may 
started with a simple PCI controller in 405 then a PCIX controller in 440 
and finally PCIe controllers in addition to PCIX in 460EX (although there 
also exists a 405EX so later these were just mix and matched in different 
SoCs). These all work slightly differently but are also similar in some 
ways as likely these were designed based on the previous one. Also 
modeling them was probebly done independently and only partially so we 
ended up with these devices. Maybe at one point we can clean it up and 
refactor these but at least for the PCIe controller I could not find any 
docs so it's not easy to find out how could these be rationalised.

> Matching the macro names with the actual QOM name is a step in the right 
> direction though.

Originally I wanted to improve PCIe emulation for sam460ex but I could not 
yet make that work (missing some details on how it should work without 
docs) so for now only these clean ups came out of that which should be 
steps forward but not all the way there yet. I try to continue when I will 
have time for it again sometimes but I think these at least should clean 
it up this a bit and we can then continue from here.

> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Thanks, Regards,
BALATON Zoltan


>>   hw/ppc/ppc440_bamboo.c  | 3 +--
>>   hw/ppc/ppc4xx_pci.c     | 6 +++---
>>   include/hw/ppc/ppc4xx.h | 2 +-
>>   3 files changed, 5 insertions(+), 6 deletions(-)
>> 
>> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
>> index f061b8cf3b..45f409c838 100644
>> --- a/hw/ppc/ppc440_bamboo.c
>> +++ b/hw/ppc/ppc440_bamboo.c
>> @@ -205,8 +205,7 @@ static void bamboo_init(MachineState *machine)
>>       ppc4xx_sdram_ddr_enable(PPC4xx_SDRAM_DDR(dev));
>>         /* PCI */
>> -    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST_BRIDGE,
>> -                                PPC440EP_PCI_CONFIG,
>> +    dev = sysbus_create_varargs(TYPE_PPC4xx_PCI_HOST, PPC440EP_PCI_CONFIG,
>>                                   qdev_get_gpio_in(uicdev, pci_irq_nrs[0]),
>>                                   qdev_get_gpio_in(uicdev, pci_irq_nrs[1]),
>>                                   qdev_get_gpio_in(uicdev, pci_irq_nrs[2]),
>> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
>> index 1d4a50fa7c..fbdf8266d8 100644
>> --- a/hw/ppc/ppc4xx_pci.c
>> +++ b/hw/ppc/ppc4xx_pci.c
>> @@ -46,7 +46,7 @@ struct PCITargetMap {
>>       uint32_t la;
>>   };
>>   -OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST_BRIDGE)
>> +OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, PPC4xx_PCI_HOST)
>>     #define PPC4xx_PCI_NR_PMMS 3
>>   #define PPC4xx_PCI_NR_PTMS 2
>> @@ -321,7 +321,7 @@ static void ppc4xx_pcihost_realize(DeviceState *dev, 
>> Error **errp)
>>       int i;
>>         h = PCI_HOST_BRIDGE(dev);
>> -    s = PPC4xx_PCI_HOST_BRIDGE(dev);
>> +    s = PPC4xx_PCI_HOST(dev);
>>         for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
>>           sysbus_init_irq(sbd, &s->irq[i]);
>> @@ -386,7 +386,7 @@ static void ppc4xx_pcihost_class_init(ObjectClass 
>> *klass, void *data)
>>   }
>>     static const TypeInfo ppc4xx_pcihost_info = {
>> -    .name          = TYPE_PPC4xx_PCI_HOST_BRIDGE,
>> +    .name          = TYPE_PPC4xx_PCI_HOST,
>>       .parent        = TYPE_PCI_HOST_BRIDGE,
>>       .instance_size = sizeof(PPC4xxPCIState),
>>       .class_init    = ppc4xx_pcihost_class_init,
>> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
>> index 39ca602442..e053b9751b 100644
>> --- a/include/hw/ppc/ppc4xx.h
>> +++ b/include/hw/ppc/ppc4xx.h
>> @@ -29,7 +29,7 @@
>>   #include "exec/memory.h"
>>   #include "hw/sysbus.h"
>>   -#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
>> +#define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
>>   #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
>>     /*
>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [RESEND PATCH v3 0/4] PPC440 devices misc clean up
  2023-07-06 11:16 ` [RESEND PATCH " BALATON Zoltan
                   ` (4 preceding siblings ...)
  (?)
@ 2023-07-07  7:19 ` Daniel Henrique Barboza
  -1 siblings, 0 replies; 15+ messages in thread
From: Daniel Henrique Barboza @ 2023-07-07  7:19 UTC (permalink / raw)
  To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: philmd

Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 7/6/23 08:16, BALATON Zoltan wrote:
> These are some small misc clean ups to PPC440 related device models
> which is all I have ready for now.
> 
> v3:
> - rebased on ppc-next moving already reviewed patch to front
> 
> v2:
> - Added R-b tags from Philippe
> - Addressed review comments
> - Added new patch to rename parent field of PPC460EXPCIEState to parent_obj
> 
> BALATON Zoltan (4):
>    ppc440_pcix: Stop using system io region for PCI bus
>    ppc4xx_pci: Rename QOM type name define
>    ppc4xx_pci: Add define for ppc4xx-host-bridge type name
>    ppc440_pcix: Rename QOM type define abd move it to common header
> 
>   hw/ppc/ppc440_bamboo.c  |  3 +--
>   hw/ppc/ppc440_pcix.c    | 21 ++++++++++++---------
>   hw/ppc/ppc4xx_pci.c     | 10 +++++-----
>   hw/ppc/sam460ex.c       |  8 ++------
>   include/hw/ppc/ppc4xx.h |  4 +++-
>   5 files changed, 23 insertions(+), 23 deletions(-)
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-07-07  7:20 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-06 11:07 [PATCH v3 0/4] PPC440 devices misc clean up BALATON Zoltan
2023-07-06 11:16 ` [RESEND PATCH " BALATON Zoltan
2023-07-03 16:48 ` [PATCH v3 1/4] ppc440_pcix: Stop using system io region for PCI bus BALATON Zoltan
2023-07-06 11:16   ` [RESEND PATCH " BALATON Zoltan
2023-07-03 21:09 ` [PATCH v3 2/4] ppc4xx_pci: Rename QOM type name define BALATON Zoltan
2023-07-06 11:16   ` [RESEND PATCH " BALATON Zoltan
2023-07-06 20:33   ` Daniel Henrique Barboza
2023-07-06 22:42     ` BALATON Zoltan
2023-07-03 21:16 ` [PATCH v3 3/4] ppc4xx_pci: Add define for ppc4xx-host-bridge type name BALATON Zoltan
2023-07-06 11:16   ` [RESEND PATCH " BALATON Zoltan
2023-07-06 20:33   ` Daniel Henrique Barboza
2023-07-03 21:21 ` [PATCH v3 4/4] ppc440_pcix: Rename QOM type define abd move it to common header BALATON Zoltan
2023-07-06 11:16   ` [RESEND PATCH " BALATON Zoltan
2023-07-06 20:33   ` Daniel Henrique Barboza
2023-07-07  7:19 ` [RESEND PATCH v3 0/4] PPC440 devices misc clean up Daniel Henrique Barboza

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.