Patchwork [v4,1/6] pci: use PCI_DEVFN() where appropriate.

login
register
mail settings
Submitter Isaku Yamahata
Date June 21, 2010, 6:03 a.m.
Message ID <748531d265b3d915ddac6316fdcd5bb7f9475ab4.1277100005.git.yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/56292/
State New
Headers show

Comments

Isaku Yamahata - June 21, 2010, 6:03 a.m.
Use PCI_DEVFN() and PCI_FUNC_MAX where appropriate.
This patch make it clear that func = 0 and
added assert() to ensure it.
This patch guarantees that auto assigned function is always
single function device at function = 0.

Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yu Liu <yu.liu@freescale.com>
Cc: Paul Brook <paul@codesourcery.com>
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/gt64xxx.c       |    2 +-
 hw/pci.c           |    4 +++-
 hw/pci.h           |    1 +
 hw/ppce500_pci.c   |    3 ++-
 hw/unin_pci.c      |   12 ++++++------
 hw/versatile_pci.c |    2 +-
 6 files changed, 14 insertions(+), 10 deletions(-)
Michael S. Tsirkin - June 21, 2010, 11:57 a.m.
On Mon, Jun 21, 2010 at 03:03:56PM +0900, Isaku Yamahata wrote:
> Use PCI_DEVFN() and PCI_FUNC_MAX where appropriate.
> This patch make it clear that func = 0 and
> added assert() to ensure it.
> This patch guarantees that auto assigned function is always
> single function device at function = 0.
> 
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Yu Liu <yu.liu@freescale.com>
> Cc: Paul Brook <paul@codesourcery.com>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

How was this tested? I suggest splitting the asserts
to a separate patch, then you can compare
stripped object files before and after the change,
should be identical.


> ---
>  hw/gt64xxx.c       |    2 +-
>  hw/pci.c           |    4 +++-
>  hw/pci.h           |    1 +
>  hw/ppce500_pci.c   |    3 ++-
>  hw/unin_pci.c      |   12 ++++++------
>  hw/versatile_pci.c |    2 +-
>  6 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> index 7691e1d..8e2cf14 100644
> --- a/hw/gt64xxx.c
> +++ b/hw/gt64xxx.c
> @@ -1115,7 +1115,7 @@ PCIBus *pci_gt64120_init(qemu_irq *pic)
>  
>      s->pci->bus = pci_register_bus(NULL, "pci",
>                                     pci_gt64120_set_irq, pci_gt64120_map_irq,
> -                                   pic, 144, 4);
> +                                   pic, PCI_DEVFN(18, 0), 4);
>      s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
>      d = pci_register_device(s->pci->bus, "GT64120 PCI Bus", sizeof(PCIDevice),
>                              0, NULL, NULL);
> diff --git a/hw/pci.c b/hw/pci.c
> index ef17eb4..28ba9cf 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -224,6 +224,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
>                           const char *name, int devfn_min)
>  {
>      qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
> +    assert(PCI_FUNC(devfn_min) == 0);
>      bus->devfn_min = devfn_min;
>      QLIST_INIT(&bus->child);
>      vmstate_register(-1, &vmstate_pcibus, bus);
> @@ -600,8 +601,9 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>                                           uint8_t header_type)
>  {
>      if (devfn < 0) {
> +        assert(PCI_FUNC(bus->devfn_min) == 0);

I think that the assert when bus is created is enough.

>          for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
> -            devfn += 8) {
> +            devfn += PCI_FUNC_MAX) {
>              if (!bus->devices[devfn])
>                  goto found;
>          }
> diff --git a/hw/pci.h b/hw/pci.h
> index 6a2bc6a..077387d 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -14,6 +14,7 @@
>  #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
>  #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
>  #define PCI_FUNC(devfn)         ((devfn) & 0x07)
> +#define PCI_FUNC_MAX            8
>  
>  /* Class, Vendor and Device IDs from Linux's pci_ids.h */
>  #include "pci_ids.h"
> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
> index 336d284..f949fe3 100644
> --- a/hw/ppce500_pci.c
> +++ b/hw/ppce500_pci.c
> @@ -279,7 +279,8 @@ PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
>      controller->pci_state.bus = pci_register_bus(NULL, "pci",
>                                                   mpc85xx_pci_set_irq,
>                                                   mpc85xx_pci_map_irq,
> -                                                 pci_irqs, 0x88, 4);
> +                                                 pci_irqs, PCI_DEVFN(0x11, 0),
> +                                                 4);
>      d = pci_register_device(controller->pci_state.bus,
>                              "host bridge", sizeof(PCIDevice),
>                              0, NULL, NULL);
> diff --git a/hw/unin_pci.c b/hw/unin_pci.c
> index f0a773d..0ecf40f 100644
> --- a/hw/unin_pci.c
> +++ b/hw/unin_pci.c
> @@ -228,10 +228,10 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
>      d = FROM_SYSBUS(UNINState, s);
>      d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>                                           pci_unin_set_irq, pci_unin_map_irq,
> -                                         pic, 11 << 3, 4);
> +                                         pic, PCI_DEVFN(11, 0), 4);
>  
>  #if 0
> -    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north");
>  #endif
>  
>      sysbus_mmio_map(s, 0, 0xf2800000);
> @@ -240,11 +240,11 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
>      /* DEC 21154 bridge */
>  #if 0
>      /* XXX: not activated as PPC BIOS doesn't handle multiple buses properly */
> -    pci_create_simple(d->host_state.bus, 12 << 3, "dec-21154");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(12, 0), "dec-21154");
>  #endif
>  
>      /* Uninorth AGP bus */
> -    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north-agp");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north-agp");
>      dev = qdev_create(NULL, "uni-north-agp");
>      qdev_init_nofail(dev);
>      s = sysbus_from_qdev(dev);
> @@ -254,7 +254,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
>      /* Uninorth internal bus */
>  #if 0
>      /* XXX: not needed for now */
> -    pci_create_simple(d->host_state.bus, 14 << 3, "uni-north-pci");
> +    pci_create_simple(d->host_state.bus, PCI_DEVFN(14, 0), "uni-north-pci");
>      dev = qdev_create(NULL, "uni-north-pci");
>      qdev_init_nofail(dev);
>      s = sysbus_from_qdev(dev);
> @@ -280,7 +280,7 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic)
>  
>      d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
>                                           pci_unin_set_irq, pci_unin_map_irq,
> -                                         pic, 11 << 3, 4);
> +                                         pic, PCI_DEVFN(11, 0), 4);
>  
>      sysbus_mmio_map(s, 0, 0xf0800000);
>      sysbus_mmio_map(s, 1, 0xf0c00000);
> diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
> index 199bc19..a76bdfa 100644
> --- a/hw/versatile_pci.c
> +++ b/hw/versatile_pci.c
> @@ -127,7 +127,7 @@ 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_DEVFN(11, 0), 4);
>  
>      /* ??? Register memory space.  */
>  
> -- 
> 1.6.6.1

