Patch set 3:Code-Review +1
4 comments:
Should we provide a user-visible prompt for this? Note that if we do, we should not select this option from other places, but have it `default y` instead.
Patch Set #3, Line 330: default 7
Ideally, this should be constrained to the valid loglevel values. It would be nice if we could reuse the prompt right below. Currently, it is hidden when CONSOLE_OVERRIDE_LOGLEVEL is selected, but we can change that:
if !CONSOLE_OVERRIDE_LOGLEVEL || CONSOLE_VPD_OVERRIDE_LOGLEVEL
choice
[...]
endchoice
config DEFAULT_CONSOLE_LOGLEVEL
int
default 0 if DEFAULT_CONSOLE_LOGLEVEL_0
default 1 if DEFAULT_CONSOLE_LOGLEVEL_1
default 2 if DEFAULT_CONSOLE_LOGLEVEL_2
default 3 if DEFAULT_CONSOLE_LOGLEVEL_3
default 4 if DEFAULT_CONSOLE_LOGLEVEL_4
default 5 if DEFAULT_CONSOLE_LOGLEVEL_5
default 6 if DEFAULT_CONSOLE_LOGLEVEL_6
default 7 if DEFAULT_CONSOLE_LOGLEVEL_7
default 8 if DEFAULT_CONSOLE_LOGLEVEL_8
help
Map the log level config names to an integer.
endif ## !CONSOLE_OVERRIDE_LOGLEVEL || CONSOLE_VPD_OVERRIDE_LOGLEVEL
Then, the CONSOLE_VPD_DEFAULT_LOGLEVEL symbol can be replaced with DEFAULT_CONSOLE_LOGLEVEL.
File src/console/loglevel_vpd.c:
Patch Set #3, Line 12: uint8_t
To use this type, you should include it: #include <stdint.h>
However, if we're returning an int, we might as well use an int here for consistency.
uint8_t val;
char val_str[VPD_LEN];
if (vpd_gets(COREBOOT_LOG_LEVEL, val_str, VPD_LEN, VPD_RW_THEN_RO)) {
val = (uint8_t)atol(val_str);
if (val >= BIOS_NEVER)
return CONFIG_CONSOLE_VPD_DEFAULT_LOGLEVEL;
return val;
} else {
return CONFIG_CONSOLE_VPD_DEFAULT_LOGLEVEL;
}
I'd say this would look better with a single `return` statement:
int log_level = CONFIG_CONSOLE_VPD_DEFAULT_LOGLEVEL;
char val_str[VPD_LEN];
if (vpd_gets(COREBOOT_LOG_LEVEL, val_str, VPD_LEN, VPD_RW_THEN_RO)) {
log_level = atol(val_str);
if (log_level < 0 || log_level >= BIOS_NEVER)
log_level = CONFIG_CONSOLE_VPD_DEFAULT_LOGLEVEL;
}
return log_level;
Also, we can rename `val` to `log_level` for clarity.
To view, visit change 45134. To unsubscribe, or for help writing mail filters, visit settings.