Re: [4/5] powerpc: PA Semi PWRficient platform support

[Olof Johansson <olof@lixom.net>]

On Wed, 6 Sep 2006 15:38:54 +0200 Segher Boessenkool <segher@kernel.crashing.org> wrote:

> > +#define PA_PXP_CFA(bus, devfn, off) (((bus) << 20) | ((devfn) <<  
> > 12) | (off))
> 
> > +	hose->cfg_data = ioremap(0xe0000000, 0x1000000);
> 
> Shouldn't that length be 0x10000000 then?  Or can't you have
> more than 0x10 busses?

Yes, typo that hasn't hit us yet. Thanks.

> > +	bus_range = (int *) get_property(dev, "bus-range", &len);
> > +	if (bus_range == NULL || len < 2 * sizeof(int)) {
> > +		printk(KERN_WARNING "Can't get bus-range for %s, assume bus 0\n",
> > +		dev->full_name);
> > +	}
> 
> Just kill all this "bus-range" stuff?  Or do you really need it?

Not at this time. Removed.

> > +	np = of_find_node_by_type(np, "open-pic");
> > +	if (!np) {
> > +		printk(KERN_ERR "No interrupt controller in device tree.\n");
> > +		return;
> > +	}
> 
> "device_type" for the MPIC should be "interrupt-controller", "open-pic"
> should be in the "compatible" property instead.

Thanks, I now use the same logic as Maple.

> > +	/* Find address list in /platform-open-pic */
> 
> Erm, can't you get this info from the open-pic node itself?
> I know maple does it like this but that should be fixed ;-)

How about I keep it this way until Maple is fixed then? It's better to keep it
fairly common anyway. Functionally there's no difference.

> > +	mpic = mpic_alloc(mpic_node, openpic_addr, MPIC_PRIMARY, 0, 0,
> > +			  " PAS-OPIC  ");
> > +	BUG_ON(mpic == NULL);
> > +
> > +	mpic_assign_isu(mpic, 0, openpic_addr + 0x10000);
> > +	mpic_init(mpic);
> 
> Very nice, I'll convert maple to do the same (unless someone beats
> me to it, heh).

Maple isn't that different there. It just has to deal with multiple ISUs, 
while we currently only have one.


-Olof