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:
(2 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); : }
I think I would need to do it in another pass after everything has been processed so that override options get checked too.
That's right.
I can at least close autohead/autogen before exit.
Sounds good.
https://review.coreboot.org/c/coreboot/+/41440/14/util/sconfig/main.c@1495 PS14, Line 1495:
I did this intentionally because this allows an override to remove all the probe entries and/or define it's own.
You would still need something like this to update the probe list for device in base tree: base_dev->probe = override_dev->probe;
Otherwise there is no way for an override device to remove a probe from the list.
I can change it if you think that this isn't something we need to accommodate. I should update the doc either way so it is clear how overrides work for probing...
I think it is fine to override all the probe properties provided by the device in the override tree. Good idea about updating the doc since it would be helpful as probe properties start getting used.
The downside is that an override has to keep that in mind and define the probe list every time it wants to override a device for anything, which is not great either..
That is right. It is slightly different than how we handle registers, but I think probe properties would mostly be used in override trees. So, it should be okay to have the probe properties be completely removed from base and use the ones provided by override.