Patch

diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index 7691e1d..8e2cf14 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -1115,7 +1115,7 @@  PCIBus *pci_gt64120_init(qemu_irq *pic)
 
     s->pci->bus = pci_register_bus(NULL, "pci",
                                    pci_gt64120_set_irq, pci_gt64120_map_irq,
-                                   pic, 144, 4);
+                                   pic, PCI_DEVFN(18, 0), 4);
     s->ISD_handle = cpu_register_io_memory(gt64120_read, gt64120_write, s);
     d = pci_register_device(s->pci->bus, "GT64120 PCI Bus", sizeof(PCIDevice),
                             0, NULL, NULL);
diff --git a/hw/pci.c b/hw/pci.c
index ef17eb4..28ba9cf 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -224,6 +224,7 @@  void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
                          const char *name, int devfn_min)
 {
     qbus_create_inplace(&bus->qbus, &pci_bus_info, parent, name);
+    assert(PCI_FUNC(devfn_min) == 0);
     bus->devfn_min = devfn_min;
     QLIST_INIT(&bus->child);
     vmstate_register(-1, &vmstate_pcibus, bus);
@@ -600,8 +601,9 @@  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
                                          uint8_t header_type)
 {
     if (devfn < 0) {
+        assert(PCI_FUNC(bus->devfn_min) == 0);
         for(devfn = bus->devfn_min ; devfn < ARRAY_SIZE(bus->devices);
-            devfn += 8) {
+            devfn += PCI_FUNC_MAX) {
             if (!bus->devices[devfn])
                 goto found;
         }
diff --git a/hw/pci.h b/hw/pci.h
index 6a2bc6a..077387d 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -14,6 +14,7 @@ 
 #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
+#define PCI_FUNC_MAX            8
 
 /* Class, Vendor and Device IDs from Linux's pci_ids.h */
 #include "pci_ids.h"
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 336d284..f949fe3 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -279,7 +279,8 @@  PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
     controller->pci_state.bus = pci_register_bus(NULL, "pci",
                                                  mpc85xx_pci_set_irq,
                                                  mpc85xx_pci_map_irq,
-                                                 pci_irqs, 0x88, 4);
+                                                 pci_irqs, PCI_DEVFN(0x11, 0),
+                                                 4);
     d = pci_register_device(controller->pci_state.bus,
                             "host bridge", sizeof(PCIDevice),
                             0, NULL, NULL);
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index f0a773d..0ecf40f 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -228,10 +228,10 @@  PCIBus *pci_pmac_init(qemu_irq *pic)
     d = FROM_SYSBUS(UNINState, s);
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_unin_set_irq, pci_unin_map_irq,
-                                         pic, 11 << 3, 4);
+                                         pic, PCI_DEVFN(11, 0), 4);
 
 #if 0
-    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north");
 #endif
 
     sysbus_mmio_map(s, 0, 0xf2800000);
@@ -240,11 +240,11 @@  PCIBus *pci_pmac_init(qemu_irq *pic)
     /* DEC 21154 bridge */
 #if 0
     /* XXX: not activated as PPC BIOS doesn't handle multiple buses properly */
-    pci_create_simple(d->host_state.bus, 12 << 3, "dec-21154");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(12, 0), "dec-21154");
 #endif
 
     /* Uninorth AGP bus */
-    pci_create_simple(d->host_state.bus, 11 << 3, "uni-north-agp");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(11, 0), "uni-north-agp");
     dev = qdev_create(NULL, "uni-north-agp");
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
@@ -254,7 +254,7 @@  PCIBus *pci_pmac_init(qemu_irq *pic)
     /* Uninorth internal bus */
 #if 0
     /* XXX: not needed for now */
-    pci_create_simple(d->host_state.bus, 14 << 3, "uni-north-pci");
+    pci_create_simple(d->host_state.bus, PCI_DEVFN(14, 0), "uni-north-pci");
     dev = qdev_create(NULL, "uni-north-pci");
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
@@ -280,7 +280,7 @@  PCIBus *pci_pmac_u3_init(qemu_irq *pic)
 
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_unin_set_irq, pci_unin_map_irq,
-                                         pic, 11 << 3, 4);
+                                         pic, PCI_DEVFN(11, 0), 4);
 
     sysbus_mmio_map(s, 0, 0xf0800000);
     sysbus_mmio_map(s, 1, 0xf0c00000);
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 199bc19..a76bdfa 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -127,7 +127,7 @@  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_DEVFN(11, 0), 4);
 
     /* ??? Register memory space.  */