Re: [PATCH 16/25] powerpc: EP405 boards support for arch/powerpc

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2007-12-06 at 21:05 -0600, Josh Boyer wrote:
> On Thu, 06 Dec 2007 19:00:15 +1100
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> 
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > Index: linux-work/arch/powerpc/boot/dts/ep405.dts
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ linux-work/arch/powerpc/boot/dts/ep405.dts	2007-12-03 12:58:45.000000000 +1100
> > @@ -0,0 +1,221 @@
> > +/*
> > + * Device Tree Source for EP405
> > + *
> > + * Copyright 2007 IBM Corp.
> > + * Josh Boyer <jwboyer@linux.vnet.ibm.com>
> 
> I still don't think I wrote this ;)

Yeah, right, I'll fix that, easy enough.


 .../...

> > +	if (!machine_is(ep405))
> > +		return 0;
> > +
> > +	/* FIXME: do bus probe here */
> 
> I should really remove this stupid FIXME from my files so people stop
> copying it into theirs ;)

Yup :-) Your fault ;-)

  .../...

> > +	/* Find the bloody thing & map it */
> > +	bcsr_node = of_find_compatible_node(NULL, NULL, "ep405-bcsr");
> > +	if (bcsr_node == NULL) {
> > +		printk(KERN_ERR "EP405 BCSR not found !\n");
> > +		return;
> > +	}
> > +	bcsr_regs = of_iomap(bcsr_node, 0);
> > +	if (bcsr_regs == NULL) {
> > +		printk(KERN_ERR "EP405 BCSR failed to map !\n");
> > +		return;
> > +	}
> 
> Is there a reason you have bcsr_node and bcsr_regs as static globals
> and leave the mapping present?  I can't see another use of them outside
> of this function, which only gets called once.

For future use mostly. There's truckloads of things on this board going
trough this FPGA and It's likely that a more complete port will need to
use this things. In fact, the CPLD can more/less be used as a cascaded
IRQ on the PCI interrupts on that thing.

Ben.