jonzhang@fb.com has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34634 )
Change subject: drivers/vpd: set bool type variable matching with key/value pair ......................................................................
Patch Set 3:
Patch Set 3:
I still don't understand why you need to add all those complicated routes. Seems like you are using the VPD format that is compatible with Google VPD 2.0, then why not just call vpd_gets?
Even if we want to add a , this still looks bloated.
If you want to add a simple wrapper for identifying 1/0 values a better implementation is:
bool vpd_get_bool(const char *key, bool default_value);
or
int vpd_get_int(const char *key, int *result); // returns -1 on failure so we can detect 'non-exist'.
The reason is that this functionality is used before FSP API is called. At that time, DRAM is not initialized, and thus the (more convenient) functions such as vpd_gets() as defined in src/drivers/vpd/vpd.h are not available. src/drivers/vpd/vpd.c is not built for romstage.
After the split of a single patch into a patch set, this design constraint looks less obvious. I will update the commit message to state such constraint explicitly. Thanks!