[XL] Change in coreboot[main]: soc/intel/ptl: Add GPIOs for Panther Lake SOC

[Cliff Huang (Code Review)]

Attention is currently required from: Anil Kumar K, Bora Guvendik, Elyes Haouas, 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 Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83789?usp=email )

Change subject: soc/intel/ptl: Add GPIOs for Panther Lake SOC
......................................................................

Patch Set 66:

(5 comments)

File src/soc/intel/pantherlake/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/83789/comment/11118e0c_7b0d127b?usp... :
PS49, Line 436: Name (JTAG, Package (0x02)
> this is mostly for FW debugging. […]

I have asked our kernel pinctrl owner and we cannot skip PADs within a group, so if any of PADs needs to be exposed, the entire group needs to be added. I didn't get the reason, though. To be conservative and consistent, I follow what we did for the past SOCs and LNL (first with this new schema) for now as it requires more validation and finding out the corner cases if we drop any of those non GPP_[group]_[num] group. Will it be okay if we add this in the TODO list for further revisiting?

File src/soc/intel/pantherlake/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/83789/comment/db02c6e2_22343998?usp... :
PS55, Line 135: GPPV
> > Acknowledged […]

ouch:( adding comments.

File src/soc/intel/pantherlake/acpi/gpio.asl:

https://review.coreboot.org/c/coreboot/+/83789/comment/46f1e687_f6ed1cd9?usp... :
PS65, Line 770: GPIO
> `VGP0` aka virtual GPIO

okay. this name comes from BIOS and LNL GPIO ASL. VGP0 sounds much better than GPIO. I was also thinking VGP_.

https://review.coreboot.org/c/coreboot/+/83789/comment/1445aee3_bea4d02b?usp... :
PS65, Line 805: GADD
> can you please move these ACPI methods before exposing the `GPIO` device ?

Done

File src/soc/intel/pantherlake/include/soc/gpio_soc_defs.h:

https://review.coreboot.org/c/coreboot/+/83789/comment/bd59a2e6_6ce4f0a1?usp... :
PS49, Line 31: 1
> > We don't have COMM2, so GPP_COMM3 cannot use this macro, though. […]

Subrata,
Currently, we use _UID to describe their community number. 
from PTL, we have:
INTC10BC:00/uid:0
INTC10BC:01/uid:1
INTC10BC:02/uid:3
INTC10BC:03/uid:4
INTC10BC:04/uid:5
I also checked BIOS for other four SOCs, including LNL, that use this new GPIO schema, and they use _UID for the community number. LNL CB also does the same.

Name (_UID, 0)          // numeric value, GPIO Community
    Name (_UID, 1)          // numeric value, GPIO Community
    Name (_UID, 3)          // numeric value, GPIO Community
    Name (_UID, 4)          // numeric value, GPIO Community
    Name (_UID, 5)          // numeric value, GPIO Community

As their instance already with :0[n] for index number, should we keep this consistent among these SOCs that uses this new schema? Also, I was told that there is no value/benefit/security implications for hiding their actual community number. At the same time, it is not harmful to use the sequential number for their _UID as you suggest.