Hi Paul, list,
On Thu, Jun 8, 2023, 16:16 Paul Menzel pmenzel@molgen.mpg.de wrote:
[Annie, Yiwei, I only added you to Cc. It’d be great if you made sure that all involved people are subscribed to the coreboot mailing list.]
Dear coreboot folks,
Two server boards based on Intel Archer City board, commit 30e743e7cc7f (mb/ibm: Add 4 SPR sockets server board IBM SBP1) [1], were pushed to Gerrit for review:
- mb/inventec: Add Intel SPR server board Inventec Transformers [2] Change-Id: Ic9d99c3aadaa9f69e6d14d4b1a6c5157f5590684
- bytedance/bd_egs (Eaglestream) [3] Change-Id: I091bc78e39cd76b3c6b9a10a1fcf58e9d671ef5d
Some code seems to be copied – like bootblock.c [4] – and almost identical to the reference platform. To avoid future maintenance burden, could more knowledgeable people comment, if server boards differ drastically so separate boards are justified or if they should be made variants.
A variant setup with boards from different vendors sounds like a maintenance nightmare. We still have a common place to put the redundant code, though: in chipset (SoC) code. The bootblock LPC decode can definitely be factored out.
That being said, it may be easier to factor things out after these three board ports have been submitted. The boot
I also noticed `mainboard_config_iio()`. Should that be moved to the
devicetree?
If the settings are only used in ramstage (where the devicetree is R/W), they could be moved to the devicetree. However, if these settings are used in romstage, the devicetree would negatively impact runtime-dependent configuration (e.g. when using straps to control IIO bifurcation depending on slot occupancy), as the devicetree cannot be updated at runtime in romstage (or earlier).
Kind regards,
Paul
[4]:
https://review.coreboot.org/c/coreboot/+/75598/5/src/mainboard/inventec/tran... _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
Best regards, Angel