Attention is currently required from: Felix Singer, Michał Żygowski, Michał Kopeć, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62498 )
Change subject: mb/clevo/tgl-u: Add Clevo NV4x Tiger Lake laptop support ......................................................................
Patch Set 6:
(12 comments)
File src/mainboard/clevo/tgl-u/Kconfig:
https://review.coreboot.org/c/coreboot/+/62498/comment/6f4bb8e9_9b99da49 PS6, Line 60: GPP_C14 GPP_C14_IRQ
File src/mainboard/clevo/tgl-u/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/62498/comment/0e7d1192_a332e8a7 PS6, Line 7: MB/ME/MZ nit: MZ at first, since it's the base for the other two?
File src/mainboard/clevo/tgl-u/variants/nv40mz/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/62498/comment/1e842f25_a65fae78 PS6, Line 3: bootblock-y += gpio.c : : romstage-y += romstage.c : : ramstage-y += gpio.c : ramstage-y += hda_verb.c : not needed. covered by tgl-u/Makefile.inc
File src/mainboard/clevo/tgl-u/variants/nv40mz/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/62498/comment/b2422dc4_024d9a56 PS6, Line 17: register "external_bypass" = "1" could you check if PU5, PU8 are populated on the board?
https://review.coreboot.org/c/coreboot/+/62498/comment/67a9a59d_cc5348c1 PS6, Line 22: required
Is S0ix actually required?
S3 is deprecated on TGl
https://review.coreboot.org/c/coreboot/+/62498/comment/d601b788_bae157c8 PS6, Line 83: register "CnviBtCore" = "true" move down to cnvi dev
https://review.coreboot.org/c/coreboot/+/62498/comment/9fd58a95_f477e606 PS6, Line 86: GPE configuration : register "pmc_gpe0_dw0" = "PMC_GPP_C" : register "pmc_gpe0_dw1" = "PMC_GPP_E" : register "pmc_gpe0_dw2" = "PMC_GPD" : move down to pmc dev?
File src/mainboard/clevo/tgl-u/variants/nv40mz/gpio.c:
https://review.coreboot.org/c/coreboot/+/62498/comment/152560df_a6f1058d PS6, Line 9: add some comments?
https://review.coreboot.org/c/coreboot/+/62498/comment/8e609dbd_8d46bfba PS6, Line 129: //PAD_CFG_TERM_GPO(GPP_U4, 1, NONE, DEEP), /* DGPU_RST#_PCH */ : //PAD_CFG_TERM_GPO(GPP_U5, 1, NONE, DEEP), /* DGPU_PWR_EN */ : configured in gpio_early
https://review.coreboot.org/c/coreboot/+/62498/comment/3fbe94d7_8833a213 PS6, Line 210: PAD_CFG_GPI_SMI(GPP_E7, NONE, PLTRST, EDGE_SINGLE, INVERT), board uses espi vwire -> don't configure the gpio
File src/mainboard/clevo/tgl-u/variants/nv40mz/ramstage.c:
https://review.coreboot.org/c/coreboot/+/62498/comment/a7748900_d2bd98af PS6, Line 9: * Disable AER for the SSD slot to support S0ix with SSDs running : * buggy firmware : any more details on this?
https://review.coreboot.org/c/coreboot/+/62498/comment/a10fe5fe_37e41abd PS6, Line 12: params->CpuPcieRpAdvancedErrorReporting[0] = 0;
I'd also add this: […]
also params->CpuPcieRpLtrEnable[0] = 1;