On 18/09/2025 19:50, BALATON Zoltan wrote:
Instead of passing unneeded enclosing objects to the config direct
access ops that only need the bus we can pass that directly thus
simplifying the functions.
Signed-off-by: BALATON Zoltan <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
---
hw/pci-host/raven.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index d7a0bde382..2057a1869f 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -65,16 +65,16 @@ static inline uint32_t raven_idsel_to_addr(hwaddr addr)
static void raven_mmcfg_write(void *opaque, hwaddr addr, uint64_t val,
unsigned int size)
{
- PREPPCIState *s = opaque;
- PCIHostState *phb = PCI_HOST_BRIDGE(s);
- pci_data_write(phb->bus, raven_idsel_to_addr(addr), val, size);
+ PCIBus *hbus = opaque;
+
+ pci_data_write(hbus, raven_idsel_to_addr(addr), val, size);
}
static uint64_t raven_mmcfg_read(void *opaque, hwaddr addr, unsigned int size)
{
- PREPPCIState *s = opaque;
- PCIHostState *phb = PCI_HOST_BRIDGE(s);
- return pci_data_read(phb->bus, raven_idsel_to_addr(addr), size);
+ PCIBus *hbus = opaque;
+
+ return pci_data_read(hbus, raven_idsel_to_addr(addr), size);
}
static const MemoryRegionOps raven_mmcfg_ops = {
@@ -233,7 +233,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error
**errp)
"pci-conf-data", 4);
memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
- memory_region_init_io(&h->mmcfg, OBJECT(s), &raven_mmcfg_ops, s,
+ memory_region_init_io(&h->mmcfg, OBJECT(h), &raven_mmcfg_ops, h->bus,
"pci-mmcfg", 0x00400000);
memory_region_add_subregion(address_space_mem, 0x80800000, &h->mmcfg);
I find this confusing as a reviewer since the general expectation is that the device
is passed as the opaque for the memory region rather than the bus. What is the reason
for trying to change an existing convention here?
You could simplify what is there by dropping the PREPPCIState reference and
simply doing:
static uint64_t raven_mmcfg_read(void *opaque, hwaddr addr, unsigned int size)
{
PCIHostState *phb = PCI_HOST_BRIDGE(opaque);
return pci_data_read(phb->bus, raven_idsel_to_addr(addr), size);
}
etc.
ATB,
Mark.
