[coreboot-gerrit] Change in coreboot[master]: cbfs: Make cbfs_default_props() hidden again

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36867 )

Change subject: cbfs: Make cbfs_default_props() hidden again
......................................................................

Patch Set 1:
> Patch Set 1:
> > Patch Set 1:
> >
> > Why not leave it like this for the next time someone may need it? (Technically, for the apollolake implementation it would also be a bit cleaner to call into this rather than reimplement it.)
>
> I was just trying to clean up the surface area. I'll make apollolake use it.
>
> Another question for you:
> I looked at this for a few mins earlier. We're currently utilizing offset/size properties. Should we make this API fill out a region device?
>
> int (*locate)(const struct region_device *boot_ro, struct region_device *cbfs_rdev);
>
> the only other place we use offset/size directly is in the coreboot table formation, but I was wondering if there's any added benefit to the change. One side effect would be that rdev_chain() has to be done within each of the callbacks instead of cbfs_boot_locate().

diff looks like this:

diff --git a/src/soc/intel/apollolake/mmap_boot.c b/src/soc/intel/apollolake/mmap_boot.c
index 631a834160..8243dc4d1f 100644
--- a/src/soc/intel/apollolake/mmap_boot.c
+++ b/src/soc/intel/apollolake/mmap_boot.c
@@ -102,14 +102,14 @@ const struct region_device *boot_device_ro(void)
 static int iafw_boot_region_properties(struct cbfs_props *props)
 {
        struct region *real_dev_reg;
-       struct region regn;

-       /* use fmap to locate CBFS area */
-       if (fmap_locate_area("COREBOOT", &regn))
+       if (cbfs_default_props(props))
                return -1;

-       props->offset = region_offset(&regn);
-       props->size = region_sz(&regn);
+       struct region regn = {
+               .offset = props->offset,
+               .size = props->size,
+       };

/* Check that we are within the memory mapped area. It's too
           easy to forget the SRAM mapping when crafting an FMAP file. */

It feels off/gross to me. Thoughts?