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 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36685/3/src/mainboard/intel/coffeel... File src/mainboard/intel/coffeelake_rvp/chromeos_32MB.fmd:
https://review.coreboot.org/c/coreboot/+/36685/3/src/mainboard/intel/coffeel... PS3, Line 5: SI_GBE@0x81000 0x2000
Arthur, is this correct now?
It's not modified by this patch now.
https://review.coreboot.org/c/coreboot/+/36685/3/src/mainboard/intel/coffeel... File src/mainboard/intel/coffeelake_rvp/variants/cml_s/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/36685/3/src/mainboard/intel/coffeel... PS3, Line 164:
Is this still applicable?
it's moved to overridetree, will ensure no newline at the end.
https://review.coreboot.org/c/coreboot/+/36685/10/src/mainboard/intel/coffee... File src/mainboard/intel/coffeelake_rvp/variants/cml_s/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/36685/10/src/mainboard/intel/coffee... PS10, Line 73: register "PcieRpEnable[0]" = "0"
I think you can omit disabled PCIe root ports, as the default value is zero
will revise it
https://review.coreboot.org/c/coreboot/+/36685/10/src/mainboard/intel/coffee... PS10, Line 91: register "PcieRpEnable[4]" = "1"
Maybe add a comment that these are for the Thunderbolt controller
will add a comment
https://review.coreboot.org/c/coreboot/+/36685/10/src/mainboard/intel/coffee... PS10, Line 151: Tunderbolt
typo: missing "h" in Thunderbolt
Thanks. Good catch.