Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47956 )
Change subject: fw_config: Default to undefined fw_config value ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/47956/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47956/3//COMMIT_MSG@9 PS3, Line 9: In order to support the use case of leaving runtime probing of : multi-sourced peripherals to the payload (or beyond) before a device is : provisioned
hmm... i'm not sure i followed that. […]
I'll try to reword it... this is for the factory, before the device is provisioned. I checked and the EC will return an error on the host command when there is no FW_CONFIG tag defined in CBI yet.
We want all devices listed in the ACPI tables when it's in the factory, so all peripherals that can be probed will be, and then the FW_CONFIG value is decided from that information on the factory floor. That's the reason for having fw_config_init() use fw_config_probe() which will return true for UNDEFINED_FW_CONFIG.
https://review.coreboot.org/c/coreboot/+/47956/3/src/lib/fw_config.c File src/lib/fw_config.c:
PS3:
please update Documentation/lib/fw_config.md to match.
Fair point, will do.
https://review.coreboot.org/c/coreboot/+/47956/3/src/lib/fw_config.c@17 PS3, Line 17: ;
can we simply start with […]
Ack
https://review.coreboot.org/c/coreboot/+/47956/3/src/lib/fw_config.c@75 PS3, Line 75: bool fw_config_probe(const struct fw_config *match)
is there ever a case where we would still want to use this version?
see below.
https://review.coreboot.org/c/coreboot/+/47956/3/src/lib/fw_config.c@132 PS3, Line 132: fw_config_probe
fw_config_probe_nodefault
Nope, this is the case when we *do* want all devices enabled. See my response above in the commit msg. Here's the way I see this being used:
1) fw_config_init() uses _probe() because we want all devices in the ACPI tables on the factory floor so we can probe for everything and write FW_CONFIG 2) GPIO programming should use _probe_nodefault() in general (see next patch), because it's safest to leave GPIOs in the NC state than to assign invalid NFs