Attention is currently required from: Saurabh Mishra, Subrata Banik.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81847?usp=email )
Change subject: soc/intel/{common, lunarlake}: Add support for new MCH ......................................................................
Patch Set 2:
(7 comments)
Commit Message:
PS2: Just reading the commit message, I would have thought the diff to be different. Maybe mention, that it’s another stepping(?) for Lunar Lake. Now idea about the correct term.
https://review.coreboot.org/c/coreboot/+/81847/comment/801147d4_8bf70cee : PS2, Line 7: soc/intel/{common, lunarlake}: Add support for new MCH The prefix does not need to be the path. Maybe:
soc/intel: Add support for new MCH Lunar Lake (0x6410)
After reading the diff:
soc/intel/lunarlake: Support stepping A0_2
https://review.coreboot.org/c/coreboot/+/81847/comment/f0a3bc46_b74e28a0 : PS2, Line 10: (ID:0x6410) I’d write: … with id 0x640
But it’s a minor comment. Feel free to ignore.
https://review.coreboot.org/c/coreboot/+/81847/comment/15251af7_6e0c3d3e : PS2, Line 10: The patch adds “The patch” is redundant [1]. Just use the imperative mood:
Add support for …
[1]: https://cbea.ms/git-commit/
https://review.coreboot.org/c/coreboot/+/81847/comment/7b526f30_7eb5192d : PS2, Line 11: Add new CPU ID (ID:0xb06d1) Add new CPU id 0xb06d1
https://review.coreboot.org/c/coreboot/+/81847/comment/b9b0904e_60c79d93 : PS2, Line 16: TEST=Build and boot the system having MCH ID:0x6410. Which log line is that?
File src/include/device/pci_ids.h:
https://review.coreboot.org/c/coreboot/+/81847/comment/734a48a2_9566e63b : PS2, Line 4292: #define PCI_DID_INTEL_LNL_M_ID_1 0x6410 Name the above …1, and this one _2 for alignment reasons?