Remove ifdefs from setup_arch()

Remove ifdefs from setup_arch()

[Gerald Champagne]

I have been modifying the linux kernel to support a custom hardware board of 
ours, and I'm trying to minimize additional changes and ifdefs in the kernel.
I noticed that the setup_arch function in arch/mips/kernel/setup.c has a new
ifdef for each board type that is supported, and it looks like this could be
simplified.  The code looks something like this:

-----------------
void __init setup_arch(char **cmdline_p)
{
	void boardname1_setup(void);
	void boardname1_setup(void);
	void boardname1_setup(void);

	...

	switch (mips_machgroup)
	{
#ifdef CONFIG_BOARDNAME1
	case: MACH_GROUP_WHATEVER1:
		boardname1_setup();
		break;
#endif

#ifdef CONFIG_BOARDNAME2
	case: MACH_GROUP_WHATEVER2:
		boardname2_setup();
		break;
#endif

#ifdef CONFIG_BOARDNAME3
	case: MACH_GROUP_WHATEVER3:
		boardname3_setup();
		break;
#endif

	default:
		panic("Unsupported architecture");
	}
	...
-----------------


For each configuration, only one case is compiled in.  Wouldn't it
be simpler to just give the board-specific setup function a common name
and consider it part of the board-specific api like all the other
board-specific functions.  Can this be changed to just this:

-----------------
void __init setup_arch(char **cmdline_p)
{
	void foo_setup(void);

	...

	foo_setup();  /* someone pick a name for this */
	...
-----------------

I'm trying to document an api for supporting an arbitrary board, and little
things like this make it more difficult to define something along the lines
of a bsp interface.  Any suggestions?  Any objections?

Thanks.

Gerald

Re: Remove ifdefs from setup_arch()

[Ralf Baechle]

On Wed, Oct 03, 2001 at 02:11:26PM -0500, Gerald Champagne wrote:

> For each configuration, only one case is compiled in.  Wouldn't it
> be simpler to just give the board-specific setup function a common name
> and consider it part of the board-specific api like all the other
> board-specific functions.  Can this be changed to just this:
> 
> -----------------
> void __init setup_arch(char **cmdline_p)
> {
> 	void foo_setup(void);
> 
> 	...
> 
> 	foo_setup();  /* someone pick a name for this */
> 	...
> -----------------
> 
> I'm trying to document an api for supporting an arbitrary board, and little
> things like this make it more difficult to define something along the lines
> of a bsp interface.  Any suggestions?  Any objections?

We used to allow support for multiple boards in one kernel binary though
that usually doesn't work for MIPS due to the large number of very different
systems.  People have asked to resurrect this option, so I'd like to go
for an option that only removes all those awful #ifdefs.  Something based
on ELF sections looks like a way to do this.

  Ralf

Re: Remove ifdefs from setup_arch()

[Jun Sun]

Gereald,

The following is copied from one of my previous posts in linux-mips mailing
list.  I think this is a reasonable idea.

BTW, do_initcalls() scheme is the same thing as Ralf is referring to as ELF
section.

Jun

.....
I talked about machine detection a while back.  My idea is following:

0. all machines that are *configured* into the image will supply <my>_detect()
and <my>_setup() functions.

1. at MIPS start up, we loop through all <my>_detect(), which returns three
values, a) run-time detection negative, b) run-time detection positive, and c)
no run-time detection code, but I return positive because I am configured in.

2. the startup code resolves conflicts (which sometimes may panic); and decide
on one machine.

3. then the startup code calls the right <my>_setup() code which will set up
the mach_type and other stuff. 

BTW, a side benenfit of doing this is that we can remove all the machine
specific code in common files such as bootinfo.h, setup.c, etc, which are
getting harder and harder to maintain as we are adding more and more embedded
boards to the tree.  

If we push it to an extreme by using mechnism like do_initcalls(), we can
achieve zero common source file modification when adding a new a machine. 
This will greatly facilitate embedded development as it allows more parallel
development and allow people to maintain their own board code much easier
before the code is submitted to the tree.

Jun




