Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36685 )
Change subject: src/mb/intel/coffeelake_rvp: Add mainboard for CML-S RVP8 ......................................................................
Patch Set 13:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... File src/mainboard/intel/coffeelake_rvp/variants/cml_s/include/variant/hda_verb.h:
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... PS13, Line 1: /* : * This file is part of the coreboot project. : * : * Copyright (C) 2018 Damien Zammit damien@zamaudio.com : * Copyright 2018 Intel Corporation. : * : * This program is free software; you can redistribute it and/or : * modify it under the terms of the GNU General Public License as : * published by the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
Please use SPDX: […]
Ack
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... File src/mainboard/intel/coffeelake_rvp/variants/cml_s/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... PS13, Line 89: register "PcieRpEnable[4]" = "1"
Does PcieRpEnable refer to the PCI device, or to the PCIe lane? IIRC, Thunderbolt uses only one root […]
Yes, it is x4.
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... PS13, Line 102: GBe
Gigabit Ethernet: GbE
Ack
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... PS13, Line 141: device pci 19.0 off end # I2C #4 (Not available on PCH-H)
If these PCI devices do not exist, adding them to the devicetree isn't necessary
Ack
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... PS13, Line 164:
nit: Add an extra space to preserve alignment?
Ack