On Mon, Oct 19, 2009 at 03:18:07PM -0600, Myles Watson wrote: > Define HAVE_INIT_TIMER to only exclude the three boards that define it to be > 0 in newconfig.
What exactly does HAVE_INIT_TIMER configure? Why do those three boards _not_ set it? It sounds like all boards would want to set it? > MOVNTI is a performance enhancement, and should default to 0 so it doesn't > break boards that forget to define it. Are all Kconfig files setting to y (via "select" preferrably) where needed? > Index: svn/src/Kconfig > =================================================================== > --- svn.orig/src/Kconfig > +++ svn/src/Kconfig > @@ -74,10 +74,6 @@ config CPU_ADDR_BITS > int > default 36 > > -config AGP_APERTURE_SIZE > - hex > - default 0x0 > - Why is this removed? > +config HAVE_LOW_TABLES > + bool > + default y > + help > + This Option is unused in the code. Since two boards try to set it to > + 'n', they may be broken. We either need to make the option useful or > + get rid of it. The broken boards are: > + asus/m2v-mx_se > + supermicro/h8dme Hm, indeed. Why was this added? Are there situations where we might not want LOW_TABLES on? > +config HAVE_HIGH_TABLES > + bool > + default n > + help > + This variable specifies whether a given northbridge has high table > + support. > + It is set in northbridge/*/Kconfig. > + Whether or not the high tables are actually written by coreboot is > + configurable by the user via WRITE_HIGH_TABLES. > + > config HAVE_ACPI_TABLES > bool > help > @@ -262,14 +280,30 @@ config HAVE_PIRQ_TABLE > Whether or not the PIRQ table is actually generated by coreboot > is configurable by the user via GENERATE_PIRQ_TABLE. > > -config HAVE_HIGH_TABLES > +#These Options are here to avoid "undefined" warnings. > +#The actual selection and help texts are in the following menu. > Index: svn/src/northbridge/amd/amdfam10/Kconfig > =================================================================== > --- svn.orig/src/northbridge/amd/amdfam10/Kconfig > +++ svn/src/northbridge/amd/amdfam10/Kconfig > @@ -21,11 +21,35 @@ config NORTHBRIDGE_AMD_AMDFAM10 > bool > select HAVE_HIGH_TABLES > select HYPERTRANSPORT_PLUGIN_SUPPORT > - select HT3_SUPPORT [...] > +config HT3_SUPPORT > + bool > + default y > + depends on NORTHBRIDGE_AMD_AMDFAM10 Why this? "select HT3_SUPPORT" should work just fine, no? Even if that's not the case I think it would be nicer to set it in some global Kconfig file so we can "select" it. IMHO _all_ y variables should be "select"ed so we avoid the 4-line chunks... > +config AMDMCT > + bool > + default y > + depends on NORTHBRIDGE_AMD_AMDFAM10 Ditto. > +config MEM_TRAIN_SEQ > +config HW_MEM_HOLE_SIZEK > +config HW_MEM_HOLE_SIZE_AUTO_INC > Index: svn/src/northbridge/amd/amdk8/Kconfig > =================================================================== > --- svn.orig/src/northbridge/amd/amdk8/Kconfig > +++ svn/src/northbridge/amd/amdk8/Kconfig > +config MEM_TRAIN_SEQ > +config HW_MEM_HOLE_SIZEK > +config HW_MEM_HOLE_SIZE_AUTO_INC Why are these three defined twice? > Index: svn/src/southbridge/via/vt8237r/vt8237r.h > =================================================================== > --- svn.orig/src/southbridge/via/vt8237r/vt8237r.h > +++ svn/src/southbridge/via/vt8237r/vt8237r.h > @@ -65,7 +65,7 @@ > #define I2C_TRANS_CMD 0x40 > #define CLOCK_SLAVE_ADDRESS 0x69 > > -#if DEBUG_SMBUS == 1 > +#if defined(DEBUG_SMBUS) && DEBUG_SMBUS == 1 > #define PRINT_DEBUG(x) print_debug(x) > #define PRINT_DEBUG_HEX16(x) print_debug_hex16(x) > #else Please drop this hunk, I have a patch in the queue to redo DEBUG_SMBUS and other DEBUG options globally anyway. > Index: svn/src/cpu/amd/model_10xxx/init_cpus.c > =================================================================== > --- svn.orig/src/cpu/amd/model_10xxx/init_cpus.c > +++ svn/src/cpu/amd/model_10xxx/init_cpus.c > @@ -473,8 +473,8 @@ static void start_node(u8 node) > /* Enable routing table */ > printk_debug("Start node %02x", node); > > -#if CONFIG_CAR_FAM10 == 1 > - /* For CONFIG_CAR_FAM10 support, we need to set Dram base/limit for the > new node */ > +#if CONFIG_NORTHBRIDGE_AMD_AMDFAM10 == 1 #if CONFIG_NORTHBRIDGE_AMD_AMDFAM10 should suffice, I think. Uwe. -- http://www.hermann-uwe.de | http://www.randomprojects.org http://www.crazy-hacks.org | http://www.unmaintained-free-software.org -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot