[PATCH] powerpc: make U4 PCIe work

[Segher Boessenkool]

> +        if (bus == hose->first_busno) {
> +                caddr = U4_PCIE_CFA0(dev_fn, offset);
> +        } else
> +                caddr = U4_PCIE_CFA1(bus, dev_fn, offset);

Please fold CFA0 and CFA1 into one inline function, much
cleaner.

You also should check (at init time) whether indirect config
access mode is actually enabled -- or just enable it yourself.

> +        /* Uninorth will return garbage if we don't read back the  
> value ! */
> +        do {
> +                out_le32(hose->cfg_addr, caddr);
> +        } while (in_le32(hose->cfg_addr) != caddr);

Not an issue on U4, kill this.

> +static int u4_pcie_read_config(struct pci_bus *bus, unsigned int  
> devfn,
> +                               int offset, int len, u32 *val)
> +{
> +        struct pci_controller *hose;
> +        volatile void __iomem *addr;
> +
> +        hose = pci_bus_to_host(bus);
> +        if (hose == NULL)
> +                return PCIBIOS_DEVICE_NOT_FOUND;

Is this check needed?  Can this code ever be called if hose would
be 0?  The variable isn't actually used anywhere else in this
function either.

> +        struct pci_controller *hose;
> +        volatile void __iomem *addr;

No need for the volatile, the inXX()/outXX() things should handle
this just fine.

> +        /* The bus contains a bridge from root -> device, we need to
> +         * make it visible on bus 0 so that we pick the right type
> +         * of config cycles. If we didn't, we would have to force all
> +         * config cycles to be type 1. So we override the "bus-range"
> +         * property here
> +         */
> +        hose->first_busno = 0x00;
> +        hose->last_busno = 0xff;

I don't understand the comment, nor the need for this code.  Is this
just old garbage ported over from PowerMac?

> +	for_each_pci_dev(dev) {
> +		/* Fixup IRQ for PCIe host */
> +		if (u4_pcie != NULL && dev->bus->number == 0 &&
> +		    pci_bus_to_host(dev->bus) == u4_pcie) {
> +			printk(KERN_DEBUG "Fixup U4 PCIe IRQ\n");
> +			dev->irq = irq_create_mapping(NULL, 1);
> +			if (dev->irq != NO_IRQ)
> +				set_irq_type(dev->irq, IRQ_TYPE_LEVEL_LOW);

Please do this in the platform code for boards that actually need
it, by fixing up the interrupt tree in the device tree.  MPIC IRQ1
is the PCIe error interrupt only btw, I wonder if this code does
the right thing at all?

> +        /*
> +         * We need to call pci_setup_phb_io for the HT bridge first
> +         * so it gets the I/O port numbers starting at 0, and we
> +         * need to call it for the AGP bridge after that so it gets
> +         * small positive I/O port numbers.
> +         */

Comment out of date (doesn't mention PCIe).  Oh and when do we
finally get real PCI domain support ;-)

> +	if (!machine_is(maple))
> +		return;

So the IDE wart fixup is now only executed on "real" Maples?  Or
all 970-based non-Apple boxes?  The naming starts to be confusing,
maybe the maple platform should be en-masse renamed to "ppc970" or
such.


Segher