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 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41209/7/src/include/fw_config.h File src/include/fw_config.h:
https://review.coreboot.org/c/coreboot/+/41209/7/src/include/fw_config.h@43 PS7, Line 43: * @match_list: List of field and options to check.
Should clarify that this needs to be terminated by an empty entry. […]
Sure I will make this more clear, I think the devicetree probing one could pass a struct device instead pretty easily.
https://review.coreboot.org/c/coreboot/+/41209/7/src/lib/fw_config.c File src/lib/fw_config.c:
https://review.coreboot.org/c/coreboot/+/41209/7/src/lib/fw_config.c@11 PS7, Line 11: MAYBE_STATIC_BSS bool fw_config_value_initialized;
Actually, this doesn't work. […]
Hm yes this was supposed to be in a function and it isn't really a migrated global but I had not seen Kyosti's patch to remove MAYBE_STATIC_BSS entirely.
https://review.coreboot.org/c/coreboot/+/41209/7/src/lib/fw_config.c@27 PS7, Line 27: CONFIG_FW_CONFIG_SOURCE_CBFS_FILE
Should this be prefixed with CONFIG_CBFS_PREFIX? (I'm not sure if the whole normal/fallback thing st […]
I guess with CONFIG_CBFS_PREFIX it would be more than flexible enough without needing to also specify the name.
https://review.coreboot.org/c/coreboot/+/41209/7/src/lib/fw_config.c@33 PS7, Line 33: }
Your documentation says CBFS can override CBI (at least that's how I understood it), but the way thi […]
that is what I get for reworking things too many times, this function used to return early here.