[L] Change in coreboot[master]: util/inteltool: Add support for Alderlake P in inteltool TEST=Dump re...
Attention is currently required from: Kacper Stojek, Michał Kopeć, Stefan Reinauer, Angel Pons.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/66825 )
Change subject: util/inteltool: Add support for Alderlake P in inteltool TEST=Dump registers on Clevo NS70PU with Intel® Core™ i7-1260P ......................................................................
Patch Set 6:
(15 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/66825/comment/8c03e08a_6915029e PS3, Line 8:
Added `TEST=Dump registers on Clevo NS70PU with Intel® Core™ i7-1260P` to the commit […]
It needs a blank new line of space between the commit title
https://review.coreboot.org/c/coreboot/+/66825/comment/5f53d67a_e28500bb PS3, Line 9: Change-Id: I2ba4ef7eee33d4dd762a05dd755de5e4d2e566dd
Now it should be signed.
Ack
File util/inteltool/gpio.c:
https://review.coreboot.org/c/coreboot/+/66825/comment/4d2b3840_98dcf9db PS2, Line 1110: case PCI_DEVICE_ID_INTEL_ADL_M:
According to my documentation and other repositories, yes. For example in coreboot https://github. […]
Ack
File util/inteltool/gpio_names/alderlake_p.h:
PS6: This whole file needs formatting. TABs should be used for indentation neverywhere. Please use `make test-lint` to check if the formatting is good.
https://review.coreboot.org/c/coreboot/+/66825/comment/560fc712_a59e0268 PS6, Line 373: "GPP_CPU_RSVD_1", "GPP_CPU_RSVD_1", : "GPP_CPU_RSVD_2", "GPP_CPU_RSVD_2", : "GPP_CPU_RSVD_3", "GPP_CPU_RSVD_3", : "GPP_CPU_RSVD_4", "GPP_CPU_RSVD_4", : "GPP_CPU_RSVD_5", "GPP_CPU_RSVD_5", : "GPP_CPU_RSVD_6", "GPP_CPU_RSVD_6", : "GPP_CPU_RSVD_7", "GPP_CPU_RSVD_7", : "GPP_CPU_RSVD_8", "GPP_CPU_RSVD_8", : "GPP_CPU_RSVD_9", "GPP_CPU_RSVD_9", : "GPP_CPU_RSVD_10", "GPP_CPU_RSVD_10", : "GPP_CPU_RSVD_11", "GPP_CPU_RSVD_11", : "GPP_CPU_RSVD_12", "GPP_CPU_RSVD_12", : "GPP_CPU_RSVD_13", "GPP_CPU_RSVD_13", : "GPP_CPU_RSVD_14", "GPP_CPU_RSVD_14", : "GPP_CPU_RSVD_15", "GPP_CPU_RSVD_15", : "GPP_CPU_RSVD_16", "GPP_CPU_RSVD_16", : "GPP_CPU_RSVD_17", "GPP_CPU_RSVD_17", : "GPP_CPU_RSVD_18", "GPP_CPU_RSVD_18", : "GPP_CPU_RSVD_19", "GPP_CPU_RSVD_19", : "GPP_CPU_RSVD_20", "GPP_CPU_RSVD_20", : "GPP_CPU_RSVD_21", "GPP_CPU_RSVD_21", : "GPP_CPU_RSVD_22", "GPP_CPU_RSVD_22", : "GPP_CPU_RSVD_23", "GPP_CPU_RSVD_23", : "GPP_CPU_RSVD_24", "GPP_CPU_RSVD_24", Indentation
https://review.coreboot.org/c/coreboot/+/66825/comment/b414c9d3_b2a722d0 PS6, Line 407: "GPP_VGPIO_0", "GPP_VGPIO_0", : "GPP_VGPIO_4", "GPP_VGPIO_4", : "GPP_VGPIO_5", "GPP_VGPIO_5", : "GPP_VGPIO_6", "GPP_VGPIO_6", : "GPP_VGPIO_7", "GPP_VGPIO_7", : "GPP_VGPIO_8", "GPP_VGPIO_8", : "GPP_VGPIO_9", "GPP_VGPIO_9", : "GPP_VGPIO_10", "GPP_VGPIO_10", : "GPP_VGPIO_11", "GPP_VGPIO_11", : "GPP_VGPIO_12", "GPP_VGPIO_12", : "GPP_VGPIO_13", "GPP_VGPIO_13", : "GPP_VGPIO_18", "GPP_VGPIO_18", : "GPP_VGPIO_19", "GPP_VGPIO_19", : "GPP_VGPIO_20", "GPP_VGPIO_20", : "GPP_VGPIO_21", "GPP_VGPIO_21", : "GPP_VGPIO_22", "GPP_VGPIO_22", : "GPP_VGPIO_23", "GPP_VGPIO_23", : "GPP_VGPIO_24", "GPP_VGPIO_24", : "GPP_VGPIO_25", "GPP_VGPIO_25", : "GPP_VGPIO_30", "GPP_VGPIO_30", : "GPP_VGPIO_31", "GPP_VGPIO_31", : "GPP_VGPIO_32", "GPP_VGPIO_32", : "GPP_VGPIO_33", "GPP_VGPIO_33", : "GPP_VGPIO_34", "GPP_VGPIO_34", : "GPP_VGPIO_35", "GPP_VGPIO_35", : "GPP_VGPIO_36", "GPP_VGPIO_36", : "GPP_VGPIO_37", "GPP_VGPIO_37", Indentation
https://review.coreboot.org/c/coreboot/+/66825/comment/4fb13bd1_6e32891b PS6, Line 445: "GPP_CPU_RSVD_25", "GPP_CPU_RSVD_25", : "GPP_CPU_RSVD_26", "GPP_CPU_RSVD_26", : "GPP_CPU_RSVD_27", "GPP_CPU_RSVD_27", : "GPP_CPU_RSVD_28", "GPP_CPU_RSVD_28", : "GPP_CPU_RSVD_29", "GPP_CPU_RSVD_29", : "GPP_CPU_RSVD_30", "GPP_CPU_RSVD_30", : "GPP_CPU_RSVD_31", "GPP_CPU_RSVD_31", : "GPP_CPU_RSVD_32", "GPP_CPU_RSVD_32", : "GPP_CPU_RSVD_33", "GPP_CPU_RSVD_33", : "GPP_CPU_RSVD_34", "GPP_CPU_RSVD_34", : "GPP_CPU_RSVD_35", "GPP_CPU_RSVD_35", : "GPP_CPU_RSVD_36", "GPP_CPU_RSVD_36", : "GPP_CPU_RSVD_37", "GPP_CPU_RSVD_37", : "GPP_CPU_RSVD_38", "GPP_CPU_RSVD_38", : "GPP_CPU_RSVD_39", "GPP_CPU_RSVD_39", : "GPP_vGPIO_PCIE_0", "GPP_vGPIO_PCIE_0", : "GPP_vGPIO_PCIE_1", "GPP_vGPIO_PCIE_1", : "GPP_vGPIO_PCIE_2", "GPP_vGPIO_PCIE_2", : "GPP_vGPIO_PCIE_3", "GPP_vGPIO_PCIE_3", : "GPP_vGPIO_PCIE_4", "GPP_vGPIO_PCIE_4", : "GPP_vGPIO_PCIE_5", "GPP_vGPIO_PCIE_5", : "GPP_vGPIO_PCIE_6", "GPP_vGPIO_PCIE_6", : "GPP_vGPIO_PCIE_7", "GPP_vGPIO_PCIE_7", : "GPP_vGPIO_PCIE_8", "GPP_vGPIO_PCIE_8", : "GPP_vGPIO_PCIE_9", "GPP_vGPIO_PCIE_9", : "GPP_vGPIO_PCIE_10", "GPP_vGPIO_PCIE_10", : "GPP_vGPIO_PCIE_11", "GPP_vGPIO_PCIE_11", : "GPP_vGPIO_PCIE_12", "GPP_vGPIO_PCIE_12", : "GPP_vGPIO_PCIE_13", "GPP_vGPIO_PCIE_13", : "GPP_vGPIO_PCIE_14", "GPP_vGPIO_PCIE_14", : "GPP_vGPIO_PCIE_15", "GPP_vGPIO_PCIE_15", : "GPP_vGPIO_PCIE_16", "GPP_vGPIO_PCIE_16", : "GPP_vGPIO_PCIE_17", "GPP_vGPIO_PCIE_17", : "GPP_vGPIO_PCIE_18", "GPP_vGPIO_PCIE_18", : "GPP_vGPIO_PCIE_19", "GPP_vGPIO_PCIE_19", : "GPP_vGPIO_PCIE_20", "GPP_vGPIO_PCIE_20", : "GPP_vGPIO_PCIE_21", "GPP_vGPIO_PCIE_21", : "GPP_vGPIO_PCIE_22", "GPP_vGPIO_PCIE_22", : "GPP_vGPIO_PCIE_23", "GPP_vGPIO_PCIE_23", : "GPP_vGPIO_PCIE_24", "GPP_vGPIO_PCIE_24", : "GPP_vGPIO_PCIE_25", "GPP_vGPIO_PCIE_25", : "GPP_vGPIO_PCIE_26", "GPP_vGPIO_PCIE_26", : "GPP_vGPIO_PCIE_27", "GPP_vGPIO_PCIE_27", : "GPP_vGPIO_PCIE_28", "GPP_vGPIO_PCIE_28", : "GPP_vGPIO_PCIE_29", "GPP_vGPIO_PCIE_29", : "GPP_vGPIO_PCIE_30", "GPP_vGPIO_PCIE_30", : "GPP_vGPIO_PCIE_31", "GPP_vGPIO_PCIE_31", : "GPP_vGPIO_PCIE_32", "GPP_vGPIO_PCIE_32", : "GPP_vGPIO_PCIE_33", "GPP_vGPIO_PCIE_33", : "GPP_vGPIO_PCIE_34", "GPP_vGPIO_PCIE_34", : "GPP_vGPIO_PCIE_35", "GPP_vGPIO_PCIE_35", : "GPP_vGPIO_PCIE_36", "GPP_vGPIO_PCIE_36", : "GPP_vGPIO_PCIE_37", "GPP_vGPIO_PCIE_37", : "GPP_vGPIO_PCIE_38", "GPP_vGPIO_PCIE_38", : "GPP_vGPIO_PCIE_39", "GPP_vGPIO_PCIE_39", : "GPP_vGPIO_PCIE_40", "GPP_vGPIO_PCIE_40", : "GPP_vGPIO_PCIE_41", "GPP_vGPIO_PCIE_41", : "GPP_vGPIO_PCIE_42", "GPP_vGPIO_PCIE_42", : "GPP_vGPIO_PCIE_43", "GPP_vGPIO_PCIE_43", : "GPP_vGPIO_PCIE_44", "GPP_vGPIO_PCIE_44", : "GPP_vGPIO_PCIE_45", "GPP_vGPIO_PCIE_45", : "GPP_vGPIO_PCIE_46", "GPP_vGPIO_PCIE_46", : "GPP_vGPIO_PCIE_47", "GPP_vGPIO_PCIE_47", : "GPP_vGPIO_PCIE_48", "GPP_vGPIO_PCIE_48", : "GPP_vGPIO_PCIE_49", "GPP_vGPIO_PCIE_49", : "GPP_vGPIO_PCIE_50", "GPP_vGPIO_PCIE_50", : "GPP_vGPIO_PCIE_51", "GPP_vGPIO_PCIE_51", : "GPP_vGPIO_PCIE_52", "GPP_vGPIO_PCIE_52", : "GPP_vGPIO_PCIE_53", "GPP_vGPIO_PCIE_53", : "GPP_vGPIO_PCIE_54", "GPP_vGPIO_PCIE_54", : "GPP_vGPIO_PCIE_55", "GPP_vGPIO_PCIE_55", : "GPP_vGPIO_PCIE_56", "GPP_vGPIO_PCIE_56", : "GPP_vGPIO_PCIE_57", "GPP_vGPIO_PCIE_57", : "GPP_vGPIO_PCIE_58", "GPP_vGPIO_PCIE_58", : "GPP_vGPIO_PCIE_59", "GPP_vGPIO_PCIE_59", : "GPP_vGPIO_PCIE_60", "GPP_vGPIO_PCIE_60", : "GPP_vGPIO_PCIE_61", "GPP_vGPIO_PCIE_61", : "GPP_vGPIO_PCIE_62", "GPP_vGPIO_PCIE_62", : "GPP_vGPIO_PCIE_63", "GPP_vGPIO_PCIE_63", : "GPP_vGPIO_PCIE_64", "GPP_vGPIO_PCIE_64", : "GPP_vGPIO_PCIE_65", "GPP_vGPIO_PCIE_65", : "GPP_vGPIO_PCIE_66", "GPP_vGPIO_PCIE_66", : "GPP_vGPIO_PCIE_67", "GPP_vGPIO_PCIE_67", : "GPP_vGPIO_PCIE_68", "GPP_vGPIO_PCIE_68", : "GPP_vGPIO_PCIE_69", "GPP_vGPIO_PCIE_69", : "GPP_vGPIO_PCIE_70", "GPP_vGPIO_PCIE_70", : "GPP_vGPIO_PCIE_71", "GPP_vGPIO_PCIE_71", : "GPP_vGPIO_PCIE_72", "GPP_vGPIO_PCIE_72", : "GPP_vGPIO_PCIE_73", "GPP_vGPIO_PCIE_73", : "GPP_vGPIO_PCIE_74", "GPP_vGPIO_PCIE_74", : "GPP_vGPIO_PCIE_75", "GPP_vGPIO_PCIE_75", : "GPP_vGPIO_PCIE_76", "GPP_vGPIO_PCIE_76", : "GPP_vGPIO_PCIE_77", "GPP_vGPIO_PCIE_77", : "GPP_vGPIO_PCIE_78", "GPP_vGPIO_PCIE_78", : "GPP_vGPIO_PCIE_79", "GPP_vGPIO_PCIE_79", : "GPP_CPU_RSVD_40", "GPP_CPU_RSVD_40", : "GPP_CPU_RSVD_41", "GPP_CPU_RSVD_41", : "GPP_CPU_RSVD_42", "GPP_CPU_RSVD_42", : "GPP_CPU_RSVD_43", "GPP_CPU_RSVD_43", : "GPP_CPU_RSVD_44", "GPP_CPU_RSVD_44", : "GPP_CPU_RSVD_45", "GPP_CPU_RSVD_45", : "GPP_CPU_RSVD_46", "GPP_CPU_RSVD_46", : "GPP_CPU_RSVD_47", "GPP_CPU_RSVD_47", : "GPP_vGPIO_PCIE_80", "GPP_vGPIO_PCIE_80", : "GPP_vGPIO_PCIE_81", "GPP_vGPIO_PCIE_81", : "GPP_vGPIO_PCIE_82", "GPP_vGPIO_PCIE_82", : "GPP_vGPIO_PCIE_83", "GPP_vGPIO_PCIE_83", Indentation
https://review.coreboot.org/c/coreboot/+/66825/comment/faa42afb_a5a15729 PS6, Line 577: &alderlake_pch_p_group_d, : &alderlake_pch_p_group_reserved, : &alderlake_pch_p_group_vgpio, Indentation
https://review.coreboot.org/c/coreboot/+/66825/comment/cb6e331f_22413855 PS6, Line 591: &alderlake_pch_p_group_pcie_vgpio, Indentation
https://review.coreboot.org/c/coreboot/+/66825/comment/afcf1419_48928d11 PS6, Line 603: &alderlake_pch_p_group_f, : &alderlake_pch_p_group_hvcmos, : &alderlake_pch_p_group_e, Indentation
https://review.coreboot.org/c/coreboot/+/66825/comment/3dbc090b_435906cd PS6, Line 616: &alderlake_pch_p_group_r, Indentation
File util/inteltool/inteltool.h:
https://review.coreboot.org/c/coreboot/+/66825/comment/da46b17c_9d223526 PS3, Line 290: 0x5182
PCI ID's are defined in `coreboot/src/include/device/pci_ids. […]
Ack
File util/inteltool/inteltool.c:
https://review.coreboot.org/c/coreboot/+/66825/comment/1fd5128c_243249db PS2, Line 548: { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ADL_P, : "12th generation (Alder Lake P family) Core Processor"}, : { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ADL_M, : "12th generation (Alder Lake M family) Core Processor"}, : { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_RPL_P, : "12th generation (Raptor Lake P family) Core Processor"},
I've moved them a bit higher up, now they are together with other Alder Lake processors.
Ack
https://review.coreboot.org/c/coreboot/+/66825/comment/b21720aa_4f7b963e PS2, Line 553: 12th
Yes, my bad. Now it says `13th`.
Ack
File util/inteltool/inteltool.c:
https://review.coreboot.org/c/coreboot/+/66825/comment/928a3d21_885bd37f PS6, Line 173: { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ADL_P, There are many many more IDs here for ADP and RPP eSPI. But... In this block of IDs we use "/* Host bridges /DRAM controllers integrated in CPUs */"
So you should be using these from pci_ids.h:
#define PCI_DID_INTEL_ADL_P_ID_1 0x4629 #define PCI_DID_INTEL_ADL_P_ID_3 0x4641 #define PCI_DID_INTEL_ADL_P_ID_4 0x4649 #define PCI_DID_INTEL_ADL_P_ID_5 0x4621 #define PCI_DID_INTEL_ADL_P_ID_6 0x4609 #define PCI_DID_INTEL_ADL_P_ID_7 0x4601 #define PCI_DID_INTEL_ADL_P_ID_8 0x4661 #define PCI_DID_INTEL_ADL_P_ID_9 0x467f #define PCI_DID_INTEL_ADL_P_ID_10 0x4619 #define PCI_DID_INTEL_ADL_M_ID_1 0x4602 #define PCI_DID_INTEL_ADL_M_ID_2 0x460a #define PCI_DID_INTEL_ADL_N_ID_1 0x4617 #define PCI_DID_INTEL_ADL_N_ID_2 0x461B #define PCI_DID_INTEL_ADL_N_ID_3 0x461c #define PCI_DID_INTEL_ADL_N_ID_4 0x4614 #define PCI_DID_INTEL_MTL_M_ID 0x7D00 #define PCI_DID_INTEL_MTL_P_ID_1 0x7D01 #define PCI_DID_INTEL_MTL_P_ID_2 0x7D02 #define PCI_DID_INTEL_MTL_P_ID_3 0x7d14 #define PCI_DID_INTEL_RPL_P_ID_1 0xa706 #define PCI_DID_INTEL_RPL_P_ID_2 0xa707 #define PCI_DID_INTEL_RPL_P_ID_3 0xa708
Probing for PCR address is ok based on ESPI device ID. However there are more eSPI IDS to add as well. Refer to documents what are the possible eSPI IDs for production devices.
-
Michał Żygowski (Code Review)