Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31370 )
Change subject: [WIP] console: Split loglevel for fast and slow ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/#/c/31370/2/src/console/init.c File src/console/init.c:
https://review.coreboot.org/#/c/31370/2/src/console/init.c@73 PS2, Line 73: int log_level = get_log_level(); Hmm... it still bugs me a bit that this will call get_log_level() twice for every print now, and since it's split across two files I don't think the compiler can optimize much out there either.
How about you instead change the result of console_log_level() to an enum with CONSOLE_LOG_NONE = 0, CONSOLE_LOG_FAST = 1 and CONSOLE_LOG_ALL = 2?
https://review.coreboot.org/#/c/31370/2/src/console/printk.c File src/console/printk.c:
https://review.coreboot.org/#/c/31370/2/src/console/printk.c@44 PS2, Line 44: console_tx_byte_cbmemc(byte); nit: why not just cbmemc_tx_byte() here right away?
https://review.coreboot.org/#/c/31370/2/src/console/printk.c@104 PS2, Line 104: do_vtxprintf(msg_level, fmt, args); Really, I'm pretty sure this should also go through all the extra checks and locks in do_printk(), it just wasn't done right when this was added (or maybe those things were added to do_printk() later and people didn't notice).
I think the cleaner solution to all of this would be to put everything that's currently in do_printk() into do_printk_va_list (maybe rename that to vprintk() because that's the standard naming for this kind of thing), and then change printk/do_printk to a static inline which unpacks the varargs and calls into vprintk().
I know this is only tangentially related to your stuff, but doing that would make the code you're adding simpler and it would be better if we clean this up at some point before we keep throwing more and more logic into a wrong design.