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: Code-Review-1
(1 comment)
Sorry, I really don't like this. :/ If we can't agree on where to put it can we at least agree to remove it entirely, maybe?
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
You are right. […]
Well, I'm still really not a fan of making a separate FMAP section for this (trying to merge discussion from CB:40376). And now you're saying some boards aren't even going to have it? What good is an ID when you can't rely on it being there? And in return if someone tried to build without vboot for a Chrome OS FMAP they're gonna get weird errors.
If we think this information isn't useful, we might as well drop it completely. I'm pretty sure nobody is using it right now. (The show_id() function in Chromium flashrom is dead code, and it upstream flashrom I can't find it at all. Also, it would've never worked on non-x86 boards in the current state. I don't know what flashrom would need this info for anyway.)
If we want to keep this in the image as separate FYI information (like we already do with the 'config' and 'revision' files), I think that's fine, but then the right place to put it is CBFS. FMAP sections are clunky and high touch and we shouldn't create more of them just for something we don't really need anyway. There's a reason we don't just put every file into a separate FMAP section.