Attention is currently required from: Nico Huber, Arthur Heymans, Michael Niewöhner, Patrick Rudolph. Angel Pons 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:
(7 comments)
File src/mainboard/hp/280_g2/Kconfig:
https://review.coreboot.org/c/coreboot/+/48386/comment/0fdd638d_50035728 PS3, Line 18:
for serial via superio, you'd need `select DRIVERS_UART_8250IO`
Yeah, and also initialise the Super I/O UARTs in bootblock, and with a proper Super I/O driver. The Super I/O on my board doesn't have any UARTs, though, so I can't test.
https://review.coreboot.org/c/coreboot/+/48386/comment/70ac752b_3626c1fd PS3, Line 41:
add a decent default for CBFS_SIZE?
The default value (2 MiB) is decent enough. Reflashing the whole BIOS region on this board takes a while
File src/mainboard/hp/280_g2/board_info.txt:
https://review.coreboot.org/c/coreboot/+/48386/comment/217c856c_779fb1a4 PS3, Line 7:
maybe add vendor url?
https://support.hp.com/us-en/document/c04955444
(will add later)
File src/mainboard/hp/280_g2/bootblock.c:
https://review.coreboot.org/c/coreboot/+/48386/comment/6f790f16_f2b3aae4 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?
They're not required in bootblock, but I prefer to configure all GPIOs in one go. Did you notice that there isn't any ramstage GPIO table? 😄
File src/mainboard/hp/280_g2/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48386/comment/2f125396_0f5e47a9 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
Reminds me I should check if these need to be adjusted
File src/mainboard/hp/280_g2/romstage.c:
https://review.coreboot.org/c/coreboot/+/48386/comment/b9204ce8_e0b5adc8 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
I don't like relying on FSP defaults
https://review.coreboot.org/c/coreboot/+/48386/comment/86fa00f8_410b209d PS3, Line 50: RealtimeMemoryTiming
only effective when SaOcSupport=1
Hmm, good point. I'll revise