Attention is currently required from: Patrick Rudolph, Paul Menzel, David Hendricks, Shuo Liu, Nill Ge.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73392 )
Change subject: mb/ibm: Add IBM SBP1 ......................................................................
Patch Set 9: Code-Review+1
(4 comments)
File src/mainboard/ibm/sbp1/Kconfig:
https://review.coreboot.org/c/coreboot/+/73392/comment/470c7448_9b7d8ee0 PS9, Line 31: MAX_SOCKET_UPD Help text says:
This is used for configuring common SPR UPD tables which their sizes depend on the socket number. Since it's the maximal socket number for the common UPD tables, mainboard should not overwrite it.
Why does it have to be overridden here? Why not use MAX_SOCKET instead?
File src/mainboard/ibm/sbp1/bootblock.c:
https://review.coreboot.org/c/coreboot/+/73392/comment/cf532532_b7c8d18e PS9, Line 23: For ArcherCity CRB, only SUART1 is used. Drop this?
File src/mainboard/ibm/sbp1/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/73392/comment/e2939500_7539dc8d PS9, Line 10: // OEM revision Is it?
File src/mainboard/ibm/sbp1/romstage.c:
https://review.coreboot.org/c/coreboot/+/73392/comment/d916aca4_2a1e594b PS9, Line 345: sktbmp[0] = BIT(1) | BIT(2); : sktbmp[1] = BIT(1) | BIT(5); : sktbmp[2] = BIT(1) | BIT(4); : sktbmp[3] = BIT(1) | BIT(4); It might be useful to have macros for these
-
Angel Pons (Code Review)