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 17:
(13 comments)
thanx for the reviews.
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:
Remove.
Done
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.
Getting support into upstream coreboot implies that the coreboot […]
Marking as resolved, as this was discussed in mailing list.
https://review.coreboot.org/c/coreboot/+/38548/5//COMMIT_MSG@14 PS5, Line 14:
I think "Skylake" is the only thing soc/intel/skylake/ and soc/intel/skylake_sp/ have in common
The uncore is vastly different, but cpu core may be same.
https://review.coreboot.org/c/coreboot/+/38548/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38548/16//COMMIT_MSG@7 PS16, Line 7: SkyLake
The `L` should not be uppercase: Skylake
Done
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/K... File src/soc/intel/skylake_sp/Kconfig:
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/K... PS15, Line 154: Do not change this value
Ack
Done
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... File src/soc/intel/skylake_sp/acpi.c:
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... PS5, Line 4: .
Remove.
Done
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... File src/soc/intel/skylake_sp/acpi/iiostack.asl:
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... PS5, Line 17: #define MAKE_IIO_DEV(id,rt) \
Why add the blank lines?
Done
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... File src/soc/intel/skylake_sp/acpi/pci_irq.asl:
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... PS5, Line 6: .
Remove.
Done
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... PS5, Line 70: #define MAKE_LINK_DEV(id,uid) \
Please align the last .
Done
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... File src/soc/intel/skylake_sp/acpi/uncore.asl:
https://review.coreboot.org/c/coreboot/+/38548/5/src/soc/intel/skylake_sp/ac... PS5, Line 4: .
Remove.
Done
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/a... File src/soc/intel/skylake_sp/acpi/uncore_irq.asl:
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/a... PS15, Line 50: Package (0x04) { 0xFFFF, 0x00, LNKA, 0x00 },
Ack. […]
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 578: return "";
that's not correct
Done
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/i... File src/soc/intel/skylake_sp/include/soc/ramstage.h:
https://review.coreboot.org/c/coreboot/+/38548/15/src/soc/intel/skylake_sp/i... PS15, Line 16:
double blank line
Done