Thanks! I think this is pretty close to what I'd expect now.
Just few more nits so we can make the interface more clean:

View Change

5 comments:

• File src/drivers/vpd/vpd.h:

  • Patch Set #12, 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 vpd_blob., i.e,

     void vpd_load_blob(struct vpd_blob *blob);

  • Patch Set #12, 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?

  • Patch Set #12, 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.

    If we can use static variable in pre-ram, this can even be

     const struct vpd_blob *vpd_get_blobs(void);

    And the caller can simply call it to figure out the buffer information.

• File src/drivers/vpd/vpd.c:

  • Patch Set #12, Line 52:

    for consistency we want to keep <=80 columns.

  • Patch Set #12, Line 89:

    if (blob->initialized == false)
    		return;
    

    so who's going to update blob->initialized?

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