Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37516 )
Change subject: mb/facebook/monolith: Add Facebook Monolith ......................................................................
Patch Set 7: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/37516/7/src/mainboard/facebook/mono... File src/mainboard/facebook/monolith/onboard.h:
https://review.coreboot.org/c/coreboot/+/37516/7/src/mainboard/facebook/mono... PS7, Line 22: #define ITE8528_DATA_PORT 0x6F off-topic: I can see these are defined the same way for fbg1701 and portwell/m107, and google/jecht also has onboard.h. I don't really like that these mirror information that is present in devicetree.cb and I might push a followup to move these super-io (configuration) base addresses into bootblock.c files directly.
Motivation is to not expose these to later stages and force the use of devicetree there.
https://review.coreboot.org/c/coreboot/+/37516/7/src/mainboard/facebook/mono... File src/mainboard/facebook/monolith/spd/spd.h:
https://review.coreboot.org/c/coreboot/+/37516/7/src/mainboard/facebook/mono... PS7, Line 25: void mainboard_fill_rcomp_strength_data(void *rcomp_strength_ptr); Looks like 4th or 5th copy of these prototypes in the tree, maybe have a look if there is need to define these more globally. They seem to fill FSP data so <fsp1.1/spd.h> might be viable.