Julius Werner 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 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44782/1/src/lib/fw_config.c File src/lib/fw_config.c:
https://review.coreboot.org/c/coreboot/+/44782/1/src/lib/fw_config.c@13 PS1, Line 13: #if ENV_RAMSTAGE After moving things around I think you can just merge this to the ENV_RAMSTAGE block below.
https://review.coreboot.org/c/coreboot/+/44782/1/src/lib/fw_config.c@20 PS1, Line 20: static struct fw_config cached_configs[MAX_CACHE_ELEMENTS]; If you tie this to fw_config_init() you could just store pointers here, rather than having to memcpy the whole thing.
https://review.coreboot.org/c/coreboot/+/44782/1/src/lib/fw_config.c@21 PS1, Line 21: static size_t cache_count; Rather than the increasing count I would consider using __ffs(match->mask) as the index. That way, duplicates will automatically map to the same slot. You can also easily implement the fw_config_get_cached() API that way if you want.
https://review.coreboot.org/c/coreboot/+/44782/1/src/lib/fw_config.c@112 PS1, Line 112: match = true; I think you should put this here, since it's supposed to be tied to the device tree parsing. Running it for manually called fw_config_probe() doesn't seem useful and would only add unpredictability.