[coreboot] [PATCH] more Kconfig selects

[Myles Watson]

On Tue, Oct 20, 2009 at 9:42 AM, Uwe Hermann <uwe at hermann-uwe.de> wrote:

> On Tue, Oct 20, 2009 at 09:19:37AM -0600, Myles Watson wrote:
> > I compared the config variables that we select with the list that we
> define.
> >
> > I removed CONFIG_CPU_AMD_FAM10 & CONFIG_CPU_AMDK8 from mainboards.  They
> > should be selected in sockets, and they aren't used yet.
> >
> > I added a couple of variables to src/Kconfig for lack of a better place
> so
> > that their selects would work.
> > I added select statements according to newconfig for some variables that
> > were defined but never selected in mainboard configs.
> >
> > Signed-off-by: Myles Watson <mylesgw at gmail.com>
>
> Acked-by: Uwe Hermann <uwe at hermann-uwe.de>
>
Thanks for the detailed review.  Rev 4816.


> But see below for material for discussions and/or a followup patch.
>


> include/pc80/vga.h:#if (CONFIG_VGA == 1)
> include/pc80/vga.h:#endif /* (CONFIG_VGA == 1) */
>
> These should probably be changed to
>  #if CONFIG_VGA
>
Fixed

mainboard/intel/eagleheights/Options.lb:uses CONFIG_VGA
> mainboard/intel/eagleheights/Options.lb:#default CONFIG_VGA=0
>
> Unused here, so can be dropped from Options.lb?
>
Could be, but I'm trying not to touch newconfig.  It should be killed sooner
rather than later, and it won't help our comparisons if we modify it.


> These should probably be changed to
>  #if CONFIG_VGA
>
This variable should probably be something more specific, since only one
board uses it, but I don't know enough about it to rename it.


> config/Options.lb:define CONFIG_USE_WATCHDOG_ON_BOOT
>
>  The comment says "Use the watchdog on booting" which is not really
>  descriptive. What does "use" mean here? Disable it? We should fix
>  the comment I guess (and the resp. Kconfig help text).
>
I expect that it means enable it, but I don't know.  Again I was just trying
to match newconfig.

mainboard/lippert/spacerunner-lx/cache_as_ram_auto.c:#if
> !CONFIG_USE_WATCHDOG_ON_BOOT
> mainboard/lippert/roadrunner-lx/cache_as_ram_auto.c:#if
> !CONFIG_USE_WATCHDOG_ON_BOOT
>
> Not sure about these two. They use the variable but never add a "uses"
> line in Option.lb?
>
You don't have to have a uses line in Options.lb to reference a CONFIG
variable in the code.  If you try to set its value it will break, but
testing its value in C works fine (or generates a warning if it isn't always
exported.)

Thanks,
Myles
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20091020/d3c527b3/attachment.html>