Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40377 )
Change subject: coreboot-id: Add coreboot id based on FMAP entries ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40377/2/src/mainboard/siemens/mc_ap... File src/mainboard/siemens/mc_apl1/mc_apl_vboot.fmd:
https://review.coreboot.org/c/coreboot/+/40377/2/src/mainboard/siemens/mc_ap... PS2, Line 9: COREBOOT_ID 0x40 : RO_FRID_PAD 0x780
Maybe we can discuss on mailing list, but I'm curious to hear more about your 'clunky and high touch' assessment.
I mean that you have to always manually put this entry in every FMAP you write. It's one more piece of boilerplate to carry around. In fact I bet the main reason Furquan is excluding CONFIG_CHROMEOS from this is to avoid having to update a hundred FMAPs of old Chrome OS boards, which leads us to the weird inconsistency where some configurations will have this ID and some won't (which I think is a bad idea... if we want to have this ID, it should be consistent everywhere, otherwise it's pretty useless). I think that's somewhat proving my point that putting things in FMAP is high-touch and we shouldn't add more stuff there without good reason.
I think with two layout mechanisms we should have clear reasons what goes in which one and shouldn't be putting things here or there arbitrarily. CBFS is generally more efficient and convenient, but makes precise placement more difficult. So basically stuff that needs to be placed in a specific spot or aligned to an erase block should be an FMAP section, everything else should be in CBFS (and yes, that means some of our legacy FMAP regions like GBB or FRID should would probably also be better placed in CBFS, and I'm considering to move them when I have to touch all that stuff anyway). This ID block has no placement requirements. It is functionally the same thing as the 'config' and 'revision' files we already place in CBFS.