Attention is currently required from: Tim Crawford, Jeremy Soller. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52349 )
Change subject: mb/system76/darp7: Add System76 Darter Pro 7 ......................................................................
Patch Set 12: Code-Review+1
(7 comments)
File src/mainboard/system76/darp7/Kconfig:
https://review.coreboot.org/c/coreboot/+/52349/comment/22923683_4ef54b11 PS12, Line 34: config MAINBOARD_SMBIOS_PRODUCT_NAME : string : default "Darter Pro" : : config MAINBOARD_VERSION : string : default "darp7" : : config CBFS_SIZE : default 0xA00000 : : config CONSOLE_POST : bool : default y : : config DIMM_SPD_SIZE : int : default 512 : : config POST_DEVICE : bool : default n : : config UART_FOR_CONSOLE : int : default 2 It shouldn't be necessary to specify any types.
File src/mainboard/system76/darp7/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/52349/comment/ebc0e362_04bfab72 PS12, Line 3: .chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT, Needs an update after CB:56967
https://review.coreboot.org/c/coreboot/+/52349/comment/0954e1e5_630ab7ec PS12, Line 126: register "HybridStorageMode" = "0" I think this is about Optane
https://review.coreboot.org/c/coreboot/+/52349/comment/ef5db83b_b9d4d441 PS12, Line 285: # TODO: should this be GPIO_LANRTD3 or LAN_PLT_RST# ? Most likely the former.
File src/mainboard/system76/darp7/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/52349/comment/ca14bc6d_9107b55a PS12, Line 10: /* OEM revision */ I don't think it is. I'd drop the comment.
File src/mainboard/system76/darp7/gpio.h:
https://review.coreboot.org/c/coreboot/+/52349/comment/fa55cb4e_c991daf9 PS12, Line 6: #ifndef __ACPI__ Is this guard really needed?
File src/mainboard/system76/darp7/romstage.c:
https://review.coreboot.org/c/coreboot/+/52349/comment/d9031164_ae6f6fff PS12, Line 11: true Is this intentionally enabled?
-
Angel Pons (Code Review)