Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44782 )
Change subject: fw_config: Add caching to successfully probed fields ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44782/3/src/include/fw_config.h File src/include/fw_config.h:
https://review.coreboot.org/c/coreboot/+/44782/3/src/include/fw_config.h@48 PS3, Line 48: void fw_config_for_each_found(void (*cb)(const struct fw_config *config, void *arg), void *arg);
For the use case you have, wouldn't a fw_config_get_cached(field_mask) API be more useful? You're ju […]
Yeah, they're small functions, how about I just leave both of them here.
https://review.coreboot.org/c/coreboot/+/44782/3/src/lib/fw_config.c File src/lib/fw_config.c:
https://review.coreboot.org/c/coreboot/+/44782/3/src/lib/fw_config.c@78 PS3, Line 78: void fw_config_for_each_found(void (*cb)(const struct fw_config *config, void *arg), void *arg)
I think you should move this under the #if ENV_RAMSTAGE block, since it can't possibly do anything u […]
Done
https://review.coreboot.org/c/coreboot/+/44782/3/src/lib/fw_config.c@91 PS3, Line 91: return __ffs(probe->mask);
This can return -1: […]
The constants intended to be used here are generated in static.h and I believe sconfig will barf on fields that have zero bits, but I can add a check here.