Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
lib/fmap: Allow disabling all output in fmap cache
Some devices like google/meep are _very_ tight on space in the bootblock and the strings are enough to make it spill over, so compile them out.
Change-Id: I1b96a6c3563e12435d6927c6e2f91a9458badac3 Signed-off-by: Patrick Georgi pgeorgi@google.com --- M src/lib/Kconfig M src/lib/fmap.c 2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/36924/1
diff --git a/src/lib/Kconfig b/src/lib/Kconfig index cb1e4a5..2db0fcd 100644 --- a/src/lib/Kconfig +++ b/src/lib/Kconfig @@ -68,3 +68,8 @@ def_bool y
endif + +config SILENT_FMAP_CACHE + bool + help + Disable all console output in fmap cache code to save some space. diff --git a/src/lib/fmap.c b/src/lib/fmap.c index 4b4179c..8f47adb 100644 --- a/src/lib/fmap.c +++ b/src/lib/fmap.c @@ -32,10 +32,14 @@ static int fmap_print_once CAR_GLOBAL; static struct mem_region_device fmap_cache CAR_GLOBAL;
+#if CONFIG(SILENT_FMAP_CACHE) +#define print_once(...) do { } while (0) +#else #define print_once(...) do { \ if (!car_get_var(fmap_print_once)) \ printk(__VA_ARGS__); \ } while (0) +#endif
DECLARE_OPTIONAL_REGION(fmap_cache);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
Patch Set 1: Code-Review+1
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
Patch Set 1:
Is there not a better solution than reducing verbosity of somewhat random parts of coreboot? How about Adding a Kconfig to change the log verbosity per stage?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
Patch Set 1:
Patch Set 1:
Is there not a better solution than reducing verbosity of somewhat random parts of coreboot? How about Adding a Kconfig to change the log verbosity per stage?
I'm fine with a better solution, but since these printks made google_meep fail, they're what I kicked out for now.
Note that for the most part, we leave strings in when we reduce the log level, so this mechanism is different from our normal verbosity selection mode (which merely makes printk decide whether to print a string or not).
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Is there not a better solution than reducing verbosity of somewhat random parts of coreboot? How about Adding a Kconfig to change the log verbosity per stage?
I'm fine with a better solution, but since these printks made google_meep fail, they're what I kicked out for now.
Note that for the most part, we leave strings in when we reduce the log level, so this mechanism is different from our normal verbosity selection mode (which merely makes printk decide whether to print a string or not).
Looking at the history of the change some more, the strings already existed before (and the print once mechanism and everything). So yes, removal of these strings is indeed rather arbitrary but it worked to get code growth under control ¯_(ツ)_/¯
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
Patch Set 1:
This seems a bit too specific to remove only these messages and no others if you ask me. I think either the platform should select NO_BOOTBLOCK_CONSOLE or NO_FMAP_CACHE (CB:36937).
Note that for the most part, we leave strings in when we reduce the log level, so this mechanism is different from our normal verbosity selection mode (which merely makes printk decide whether to print a string or not).
Yeah, that's also something we should really fix at some point I think, it would be much better than disabling consoles all-or-nothing.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
Patch Set 1:
Patch Set 1:
This seems a bit too specific to remove only these messages and no others if you ask me. I think either the platform should select NO_BOOTBLOCK_CONSOLE or NO_FMAP_CACHE (CB:36937).
Thanks, I'll adapt the following CL to use NO_FMAP_CACHE.
Yeah, that's also something we should really fix at some point I think, it would be much better than disabling consoles all-or-nothing.
The background is that we supported that for a long time, routing printk through a bunch of macros to eliminate extraneous strings. We dropped it at some point because it confused people that they couldn't crank up the loglevel in CMOS (or whereever they configured it) and see more log information. See http://review.coreboot.org/3188
Patrick Georgi has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36924 )
Change subject: lib/fmap: Allow disabling all output in fmap cache ......................................................................
Abandoned
hack hack hack
-
Arthur Heymans (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)