* [PATCH 01/13] ppc440: Change ppc460ex_pcie_init() parameter type
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:48 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 02/13] ppc440: Add cpu link property to PCIe controller model BALATON Zoltan
` (11 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
Change parameter of ppc460ex_pcie_init() from env to cpu to allow
further refactoring.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440.h | 2 +-
hw/ppc/ppc440_uc.c | 7 ++++---
hw/ppc/sam460ex.c | 2 +-
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
index 7c24db8504..ae42bcf0c8 100644
--- a/hw/ppc/ppc440.h
+++ b/hw/ppc/ppc440.h
@@ -18,6 +18,6 @@ void ppc4xx_cpr_init(CPUPPCState *env);
void ppc4xx_sdr_init(CPUPPCState *env);
void ppc4xx_ahb_init(CPUPPCState *env);
void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
-void ppc460ex_pcie_init(CPUPPCState *env);
+void ppc460ex_pcie_init(PowerPCCPU *cpu);
#endif /* PPC440_H */
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 651263926e..8eb985d714 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -17,6 +17,7 @@
#include "hw/qdev-properties.h"
#include "hw/pci/pci.h"
#include "sysemu/reset.h"
+#include "cpu.h"
#include "ppc440.h"
/*****************************************************************************/
@@ -1108,17 +1109,17 @@ static void ppc460ex_pcie_register_dcrs(PPC460EXPCIEState *s, CPUPPCState *env)
&dcr_read_pcie, &dcr_write_pcie);
}
-void ppc460ex_pcie_init(CPUPPCState *env)
+void ppc460ex_pcie_init(PowerPCCPU *cpu)
{
DeviceState *dev;
dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE0_BASE);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
- ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), env);
+ ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), &cpu->env);
dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE1_BASE);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
- ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), env);
+ ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), &cpu->env);
}
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index cf065aae0e..aaa8d2f4a5 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -422,7 +422,7 @@ static void sam460ex_init(MachineState *machine)
usb_create_simple(usb_bus_find(-1), "usb-mouse");
/* PCI bus */
- ppc460ex_pcie_init(env);
+ ppc460ex_pcie_init(cpu);
/* 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));
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 01/13] ppc440: Change ppc460ex_pcie_init() parameter type
2023-07-03 22:02 ` [PATCH 01/13] ppc440: Change ppc460ex_pcie_init() parameter type BALATON Zoltan
@ 2023-07-04 8:48 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:48 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> Change parameter of ppc460ex_pcie_init() from env to cpu to allow
> further refactoring.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440.h | 2 +-
> hw/ppc/ppc440_uc.c | 7 ++++---
> hw/ppc/sam460ex.c | 2 +-
> 3 files changed, 6 insertions(+), 5 deletions(-)
FWIW I'd rather move ppc460ex_pcie_register_dcrs() in
this patch. Regardless:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 02/13] ppc440: Add cpu link property to PCIe controller model
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
2023-07-03 22:02 ` [PATCH 01/13] ppc440: Change ppc460ex_pcie_init() parameter type BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:46 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration BALATON Zoltan
` (10 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
The PCIe controller model uses PPC DCRs but cannot be modeled with
TYPE_PPC4xx_DCR_DEVICE as it derives from TYPE_PCIE_HOST_BRIDGE. Add a
cpu link property to it similar to other DCR devices to allow
registering DCRs from the device model.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440_uc.c | 114 ++++++++++++++++++++++++---------------------
1 file changed, 62 insertions(+), 52 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 8eb985d714..b26c0cee1b 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -779,6 +779,7 @@ struct PPC460EXPCIEState {
MemoryRegion iomem;
qemu_irq irq[4];
int32_t dcrn_base;
+ PowerPCCPU *cpu;
uint64_t cfg_base;
uint32_t cfg_mask;
@@ -1001,6 +1002,58 @@ static void ppc460ex_set_irq(void *opaque, int irq_num, int level)
qemu_set_irq(s->irq[irq_num], level);
}
+static void ppc460ex_pcie_register_dcrs(PPC460EXPCIEState *s)
+{
+ CPUPPCState *env = &s->cpu->env;
+
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGBAH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGBAL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGMSK, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGBAH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGBAL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGMSK, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1BAH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1BAL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1MSKH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1MSKL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2BAH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2BAL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2MSKH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2MSKL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3BAH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3BAL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3MSKH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3MSKL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_REGBAH, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_REGBAL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_REGMSK, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_SPECIAL, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+ ppc_dcr_register(env, s->dcrn_base + PEGPL_CFG, s,
+ &dcr_read_pcie, &dcr_write_pcie);
+}
+
static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
{
PPC460EXPCIEState *s = PPC460EX_PCIE_HOST(dev);
@@ -1008,6 +1061,10 @@ static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
int i, id;
char buf[16];
+ if (!s->cpu) {
+ error_setg(errp, "cpu link property must be set");
+ return;
+ }
switch (s->dcrn_base) {
case DCRN_PCIE0_BASE:
id = 0;
@@ -1028,10 +1085,13 @@ static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
pci->bus = pci_register_root_bus(DEVICE(s), buf, ppc460ex_set_irq,
pci_swizzle_map_irq_fn, s, &s->iomem,
get_system_io(), 0, 4, TYPE_PCIE_BUS);
+ ppc460ex_pcie_register_dcrs(s);
}
static Property ppc460ex_pcie_props[] = {
DEFINE_PROP_INT32("dcrn-base", PPC460EXPCIEState, dcrn_base, -1),
+ DEFINE_PROP_LINK("cpu", PPC460EXPCIEState, cpu, TYPE_POWERPC_CPU,
+ PowerPCCPU *),
DEFINE_PROP_END_OF_LIST(),
};
@@ -1059,67 +1119,17 @@ static void ppc460ex_pcie_register(void)
type_init(ppc460ex_pcie_register)
-static void ppc460ex_pcie_register_dcrs(PPC460EXPCIEState *s, CPUPPCState *env)
-{
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGBAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGBAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGMSK, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGBAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGBAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGMSK, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1BAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1BAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1MSKH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1MSKL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2BAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2BAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2MSKH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2MSKL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3BAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3BAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3MSKH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3MSKL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_REGBAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_REGBAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_REGMSK, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_SPECIAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFG, s,
- &dcr_read_pcie, &dcr_write_pcie);
-}
-
void ppc460ex_pcie_init(PowerPCCPU *cpu)
{
DeviceState *dev;
dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE0_BASE);
+ object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
- ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), &cpu->env);
dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE1_BASE);
+ object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
- ppc460ex_pcie_register_dcrs(PPC460EX_PCIE_HOST(dev), &cpu->env);
}
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 02/13] ppc440: Add cpu link property to PCIe controller model
2023-07-03 22:02 ` [PATCH 02/13] ppc440: Add cpu link property to PCIe controller model BALATON Zoltan
@ 2023-07-04 8:46 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:46 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> The PCIe controller model uses PPC DCRs but cannot be modeled with
> TYPE_PPC4xx_DCR_DEVICE as it derives from TYPE_PCIE_HOST_BRIDGE. Add a
> cpu link property to it similar to other DCR devices to allow
> registering DCRs from the device model.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440_uc.c | 114 ++++++++++++++++++++++++---------------------
> 1 file changed, 62 insertions(+), 52 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
2023-07-03 22:02 ` [PATCH 01/13] ppc440: Change ppc460ex_pcie_init() parameter type BALATON Zoltan
2023-07-03 22:02 ` [PATCH 02/13] ppc440: Add cpu link property to PCIe controller model BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:49 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 04/13] ppc440: Rename local variable in dcr_read_pcie() BALATON Zoltan
` (9 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
It is more readable to wrap the complex call to ppc_dcr_register in a
macro when needed repeatedly.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440_uc.c | 76 +++++++++++++++++-----------------------------
1 file changed, 28 insertions(+), 48 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index b26c0cee1b..db83a0dec8 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -1002,56 +1002,36 @@ static void ppc460ex_set_irq(void *opaque, int irq_num, int level)
qemu_set_irq(s->irq[irq_num], level);
}
+#define PPC440_PCIE_DCR(s, dcrn) \
+ ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \
+ &dcr_read_pcie, &dcr_write_pcie)
+
+
static void ppc460ex_pcie_register_dcrs(PPC460EXPCIEState *s)
{
- CPUPPCState *env = &s->cpu->env;
-
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGBAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGBAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFGMSK, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGBAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGBAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_MSGMSK, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1BAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1BAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1MSKH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR1MSKL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2BAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2BAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2MSKH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR2MSKL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3BAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3BAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3MSKH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_OMR3MSKL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_REGBAH, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_REGBAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_REGMSK, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_SPECIAL, s,
- &dcr_read_pcie, &dcr_write_pcie);
- ppc_dcr_register(env, s->dcrn_base + PEGPL_CFG, s,
- &dcr_read_pcie, &dcr_write_pcie);
+ PPC440_PCIE_DCR(s, PEGPL_CFGBAH);
+ PPC440_PCIE_DCR(s, PEGPL_CFGBAL);
+ PPC440_PCIE_DCR(s, PEGPL_CFGMSK);
+ PPC440_PCIE_DCR(s, PEGPL_MSGBAH);
+ PPC440_PCIE_DCR(s, PEGPL_MSGBAL);
+ PPC440_PCIE_DCR(s, PEGPL_MSGMSK);
+ PPC440_PCIE_DCR(s, PEGPL_OMR1BAH);
+ PPC440_PCIE_DCR(s, PEGPL_OMR1BAL);
+ PPC440_PCIE_DCR(s, PEGPL_OMR1MSKH);
+ PPC440_PCIE_DCR(s, PEGPL_OMR1MSKL);
+ PPC440_PCIE_DCR(s, PEGPL_OMR2BAH);
+ PPC440_PCIE_DCR(s, PEGPL_OMR2BAL);
+ PPC440_PCIE_DCR(s, PEGPL_OMR2MSKH);
+ PPC440_PCIE_DCR(s, PEGPL_OMR2MSKL);
+ PPC440_PCIE_DCR(s, PEGPL_OMR3BAH);
+ PPC440_PCIE_DCR(s, PEGPL_OMR3BAL);
+ PPC440_PCIE_DCR(s, PEGPL_OMR3MSKH);
+ PPC440_PCIE_DCR(s, PEGPL_OMR3MSKL);
+ PPC440_PCIE_DCR(s, PEGPL_REGBAH);
+ PPC440_PCIE_DCR(s, PEGPL_REGBAL);
+ PPC440_PCIE_DCR(s, PEGPL_REGMSK);
+ PPC440_PCIE_DCR(s, PEGPL_SPECIAL);
+ PPC440_PCIE_DCR(s, PEGPL_CFG);
}
static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration
2023-07-03 22:02 ` [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration BALATON Zoltan
@ 2023-07-04 8:49 ` Philippe Mathieu-Daudé
2023-07-04 9:33 ` BALATON Zoltan
0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:49 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> It is more readable to wrap the complex call to ppc_dcr_register in a
> macro when needed repeatedly.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440_uc.c | 76 +++++++++++++++++-----------------------------
> 1 file changed, 28 insertions(+), 48 deletions(-)
> +#define PPC440_PCIE_DCR(s, dcrn) \
> + ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \
'(s), \'
> + &dcr_read_pcie, &dcr_write_pcie)
> +
> +
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration
2023-07-04 8:49 ` Philippe Mathieu-Daudé
@ 2023-07-04 9:33 ` BALATON Zoltan
2023-07-04 9:55 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-04 9:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
> On 4/7/23 00:02, BALATON Zoltan wrote:
>> It is more readable to wrap the complex call to ppc_dcr_register in a
>> macro when needed repeatedly.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/ppc/ppc440_uc.c | 76 +++++++++++++++++-----------------------------
>> 1 file changed, 28 insertions(+), 48 deletions(-)
>
>
>> +#define PPC440_PCIE_DCR(s, dcrn) \
>> + ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \
>
> '(s), \'
The parenthesis here would be superfluous as it stands alone in a function
parameter between commas so no matter what you substitue here should not
have an unwanted side effect (unless it has a comma but that's an error
anyway) so maybe this is not needed.
>> + &dcr_read_pcie, &dcr_write_pcie)
>> +
>> +
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Thanks for the quick review, I'll post a v2 in a few days to wait a bit if
anobody else has any other request.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration
2023-07-04 9:33 ` BALATON Zoltan
@ 2023-07-04 9:55 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 9:55 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
On 4/7/23 11:33, BALATON Zoltan wrote:
> On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
>> On 4/7/23 00:02, BALATON Zoltan wrote:
>>> It is more readable to wrap the complex call to ppc_dcr_register in a
>>> macro when needed repeatedly.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/ppc/ppc440_uc.c | 76 +++++++++++++++++-----------------------------
>>> 1 file changed, 28 insertions(+), 48 deletions(-)
>>
>>
>>> +#define PPC440_PCIE_DCR(s, dcrn) \
>>> + ppc_dcr_register(&(s)->cpu->env, (s)->dcrn_base + (dcrn), s, \
>>
>> '(s), \'
>
> The parenthesis here would be superfluous as it stands alone in a
> function parameter between commas so no matter what you substitue here
> should not have an unwanted side effect (unless it has a comma but
> that's an error anyway) so maybe this is not needed.
Well I noticed because you used it for the 2 other cases, so I'm
just trying to be consistent here. Besides, not using parenthesis
for macro arguments is a bad practice. Problems happen when others
copy code.
>
>>> + &dcr_read_pcie, &dcr_write_pcie)
>>> +
>>> +
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Thanks for the quick review, I'll post a v2 in a few days to wait a bit
> if anobody else has any other request.
>
> Regards,
> BALATON Zoltan
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 04/13] ppc440: Rename local variable in dcr_read_pcie()
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (2 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 03/13] ppc440: Add a macro to shorten PCIe controller DCR registration BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:50 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 05/13] ppc440: Stop using system io region for PCIe buses BALATON Zoltan
` (8 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
Rename local variable storing state struct in dcr_read_pcie() for
brevity and consistency with other functions.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440_uc.c | 50 +++++++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index db83a0dec8..38ee27f437 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -828,78 +828,78 @@ enum {
static uint32_t dcr_read_pcie(void *opaque, int dcrn)
{
- PPC460EXPCIEState *state = opaque;
+ PPC460EXPCIEState *s = opaque;
uint32_t ret = 0;
- switch (dcrn - state->dcrn_base) {
+ switch (dcrn - s->dcrn_base) {
case PEGPL_CFGBAH:
- ret = state->cfg_base >> 32;
+ ret = s->cfg_base >> 32;
break;
case PEGPL_CFGBAL:
- ret = state->cfg_base;
+ ret = s->cfg_base;
break;
case PEGPL_CFGMSK:
- ret = state->cfg_mask;
+ ret = s->cfg_mask;
break;
case PEGPL_MSGBAH:
- ret = state->msg_base >> 32;
+ ret = s->msg_base >> 32;
break;
case PEGPL_MSGBAL:
- ret = state->msg_base;
+ ret = s->msg_base;
break;
case PEGPL_MSGMSK:
- ret = state->msg_mask;
+ ret = s->msg_mask;
break;
case PEGPL_OMR1BAH:
- ret = state->omr1_base >> 32;
+ ret = s->omr1_base >> 32;
break;
case PEGPL_OMR1BAL:
- ret = state->omr1_base;
+ ret = s->omr1_base;
break;
case PEGPL_OMR1MSKH:
- ret = state->omr1_mask >> 32;
+ ret = s->omr1_mask >> 32;
break;
case PEGPL_OMR1MSKL:
- ret = state->omr1_mask;
+ ret = s->omr1_mask;
break;
case PEGPL_OMR2BAH:
- ret = state->omr2_base >> 32;
+ ret = s->omr2_base >> 32;
break;
case PEGPL_OMR2BAL:
- ret = state->omr2_base;
+ ret = s->omr2_base;
break;
case PEGPL_OMR2MSKH:
- ret = state->omr2_mask >> 32;
+ ret = s->omr2_mask >> 32;
break;
case PEGPL_OMR2MSKL:
- ret = state->omr3_mask;
+ ret = s->omr3_mask;
break;
case PEGPL_OMR3BAH:
- ret = state->omr3_base >> 32;
+ ret = s->omr3_base >> 32;
break;
case PEGPL_OMR3BAL:
- ret = state->omr3_base;
+ ret = s->omr3_base;
break;
case PEGPL_OMR3MSKH:
- ret = state->omr3_mask >> 32;
+ ret = s->omr3_mask >> 32;
break;
case PEGPL_OMR3MSKL:
- ret = state->omr3_mask;
+ ret = s->omr3_mask;
break;
case PEGPL_REGBAH:
- ret = state->reg_base >> 32;
+ ret = s->reg_base >> 32;
break;
case PEGPL_REGBAL:
- ret = state->reg_base;
+ ret = s->reg_base;
break;
case PEGPL_REGMSK:
- ret = state->reg_mask;
+ ret = s->reg_mask;
break;
case PEGPL_SPECIAL:
- ret = state->special;
+ ret = s->special;
break;
case PEGPL_CFG:
- ret = state->cfg;
+ ret = s->cfg;
break;
}
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 04/13] ppc440: Rename local variable in dcr_read_pcie()
2023-07-03 22:02 ` [PATCH 04/13] ppc440: Rename local variable in dcr_read_pcie() BALATON Zoltan
@ 2023-07-04 8:50 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:50 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> Rename local variable storing state struct in dcr_read_pcie() for
> brevity and consistency with other functions.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440_uc.c | 50 +++++++++++++++++++++++-----------------------
> 1 file changed, 25 insertions(+), 25 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 05/13] ppc440: Stop using system io region for PCIe buses
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (3 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 04/13] ppc440: Rename local variable in dcr_read_pcie() BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:51 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 06/13] sam460ex: Remove address_space_mem local variable BALATON Zoltan
` (7 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
Add separate memory regions for the mem and io spaces of the PCIe bus
to avoid different buses using the same system io region.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440_uc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 38ee27f437..0c5d999878 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -776,6 +776,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC460EXPCIEState, PPC460EX_PCIE_HOST)
struct PPC460EXPCIEState {
PCIExpressHost host;
+ MemoryRegion busmem;
MemoryRegion iomem;
qemu_irq irq[4];
int32_t dcrn_base;
@@ -1056,15 +1057,17 @@ static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
error_setg(errp, "invalid PCIe DCRN base");
return;
}
+ snprintf(buf, sizeof(buf), "pcie%d-mem", id);
+ memory_region_init(&s->busmem, OBJECT(s), buf, UINT64_MAX);
snprintf(buf, sizeof(buf), "pcie%d-io", id);
- memory_region_init(&s->iomem, OBJECT(s), buf, UINT64_MAX);
+ memory_region_init(&s->iomem, OBJECT(s), buf, 0x10000);
for (i = 0; i < 4; i++) {
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
}
snprintf(buf, sizeof(buf), "pcie.%d", id);
pci->bus = pci_register_root_bus(DEVICE(s), buf, ppc460ex_set_irq,
- pci_swizzle_map_irq_fn, s, &s->iomem,
- get_system_io(), 0, 4, TYPE_PCIE_BUS);
+ pci_swizzle_map_irq_fn, s, &s->busmem,
+ &s->iomem, 0, 4, TYPE_PCIE_BUS);
ppc460ex_pcie_register_dcrs(s);
}
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 05/13] ppc440: Stop using system io region for PCIe buses
2023-07-03 22:02 ` [PATCH 05/13] ppc440: Stop using system io region for PCIe buses BALATON Zoltan
@ 2023-07-04 8:51 ` Philippe Mathieu-Daudé
2023-07-04 9:48 ` BALATON Zoltan
0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:51 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> Add separate memory regions for the mem and io spaces of the PCIe bus
> to avoid different buses using the same system io region.
"Reduce the I/O space to 64K."
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440_uc.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
> index 38ee27f437..0c5d999878 100644
> --- a/hw/ppc/ppc440_uc.c
> +++ b/hw/ppc/ppc440_uc.c
> @@ -776,6 +776,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC460EXPCIEState, PPC460EX_PCIE_HOST)
> struct PPC460EXPCIEState {
> PCIExpressHost host;
>
> + MemoryRegion busmem;
> MemoryRegion iomem;
> qemu_irq irq[4];
> int32_t dcrn_base;
> @@ -1056,15 +1057,17 @@ static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
> error_setg(errp, "invalid PCIe DCRN base");
> return;
> }
> + snprintf(buf, sizeof(buf), "pcie%d-mem", id);
> + memory_region_init(&s->busmem, OBJECT(s), buf, UINT64_MAX);
> snprintf(buf, sizeof(buf), "pcie%d-io", id);
> - memory_region_init(&s->iomem, OBJECT(s), buf, UINT64_MAX);
> + memory_region_init(&s->iomem, OBJECT(s), buf, 0x10000);
64 * KiB
> for (i = 0; i < 4; i++) {
> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
> }
> snprintf(buf, sizeof(buf), "pcie.%d", id);
> pci->bus = pci_register_root_bus(DEVICE(s), buf, ppc460ex_set_irq,
> - pci_swizzle_map_irq_fn, s, &s->iomem,
> - get_system_io(), 0, 4, TYPE_PCIE_BUS);
> + pci_swizzle_map_irq_fn, s, &s->busmem,
> + &s->iomem, 0, 4, TYPE_PCIE_BUS);
> ppc460ex_pcie_register_dcrs(s);
> }
>
With the changes addressed:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 05/13] ppc440: Stop using system io region for PCIe buses
2023-07-04 8:51 ` Philippe Mathieu-Daudé
@ 2023-07-04 9:48 ` BALATON Zoltan
0 siblings, 0 replies; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-04 9:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
> On 4/7/23 00:02, BALATON Zoltan wrote:
>> Add separate memory regions for the mem and io spaces of the PCIe bus
>> to avoid different buses using the same system io region.
>
> "Reduce the I/O space to 64K."
Unlike the other similar patch this does not reduce the IO space size
because get_system_io() was that size already. I've changed the size below
to use KiB.
Regards,
BALATON Zoltan
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/ppc/ppc440_uc.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
>> index 38ee27f437..0c5d999878 100644
>> --- a/hw/ppc/ppc440_uc.c
>> +++ b/hw/ppc/ppc440_uc.c
>> @@ -776,6 +776,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC460EXPCIEState,
>> PPC460EX_PCIE_HOST)
>> struct PPC460EXPCIEState {
>> PCIExpressHost host;
>> + MemoryRegion busmem;
>> MemoryRegion iomem;
>> qemu_irq irq[4];
>> int32_t dcrn_base;
>> @@ -1056,15 +1057,17 @@ static void ppc460ex_pcie_realize(DeviceState *dev,
>> Error **errp)
>> error_setg(errp, "invalid PCIe DCRN base");
>> return;
>> }
>> + snprintf(buf, sizeof(buf), "pcie%d-mem", id);
>> + memory_region_init(&s->busmem, OBJECT(s), buf, UINT64_MAX);
>> snprintf(buf, sizeof(buf), "pcie%d-io", id);
>> - memory_region_init(&s->iomem, OBJECT(s), buf, UINT64_MAX);
>> + memory_region_init(&s->iomem, OBJECT(s), buf, 0x10000);
>
> 64 * KiB
>
>> for (i = 0; i < 4; i++) {
>> sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
>> }
>> snprintf(buf, sizeof(buf), "pcie.%d", id);
>> pci->bus = pci_register_root_bus(DEVICE(s), buf, ppc460ex_set_irq,
>> - pci_swizzle_map_irq_fn, s, &s->iomem,
>> - get_system_io(), 0, 4, TYPE_PCIE_BUS);
>> + pci_swizzle_map_irq_fn, s, &s->busmem,
>> + &s->iomem, 0, 4, TYPE_PCIE_BUS);
>> ppc460ex_pcie_register_dcrs(s);
>> }
>>
>
> With the changes addressed:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 06/13] sam460ex: Remove address_space_mem local variable
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (4 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 05/13] ppc440: Stop using system io region for PCIe buses BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-03 22:02 ` [PATCH 07/13] ppc440: Add busnum property to PCIe controller model BALATON Zoltan
` (6 subsequent siblings)
12 siblings, 0 replies; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
Some places already use get_system_memory() directly so replace the
remaining uses and drop the local variable.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/sam460ex.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index aaa8d2f4a5..f098226974 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -266,7 +266,6 @@ static void main_cpu_reset(void *opaque)
static void sam460ex_init(MachineState *machine)
{
- MemoryRegion *address_space_mem = get_system_memory();
MemoryRegion *isa = g_new(MemoryRegion, 1);
MemoryRegion *l2cache_ram = g_new(MemoryRegion, 1);
DeviceState *uic[4];
@@ -406,7 +405,8 @@ static void sam460ex_init(MachineState *machine)
/* FIXME: remove this after fixing l2sram mapping in ppc440_uc.c? */
memory_region_init_ram(l2cache_ram, NULL, "ppc440.l2cache_ram", 256 * KiB,
&error_abort);
- memory_region_add_subregion(address_space_mem, 0x400000000LL, l2cache_ram);
+ memory_region_add_subregion(get_system_memory(), 0x400000000LL,
+ l2cache_ram);
/* USB */
sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400,
@@ -444,13 +444,13 @@ static void sam460ex_init(MachineState *machine)
/* SoC has 4 UARTs
* but board has only one wired and two are present in fdt */
if (serial_hd(0) != NULL) {
- serial_mm_init(address_space_mem, 0x4ef600300, 0,
+ serial_mm_init(get_system_memory(), 0x4ef600300, 0,
qdev_get_gpio_in(uic[1], 1),
PPC_SERIAL_MM_BAUDBASE, serial_hd(0),
DEVICE_BIG_ENDIAN);
}
if (serial_hd(1) != NULL) {
- serial_mm_init(address_space_mem, 0x4ef600400, 0,
+ serial_mm_init(get_system_memory(), 0x4ef600400, 0,
qdev_get_gpio_in(uic[0], 1),
PPC_SERIAL_MM_BAUDBASE, serial_hd(1),
DEVICE_BIG_ENDIAN);
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 07/13] ppc440: Add busnum property to PCIe controller model
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (5 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 06/13] sam460ex: Remove address_space_mem local variable BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:52 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 08/13] ppc440: Remove ppc460ex_pcie_init legacy init function BALATON Zoltan
` (5 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
Instead of guessing controller number from dcrn_base add a property so
the device does not need knowledge about where it is used.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440_uc.c | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 0c5d999878..61782a5c1e 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -779,6 +779,7 @@ struct PPC460EXPCIEState {
MemoryRegion busmem;
MemoryRegion iomem;
qemu_irq irq[4];
+ int32_t num;
int32_t dcrn_base;
PowerPCCPU *cpu;
@@ -1039,32 +1040,25 @@ static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
{
PPC460EXPCIEState *s = PPC460EX_PCIE_HOST(dev);
PCIHostState *pci = PCI_HOST_BRIDGE(dev);
- int i, id;
- char buf[16];
+ int i;
+ char buf[20];
if (!s->cpu) {
error_setg(errp, "cpu link property must be set");
return;
}
- switch (s->dcrn_base) {
- case DCRN_PCIE0_BASE:
- id = 0;
- break;
- case DCRN_PCIE1_BASE:
- id = 1;
- break;
- default:
- error_setg(errp, "invalid PCIe DCRN base");
+ if (s->num < 0 || s->dcrn_base < 0) {
+ error_setg(errp, "busnum and dcrn-base properties must be set");
return;
}
- snprintf(buf, sizeof(buf), "pcie%d-mem", id);
+ snprintf(buf, sizeof(buf), "pcie%d-mem", s->num);
memory_region_init(&s->busmem, OBJECT(s), buf, UINT64_MAX);
- snprintf(buf, sizeof(buf), "pcie%d-io", id);
+ snprintf(buf, sizeof(buf), "pcie%d-io", s->num);
memory_region_init(&s->iomem, OBJECT(s), buf, 0x10000);
for (i = 0; i < 4; i++) {
sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
}
- snprintf(buf, sizeof(buf), "pcie.%d", id);
+ snprintf(buf, sizeof(buf), "pcie.%d", s->num);
pci->bus = pci_register_root_bus(DEVICE(s), buf, ppc460ex_set_irq,
pci_swizzle_map_irq_fn, s, &s->busmem,
&s->iomem, 0, 4, TYPE_PCIE_BUS);
@@ -1072,6 +1066,7 @@ static void ppc460ex_pcie_realize(DeviceState *dev, Error **errp)
}
static Property ppc460ex_pcie_props[] = {
+ DEFINE_PROP_INT32("busnum", PPC460EXPCIEState, num, -1),
DEFINE_PROP_INT32("dcrn-base", PPC460EXPCIEState, dcrn_base, -1),
DEFINE_PROP_LINK("cpu", PPC460EXPCIEState, cpu, TYPE_POWERPC_CPU,
PowerPCCPU *),
@@ -1107,11 +1102,13 @@ void ppc460ex_pcie_init(PowerPCCPU *cpu)
DeviceState *dev;
dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
+ qdev_prop_set_int32(dev, "busnum", 0);
qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE0_BASE);
object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
+ qdev_prop_set_int32(dev, "busnum", 1);
qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE1_BASE);
object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH 08/13] ppc440: Remove ppc460ex_pcie_init legacy init function
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (6 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 07/13] ppc440: Add busnum property to PCIe controller model BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:53 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 09/13] ppc4xx_pci: Rename QOM type name define BALATON Zoltan
` (4 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
After previous changes we can now remove the legacy init function and
move the device creation to board code.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440.h | 1 -
hw/ppc/ppc440_uc.c | 21 ---------------------
hw/ppc/sam460ex.c | 17 ++++++++++++++++-
include/hw/ppc/ppc4xx.h | 1 +
4 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/hw/ppc/ppc440.h b/hw/ppc/ppc440.h
index ae42bcf0c8..909373fb38 100644
--- a/hw/ppc/ppc440.h
+++ b/hw/ppc/ppc440.h
@@ -18,6 +18,5 @@ void ppc4xx_cpr_init(CPUPPCState *env);
void ppc4xx_sdr_init(CPUPPCState *env);
void ppc4xx_ahb_init(CPUPPCState *env);
void ppc4xx_dma_init(CPUPPCState *env, int dcr_base);
-void ppc460ex_pcie_init(PowerPCCPU *cpu);
#endif /* PPC440_H */
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 61782a5c1e..545f36edce 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -770,7 +770,6 @@ void ppc4xx_dma_init(CPUPPCState *env, int dcr_base)
*/
#include "hw/pci/pcie_host.h"
-#define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
OBJECT_DECLARE_SIMPLE_TYPE(PPC460EXPCIEState, PPC460EX_PCIE_HOST)
struct PPC460EXPCIEState {
@@ -799,9 +798,6 @@ struct PPC460EXPCIEState {
uint32_t cfg;
};
-#define DCRN_PCIE0_BASE 0x100
-#define DCRN_PCIE1_BASE 0x120
-
enum {
PEGPL_CFGBAH = 0x0,
PEGPL_CFGBAL,
@@ -1096,20 +1092,3 @@ static void ppc460ex_pcie_register(void)
}
type_init(ppc460ex_pcie_register)
-
-void ppc460ex_pcie_init(PowerPCCPU *cpu)
-{
- DeviceState *dev;
-
- dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
- qdev_prop_set_int32(dev, "busnum", 0);
- qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE0_BASE);
- object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
- sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-
- dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
- qdev_prop_set_int32(dev, "busnum", 1);
- qdev_prop_set_int32(dev, "dcrn-base", DCRN_PCIE1_BASE);
- object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
- sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-}
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index f098226974..d446cfc37b 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -45,6 +45,9 @@
/* dd bs=1 skip=$(($(stat -c '%s' updater/updater-460) - 0x80000)) \
if=updater/updater-460 of=u-boot-sam460-20100605.bin */
+#define PCIE0_DCRN_BASE 0x100
+#define PCIE1_DCRN_BASE 0x120
+
/* from Sam460 U-Boot include/configs/Sam460ex.h */
#define FLASH_BASE 0xfff00000
#define FLASH_BASE_H 0x4
@@ -421,8 +424,20 @@ static void sam460ex_init(MachineState *machine)
usb_create_simple(usb_bus_find(-1), "usb-kbd");
usb_create_simple(usb_bus_find(-1), "usb-mouse");
+ /* PCIe buses */
+ dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
+ qdev_prop_set_int32(dev, "busnum", 0);
+ qdev_prop_set_int32(dev, "dcrn-base", PCIE0_DCRN_BASE);
+ object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
+ dev = qdev_new(TYPE_PPC460EX_PCIE_HOST);
+ qdev_prop_set_int32(dev, "busnum", 1);
+ qdev_prop_set_int32(dev, "dcrn-base", PCIE1_DCRN_BASE);
+ object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
/* PCI bus */
- ppc460ex_pcie_init(cpu);
/* 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));
diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
index f8c86e09ec..39ca602442 100644
--- a/include/hw/ppc/ppc4xx.h
+++ b/include/hw/ppc/ppc4xx.h
@@ -30,6 +30,7 @@
#include "hw/sysbus.h"
#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pcihost"
+#define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
/*
* Generic DCR device
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 08/13] ppc440: Remove ppc460ex_pcie_init legacy init function
2023-07-03 22:02 ` [PATCH 08/13] ppc440: Remove ppc460ex_pcie_init legacy init function BALATON Zoltan
@ 2023-07-04 8:53 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:53 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> After previous changes we can now remove the legacy init function and
> move the device creation to board code.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440.h | 1 -
> hw/ppc/ppc440_uc.c | 21 ---------------------
> hw/ppc/sam460ex.c | 17 ++++++++++++++++-
> include/hw/ppc/ppc4xx.h | 1 +
> 4 files changed, 17 insertions(+), 23 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 09/13] ppc4xx_pci: Rename QOM type name define
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (7 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 08/13] ppc440: Remove ppc460ex_pcie_init legacy init function BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:56 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name BALATON Zoltan
` (3 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
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] 33+ messages in thread* Re: [PATCH 09/13] ppc4xx_pci: Rename QOM type name define
2023-07-03 22:02 ` [PATCH 09/13] ppc4xx_pci: Rename QOM type name define BALATON Zoltan
@ 2023-07-04 8:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:56 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, 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>
> ---
> 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"
This is the host bridge however... Maybe we want to rename:
#define TYPE_PPC4xx_PCI_HOST_BRIDGE "ppc4xx-pci-hostbridge"
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (8 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 09/13] ppc4xx_pci: Rename QOM type name define BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:57 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 11/13] ppc440_pcix: Rename QOM type define abd move it to common header BALATON Zoltan
` (2 subsequent siblings)
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
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 f10f93c533..dfec25ac83 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -495,7 +495,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
ppc440_pcix_map_irq, &s->irq, &s->busmem,
get_system_io(), 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] 33+ messages in thread* Re: [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
2023-07-03 22:02 ` [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name BALATON Zoltan
@ 2023-07-04 8:57 ` Philippe Mathieu-Daudé
2023-07-04 9:36 ` BALATON Zoltan
0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:57 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, 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>
> ---
> 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 f10f93c533..dfec25ac83 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -495,7 +495,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
> ppc440_pcix_map_irq, &s->irq, &s->busmem,
> get_system_io(), 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"
This is the function #0 of the host bridge, maybe:
#define TYPE_PPC4xx_HOST_BRIDGE_FN0 "ppc4xx-pci-host-bridge-fn0"
> #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
> #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name
2023-07-04 8:57 ` Philippe Mathieu-Daudé
@ 2023-07-04 9:36 ` BALATON Zoltan
0 siblings, 0 replies; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-04 9:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
> On 4/7/23 00:02, 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>
>> ---
>> 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 f10f93c533..dfec25ac83 100644
>> --- a/hw/ppc/ppc440_pcix.c
>> +++ b/hw/ppc/ppc440_pcix.c
>> @@ -495,7 +495,8 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
>> **errp)
>> ppc440_pcix_map_irq, &s->irq, &s->busmem,
>> get_system_io(), 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"
>
> This is the function #0 of the host bridge, maybe:
>
> #define TYPE_PPC4xx_HOST_BRIDGE_FN0 "ppc4xx-pci-host-bridge-fn0"
That's way too long so I'd drop bridge from all of these and maybe name
this ppc4xx-pci-host-fn0 or ppc4xx-pci-host-func (there are no other
functions of the bridge so this shows this is the PCI side of it). I'd
still go for shorted defines and not changing the string types too much.
Would that be acceptable?
Regards,
BALATON Zoltan
>> #define TYPE_PPC4xx_PCI_HOST "ppc4xx-pci-host"
>> #define TYPE_PPC460EX_PCIE_HOST "ppc460ex-pcie-host"
>>
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 11/13] ppc440_pcix: Rename QOM type define abd move it to common header
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (9 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 10/13] ppc4xx_pci: Add define for ppc4xx-host-bridge type name BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:58 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 12/13] ppc440_pcix: Don't use iomem for regs BALATON Zoltan
2023-07-03 22:02 ` [PATCH 13/13] ppc440_pcix: Stop using system io region for PCI bus BALATON Zoltan
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
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 dfec25ac83..adfecf1e76 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -44,8 +44,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
@@ -397,7 +396,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++) {
@@ -487,7 +486,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 bus memory", UINT64_MAX);
@@ -525,7 +524,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 d446cfc37b..8d0e551d14 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -439,7 +439,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));
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] 33+ messages in thread* Re: [PATCH 11/13] ppc440_pcix: Rename QOM type define abd move it to common header
2023-07-03 22:02 ` [PATCH 11/13] ppc440_pcix: Rename QOM type define abd move it to common header BALATON Zoltan
@ 2023-07-04 8:58 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:58 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, 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>
> ---
> 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 dfec25ac83..adfecf1e76 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -44,8 +44,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
> @@ -397,7 +396,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++) {
> @@ -487,7 +486,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 bus memory", UINT64_MAX);
> @@ -525,7 +524,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 d446cfc37b..8d0e551d14 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -439,7 +439,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));
> 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"
Again, this is a host bridge, why not name it HOST_BRIDGE?
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (10 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 11/13] ppc440_pcix: Rename QOM type define abd move it to common header BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 8:59 ` Philippe Mathieu-Daudé
2023-07-03 22:02 ` [PATCH 13/13] ppc440_pcix: Stop using system io region for PCI bus BALATON Zoltan
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
The iomem memory region is better used for the PCI IO space but
currently used for registers. Stop using it for that to allow this to
be cleaned up in the next patch.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/ppc440_pcix.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index adfecf1e76..ee2dc44f67 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
PPC440PCIXState *s;
PCIHostState *h;
+ MemoryRegion *regs = g_new(MemoryRegion, 1);
h = PCI_HOST_BRIDGE(dev);
s = PPC440_PCIX_HOST(dev);
@@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
h, "pci-conf-idx", 4);
memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops,
h, "pci-conf-data", 4);
- memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
- "pci.reg", PPC440_REG_SIZE);
+ memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg",
+ PPC440_REG_SIZE);
memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem);
memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem);
- memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem);
+ memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs);
sysbus_init_mmio(sbd, &s->container);
}
--
2.30.9
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
2023-07-03 22:02 ` [PATCH 12/13] ppc440_pcix: Don't use iomem for regs BALATON Zoltan
@ 2023-07-04 8:59 ` Philippe Mathieu-Daudé
2023-07-04 9:37 ` BALATON Zoltan
0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 8:59 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> The iomem memory region is better used for the PCI IO space but
> currently used for registers. Stop using it for that to allow this to
> be cleaned up in the next patch.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440_pcix.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index adfecf1e76..ee2dc44f67 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> PPC440PCIXState *s;
> PCIHostState *h;
> + MemoryRegion *regs = g_new(MemoryRegion, 1);
Why not hold it within PPC440PCIXState?
> h = PCI_HOST_BRIDGE(dev);
> s = PPC440_PCIX_HOST(dev);
> @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
> h, "pci-conf-idx", 4);
> memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops,
> h, "pci-conf-data", 4);
> - memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
> - "pci.reg", PPC440_REG_SIZE);
> + memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg",
> + PPC440_REG_SIZE);
> memory_region_add_subregion(&s->container, PCIC0_CFGADDR, &h->conf_mem);
> memory_region_add_subregion(&s->container, PCIC0_CFGDATA, &h->data_mem);
> - memory_region_add_subregion(&s->container, PPC440_REG_BASE, &s->iomem);
> + memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs);
> sysbus_init_mmio(sbd, &s->container);
> }
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
2023-07-04 8:59 ` Philippe Mathieu-Daudé
@ 2023-07-04 9:37 ` BALATON Zoltan
2023-07-04 9:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-04 9:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 1951 bytes --]
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
> On 4/7/23 00:02, BALATON Zoltan wrote:
>> The iomem memory region is better used for the PCI IO space but
>> currently used for registers. Stop using it for that to allow this to
>> be cleaned up in the next patch.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/ppc/ppc440_pcix.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
>> index adfecf1e76..ee2dc44f67 100644
>> --- a/hw/ppc/ppc440_pcix.c
>> +++ b/hw/ppc/ppc440_pcix.c
>> @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
>> **errp)
>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> PPC440PCIXState *s;
>> PCIHostState *h;
>> + MemoryRegion *regs = g_new(MemoryRegion, 1);
>
> Why not hold it within PPC440PCIXState?
Because it's never needed after this function.
Regards,
BALATON Zoltan
>> h = PCI_HOST_BRIDGE(dev);
>> s = PPC440_PCIX_HOST(dev);
>> @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev,
>> Error **errp)
>> h, "pci-conf-idx", 4);
>> memory_region_init_io(&h->data_mem, OBJECT(s), &pci_host_data_le_ops,
>> h, "pci-conf-data", 4);
>> - memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
>> - "pci.reg", PPC440_REG_SIZE);
>> + memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg",
>> + PPC440_REG_SIZE);
>> memory_region_add_subregion(&s->container, PCIC0_CFGADDR,
>> &h->conf_mem);
>> memory_region_add_subregion(&s->container, PCIC0_CFGDATA,
>> &h->data_mem);
>> - memory_region_add_subregion(&s->container, PPC440_REG_BASE,
>> &s->iomem);
>> + memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs);
>> sysbus_init_mmio(sbd, &s->container);
>> }
>>
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
2023-07-04 9:37 ` BALATON Zoltan
@ 2023-07-04 9:57 ` Philippe Mathieu-Daudé
2023-07-04 10:14 ` BALATON Zoltan
0 siblings, 1 reply; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 9:57 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
On 4/7/23 11:37, BALATON Zoltan wrote:
> On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
>> On 4/7/23 00:02, BALATON Zoltan wrote:
>>> The iomem memory region is better used for the PCI IO space but
>>> currently used for registers. Stop using it for that to allow this to
>>> be cleaned up in the next patch.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/ppc/ppc440_pcix.c | 7 ++++---
>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
>>> index adfecf1e76..ee2dc44f67 100644
>>> --- a/hw/ppc/ppc440_pcix.c
>>> +++ b/hw/ppc/ppc440_pcix.c
>>> @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev,
>>> Error **errp)
>>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> PPC440PCIXState *s;
>>> PCIHostState *h;
>>> + MemoryRegion *regs = g_new(MemoryRegion, 1);
>>
>> Why not hold it within PPC440PCIXState?
>
> Because it's never needed after this function.
But we can't free() it because it has to stay valid as long as
PPC440PCIXState is in use. So it seems to belong there.
>
> Regards,
> BALATON Zoltan
>
>>> h = PCI_HOST_BRIDGE(dev);
>>> s = PPC440_PCIX_HOST(dev);
>>> @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState
>>> *dev, Error **errp)
>>> h, "pci-conf-idx", 4);
>>> memory_region_init_io(&h->data_mem, OBJECT(s),
>>> &pci_host_data_le_ops,
>>> h, "pci-conf-data", 4);
>>> - memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
>>> - "pci.reg", PPC440_REG_SIZE);
>>> + memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg",
>>> + PPC440_REG_SIZE);
>>> memory_region_add_subregion(&s->container, PCIC0_CFGADDR,
>>> &h->conf_mem);
>>> memory_region_add_subregion(&s->container, PCIC0_CFGDATA,
>>> &h->data_mem);
>>> - memory_region_add_subregion(&s->container, PPC440_REG_BASE,
>>> &s->iomem);
>>> + memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs);
>>> sysbus_init_mmio(sbd, &s->container);
>>> }
>>>
>>
>>
>>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 12/13] ppc440_pcix: Don't use iomem for regs
2023-07-04 9:57 ` Philippe Mathieu-Daudé
@ 2023-07-04 10:14 ` BALATON Zoltan
0 siblings, 0 replies; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-04 10:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 2486 bytes --]
On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
> On 4/7/23 11:37, BALATON Zoltan wrote:
>> On Tue, 4 Jul 2023, Philippe Mathieu-Daudé wrote:
>>> On 4/7/23 00:02, BALATON Zoltan wrote:
>>>> The iomem memory region is better used for the PCI IO space but
>>>> currently used for registers. Stop using it for that to allow this to
>>>> be cleaned up in the next patch.
>>>>
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> ---
>>>> hw/ppc/ppc440_pcix.c | 7 ++++---
>>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
>>>> index adfecf1e76..ee2dc44f67 100644
>>>> --- a/hw/ppc/ppc440_pcix.c
>>>> +++ b/hw/ppc/ppc440_pcix.c
>>>> @@ -484,6 +484,7 @@ static void ppc440_pcix_realize(DeviceState *dev,
>>>> Error **errp)
>>>> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>> PPC440PCIXState *s;
>>>> PCIHostState *h;
>>>> + MemoryRegion *regs = g_new(MemoryRegion, 1);
>>>
>>> Why not hold it within PPC440PCIXState?
>>
>> Because it's never needed after this function.
>
> But we can't free() it because it has to stay valid as long as
> PPC440PCIXState is in use. So it seems to belong there.
OK, moved it to PPC440PCIXState.
Regards,
BALATON Zoltan
>>>> h = PCI_HOST_BRIDGE(dev);
>>>> s = PPC440_PCIX_HOST(dev);
>>>> @@ -507,11 +508,11 @@ static void ppc440_pcix_realize(DeviceState *dev,
>>>> Error **errp)
>>>> h, "pci-conf-idx", 4);
>>>> memory_region_init_io(&h->data_mem, OBJECT(s),
>>>> &pci_host_data_le_ops,
>>>> h, "pci-conf-data", 4);
>>>> - memory_region_init_io(&s->iomem, OBJECT(s), &pci_reg_ops, s,
>>>> - "pci.reg", PPC440_REG_SIZE);
>>>> + memory_region_init_io(regs, OBJECT(s), &pci_reg_ops, s, "pci-reg",
>>>> + PPC440_REG_SIZE);
>>>> memory_region_add_subregion(&s->container, PCIC0_CFGADDR,
>>>> &h->conf_mem);
>>>> memory_region_add_subregion(&s->container, PCIC0_CFGDATA,
>>>> &h->data_mem);
>>>> - memory_region_add_subregion(&s->container, PPC440_REG_BASE,
>>>> &s->iomem);
>>>> + memory_region_add_subregion(&s->container, PPC440_REG_BASE, regs);
>>>> sysbus_init_mmio(sbd, &s->container);
>>>> }
>>>>
>>>
>>>
>>>
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 13/13] ppc440_pcix: Stop using system io region for PCI bus
2023-07-03 22:02 [PATCH 00/13] PPC440 devices misc clean up BALATON Zoltan
` (11 preceding siblings ...)
2023-07-03 22:02 ` [PATCH 12/13] ppc440_pcix: Don't use iomem for regs BALATON Zoltan
@ 2023-07-03 22:02 ` BALATON Zoltan
2023-07-04 9:01 ` Philippe Mathieu-Daudé
12 siblings, 1 reply; 33+ messages in thread
From: BALATON Zoltan @ 2023-07-03 22:02 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
Use the iomem region 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>
---
hw/ppc/ppc440_pcix.c | 8 +++++---
hw/ppc/sam460ex.c | 6 +-----
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index ee2dc44f67..cca8a72c72 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -490,10 +490,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
s = PPC440_PCIX_HOST(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", 0x10000);
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),
TYPE_PPC4xx_HOST_BRIDGE);
@@ -514,6 +515,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, 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 8d0e551d14..1e615b8d35 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(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"));
- 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] 33+ messages in thread* Re: [PATCH 13/13] ppc440_pcix: Stop using system io region for PCI bus
2023-07-03 22:02 ` [PATCH 13/13] ppc440_pcix: Stop using system io region for PCI bus BALATON Zoltan
@ 2023-07-04 9:01 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 33+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-07-04 9:01 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Daniel Henrique Barboza
On 4/7/23 00:02, BALATON Zoltan wrote:
> Use the iomem region for the PCI io space and map it directly from the
> board without an intermediate alias that is not really needed.
"Reduce the I/O region to 64K."
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/ppc440_pcix.c | 8 +++++---
> hw/ppc/sam460ex.c | 6 +-----
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index ee2dc44f67..cca8a72c72 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -490,10 +490,11 @@ static void ppc440_pcix_realize(DeviceState *dev, Error **errp)
> s = PPC440_PCIX_HOST(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", 0x10000);
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),
> TYPE_PPC4xx_HOST_BRIDGE);
> @@ -514,6 +515,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, regs);
> sysbus_init_mmio(sbd, &s->container);
> + sysbus_init_mmio(sbd, &s->iomem);
> }
With the changes requested:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 33+ messages in thread