Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38463 )
Change subject: src/mainboard/system76: Add System76 Lemur Pro (lemp9) ......................................................................
Patch Set 5:
(6 comments)
I do appreciate the formatting/commenting, esp in devicetree and gpio.h :)
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?
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 reason for this when there's no variant/baseboard setup? coming later?
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 same here
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 I think typically these are put on separate lines for readability
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 still true?
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: ## this device uses fixed RAM vs sodimms?