On Tue, 3 Jun 2025, Philippe Mathieu-Daudé wrote:
Hi Zoltan,

On 4/5/25 18:01, BALATON Zoltan wrote:
The raven PCI device does not need a state struct as it has no data to
store there any more so we can remove that to simplify code.

Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
  hw/pci-host/raven.c | 30 +-----------------------------
  1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index f8c0be5d21..172f01694c 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -31,7 +31,6 @@
  #include "hw/pci/pci_bus.h"
  #include "hw/pci/pci_host.h"
  #include "hw/qdev-properties.h"
-#include "migration/vmstate.h"
  #include "hw/intc/i8259.h"
  #include "hw/irq.h"
  #include "hw/or-irq.h"
@@ -40,12 +39,6 @@
  #define TYPE_RAVEN_PCI_DEVICE "raven"
  #define TYPE_RAVEN_PCI_HOST_BRIDGE "raven-pcihost"
  -OBJECT_DECLARE_SIMPLE_TYPE(RavenPCIState, RAVEN_PCI_DEVICE)
-
-struct RavenPCIState {
-    PCIDevice dev;
-};
-
  typedef struct PRePPCIState PREPPCIState;
  DECLARE_INSTANCE_CHECKER(PREPPCIState, RAVEN_PCI_HOST_BRIDGE,
                           TYPE_RAVEN_PCI_HOST_BRIDGE)
@@ -65,7 +58,6 @@ struct PRePPCIState {
      MemoryRegion bm_ram_alias;
      MemoryRegion bm_pci_memory_alias;
      AddressSpace bm_as;
-    RavenPCIState pci_dev;

I'd rather the PF0 be reachable from PHB:

      PCIDevice *func0;

        int contiguous_map;
  };
@@ -268,8 +260,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error **errp)
                            "pci-intack", 1);
memory_region_add_subregion(address_space_mem, 0xbffffff0, &s->pci_intack);
  -    /* TODO Remove once realize propagates to child devices. */
-    qdev_realize(DEVICE(&s->pci_dev), BUS(&s->pci_bus), errp);

  func0 =

+ pci_create_simple(&s->pci_bus, PCI_DEVFN(0, 0), TYPE_RAVEN_PCI_DEVICE);
  }

This would be a variable set but not used. Why do we store a value that we don't need and easy to look up when needed?

Regards,
BALATON Zoltan

If you don't object, I can amend if queuing; otherwise:

Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>

Reply via email to