Hi Paul, list, On Thu, Jun 8, 2023, 16:16 Paul Menzel <[email protected]> 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: > > 1. mb/inventec: Add Intel SPR server board Inventec Transformers [2] > Change-Id: Ic9d99c3aadaa9f69e6d14d4b1a6c5157f5590684 > 2. 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 > > > [1]: https://review.coreboot.org/c/coreboot/+/73392 > "mb/ibm: Add 4 SPR sockets server board IBM SBP1" > [2]: https://review.coreboot.org/c/coreboot/+/75598 > "mb/inventec: Add Intel SPR server board Inventec Transformers" > [3]: https://review.coreboot.org/c/coreboot/+/75722 > "mb/bytedance: Add 2 SPR sockets server board bd_egs" > [4]: > > https://review.coreboot.org/c/coreboot/+/75598/5/src/mainboard/inventec/transformers/bootblock.c#18 > _______________________________________________ > coreboot mailing list -- [email protected] > To unsubscribe send an email to [email protected] Best regards, Angel
_______________________________________________ coreboot mailing list -- [email protected] To unsubscribe send an email to [email protected]