Ralf Baechle wrote:
> 
> On Wed, Oct 03, 2001 at 02:11:26PM -0500, Gerald Champagne wrote:
> 
> > For each configuration, only one case is compiled in.  Wouldn't it
> > be simpler to just give the board-specific setup function a common name
> > and consider it part of the board-specific api like all the other
> > board-specific functions.  Can this be changed to just this:
> >
> > -----------------
> > void __init setup_arch(char **cmdline_p)
> > {
> >       void foo_setup(void);
> >
> >       ...
> >
> >       foo_setup();  /* someone pick a name for this */
> >       ...
> > -----------------
> >
> > I'm trying to document an api for supporting an arbitrary board, and little
> > things like this make it more difficult to define something along the lines
> > of a bsp interface.  Any suggestions?  Any objections?
> 
> We used to allow support for multiple boards in one kernel binary though
> that usually doesn't work for MIPS due to the large number of very different
> systems.  People have asked to resurrect this option, so I'd like to go
> for an option that only removes all those awful #ifdefs.  Something based
> on ELF sections looks like a way to do this.
> 
>   Ralf

Re: Remove ifdefs from setup_arch()

[Geert Uytterhoeven]

On Wed, 3 Oct 2001, Jun Sun wrote:
> I talked about machine detection a while back.  My idea is following:
> 
> 0. all machines that are *configured* into the image will supply <my>_detect()
> and <my>_setup() functions.
> 
> 1. at MIPS start up, we loop through all <my>_detect(), which returns three
> values, a) run-time detection negative, b) run-time detection positive, and c)
> no run-time detection code, but I return positive because I am configured in.
> 
> 2. the startup code resolves conflicts (which sometimes may panic); and decide
> on one machine.
> 
> 3. then the startup code calls the right <my>_setup() code which will set up
> the mach_type and other stuff. 

Nice!

I suppose you want to have struct containing pointers to both the detect() and
setup() functions, so you know which setup() function you have to call?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: Remove ifdefs from setup_arch()

[Jun Sun]

Geert Uytterhoeven wrote:
> 
> On Wed, 3 Oct 2001, Jun Sun wrote:
> > I talked about machine detection a while back.  My idea is following:
> >
> > 0. all machines that are *configured* into the image will supply <my>_detect()
> > and <my>_setup() functions.
> >
> > 1. at MIPS start up, we loop through all <my>_detect(), which returns three
> > values, a) run-time detection negative, b) run-time detection positive, and c)
> > no run-time detection code, but I return positive because I am configured in.
> >
> > 2. the startup code resolves conflicts (which sometimes may panic); and decide
> > on one machine.
> >
> > 3. then the startup code calls the right <my>_setup() code which will set up
> > the mach_type and other stuff.
> 
> Nice!
> 
> I suppose you want to have struct containing pointers to both the detect() and
> setup() functions, so you know which setup() function you have to call?
> 

The actual mechanism can vary and be flexible, but here is more detail what I
had in mind:

1. <my>_detect is placed in a special ELF section (mips_mach_detect), using
similar mechanism as .initcall.init section and __setup() macro.

2. in addition to the 3 possible return value, <my>_detect also returns a
function pointer to <my>_setup.  Once a final candidate is chosen, the machine
detection code will issue the right <my>_setup call.

There are probably some other related changes which need to be made, (e.g.,
prom_init() may be eliminated, etc).

It seems like I get more and more positive feedbacks on this idea.  We should
try to implement this in 2.5.

Jun

Re: Remove ifdefs from setup_arch()

[Gerald Champagne]

Can you wrap this code into small functions like machine_detect() and
board_setup() and put it all in one place?  That way those of us who
just want a simple system supporting only one board can just replace
those two functions with defines that alias the proper machine_detect
and board_setup fuctions.  Then all of the special elf sections and
function pointers go away.

If it's all in one place like this, then maybe it could be configured
in the config.in file.  I can ifdef it out for boards like mine or other
boards that can't possibly support more than one system in a given binary
image.  config.in could ifdef it in for configurations that could possibly
support more than one configuration in a given binary image.

Gerald


Jun Sun wrote:
> The actual mechanism can vary and be flexible, but here is more detail what I
> had in mind:
> 
> 1. <my>_detect is placed in a special ELF section (mips_mach_detect), using
> similar mechanism as .initcall.init section and __setup() macro.
> 
> 2. in addition to the 3 possible return value, <my>_detect also returns a
> function pointer to <my>_setup.  Once a final candidate is chosen, the machine
> detection code will issue the right <my>_setup call.
> 
> There are probably some other related changes which need to be made, (e.g.,
> prom_init() may be eliminated, etc).
> 
> It seems like I get more and more positive feedbacks on this idea.  We should
> try to implement this in 2.5.
> 
> Jun
> 
> 
> 

Re: Remove ifdefs from setup_arch()

[Ralf Baechle]

On Fri, Oct 12, 2001 at 01:13:29PM -0500, Gerald Champagne wrote:

> Can you wrap this code into small functions like machine_detect() and
> board_setup() and put it all in one place?  That way those of us who
> just want a simple system supporting only one board can just replace
> those two functions with defines that alias the proper machine_detect
> and board_setup fuctions.  Then all of the special elf sections and
> function pointers go away.
> 
> If it's all in one place like this, then maybe it could be configured
> in the config.in file.  I can ifdef it out for boards like mine or other
> boards that can't possibly support more than one system in a given binary
> image.  config.in could ifdef it in for configurations that could possibly
> support more than one configuration in a given binary image.

That would all be initcode / initdata so effective bloat zero.

  Ralf