Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Jayvik Desai, Julius Werner, Jérémy Compostella, Paul Menzel, Ronak Kanabar.
Subrata Banik 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 9:
(1 comment)
File src/drivers/intel/fsp2_0/debug.c:
https://review.coreboot.org/c/coreboot/+/86001/comment/a1d785f8_39c76d44?usp... : PS6, Line 185: if (!CONFIG(USE_CBFS_FILE_OPTION_BACKEND))
Not really sure which comment you're referring to but it's possible I may have missed something.
I still don't really understand why you need this. As far as I can tell, the previous code in `fsp_control_log_level()` would just always return `fsp_map_console_log_level()`. With the implementation I proposed, an option file could override that behavior, but if the file doesn't exist it will still fall back to `fsp_map_console_log_level()`. So the default behavior doesn't change?
If your concern is that after this change you want to build ChromeOS images that have the debug FSP built-in but don't enable the debug output by default (unless someone manually overwrote the setting), I think the obvious answer is to just have our ebuild add an option file with value 0 by default?
I guess what you are asking is to inject CBFS option file by default with value 0 to ensure that FSP log-level stays disabled unless someone overrides it. Wondering if we really need to do this (what if someone deleted those option files then the log-level will be again chatty)? In my latest patch set, I have used FSP_DYNAMIC_ Kconfig which will ensure that the default log level is 0 and that might eliminate the need for any ebuild change. WDYT ?
Please take a look at crrev/c/6189176 which is meant to address your review comment. marking it resolved and please feel free to open it otherwise.