Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Hannah Williams, Jamie Ryu, Kapil Porwal, Pranava Y N, Ravishankar Sarawadi, Saurabh Mishra, Wonkyu Kim.
Subrata Banik 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 31:
(8 comments)
File src/soc/intel/pantherlake/acpi/pcie.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/fd602db3_b3ac859a?usp... : PS23, Line 237: Name (_ADR, 0x001D0000)
Hi Subrata, we have reviwed this change to be corrected, but looks like the patch missed to add the changes. i have added the required correction.
I don't follow your argument here, these RP numbers were always wrong since patchset 1 of this CL and only correct with my code review. Therefore, I request to be more careful with the code change and try to be more meaningful.
File src/soc/intel/pantherlake/acpi/southbridge.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/4e61f1c3_e523f18b?usp... : PS23, Line 47: /* UFS 0:17:0 */ : #include "ufs.asl"
Acknowledged
i would request to add a TODO so, we know that UFS ASL entry is pending.
Why don't you add things properly in this CL itself. You need one Kconfig which will be selected for PTL-U based boards alone and then include ufs.asl based on the same Kconfig
File src/soc/intel/pantherlake/acpi/tcss_dma.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/0ed1a030_ae9d117b?usp... : PS23, Line 9: Offset (0x85), : PMEE, 1, /* 8, PME_EN */
Done
do you know why it was added in the first place ? someone referred to the wrong EDS ?
File src/soc/intel/pantherlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/05e9a108_93c52518?usp... : PS23, Line 56: 0xBAC
Acknowledged
sorry, unable to follow. Why not use this CL and rather prefer to use another CL for converting harcoded values into macro ?
File src/soc/intel/pantherlake/acpi/tcss_pcierp.asl:
https://review.coreboot.org/c/coreboot/+/83772/comment/0db92d3b_7c05d0b1?usp... : PS31, Line 27: still space and not tab
https://review.coreboot.org/c/coreboot/+/83772/comment/11bebdc0_e89a24cc?usp... : PS31, Line 28: same
https://review.coreboot.org/c/coreboot/+/83772/comment/a97986ff_bc424f24?usp... : PS31, Line 29: same
https://review.coreboot.org/c/coreboot/+/83772/comment/0b84ea59_6dc93705?usp... : PS31, Line 32: Offset (0x404), : LSOE, 1, : LNSE, 1, same