Attention is currently required from: Marc Jones, Nico Huber, Stefan Reinauer, Patrick Rudolph. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45208 )
Change subject: console: Allow configuring log level through CBFS ......................................................................
Patch Set 17: Code-Review+2
(3 comments)
Patchset:
PS17: Sorry for missing the June replies, they must have somehow slipped through my queue. I think this looks pretty good now.
File src/console/Kconfig:
https://review.coreboot.org/c/coreboot/+/45208/comment/f86fa98c_b343964a PS14, Line 43: config OPTIONAL_SLOW_CONSOLE
I dislike options so now that we cut down on the code, I made it a default behavior.
Hmm... well, the cost for that extra lookup should be really trivial, so I guess as long as nobody complains it should be fine to have it on unconditionally.
File tests/console/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/45208/comment/89da114a_beea8615 PS14, Line 10: routing-with-cbmemcons-test-cflags += -I 3rdparty/vboot/firmware/include
because we now draw in cbfs, and through that, vboot, and the unit testing framework minimizes API e […]
I still don't really understand this. I think the include path for unit test builds should be the same as the one for normal coreboot builds (plus extra test-only header directories). If vboot is in the default include path for coreboot itself, I think we should also put it in the path for unit tests rather than requiring all tests that touch any file including vboot stuff to add custom cflags like this.
-
Julius Werner (Code Review)