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 <bala...@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 <danielhb...@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"
/*
