Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40581 )
Change subject: mb/clevo/n141cu: Add new Comet Lake mainboard ......................................................................
Patch Set 31:
(7 comments)
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... File src/mainboard/clevo/cml-u/Kconfig:
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... PS31, Line 18: select ONBOARD_VGA_IS_PRIMARY does it really have ga?
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... PS31, Line 38: default "clevo/cml-u" : : config VARIANT_DIR : default "n141cu" : : config MAINBOARD_FAMILY : default "Comet Lake" : hmm, are there multiple latops that are expected to be mostly idenatical or at least very similar?
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... File src/mainboard/clevo/cml-u/board_info.txt:
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... PS31, Line 2: Board name: cml-u that's not the board/laptop name, is it?
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... File src/mainboard/clevo/cml-u/ramstage.c:
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... PS31, Line 11: Make M.2 port configurable (SATA <> have a look at intel flex i/o. that should be possible to be handled outmagically if they didn't mess it up 😊
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... File src/mainboard/clevo/cml-u/variants/n141cu/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... PS31, Line 169: register "deep_s3_enable_ac" = "0" you can drop the 0 values, since they are default
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... File src/mainboard/clevo/cml-u/variants/n141cu/gma-mainboard.ads:
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... PS31, Line 13: P1, : DP2, : DP3, : HDMI1, : HDMI2, : HDMI3, : eDP, : others so many ports?
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... File src/mainboard/clevo/cml-u/variants/n141cu/include/gpio_table.h:
https://review.coreboot.org/c/coreboot/+/40581/31/src/mainboard/clevo/cml-u/... PS31, Line 91: 0x04000201 this will trigger an error "gpio_pad_reset_config_override: Logical to Chipset mapping not found" due to the reset mapping; replace 0x0... with 0xc...