Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38548 )
Change subject: soc/intel: Add Intel Skylake Scalable Processor support ......................................................................
Patch Set 18:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38548/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38548/5//COMMIT_MSG@13 PS5, Line 13: that will not be shared in public, at least for the time being.
Marking as resolved, as this was discussed in mailing list.
Done
https://review.coreboot.org/c/coreboot/+/38548/10/src/soc/intel/skylake_sp/c... File src/soc/intel/skylake_sp/chip.c:
https://review.coreboot.org/c/coreboot/+/38548/10/src/soc/intel/skylake_sp/c... PS10, Line 88: static void skxsp_pci_dev_iterator(struct bus *bus,
I hope not to make the scope of initial commit bigger. That being said, let's discuss the details. […]
We plan to improve the code in 3 phases: a. Get initial code in good shape. Such initial shape supports a Xeon-SP processor (eg. SKX-SP), it is good enough to boot into target OS. b. Extract Xeon-SP processor common code out. With such common code, one additional Xeon-SP processor will be supported, in addition to SKX-SP. c. Update generic code (such as device/pci_*) to work with multiple domains.
The being said, I went through the code, and made some changes: a. Got rid of soc_set_subsystem(). The generic one implemented in device/pci_device.c works fine. b. Reuse round() as defined in device/device.c.
https://review.coreboot.org/c/coreboot/+/38548/16/src/soc/intel/skylake_sp/c... File src/soc/intel/skylake_sp/chip.c:
https://review.coreboot.org/c/coreboot/+/38548/16/src/soc/intel/skylake_sp/c... PS16, Line 374: if (!(res->flags & IORESOURCE_BRIDGE) ||
Can this be moved out into a function? It would avoid so many tabs
Done