[PATCH 10/16] hw/pci-host/raven: Use correct parameter in direct access ops

BALATON Zoltan posted 16 patches 7 months, 2 weeks ago
Maintainers: "Hervé Poussineau" <hpoussin@reactos.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
There is a newer version of this series
[PATCH 10/16] hw/pci-host/raven: Use correct parameter in direct access ops
Posted by BALATON Zoltan 7 months, 2 weeks ago
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 <balaton@eik.bme.hu>
---
 hw/pci-host/raven.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index d7a0bde382..c39e95b45f 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -65,16 +65,12 @@ 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);
+    pci_data_write(opaque, 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);
+    return pci_data_read(opaque, raven_idsel_to_addr(addr), size);
 }
 
 static const MemoryRegionOps raven_mmcfg_ops = {
@@ -233,7 +229,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);
 
-- 
2.41.3
Re: [PATCH 10/16] hw/pci-host/raven: Use correct parameter in direct access ops
Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago
On 4/5/25 18:01, 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 <balaton@eik.bme.hu>
> ---
>   hw/pci-host/raven.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
> index d7a0bde382..c39e95b45f 100644
> --- a/hw/pci-host/raven.c
> +++ b/hw/pci-host/raven.c
> @@ -65,16 +65,12 @@ 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);

Something like this is clearer IMHO:

        PCIBus *hbus = opaque;

        pci_data_write(hbus, raven_idsel_to_addr(addr), val, size);

> +    pci_data_write(opaque, 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);

Ditto.

> +    return pci_data_read(opaque, raven_idsel_to_addr(addr), size);
>   }
>   
>   static const MemoryRegionOps raven_mmcfg_ops = {
> @@ -233,7 +229,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);
>   

Casting the opaque:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>