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 36:
(8 comments)
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... File src/mainboard/clevo/cml-u/variants/n141cu/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 3: todo: - i2c soc config (mainly touchpad) - touchpad -
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 19: ptf_enable" = "0" : register "PmTimerDisabled" = "0" : register "s0ix_enable" = "0" disabled -> drop
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 26: register "Device4Enable" = "0" should be enabled; we're still missing the devtree patches for cnl, so this can be dropped, when they have been added
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 42: register "tcc_offset" = "12" woops.. really that much?
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 79: register "SataMode" = "Sata_AHCI" could be dropped, since Sata_AHCI == 0
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 80: register "SataSalpSupport" = "0" drop if 0; but this could be enabled as well
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 157: register "PchPmSlpS3MinAssert" = "3" # 50ms : register "PchPmSlpS4MinAssert" = "1" # 1s : register "PchPmSlpSusMinAssert" = "2" # 500ms : register "PchPmSlpAMinAssert" = "4" # 2s : use the right macros here
https://review.coreboot.org/c/coreboot/+/40581/36/src/mainboard/clevo/cml-u/... PS36, Line 167: register "AcousticNoiseMitigation" = "1" AcousticNoiseMitigation is not for HDA iirc
-
Michael Niewöhner (Code Review)