<br><br><div class="gmail_quote">On Tue, Oct 20, 2009 at 9:42 AM, Uwe Hermann <span dir="ltr"><<a href="mailto:uwe@hermann-uwe.de">uwe@hermann-uwe.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="h5">On Tue, Oct 20, 2009 at 09:19:37AM -0600, Myles Watson wrote:<br>
> I compared the config variables that we select with the list that we define.<br>
><br>
> I removed CONFIG_CPU_AMD_FAM10 & CONFIG_CPU_AMDK8 from mainboards.  They<br>
> should be selected in sockets, and they aren't used yet.<br>
><br>
> I added a couple of variables to src/Kconfig for lack of a better place so<br>
> that their selects would work.<br>
> I added select statements according to newconfig for some variables that<br>
> were defined but never selected in mainboard configs.<br>
><br>
> Signed-off-by: Myles Watson <<a href="mailto:mylesgw@gmail.com">mylesgw@gmail.com</a>><br>
<br>
</div></div>Acked-by: Uwe Hermann <<a href="mailto:uwe@hermann-uwe.de">uwe@hermann-uwe.de</a>><br></blockquote><div>Thanks for the detailed review.  Rev 4816.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

But see below for material for discussions and/or a followup patch.<br></blockquote><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

include/pc80/vga.h:#if (CONFIG_VGA == 1)<br>
include/pc80/vga.h:#endif /* (CONFIG_VGA == 1) */<br>
<br>
These should probably be changed to<br>
  #if CONFIG_VGA<br></blockquote><div>Fixed <br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
mainboard/intel/eagleheights/Options.lb:uses CONFIG_VGA<br>
mainboard/intel/eagleheights/Options.lb:#default CONFIG_VGA=0<br>
<br>
Unused here, so can be dropped from Options.lb?<br></blockquote><div>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.<br>
 <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
These should probably be changed to<br>
  #if CONFIG_VGA<br></blockquote><div>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.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

config/Options.lb:define CONFIG_USE_WATCHDOG_ON_BOOT<br>
<br>
  The comment says "Use the watchdog on booting" which is not really<br>
  descriptive. What does "use" mean here? Disable it? We should fix<br>
  the comment I guess (and the resp. Kconfig help text).<br></blockquote><div>I expect that it means enable it, but I don't know.  Again I was just trying to match newconfig.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

mainboard/lippert/spacerunner-lx/cache_as_ram_auto.c:#if !CONFIG_USE_WATCHDOG_ON_BOOT<br>
mainboard/lippert/roadrunner-lx/cache_as_ram_auto.c:#if !CONFIG_USE_WATCHDOG_ON_BOOT<br>
<br>
Not sure about these two. They use the variable but never add a "uses"<br>
line in Option.lb?<br></blockquote><div>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.)<br>
<br>Thanks,<br>Myles<br></div></div><br>