Re: [PATCH v4 04/13] [POWERPC] Add generic support for simple MPC5200 based boards

[Stephen Rothwell <sfr@canb.auug.org.au>]

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]

On Mon, 12 Nov 2007 23:22:16 +0100 Marian Balakowicz <m8@semihalf.com> wrote:
>
> Stephen Rothwell wrote:
> > On Fri, 09 Nov 2007 18:12:02 +0100 Marian Balakowicz <m8@semihalf.com> wrote:
> >> +++ b/arch/powerpc/platforms/52xx/mpc5200_simple.c
> >>
> >> +static int __init mpc5200_simple_probe(void)
> >> +{
> >> +	unsigned long node = of_get_flat_dt_root();
> > 
> > You need to include asm/prom.h to use the flattened device tree accessors.
> 
> Right, but this one is already included in mpc52xx.h.

But you should directly include it since you are using stuff declared in
there.  Depending on indirect includes is fragile (someone might take it
out of mpc52xx.h one day).

> >> +	/* list of the supported boards */
> >> +	const char *board[] = {	
> >> +		"tqc,tqm5200",
> >> +		"promess,motionpro",
> >> +		"schindler,cm5200",
> >> +		NULL
> >> +	};
> > 
> > Make that static.
> 
> Board table is no longer needed after kernel is initialized, it would
> be nice to declare it static and __initdata, but that would be quite
> ugly as it's a table of pointers and each string would require
> separate statement too. If we don't do it then what's the benefit of
> making it static?

Except the way it currently is, the data stays in the kernel image.  If
you declared it static and __initdata then at least the table would
actually go away at run time.  True, to make the strings go away is
harder.

Doesn't actually matter much.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]