Subject: Re: Changing ibm4xx device tree.
To: Shigeyuki Fukushima <shige@NetBSD.org>
From: Jachym Holecek <freza@dspfpga.com>
List: port-powerpc
Date: 05/07/2006 04:22:59
# Shigeyuki Fukushima 2006-05-06:
> But I don't agree that we will add new declarations (such as (*1)) to
> plb and opb driver and update plb.c and opb.c when supporting for new
> ibm4xx cpu.

Well, at the moment it's only used by the 405GP/GPr. A quick look at
AMCCs datasheets suggests that most models could be quite elegantly
supported with just some header file tweaking (looks like we could
put most 405s and the 440EP to a single header file without major harm).

The 440SP/GX/GP are quite different beasts (they'd need some changes at
the pmap layer AFAIU, so wondering about address space layout sounds
premature ;-).

One detail that I think could be improved though is to change the opb
device table from:

        { IBM405GP,     "com",  IBM405GP_UART0_BASE,     0, 0 },
	...
        { IBM405GPR,    "com",  IBM405GP_UART0_BASE,     0, 0 },

to something like:

static int
cpu_is_405gpx(int pvr)
{
	return (pvr == IBM405GP || pvr == IBM405GPR)
}
..
        { cpu_is_405gpx,    "com",  IBM405GP_UART0_BASE,     0, 0 },

so that duplicate entries can be avoided.

> (*1) [OPB/PLB device table per CPU model]
> 
> I want to separate it from plb and opb driver.
> I developed a patch (attached with this e-mail).
> 
> Are this idea and patch ok?

A couple of random notes regarding the patch:

  * powerpc/include/ibm4xx/ibm405gpr.h is a copy of ibm405gp.h

  * separating GP and GPr doesn't make much sense to me -- the only
    difference between them is in cache size and clocking frequencies,
    at least as per AMCCs product brief

  * unless I'm missing something, it silently makes the choice of 4xx
    CPU model into a compile time option -- I think it's OK for this class
    of systems, but people may think otherwise

  * I don't like the way device/PCI-config tables are passed -- I'd just
    declare 'extern table' in the driver and let ibm${foo}.c provide it
    in the compile-time case, or use device properties (or whatever it's
    called now ;) in the runtime case

Anyway, just my 0.05EUR.

	-- Jachym