Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35612 )
Change subject: mb/google/kukui: Extend FMAP to 8MB layout ......................................................................
Patch Set 7: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/35612/6/src/mainboard/google/kukui/... File src/mainboard/google/kukui/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/35612/6/src/mainboard/google/kukui/... PS6, Line 40: RW_SHARED 36K {
I tried that before, but the fmt tool currently seem to not support having two unspecified region i […]
That is... very odd. I was pretty sure it can do that (because IIRC it depth-first searches the nodes and then fixes up the variable one after it bubbled up everything from the others). Maybe there's a specific limitation if it's directly under the root node? (Of course the size of this node is only defined when you don't have the RW_UNUSED below... did you take that out to test it?)
https://review.coreboot.org/c/coreboot/+/35612/6/src/mainboard/google/kukui/... PS6, Line 42: RW_UNUSED
RW_SHARED has special meaning and will be copied on A/B AU so I'd like to reserve some space for it. […]
It does? I'm pretty sure it doesn't, anymore. The only thing in here I'm aware of is the netboot data. If it's supposed to have anything else (even reserve for future expansion) we should explicitly specify what that is.
https://review.coreboot.org/c/coreboot/+/35612/6/src/mainboard/google/kukui/... PS6, Line 44: RW_LEGACY(CBFS) 1M # Minimal 1M.
I'd like the make the regions in more similar size so if we have to move or revise in future (so upd […]
Not quite sure what you mean here? (It's just a CBFS, it should be able to deal with being bigger or smaller as long as everything fits. There are already plenty of other RW_LEGACY regions that aren't exactly 1MB if that's what you mean.)