Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46605 )
Change subject: lib/libpayload: Replace strapping_ids with new board configuration entry ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46605/9/payloads/libpayload/include... File payloads/libpayload/include/sysinfo.h:
https://review.coreboot.org/c/coreboot/+/46605/9/payloads/libpayload/include... PS9, Line 111: UNDEFINED_FW_CONFIG
Is the payload expected to test whether the fw_config is all 1s before treating it as a valid valu […]
Agreed. As long as we have the payload check the invalid/undefined case, it should be fine.
I think it would be good to at least add a comment indicating that the payload is responsible for ensuring that it checks against UNDEFINED_FW_CONFIG before using the bits set in fw_config field.
https://review.coreboot.org/c/coreboot/+/46605/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/46605/9/src/lib/coreboot_table.c@a2... PS9, Line 222: if (bid == UNDEFINED_STRAPPING_ID)
I don't believe so; libpayload parses the entire table and all of the entries found into fields in t […]
Right. Before this change that value in lib_sysinfo would be left as 0 and now it is changed to UNDEFINED_STRAPPING_ID. Probably not an issue, but just pointing out the change in behavior.