Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40387 )
Change subject: mb/ocp/deltalake: Add OCP DeltaLake mainboard ......................................................................
Patch Set 11:
(11 comments)
Thanks!
https://review.coreboot.org/c/coreboot/+/40387/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40387/7//COMMIT_MSG@10 PS7, Line 10: CooperLake
Cooper Lake […]
Done
https://review.coreboot.org/c/coreboot/+/40387/7//COMMIT_MSG@16 PS7, Line 16: Following is
Below are the logs
Done
https://review.coreboot.org/c/coreboot/+/40387/7//COMMIT_MSG@33 PS7, Line 33: 955:returning from FspSiliconInit 176,696,186 (12,297,829,380,333,618)
That’s almost three minutes?
Yes, because we turn on lots of logging.
https://review.coreboot.org/c/coreboot/+/40387/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40387/8//COMMIT_MSG@19 PS8, Line 19: root@2620:10d:c0b9:c14:5f0f:abc3:81d8:86fb's password:
I don't think we need to have the IPv6 addresses here
Done
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... PS8, Line 2: /* This file is part of the coreboot project. */
We dropped the "This file is part of ... […]
Done
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/cpxsp_dl_gpio.h:
PS8:
Please use SPDX. […]
Done
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... PS8, Line 63: Corporation
I think we can drop the "Corporation" word to make the comments shorter
Done
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... PS8, Line 63: SkyLake-E
CPX isn't exactly Skylake, is it?
CPX-SP is 3rd generation of Xeon-SP processor family, while SKX-SP is the 2nd generation. With lspci command, "00:04.0 System peripheral: Intel Corporation Sky Lake-E CBDMA Registers (rev 0a)" is shown.
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... PS8, Line 72: Intel Corporation Device 2025
What are these devices?
Done
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... PS8, Line 78: device pci 11.1 on end # Intel Corporation Device a26d
Maybe add a comment to separate processor devices from PCH devices?
Done
https://review.coreboot.org/c/coreboot/+/40387/8/src/mainboard/ocp/deltalake... PS8, Line 90: device pci 1f.2 on end # Memory controller: Intel Corporation Device a221
Memory controller, in the PCH?
The memory controller is not in PCH, it is in processor.
-
Jonathan Zhang (Code Review)