<br><br><div class="gmail_quote">On Mon, Oct 19, 2009 at 4:17 PM, 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 class="im">On Mon, Oct 19, 2009 at 03:18:07PM -0600, Myles Watson wrote:<br>
> Define HAVE_INIT_TIMER to only exclude the three boards that define it to be<br>
> 0 in newconfig.<br>
<br>
</div>What exactly does HAVE_INIT_TIMER configure? Why do those three boards<br>
_not_ set it? It sounds like all boards would want to set it?<br></blockquote><div>I don't know.  Since one of the boards is qemu, I figured that it might not have some functionality that hardware has.<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;">
<div class="im">> MOVNTI is a performance enhancement, and should default to 0 so it doesn't<br>
> break boards that forget to define it.<br>
<br>
</div>Are all Kconfig files setting to y (via "select" preferrably) where needed?<br></blockquote><div>It's hard to check since this option is set in sockets.  That is a worthwhile follow up patch.  It just doesn't make sense to break booting for performance.<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;">
> Index: svn/src/Kconfig<br>
> ===================================================================<br>
> --- svn.orig/src/Kconfig<br>
> +++ svn/src/Kconfig<br>
> @@ -74,10 +74,6 @@ config CPU_ADDR_BITS<br>
>       int<br>
>       default 36<br>
><br>
> -config AGP_APERTURE_SIZE<br>
> -     hex<br>
> -     default 0x0<br>
> -<br>
<br>
Why is this removed?<br></blockquote><div>It shouldn't be a global option.  It's only used by amd northbridges.  Setting global defaults for specific options makes it harder to see when something is broken.<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 HAVE_LOW_TABLES<br>
> +     bool<br>
> +     default y<br>
> +     help<br>
> +       This Option is unused in the code.  Since two boards try to set it to<br>
> +       'n', they may be broken.  We either need to make the option useful or<br>
> +       get rid of it.  The broken boards are:<br>
> +       asus/m2v-mx_se<br>
> +       supermicro/h8dme<br>
<br>
Hm, indeed. Why was this added? Are there situations where we might not<br>
want LOW_TABLES on?<br></blockquote><div>Good question.  I think there might be times where you don't want to corrupt low memory.<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;">
<br>
> Index: svn/src/northbridge/amd/amdfam10/Kconfig<br>
> ===================================================================<br>
> --- svn.orig/src/northbridge/amd/amdfam10/Kconfig<br>
<div class="im">> +++ svn/src/northbridge/amd/amdfam10/Kconfig<br>
> @@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10<br>
>       bool<br>
>       select HAVE_HIGH_TABLES<br>
>       select HYPERTRANSPORT_PLUGIN_SUPPORT<br>
> -     select HT3_SUPPORT<br>
[...]<br>
</div><div class="im">> +config HT3_SUPPORT<br>
> +     bool<br>
> +     default y<br>
> +     depends on NORTHBRIDGE_AMD_AMDFAM10<br>
<br>
</div>Why this? "select HT3_SUPPORT" should work just fine, no?<br>
Even if that's not the case I think it would be nicer to set it in some<br>
global Kconfig file so we can "select" it. IMHO _all_ y variables should<br>
be "select"ed so we avoid the 4-line chunks...<br></blockquote><div>It doesn't work unless HT3_SUPPORT is defined somewhere else.  It doesn't make sense to define it anywhere else to me, since only FAM10 uses 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;">
<br>
> +config AMDMCT<br>
> +     bool<br>
<div class="im">> +     default y<br>
> +     depends on NORTHBRIDGE_AMD_AMDFAM10<br>
<br>
</div>Ditto.<br></blockquote><div>Yep.<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 MEM_TRAIN_SEQ<br>
> +config HW_MEM_HOLE_SIZEK<br>
> +config HW_MEM_HOLE_SIZE_AUTO_INC<br>
> Index: svn/src/northbridge/amd/amdk8/Kconfig<br>
> ===================================================================<br>
> --- svn.orig/src/northbridge/amd/amdk8/Kconfig<br>
> +++ svn/src/northbridge/amd/amdk8/Kconfig<br>
> +config MEM_TRAIN_SEQ<br>
> +config HW_MEM_HOLE_SIZEK<br>
> +config HW_MEM_HOLE_SIZE_AUTO_INC<br>
<br>
Why are these three defined twice?<br></blockquote><div>So that they only show up when they're used.<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;">

> Index: svn/src/southbridge/via/vt8237r/vt8237r.h<br>
> ===================================================================<br>
> --- svn.orig/src/southbridge/via/vt8237r/vt8237r.h<br>
> +++ svn/src/southbridge/via/vt8237r/vt8237r.h<br>
> @@ -65,7 +65,7 @@<br>
>  #define I2C_TRANS_CMD                        0x40<br>
>  #define CLOCK_SLAVE_ADDRESS          0x69<br>
><br>
> -#if DEBUG_SMBUS == 1<br>
> +#if defined(DEBUG_SMBUS) && DEBUG_SMBUS == 1<br>
>  #define PRINT_DEBUG(x)               print_debug(x)<br>
>  #define PRINT_DEBUG_HEX16(x) print_debug_hex16(x)<br>
>  #else<br>
<br>
Please drop this hunk, I have a patch in the queue to redo DEBUG_SMBUS<br>
and other DEBUG options globally anyway.<br></blockquote><div>Sure.  I was just trying to get most of the not defined warnings.  Dropped.<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;">

> Index: svn/src/cpu/amd/model_10xxx/init_cpus.c<br>
> ===================================================================<br>
> --- svn.orig/src/cpu/amd/model_10xxx/init_cpus.c<br>
> +++ svn/src/cpu/amd/model_10xxx/init_cpus.c<br>
> @@ -473,8 +473,8 @@ static void start_node(u8 node)<br>
>       /* Enable routing table */<br>
>       printk_debug("Start node %02x", node);<br>
><br>
> -#if CONFIG_CAR_FAM10 == 1<br>
> -     /* For CONFIG_CAR_FAM10 support, we need to set Dram base/limit for the new node */<br>
> +#if CONFIG_NORTHBRIDGE_AMD_AMDFAM10 == 1<br>
<br>
#if CONFIG_NORTHBRIDGE_AMD_AMDFAM10<br>
should suffice, I think.<br></blockquote><div>Yes.  Fixed.<br><br>Thanks,<br>Myles<br></div></div><br>