Attention is currently required from: Jakub Czapiga, Julius Werner, Nico Huber, Philipp Hug, ron minnich.
Maximilian Brune has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/coreboot/+/79907?usp=email )
Change subject: [RFC] region: Hide struct region members ......................................................................
Patch Set 8:
(2 comments)
File src/arch/arm/fit_payload.c:
https://review.coreboot.org/c/coreboot/+/79907/comment/f32dc12f_e9ac7451?usp... : PS8, Line 3: /* FIXME: should use the high-level region api */
Hmmm, the comments were probably a bit premature. The code seems to […]
I don't think we have an actual maintainer for the FIT payload support. I have only tried to use a FIT payload once (~1 year ago) and at the time it didn't even work (I think). I wouldn't be surprised if it still doesn't work and no one noticed, since no one uses it. If it were up to me I would just remove the FIT payload entirely and just use SELF (maybe generate a SELF from a FIT in build time like we do with everything else). But that is something for another time/patch. I would suggest we ignore the FIT stuff and just move on with this patch (aka marking this as resolved). One thing though: Can't `region-test.c` just use the getter/setter functions instead of directly accessing the properties?
File src/commonlib/include/commonlib/region.h:
https://review.coreboot.org/c/coreboot/+/79907/comment/f6292274_b72a16cb?usp... : PS8, Line 80: #else : #define _region_offset _region_internal_only__offset : #define _region_size _region_internal_only__size I understand that in order for code that still accesses `offset` and `size` directly we need this `#ifdef`. But for what purposes does the content of `#else` exist? Why redefine it as something else even if `REGION_INTERNAL_STRUCTURES` is not set? I feel like I am missing something.