Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41440 )
Change subject: sconfig: Add support for firmware configuration ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41440/14/util/sconfig/main.c File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/41440/14/util/sconfig/main.c@557 PS14, Line 557: field = find_fw_config_field(probe->field); : if (!field) { : printf("ERROR: fw_config_probe field %s not found\n", probe->field); : exit(1); : } : option = find_fw_config_option(field, probe->option); : if (!option) { : printf("ERROR: fw_config_probe field %s option %s not found\n", : probe->field, probe->option); : exit(1); : } Should these be done in add_fw_config_probe() instead so that we don't exit() with file opened?
https://review.coreboot.org/c/coreboot/+/41440/14/util/sconfig/main.c@1495 PS14, Line 1495: I think we will need something here(like update_fw_config_probe) to copy the probe properties of a device in override tree to the equivalent device in base tree. This would be for a condition where device exists in both base tree and override tree and the device in override tree contains additional probe properties.
https://review.coreboot.org/c/coreboot/+/41440/14/util/sconfig/sconfig.h File util/sconfig/sconfig.h:
https://review.coreboot.org/c/coreboot/+/41440/14/util/sconfig/sconfig.h@11 PS14, Line 11: nit: extra blank line not required.