Attention is currently required from: Anil Kumar K, Bora Guvendik, Hannah Williams, Jamie Ryu, Jérémy Compostella, Kapil Porwal, Paul Menzel, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra, Subrata Banik, Wonkyu Kim.
Cliff Huang has posted comments on this change by Ravishankar Sarawadi. ( https://review.coreboot.org/c/coreboot/+/83772?usp=email )
Change subject: soc/intel/ptl: Add SoC ACPI directory for Panther Lake ......................................................................
Patch Set 117:
(12 comments)
File src/soc/intel/pantherlake/acpi/camera_clock_ctl.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/9cb0e098_4c16f85d?usp... : PS104, Line 3: #define R_ICLK_PCR_CAMERA1 0x8000 : #define B_ICLK_PCR_FREQUENCY 0x1 : #define B_ICLK_PCR_REQUEST 0x2 :
can you please point me at the EDS section to compare the bit-field? […]
I add TASK 16 for TODO itme in https://partnerissuetracker.corp.google.com/issues/357011633 for IMGCLKOUT[n] register descritpion.
File src/soc/intel/pantherlake/acpi/southbridge.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/2edf4be2_4c641a18?usp... : PS81, Line 11: #if CONFIG(SOC_INTEL_COMMON_BLOCK_IOE_P2SB)
I am not familiar with this but start looking into this. […]
will need new romstage patch and then change ASL.
File src/soc/intel/pantherlake/acpi/southbridge.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/846e2196_e1fbf1d1?usp... : PS104, Line 5: #include <soc/itss.h>
please follow https://review.coreboot. […]
Done
File src/soc/intel/pantherlake/acpi/tcss.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/2db278c7_d5cb14cf?usp... : PS104, Line 396: /*OperationRegion (IOMR, SystemMemory, IOM_BASE_ADDR, 0x100)*/
do we need this ?
Done
https://review.coreboot.org/c/coreboot/+/83772/comment/a86462f6_8f3f3333?usp... : PS104, Line 779: If (TRE2 == 1) {
same
Done
https://review.coreboot.org/c/coreboot/+/83772/comment/b25cbdc9_a761f19c?usp... : PS104, Line 809: If (TRE3 == 1) {
again same problem, this PTL code is so old that you need to rebase to apply latest code changes. […]
Done
File src/soc/intel/pantherlake/acpi/tcss.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/971807ff_11f2015c?usp... : PS105, Line 32: #define MCHBAR_TCSS_DEVEN_OFFSET 0x7090
this offset needs to be changed to 0x73a8
Done
File src/soc/intel/pantherlake/acpi/tcss_dma.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/9fd96047_2e40a916?usp... : PS104, Line 13: 0xC8
use smallcase `0xc8`
apply this to all TCSS ASL files
File src/soc/intel/pantherlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/0c17f18d_d5132ec7?usp... : PS23, Line 56: 0xBAC
Subrata, I browsed across the ASL files and check how the Field under OperationRegion are written, […]
okay. following this CL: https://review.coreboot.org/c/coreboot/+/78163/10/src/soc/intel/meteorlake/a...
File src/soc/intel/pantherlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/64a05401_d0c21f8a?usp... : PS104, Line 22: Offset (0x5B),
we don't need this line
Done
https://review.coreboot.org/c/coreboot/+/83772/comment/4776bb25_b482e060?usp... : PS104, Line 15: Offset(0x5A), /* SLSTS[7:0] - Slot Status Register */ : ABPX, 1, /* 0, Attention Button Pressed */ : , 2, : PDCX, 1, /* 3, Presence Detect Changed */ : , 2, : PDSX, 1, /* 6, Presence Detect State */ : , 1, : Offset (0x5B), : DLSC, 1, /* 8, Data Link Layer State Changed */
why not this? […]
sure. based on the EDS register description, Slot Status (SLSTS) – Offset 5a is 16bit long.
https://review.coreboot.org/c/coreboot/+/83772/comment/86a04158_f94a410c?usp... : PS104, Line 24: Offset(0x60), /* RSTS - Root Status Register */ : Offset(0x62),
why not […]
sure. this is preferred as 0x60 register is 32-bit wide.
-
Cliff Huang (Code Review)