Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Ronak Kanabar, Subrata Banik.
Julius Werner has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86001?usp=email )
Change subject: drivers/intel/fsp2_0: Add option to control debug log level using CBFS ......................................................................
Patch Set 5:
(1 comment)
File src/drivers/intel/fsp2_0/debug.c:
https://review.coreboot.org/c/coreboot/+/86001/comment/b9e61927_13500f76?usp... : PS5, Line 185: if (!CONFIG(FSP_DEBUG_LOG_LEVEL_USING_CBFS)) This should be called USING_OPTIONS or something like that, since it would work with other option backends as well.
...but does this really need to be a separate Kconfig? Why don't you just write this as ``` return get_uint_option("fsp_pcd_debug_level", fsp_map_console_log_level()); ``` and then in `fsp_control_log_level()` you can simply do: ``` fsp_set_debug_level(is_enabled ? get_fsp_pcd_debug_log_level() : FSP_LOG_LEVEL_DISABLE); ``` (or maybe just inline this there if there are no other call sites). That way, if an option backend is configured in and the option exists in there, it is used. If not, you fall back to the previous behavior.