Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45134 )
Change subject: console: Override coreboot log level via VPD variable ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45134/3/src/console/Kconfig File src/console/Kconfig:
https://review.coreboot.org/c/coreboot/+/45134/3/src/console/Kconfig@319 PS3, Line 319: bool
Should we provide a user-visible prompt for this? Note that if we do, we should not select this opti […]
Updated, but I still set default n because most people don't want to enable this by default.
https://review.coreboot.org/c/coreboot/+/45134/3/src/console/Kconfig@330 PS3, Line 330: default 7
Ideally, this should be constrained to the valid loglevel values. […]
Yes it's better if we can reuse DEFAULT_CONSOLE_LOGLEVEL, thanks.
https://review.coreboot.org/c/coreboot/+/45134/3/src/console/loglevel_vpd.c File src/console/loglevel_vpd.c:
https://review.coreboot.org/c/coreboot/+/45134/3/src/console/loglevel_vpd.c@... PS3, Line 12: uint8_t
To use this type, you should include it: #include <stdint.h> […]
Done
https://review.coreboot.org/c/coreboot/+/45134/3/src/console/loglevel_vpd.c@... PS3, Line 12: 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: […]
Done