Patch Set 8:

(3 comments)

This is much better now, but I still think it'll be even better if we can have vpd_gets and vpd_getbool in same usage for both RAM stage and ROM stage. They can then live in a shared (maybe vpd_common) place.

In fact, from your current implementation I wonder if it'll make more sense to have

 vpd.c // common utils, which based on a function like vpd_load_blob(type)
 vpd_cbmem.c // fetch and build cache in CBMEM, providing vpd_load_blob from CBMEM
 vpd_romstage.c, or vpd_nocache.c // providing vpd_load_blob directly

Thanks Hung-Te. I will take care of the 3 specific comments, and I am working on a version using CAR_GLOBAL to store blobs' base address and size.

On the other hand, my understanding of your general comments is that you are thinking about a different structure from the current design. With your design, vpd.c is linked to both stages, and provides library functions to both stages. In this case, build time #ifdef ENV_ROMSTAGE (or ENV_RAMSTAGE) is needed, isn't it? For example, RAMSTAGE_CBMEM_INIT_HOOK statement should only be built into ramstage; in vpd_gets() and vpd_find(), some different actions are needed for ramstage and for romstage. Is this preferred?

View Change

To view, visit change 34634. To unsubscribe, or for help writing mail filters, visit settings.