[coreboot-gerrit] [L] Change in coreboot[main]: Post-build control of serial

[Simon Glass (Code Review)]

Attention is currently required from: Jérémy Compostella, Stefan Reinauer.

Simon Glass has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77712?usp=email )

Change subject: Post-build control of serial
......................................................................

Patch Set 4:

(6 comments)

File Documentation/util/cbfstool/ccb.md:

https://review.coreboot.org/c/coreboot/+/77712/comment/169a9e0c_d532d859 :
PS2, Line 11: ccb-set
> can we have this be more "natural"?  […]

I will try:
configure
configure-get

I don't like the last one very much. WDYT?

File src/commonlib/include/commonlib/ccb.h:

https://review.coreboot.org/c/coreboot/+/77712/comment/3767a75d_c9c4ec97 :
PS2, Line 32: struct ccb {
            : 	uint32_t magic;
            : 	uint32_t flags;
            : };
            :
> Can we / do we want to migrate GBB into this format as well? It would be nice to not have several ty […]

I thought that was a vboot-specific thing?

Also, my understanding is that GBB only supports flags, which is all this does too, right now. But this can be expanded (e.g. for log levels) but I'm not sure that GBB can?

File src/console/Kconfig:

https://review.coreboot.org/c/coreboot/+/77712/comment/06133d7c_92ba5820 :
PS2, Line 443: config CONSOLE_SILENT_IN_CCB
             : 	bool "Flag for silencing console is in coreboot control block"
             : 	default y
             : 	help
             : 	  This enables using the coreboot control block (CCB) to obtain the
             : 	  console silence flag in the bootblock. This is set to 0 by default,
             : 	  meaning that the console is active (if enabled) but can be set to 1
             : 	  by an external tool operating on the image.
> What other mechanisms would we use here instead of CCB? Can we just have one that works everywhere? […]

Yes I suppose we can drop this option and just always support it?

File src/lib/ccb.c:

https://review.coreboot.org/c/coreboot/+/77712/comment/5c04437c_44ac4292 :
PS3, Line 8: struct ccb ccb_static __attribute__((section(".init"))) = {
> `Prefer __section(".init") over __attribute__((section(".init")))`

Please fix.

File util/cbfstool/cbfstool.c:

https://review.coreboot.org/c/coreboot/+/77712/comment/a261607b_0fc307a0 :
PS4, Line 2140: 		        "Get a value from the Coreboot Control Block\n"
> `code indent should use tabs where possible`

Please fix.

https://review.coreboot.org/c/coreboot/+/77712/comment/4e28c69a_aa3fa186 :
PS4, Line 2142: 		        "Set a value in the Coreboot Control Block\n"
> `code indent should use tabs where possible`

Please fix.