On Tue, Oct 20, 2009 at 9:42 AM, Uwe Hermann <u...@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 <myle...@gmail.com> > > Acked-by: Uwe Hermann <u...@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
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot