diff mbox

[3/6] versatile_pci: user PCIHostState instead of PCIBus

Message ID 1263286378-10398-4-git-send-email-yamahata@valinux.co.jp
State New
Headers show

Commit Message

Isaku Yamahata Jan. 12, 2010, 8:52 a.m. UTC 
To use pci host framework, use PCIHostState instead of PCIBus in PCIVPBState.

Cc: Paul Brook <paul@codesourcery.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/versatile_pci.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

Comments

Paul Brook Jan. 13, 2010, 1:02 p.m. UTC | #1 
On Tuesday 12 January 2010, Isaku Yamahata wrote:
> To use pci host framework, use PCIHostState instead of PCIBus in
>  PCIVPBState.

No.

pci_host.[ch] provides very specific functionality, it is not a generic PCI 
host device. Specifically it provides indirect access to PCI config space via 
a memory mapped {address,data} pair. The versatile PCI host exposes PCI config 
space directly, so should not be using this code.

If you want a generic framework for PCI hosts then you need to use something 
else. If nothing else, assuming that a PCI host bridge is always is SysBus 
device is wrong.

Paul
Michael S. Tsirkin Jan. 13, 2010, 1:04 p.m. UTC | #2 
On Wed, Jan 13, 2010 at 01:02:50PM +0000, Paul Brook wrote:
> On Tuesday 12 January 2010, Isaku Yamahata wrote:
> > To use pci host framework, use PCIHostState instead of PCIBus in
> >  PCIVPBState.
> 
> No.
> 
> pci_host.[ch] provides very specific functionality, it is not a generic PCI 
> host device. Specifically it provides indirect access to PCI config space via 
> a memory mapped {address,data} pair. The versatile PCI host exposes PCI config 
> space directly, so should not be using this code.
> 
> If you want a generic framework for PCI hosts then you need to use something 
> else. If nothing else, assuming that a PCI host bridge is always is SysBus 
> device is wrong.
> 
> Paul

What most people seem to want is callback that will get length is a
parameter instead of supplying 3 functions. pci_host does it
but we do not need pci_host for this.
diff mbox

Patch

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 153c651..d99b7fa 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -12,7 +12,7 @@ 
 #include "pci_host.h"
 
 typedef struct {
-    SysBusDevice busdev;
+    PCIHostState pci_host;
     qemu_irq irq[4];
     int realview;
     int mem_config;
@@ -26,38 +26,43 @@  static inline uint32_t vpb_pci_config_addr(target_phys_addr_t addr)
 static void pci_vpb_config_writeb (void *opaque, target_phys_addr_t addr,
                                    uint32_t val)
 {
-    pci_data_write(opaque, vpb_pci_config_addr (addr), val, 1);
+    PCIHostState *s = opaque;
+    pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 1);
 }
 
 static void pci_vpb_config_writew (void *opaque, target_phys_addr_t addr,
                                    uint32_t val)
 {
+    PCIHostState *s = opaque;
 #ifdef TARGET_WORDS_BIGENDIAN
     val = bswap16(val);
 #endif
-    pci_data_write(opaque, vpb_pci_config_addr (addr), val, 2);
+    pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 2);
 }
 
 static void pci_vpb_config_writel (void *opaque, target_phys_addr_t addr,
                                    uint32_t val)
 {
+    PCIHostState *s = opaque;
 #ifdef TARGET_WORDS_BIGENDIAN
     val = bswap32(val);
 #endif
-    pci_data_write(opaque, vpb_pci_config_addr (addr), val, 4);
+    pci_data_write(s->bus, vpb_pci_config_addr (addr), val, 4);
 }
 
 static uint32_t pci_vpb_config_readb (void *opaque, target_phys_addr_t addr)
 {
+    PCIHostState *s = opaque;
     uint32_t val;
-    val = pci_data_read(opaque, vpb_pci_config_addr (addr), 1);
+    val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 1);
     return val;
 }
 
 static uint32_t pci_vpb_config_readw (void *opaque, target_phys_addr_t addr)
 {
+    PCIHostState *s = opaque;
     uint32_t val;
-    val = pci_data_read(opaque, vpb_pci_config_addr (addr), 2);
+    val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 2);
 #ifdef TARGET_WORDS_BIGENDIAN
     val = bswap16(val);
 #endif
@@ -66,8 +71,9 @@  static uint32_t pci_vpb_config_readw (void *opaque, target_phys_addr_t addr)
 
 static uint32_t pci_vpb_config_readl (void *opaque, target_phys_addr_t addr)
 {
+    PCIHostState *s = opaque;
     uint32_t val;
-    val = pci_data_read(opaque, vpb_pci_config_addr (addr), 4);
+    val = pci_data_read(s->bus, vpb_pci_config_addr (addr), 4);
 #ifdef TARGET_WORDS_BIGENDIAN
     val = bswap32(val);
 #endif
@@ -114,7 +120,8 @@  static void pci_vpb_map(SysBusDevice *dev, target_phys_addr_t base)
 
 static int pci_vpb_init(SysBusDevice *dev)
 {
-    PCIVPBState *s = FROM_SYSBUS(PCIVPBState, dev);
+    PCIHostState *pci_host = FROM_SYSBUS(PCIHostState, dev);
+    PCIVPBState *s = DO_UPCAST(PCIVPBState, pci_host, pci_host);
     PCIBus *bus;
     int i;
 
@@ -124,11 +131,12 @@  static int pci_vpb_init(SysBusDevice *dev)
     bus = pci_register_bus(&dev->qdev, "pci",
                            pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
                            11 << 3, 4);
+    pci_host->bus = bus;
 
     /* ??? Register memory space.  */
 
     s->mem_config = cpu_register_io_memory(pci_vpb_config_read,
-                                           pci_vpb_config_write, bus);
+                                           pci_vpb_config_write, pci_host);
     sysbus_init_mmio_cb(dev, 0x04000000, pci_vpb_map);
 
     pci_create_simple(bus, -1, "versatile_pci_host");
@@ -137,7 +145,8 @@  static int pci_vpb_init(SysBusDevice *dev)
 
 static int pci_realview_init(SysBusDevice *dev)
 {
-    PCIVPBState *s = FROM_SYSBUS(PCIVPBState, dev);
+    PCIHostState *pci_host = FROM_SYSBUS(PCIHostState, dev);
+    PCIVPBState *s = DO_UPCAST(PCIVPBState, pci_host, pci_host);
     s->realview = 1;
     return pci_vpb_init(dev);
 }