Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41209 )
Change subject: fw_config: Add firmware configuration interface ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41209/2/src/include/fw_config.h File src/include/fw_config.h:
https://review.coreboot.org/c/coreboot/+/41209/2/src/include/fw_config.h@9 PS2, Line 9:
mainboards themselves can't include code to static.c currently
I'd be curious why that is, what was the exact problem? I thought static.c is treated like any other stage source file once it is generated.
It is because static.c is entirely generated and the only #includes come from other chips. (except mainboard because it is special) So if you try to provide something from the mainboard itself it has to be included indirectly from another chip.
Just an annoyance, I had a hack to sconfig that allowed an 'include ...' token but that was messy as well, mostly because sconfig is just messy by default.
It's possible to have the generation macro spit out a #define FW_CONFIG_FIELD_AUDIO_NAME "AUDIO" as well, of course (and same for options).
Sure but that takes up space just like a string in a table.
Auto-generating the table from somewhere else also sounds fine but I think that's a pretty orthogonal issue to how it is represented in C. You could auto-generate macros or you could auto-generate structs. But at least if you don't want to auto-generate it I think we can definitely find ways to make a header file with macro definitions just as compact and readable as a struct initializer.
It solves some of the problems that I had with macros, so it isn't necessarily orthogonal if it results in the solution that people prefer.