Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34634 )
Change subject: drivers/vpd: add framework to search VPD in romstage ......................................................................
Patch Set 15:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34634/12/src/drivers/vpd/vpd.h File src/drivers/vpd/vpd.h:
https://review.coreboot.org/c/coreboot/+/34634/12/src/drivers/vpd/vpd.h@32 PS12, Line 32: void **ro_base, uint32_t *ro_size, : void **rw_base, uint32_t *rw_size
If 'struct vpd_blob' is already declared also in header file, I think we should just replace this by […]
Done
https://review.coreboot.org/c/coreboot/+/34634/12/src/drivers/vpd/vpd.h@35 PS12, Line 35: /* : * returns the size of data in a VPD 2.0 formatted fmap region, or 0. : * Also sets *base as the region's base address. : */ : int32_t get_vpd_size(const char *fmap_name, int32_t *base);
why do we need to publish this function?
Done
https://review.coreboot.org/c/coreboot/+/34634/12/src/drivers/vpd/vpd.h@45 PS12, Line 45: void **ro_base, uint32_t *ro_size, : void **rw_base, uint32_t *rw_size
... so vpd_get_buffers may use 'struct vpd_blob' as well. […]
Done
https://review.coreboot.org/c/coreboot/+/34634/12/src/drivers/vpd/vpd.c File src/drivers/vpd/vpd.c:
https://review.coreboot.org/c/coreboot/+/34634/12/src/drivers/vpd/vpd.c@52 PS12, Line 52:
for consistency we want to keep <=80 columns.
Done
https://review.coreboot.org/c/coreboot/+/34634/12/src/drivers/vpd/vpd.c@89 PS12, Line 89: if (blob->initialized == false) : return;
so who's going to update blob->initialized?
Done