Angel Pons 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: Code-Review+1
(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:
/* SPDX-License-Identifier: GPL-2.0-only */ /* This file is part of the coreboot project. */
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 port with four lanes.
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... PS13, Line 102: GBe Gigabit Ethernet: GbE
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
https://review.coreboot.org/c/coreboot/+/36685/13/src/mainboard/intel/coffee... PS13, Line 164: nit: Add an extra space to preserve alignment?