Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16840
-gerrit
commit dfdb0733a6a71b11d15006dafc13841e84fab7cd
Author: Martin Roth <martinroth(a)google.com>
Date: Fri Sep 30 15:56:27 2016 -0600
util/lint/kconfig_lint: change warning levels and text
- Add an exception for MAINBOARD_POWER_ON_AFTER_POWER_FAIL when checking
- With those exceptions set, we don't have anymore #define or #ifdef
warnings, so turn them to errors so no more can be pushed.
- Change the definition of an unused symbol from a warning to a note.
There are times when unused symbols are expected.
- Upgrade the warning for loading Kconfig files multiple times from
a warning to an error.
Change-Id: I6dcb06d4f0b099d5ccaf7643e72dd790719bdf58
Signed-off-by: Martin Roth <martinroth(a)google.com>
---
util/lint/kconfig_lint | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/util/lint/kconfig_lint b/util/lint/kconfig_lint
index 064f3db..39a0291 100755
--- a/util/lint/kconfig_lint
+++ b/util/lint/kconfig_lint
@@ -197,8 +197,11 @@ sub check_for_ifdef {
my $symbol = $3;
if ( ( exists $symbols{$symbol} ) && ( $symbols{$symbol}{type} ne "string" ) ) {
- show_warning( "#ifdef 'CONFIG_$symbol' used at $file:$lineno."
+ # TODO: Remove special check for CONFIG_MAINBOARD_POWER_ON_AFTER_POWER_FAIL
+ if ($symbol ne "MAINBOARD_POWER_ON_AFTER_POWER_FAIL") {
+ show_error( "#ifdef 'CONFIG_$symbol' used at $file:$lineno."
. " Symbols of type '$symbols{$symbol}{type}' are always defined." );
+ }
}
}
}
@@ -261,7 +264,10 @@ sub check_for_def {
my $symbol = $3;
if ( ( exists $symbols{$symbol} ) ) {
- show_warning("#define of symbol 'CONFIG_$symbol' used at $file:$lineno.");
+ # TODO: Remove special check for CONFIG_MAINBOARD_POWER_ON_AFTER_POWER_FAIL
+ if ($symbol ne "MAINBOARD_POWER_ON_AFTER_POWER_FAIL") {
+ show_error("#define of symbol 'CONFIG_$symbol' used at $file:$lineno.");
+ }
}
else {
show_warning( "#define 'CONFIG_$symbol' used at $file:$lineno."
@@ -460,7 +466,9 @@ sub check_used_symbols {
for ( my $i = 0 ; $i <= $symbols{$key}{count} ; $i++ ) {
my $filename = $symbols{$key}{$i}{file};
my $line_no = $symbols{$key}{$i}{line_no};
- show_warning("Unused symbol '$key' referenced at $filename:$line_no.");
+ if ($show_note_output) {
+ print("#!!!!! Note: Unused symbol '$key' defined at $filename:$line_no.");
+ }
}
}
}
@@ -1162,9 +1170,9 @@ sub load_kconfig_file {
#if the file exists, try to load it.
elsif ( -e "$input_file" ) {
- #throw a warning if the file has already been loaded.
+ #throw an error if the file has already been loaded.
if ( exists $loaded_files{$input_file} ) {
- show_warning("'$input_file' sourced at $loadfile:$loadline was already loaded by $loaded_files{$input_file}");
+ show_error("'$input_file' sourced at $loadfile:$loadline was already loaded by $loaded_files{$input_file}");
}
#load the file's contents and mark the file as loaded for checking later
Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16839
-gerrit
commit c771a1f56b57882ffd8fa5922ccab6eb2ef222fe
Author: Martin Roth <martinroth(a)google.com>
Date: Fri Sep 30 15:51:32 2016 -0600
util/lint/kconfig_lint: Check default types
The type of the default value wasn't being checked to make sure that it
matched the type of the Kconfig symbol.
This makes sure that the symbol is being set to either a reasonable
looking value or to another Kconfig symbol.
Change-Id: Ia01bd2d8b387f319d29f0a005d55cb8e20cd3853
Signed-off-by: Martin Roth <martinroth(a)google.com>
---
util/lint/kconfig_lint | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/util/lint/kconfig_lint b/util/lint/kconfig_lint
index c955343..064f3db 100755
--- a/util/lint/kconfig_lint
+++ b/util/lint/kconfig_lint
@@ -329,10 +329,42 @@ sub check_defaults {
next unless ( exists $symbols{$sym}{$sym_num}{default_max} );
for ( my $def_num = 0 ; $def_num <= $symbols{$sym}{$sym_num}{default_max} ; $def_num++ ) {
+ my $filename = $symbols{$sym}{$sym_num}{file};
+ my $line_no = $symbols{$sym}{$sym_num}{default}{$def_num}{default_line_no};
+
+ # skip good defaults
+ if (! ((($symbols{$sym}{type} eq "hex") && ($symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /^0x/)) ||
+ (($symbols{$sym}{type} eq "int") && ($symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /^[-0-9]+$/)) ||
+ (($symbols{$sym}{type} eq "string") && ($symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /^".*"$/)) ||
+ (($symbols{$sym}{type} eq "bool") && ($symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /^[yn]$/)))
+ ) {
+
+ my ($checksym) = $symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /(\w+)/;
+
+ if (! exists $symbols{$checksym}) {
+
+ # verify the symbol type against the default value
+ if ($symbols{$sym}{type} eq "hex") {
+ show_error("non hex default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) used for hex symbol $sym at $filename:$line_no.");
+ } elsif ($symbols{$sym}{type} eq "int") {
+ show_error("non int default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) used for int symbol $sym at $filename:$line_no.");
+ } elsif ($symbols{$sym}{type} eq "string") {
+ # TODO: Remove special MAINBOARD_DIR check
+ if ($sym ne "MAINBOARD_DIR") {
+ show_error("no quotes around default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) used for string symbol $sym at $filename:$line_no.");
+ }
+ } elsif ($symbols{$sym}{type} eq "bool") {
+ if ($symbols{$sym}{$sym_num}{default}{$def_num}{default} =~ /[01YN]/) {
+ show_warning("default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) for bool symbol $sym uses value other than y/n at $filename:$line_no.");
+ } else {
+ show_error("non bool default value ($symbols{$sym}{$sym_num}{default}{$def_num}{default}) used for bool symbol $sym at $filename:$line_no.");
+ }
+ }
+ }
+ }
+
#if a default is already set, display an error
if ($default_set) {
- my $filename = $symbols{$sym}{$sym_num}{file};
- my $line_no = $symbols{$sym}{$sym_num}{default}{$def_num}{default_line_no};
show_warning( "Default for '$sym' referenced at $filename:$line_no will never be set"
. " - overridden by default set at $default_filename:$default_line_no" );
}
Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16838
-gerrit
commit 18ff99b05b5fd7e0a37a5d2f5ad3a82c7d50d698
Author: Martin Roth <martinroth(a)google.com>
Date: Fri Sep 30 15:27:24 2016 -0600
google/gale/Kconfig: Change wording of Kconfig option
Everybody knows WHAT they're supposed to do with options, so the text
"Pick this" or "Select to" are redundant.
Change-Id: I327c5be755373e99ca0738593bd78e1084d4d492
Signed-off-by: Martin Roth <martinroth(a)google.com>
---
src/mainboard/google/gale/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mainboard/google/gale/Kconfig b/src/mainboard/google/gale/Kconfig
index 5b960cd..01e2945 100644
--- a/src/mainboard/google/gale/Kconfig
+++ b/src/mainboard/google/gale/Kconfig
@@ -37,7 +37,7 @@ config CHROMEOS
select WIPEOUT_SUPPORTED
config BOARD_VARIANT_DK01
- bool "pick this to build an image for DK01"
+ bool "Build an image for DK01"
default n
config MAINBOARD_DIR
Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16837
-gerrit
commit f4d0e8e7b6292292bcb82ef8080b71bdffc0e5d2
Author: Martin Roth <martinroth(a)google.com>
Date: Fri Sep 30 14:51:24 2016 -0600
google/gale: Remove #ifdef of Kconfig bool symbol
Kconfig symbols of type bool are ALWAYS defined, so this code was
always being included and run, which isn't what the author wanted.
Change to use IS_ENABLED(), and a regular if() instead of an #ifdef.
Change-Id: I72623fa27e47980c602135f4b73f371c7f50139b
Signed-off-by: Martin Roth <martinroth(a)google.com>
---
src/mainboard/google/gale/verstage.c | 55 ++++++++++++++++++------------------
1 file changed, 27 insertions(+), 28 deletions(-)
diff --git a/src/mainboard/google/gale/verstage.c b/src/mainboard/google/gale/verstage.c
index cdc4386..1a3f5a4 100644
--- a/src/mainboard/google/gale/verstage.c
+++ b/src/mainboard/google/gale/verstage.c
@@ -24,34 +24,33 @@
static void ipq_setup_tpm(void)
{
-#ifdef CONFIG_I2C_TPM
- gpio_tlmm_config_set(TPM_RESET_GPIO, FUNC_SEL_GPIO,
- GPIO_PULL_UP, GPIO_6MA, 1);
- gpio_set(TPM_RESET_GPIO, 0);
- udelay(100);
- gpio_set(TPM_RESET_GPIO, 1);
-
- /*
- * ----- Per the SLB 9615XQ1.2 spec -----
- *
- * 4.7.1 Reset Timing
- *
- * The TPM_ACCESS_x.tpmEstablishment bit has the correct value
- * and the TPM_ACCESS_x.tpmRegValidSts bit is typically set
- * within 8ms after RESET# is deasserted.
- *
- * The TPM is ready to receive a command after less than 30 ms.
- *
- * --------------------------------------
- *
- * I'm assuming this means "wait for 30ms"
- *
- * If we don't wait here, subsequent QUP I2C accesses
- * to the TPM either fail or timeout.
- */
- mdelay(30);
-
-#endif /* CONFIG_I2C_TPM */
+ if (IS_ENABLED(CONFIG_I2C_TPM)) {
+ gpio_tlmm_config_set(TPM_RESET_GPIO, FUNC_SEL_GPIO,
+ GPIO_PULL_UP, GPIO_6MA, 1);
+ gpio_set(TPM_RESET_GPIO, 0);
+ udelay(100);
+ gpio_set(TPM_RESET_GPIO, 1);
+
+ /*
+ * ----- Per the SLB 9615XQ1.2 spec -----
+ *
+ * 4.7.1 Reset Timing
+ *
+ * The TPM_ACCESS_x.tpmEstablishment bit has the correct value
+ * and the TPM_ACCESS_x.tpmRegValidSts bit is typically set
+ * within 8ms after RESET# is deasserted.
+ *
+ * The TPM is ready to receive a command after less than 30 ms.
+ *
+ * --------------------------------------
+ *
+ * I'm assuming this means "wait for 30ms"
+ *
+ * If we don't wait here, subsequent QUP I2C accesses
+ * to the TPM either fail or timeout.
+ */
+ mdelay(30);
+ }
}
void verstage_mainboard_init(void)
Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16836
-gerrit
commit 10c1ecba3b830532aa856d64a9799fd0b8e91ab2
Author: Martin Roth <martinroth(a)google.com>
Date: Fri Sep 30 14:49:09 2016 -0600
src/Kconfig: Remove duplicate source of fsp1_0/Kconfig
The FSP code was initially loaded in the chipset section, because that's
where it made sense, even though the source lived in the driver tree.
When the Kconfigs were updated to load files using globbing, this file
got loaded twice, once in drivers and once in the chipset. Remove the
initial location, and just load it in the drivers as the new FSP entries
do.
Change-Id: I3e7759b7b3f294ea2ae2a0c5fdba1e85da8a0699
Signed-off-by: Martin Roth <martinroth(a)google.com>
---
src/Kconfig | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/Kconfig b/src/Kconfig
index 91b27ce..a774124 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -443,8 +443,6 @@ source "src/superio/*/*/Kconfig"
comment "Embedded Controllers"
source "src/ec/acpi/Kconfig"
source "src/ec/*/*/Kconfig"
-# FIXME move to vendorcode
-source "src/drivers/intel/fsp1_0/Kconfig"
source "src/southbridge/intel/common/firmware/Kconfig"
source "src/vboot/Kconfig"
Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/16835
-gerrit
commit 322c1594474889e9ea50dca79b6222e1ddd4298c
Author: Martin Roth <martinroth(a)google.com>
Date: Fri Sep 30 14:44:54 2016 -0600
vendorcode/amd/pi/Kconfig: update AGESA_BINARY_PI_LOCATION to hex
The AGESA_BINARY_PI_LOCATION Kconfig symbol was declared as a string.
Change it to a hex value.
Change-Id: Ifd87b6c8dfcdf950aea9b15a6fea45bb72e8b4e9
Signed-off-by: Martin Roth <martinroth(a)google.com>
---
src/vendorcode/amd/pi/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vendorcode/amd/pi/Kconfig b/src/vendorcode/amd/pi/Kconfig
index 892892d..0122366 100644
--- a/src/vendorcode/amd/pi/Kconfig
+++ b/src/vendorcode/amd/pi/Kconfig
@@ -45,7 +45,7 @@ config AGESA_BINARY_PI_FILE
Specify the binary file to use for AMD platform initialization.
config AGESA_BINARY_PI_LOCATION
- string "AGESA PI binary address in ROM"
+ hex "AGESA PI binary address in ROM"
default 0xFFE00000
help
Specify the ROM address at which to store the binary Platform
the following patch was just integrated into master:
commit 80fa9d899c2dacf3b842b8a8cae28352bb874e62
Author: Martin Roth <martinroth(a)google.com>
Date: Thu Sep 29 15:10:37 2016 -0600
google/reef: Fix default values in Kconfig
These default values weren't being set with the default
keyword so were ending up with different values.
from the default generated config file before this change:
CONFIG_DRIVER_TPM_I2C_BUS=0x9
CONFIG_DRIVER_TPM_I2C_ADDR=0x2
CONFIG_DRIVER_TPM_I2C_IRQ=-1
Change-Id: I19514d0c9b2a9b7e479f003a4d3384e073f4d531
Signed-off-by: Martin Roth <martinroth(a)google.com>
Reviewed-on: https://review.coreboot.org/16828
Reviewed-by: Duncan Laurie <dlaurie(a)chromium.org>
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net>
See https://review.coreboot.org/16828 for details.
-gerrit
the following patch was just integrated into master:
commit 311fb696cfa6644ca82dca3e1ea825401779db51
Author: Martin Roth <martinroth(a)google.com>
Date: Thu Sep 29 14:46:24 2016 -0600
Kconfig: Prefix hex defaults with 0x
Because these variables had "non-hexidecimal" defaults, they
were updated by kconfig when writing defconfig files.
Change-Id: Ic1a070d340708f989157ad18ddc79de7bb92d873
Signed-off-by: Martin Roth <martinroth(a)google.com>
Reviewed-on: https://review.coreboot.org/16827
Tested-by: build bot (Jenkins)
Reviewed-by: Duncan Laurie <dlaurie(a)chromium.org>
Reviewed-by: Patrick Georgi <pgeorgi(a)google.com>
Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net>
See https://review.coreboot.org/16827 for details.
-gerrit