Attention is currently required from: Nico Huber, Angel Pons, Arthur Heymans, Patrick Rudolph. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48386 )
Change subject: mb/hp/280_g2: Add new mainboard ......................................................................
Patch Set 3:
(8 comments)
File src/mainboard/hp/280_g2/Kconfig:
https://review.coreboot.org/c/coreboot/+/48386/comment/067c91a5_300ff5bb PS3, Line 18: for serial via superio, you'd need `select DRIVERS_UART_8250IO`
https://review.coreboot.org/c/coreboot/+/48386/comment/1ee9dd17_5a244990 PS3, Line 41: add a decent default for CBFS_SIZE?
File src/mainboard/hp/280_g2/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48386/comment/5e680930_acb529df PS3, Line 1: ## nit: one is enough
File src/mainboard/hp/280_g2/board_info.txt:
https://review.coreboot.org/c/coreboot/+/48386/comment/70dbd831_5621dfbf PS3, Line 7: maybe add vendor url?
File src/mainboard/hp/280_g2/bootblock.c:
https://review.coreboot.org/c/coreboot/+/48386/comment/a158addb_bda71a29 PS3, Line 18: PAD_CFG_NF(GPP_E9, NONE, DEEP, NF1), /* USB_OC_LAN# */ : PAD_CFG_NF(GPP_E10, NONE, DEEP, NF1), /* USB3.0_OC_BACK# */ : PAD_CFG_NF(GPP_E11, NONE, DEEP, NF1), /* USB_OC_REAR2# */ : PAD_CFG_NF(GPP_E12, NONE, DEEP, NF1), /* USB_OC_FRONT1# */ : PAD_CFG_NF(GPP_F15, NONE, DEEP, NF1), /* USB_OC_FRONT2# */ : PAD_CFG_GPI(GPP_G1, NONE, PLTRST), /* LPT_DET# */ : PAD_CFG_GPO(GPP_G2, 0, PLTRST), /* AUD_AMP_ON# */ : are they needed in bootblock?
File src/mainboard/hp/280_g2/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48386/comment/d28b8d2f_dc75f761 PS3, Line 8: register "PmConfigSlpS3MinAssert" = "SLP_S3_MIN_ASSERT_50MS" : register "PmConfigSlpS4MinAssert" = "SLP_S4_MIN_ASSERT_4S" : register "PmConfigSlpSusMinAssert" = "SLP_SUS_MIN_ASSERT_4S" : register "PmConfigSlpAMinAssert" = "SLP_A_MIN_ASSERT_2S" : register "PmConfigPwrCycDur" = "RESET_POWER_CYCLE_4S" : could be moved to pmc
File src/mainboard/hp/280_g2/romstage.c:
https://review.coreboot.org/c/coreboot/+/48386/comment/d4a99456_3dc84093 PS3, Line 40: /* These settings are most likely useless if using a release build of FSP */ : mem_cfg->PcdDebugInterfaceFlags = 2; /* Enable UART */ : mem_cfg->PcdSerialIoUartNumber = 2; /* Use UART #2 */ : mem_cfg->PcdSerialDebugBaudRate = 7; /* 115200 baud */ : mem_cfg->PcdSerialDebugLevel = 3; /* Log <= Info */ : nit: these match KBL fsp defaults
https://review.coreboot.org/c/coreboot/+/48386/comment/50a535ce_d4f3e095 PS3, Line 50: RealtimeMemoryTiming only effective when SaOcSupport=1