Message ID | 748531d265b3d915ddac6316fdcd5bb7f9475ab4.1277100005.git.yamahata@valinux.co.jp |
---|---|
State | New |
Headers | show |
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
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. */
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(-)