Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31539 )
Change subject: symbols.h: Add macro to define memlayout region symbols ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/31539/2/src/include/symbols.h File src/include/symbols.h:
https://review.coreboot.org/#/c/31539/2/src/include/symbols.h@a64 PS2, Line 64: : : : : : :
Is it important to keep any of these comments?
Oh, right, I'll keep that one.
https://review.coreboot.org/#/c/31539/2/src/security/vboot/symbols.h File src/security/vboot/symbols.h:
https://review.coreboot.org/#/c/31539/2/src/security/vboot/symbols.h@19 PS2, Line 19: #include <symbols.h> : : DECLARE_REGION(vboot2_work) :
Is there any reason why we aren't just defining this in symbols. […]
I don't think so, it's not very clear cut. Some stuff is separate out and some isn't. I'd be fine with merging all the non-SoC-specific stuff back into symbols.h, but I don't want to do it in this CL to not make it even bigger (because you'd have to touch the #include everywhere).