Attention is currently required from: Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Rizwan Qureshi, Tim Wawrzynczak, Patrick Rudolph. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62581 )
Change subject: soc/intel/common: Include Meteor Lake device IDs ......................................................................
Patch Set 3:
(21 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62581/comment/475c64c8_3d9e031b PS1, Line 7: Meteorlake
Meteor Lake ?
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/48a11969_4b920a3d PS1, Line 9: Meteorlake
Meteor Lake
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/6003189f_f82bb057 PS1, Line 9: vol1(640228)
Please add a space before the (.
Ack
File src/include/device/pci_ids.h:
https://review.coreboot.org/c/coreboot/+/62581/comment/623078ec_1e1d57aa PS1, Line 3078: PCI_DEVICE_ID_INTEL_MTL_ESPI_0
This is what EDS is saying, do we need to add all of these […]
Currently we assigned 8 values for MTL M and P but they're not in external document. We'll add only assigned values to optimize common driver and add more once it's assigned.
https://review.coreboot.org/c/coreboot/+/62581/comment/8c2d0ed6_9adbe890 PS1, Line 3431: #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP9 0x7e4d : #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP10 0x7eca : #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP11 0x7ecb : #define PCI_DEVICE_ID_INTEL_MTL_PCIE_RP12 0x7ecc
I have some confusion here […]
There is no CPU RPs in MTL. All RPs are in SOC/IOE die and FSP UPD is using same array for RP1- RP12;FSP handles like PCH RPs.
So, We can use like below based on location. PCI_DEVICE_ID_INTEL_MTP_SOC_PCIE_RP1-9 PCI_DEVICE_ID_INTEL_MTP_IOE_P_PCIE_RP10-12
https://review.coreboot.org/c/coreboot/+/62581/comment/e4f46900_5f5b4104 PS1, Line 3508: #define PCI_DEVICE_ID_INTEL_MTL_SATA_2 0x7e67 : #define PCI_DEVICE_ID_INTEL_MTL_SATA_3 0x282a
you don't need these DID as for RAID mode. […]
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/4ebb913f_f685098f PS1, Line 3531: PCI_DEVICE_ID_INTEL_MTL_PMC
meaningful would be have different name macro […]
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/e1cfec51_ea431345 PS1, Line 3532: PCI_DEVICE_ID_INTEL_MTL_M_PMC
nit: PCI_DEVICE_ID_INTEL_MTL_M_IOE_PMC
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/88c4dcac_ca7adfbb PS1, Line 3533: PCI_DEVICE_ID_INTEL_MTL_P_PMC
nit: PCI_DEVICE_ID_INTEL_MTL_P_IOE_PMC
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/e6377b75_fead5e7a PS1, Line 3807: PCI_DEVICE_ID_INTEL_MTL_SPI0
suggestion: […]
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/d0626fc9_ad7fe79f PS1, Line 3962: #define PCI_DEVICE_ID_INTEL_SIMICS_GT0 0xffff
u don't need this. no probing is done for 0xFFFF DID device.
It's used for report platform for early SOC without IGD. We'll define and use in soc folder.
https://review.coreboot.org/c/coreboot/+/62581/comment/bc7ffe62_0c40c225 PS1, Line 4140: PCI_DEVICE_ID_INTEL_MTL_M_XHCI
PCI_DEVICE_ID_INTEL_MTL_M_TCSS_XHCI
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/5142f76a_e0382e06 PS1, Line 4141: PCI_DEVICE_ID_INTEL_MTL_P_XHCI
same
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/6a3dc4ce_b57d62d5 PS1, Line 4163: PCI_DEVICE_ID_INTEL_MTL_P2SB
please follow the same suggestion as PMC DID and split between SOC and IOE
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/f3e05c63_b5abf713 PS1, Line 4178: PCI_DEVICE_ID_INTEL_MTL_SRAM
same as above
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/afdd3ccc_7f6a7e0c PS1, Line 4269: #define PCI_DEVICE_ID_INTEL_MTL_CSE1 0x7e71 : #define PCI_DEVICE_ID_INTEL_MTL_CSE2 0x7e74 : #define PCI_DEVICE_ID_INTEL_MTL_CSE3 0x7e75
wondering, if we really need these HECI devices, we are keeping HECI1 alone enable. […]
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/c8e30cfb_807c6f63 PS1, Line 4292: PCI_DEVICE_ID_INTEL_MTL_M_XDCI
Please add _TCSS_ in between
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/8f5be521_a9253150 PS1, Line 4416:
can you please add CNVI Ids as well?
Ack
https://review.coreboot.org/c/coreboot/+/62581/comment/5400fa78_66cb316b PS1, Line 4424:
can you please add Crashlog ID as well?
Ack
File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/62581/comment/b549bab7_f2a29193 PS1, Line 31: CPUID_METEORLAKE_1_A0
suggestion: […]
Ack
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/62581/comment/457345f5_6a842bfe PS2, Line 1291: PCI_DID_INTEL_ADP_M_CSE3, This is missing.
-
Wonkyu Kim (Code Review)