On Sat, Oct 17, 2009 at 7:47 AM, Uwe Hermann <uwe@hermann-uwe.de> wrote:
On Sat, Oct 17, 2009 at 03:25:07PM +0200, svn@coreboot.org wrote:
> Remove CONFIG_ from #defines that aren't config variables.  Trivial.

Why this? Shouldn't they have been added to src/Kconfig (for example)
instead?
No.  Many of them are only mentioned in one place.  They probably should be removed completely.  I think it will improve the quality of Coreboot to limit configuration options to ones that actually do something.

 

> Modified: trunk/coreboot-v2/src/arch/i386/init/car.S
> ===================================================================
> --- trunk/coreboot-v2/src/arch/i386/init/car.S        2009-10-17 03:00:04 UTC (rev 4801)
> +++ trunk/coreboot-v2/src/arch/i386/init/car.S        2009-10-17 13:25:07 UTC (rev 4802)
> @@ -269,7 +269,7 @@
>
>
>       /* TODO: make this a config variable */
> -#if CONFIG_CARTEST
> +#if CARTEST

This one, for example, should likely be an option in the "Debugging"
menu, maybe only visible if EXPERT is enabled (will post EXPERT patch
soon).
I think there's too little gain for something this trivial.

I personally think that _all_ options should be kconfig options (i.e.
listed in a Kconfig file with some default). Whether or not they should
also be visible in menuconfig is another issue and depends on the
option, though.
These aren't really options.  They really are just #ifdefs.  They were never in Options.lb.

Thanks,
Myles