Jeremy Soller has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38463 )
Change subject: mainboard/system76: Add System76 Lemur Pro (lemp9) ......................................................................
Patch Set 6:
(14 comments)
https://review.coreboot.org/c/coreboot/+/38463/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38463/5//COMMIT_MSG@7 PS5, Line 7: src/
Remove.
Done
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/Kconfig:
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... PS5, Line 13: # select MAINBOARD_HAS_SPI_TPM_CR50 : # select MAINBOARD_HAS_TPM2
any reason to include since commented out?
Done
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... PS5, Line 29: config VARIANT_DIR : string : default "lemp9" if BOARD_SYSTEM76_LEMP9
per src/device/Kconfig, the default VBT path for non-variant boards is simply the mainboard folder
Done
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... PS5, Line 97: #config DRIVER_TPM_SPI_BUS : # hex : # default 0x0 : : #config DRIVER_TPM_SPI_CHIP : # int : # default 2
the latter seems fine to me
Done
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... PS5, Line 2: ramstage-y += ramstage.c hda_verb.c
Will fix
Done
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/bootblock.c:
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... PS1, Line 20: void bootblock_mainboard_init(void) {
open brace '{' following function definitions go on the next line
Done
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/ramstage.c:
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... PS1, Line 19: void mainboard_silicon_init_params(FSP_S_CONFIG *params) {
open brace '{' following function definitions go on the next line
Done
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/romstage.c:
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... PS1, Line 46: //{0x0F, 0x00}, {0xFF, 0x00}, {0xFF, 0x00}
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... PS1, Line 46: //{0x0F, 0x00}, {0xFF, 0x00}, {0xFF, 0x00}
please, no space before tabs
Done
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... PS1, Line 50: //{0x33, 0x00}, {0xFF, 0x00}, {0xFF, 0x00}
please, no space before tabs
Done
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... PS1, Line 50: //{0x33, 0x00}, {0xFF, 0x00}, {0xFF, 0x00}
code indent should use tabs where possible
Done
https://review.coreboot.org/c/coreboot/+/38463/1/src/mainboard/system76/lemp... PS1, Line 95: void mainboard_memory_init_params(FSPM_UPD *memupd) {
open brace '{' following function definitions go on the next line
Done
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/romstage.c:
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... PS5, Line 19: //TODO: find correct values
It's all figured out now, I will remove
Done
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... File src/mainboard/system76/lemp9/spd/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38463/5/src/mainboard/system76/lemp... PS5, Line 1: ##
Ack
Done