On Mon, Oct 22, 2012 at 4:17 PM, Michael S. Tsirkin <m...@redhat.com> wrote: > On Mon, Oct 22, 2012 at 03:26:24PM +0200, Andreas Färber wrote: >> Am 19.10.2012 22:43, schrieb Jason Baron: >> > From: Jason Baron <jba...@redhat.com> >> > >> > This adds support for the DECchip 21154 PCI bridge. >> > >> > Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp> >> > Signed-off-by: Jason Baron <jba...@redhat.com> >> > --- >> > hw/Makefile.objs | 2 +- >> > hw/i21154.c | 113 >> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> > hw/i21154.h | 9 ++++ >> > 3 files changed, 123 insertions(+), 1 deletions(-) >> > create mode 100644 hw/i21154.c >> > create mode 100644 hw/i21154.h >> >> Why is this creating a new file and not reusing dec_pci.c? We shouldn't >> have two parallel implementations of the same chip. >> >> Andreas > > Good point I missed this. There's a minor difference > wrt dec-21154-p2p-bridge in a couple of fields, > these could be set by properties. > Also dec_map_irq differs from the spec compliant > map function. I am guessing this is a bug. > Would appreciate testing of the patch below. > > > Are you familiar with dec_pci.c? Looking at it, it seems to > implement a pci host bridge "dec-21154-sysbus" , > a pci to pci bridge "dec-21154-p2p-bridge", > and something called "dec-21154" which sports a comment > "PCI2PCI bridge same values as PearPC - check this" - > and implements an empty init function; > what this last is and why it's useful I am not sure. > > Anyone? Blue Swirl? Anyone can test this doesn't break > things and report?
The device is only linked by PPC but the init function is not invoked. It should be also used by Sparc64 (it's present on real Ultra-5 machine and several devices should be behind the bridge) but it isn't. > > ---> > > dec_pci: irq swizzle PCI spec compliance > > Make IRQ mapping for dec PCI PCI 2 PCI Bridge compliant > with the PCI spec. > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > --- > > diff --git a/hw/dec_pci.c b/hw/dec_pci.c > index c30ade3..a49f0bd 100644 > --- a/hw/dec_pci.c > +++ b/hw/dec_pci.c > @@ -82,7 +82,7 @@ PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn) > dev = pci_create_multifunction(parent_bus, devfn, false, > "dec-21154-p2p-bridge"); > br = DO_UPCAST(PCIBridge, dev, dev); > - pci_bridge_map_irq(br, "DEC 21154 PCI-PCI bridge", dec_map_irq); dec_map_irq is now unused and should be removed to avoid build breakage. > + pci_bridge_map_irq(br, "DEC 21154 PCI-PCI bridge", > pci_swizzle_map_irq_fn); > qdev_init_nofail(&dev->qdev); > return pci_bridge_get_sec_bus(br); > } Otherwise I think the patch is fine.
