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
