Attention is currently required from: Michał Kopeć, Tim Wawrzynczak. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63467 )
Change subject: soc/intel/alderlake: add GPIO definitions for PCH-S ......................................................................
Patch Set 3:
(12 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63467/comment/52f747e3_52781fcf PS3, Line 13: - Intel PCH-S EDS Vol1 (#621483) This DOC ID is actually vol2
File src/soc/intel/alderlake/gpio_pch_s.c:
https://review.coreboot.org/c/coreboot/+/63467/comment/794163e6_afb3507e PS3, Line 15: { .logical = PAD_RESET(PWROK), .chipset = 0U << 30 }, : { .logical = PAD_RESET(DEEP), .chipset = 1U << 30 }, : { .logical = PAD_RESET(PLTRST), .chipset = 2U << 30 }, GPP has 4 reset configs in total. Question is how to map it to the outdated values in intelblock header file. Since this is only logical, maybe we should add new names in intelblocks? The valeus would overlap, but at least the names will reflect the real reset configuration
https://review.coreboot.org/c/coreboot/+/63467/comment/8272b450_073696ac PS3, Line 70: INTEL_GPP_BASE(GPP_SPI0_IO_2, GPP_C0, GPP_C23, 288), /* GPP_C */ Let's add VPIO3 for completeness
INTEL_GPP(GPP_SPI0_IO_2, GPP_VGPIO_PCIE_0, GPP_VGPIO_PCIE_83),
https://review.coreboot.org/c/coreboot/+/63467/comment/41284e2f_436a2709 PS3, Line 72: Missing community3 vw entries: GPP_A0 - GPP_A14 GPP_C0 - GPP_C23
https://review.coreboot.org/c/coreboot/+/63467/comment/7e30dfa1_4ec15864 PS3, Line 88: INTEL_GPP(GPP_D0, GPP_JTAG_TDO, GPP_CPU_TRSTB), /* JTAG */ Let's add CPU groups for completeness
INTEL_GPP(GPP_D0, GPP_HDACPU_SDI, GPP_C10_WAKE), /* CPU */
https://review.coreboot.org/c/coreboot/+/63467/comment/2db63b28_0727a849 PS3, Line 173: .name = "GPP_SPI0AC", Just GPP_AC
https://review.coreboot.org/c/coreboot/+/63467/comment/13029705_7d0128b5 PS3, Line 204: [COMM_5] = { /* GPP D, JTAG */ GPP_D, JTAG, CPU
File src/soc/intel/alderlake/include/soc/gpio_defs_pch_s.h:
https://review.coreboot.org/c/coreboot/+/63467/comment/eb22604f_df6d426a PS3, Line 323: #define HOSTSW_OWN_REG_0 0x200 This is 0x150
https://review.coreboot.org/c/coreboot/+/63467/comment/ed66c27c_d1c48111 PS3, Line 324: #define GPI_INT_STS_0 0x220 This is 0x200
https://review.coreboot.org/c/coreboot/+/63467/comment/cd31a2ea_c460c6fe PS3, Line 325: #define GPI_INT_EN_0 0x110 This is 0x220
File src/soc/intel/alderlake/include/soc/gpio_defs_pch_s.h:
https://review.coreboot.org/c/coreboot/+/63467/comment/735ffb85_7865f7e4 PS1, Line 288: #define GPP_F20_IRQ 0x70 : #define GPP_F21_IRQ 0x71 : #define GPP_F22_IRQ 0x72 : #define GPP_F23_IRQ 0x73 : : /* Group D */ : #define GPP_D0_IRQ 0x70 : #define GPP_D1_IRQ 0x71 : #define GPP_D2_IRQ 0x72 : #define GPP_D3_IRQ 0x73
So does inteltool: […]
Yes, ADL PCH-H EDS vol2 says the same
File src/soc/intel/alderlake/include/soc/gpio_soc_defs_pch_s.h:
https://review.coreboot.org/c/coreboot/+/63467/comment/1042e9a7_b5dfb29d PS3, Line 62: #define GPP_GSPI0_CLK_LOOPBK INC(GPP_I17) : #define GPP_GSPI1_CLK_LOOPBK INC(GPP_I18) #define GPP_GSPI0_CLK_LOOPBK INC(GPP_I22) #define GPP_GSPI1_CLK_LOOPBK INC(GPP_GSPI0_CLK_LOOPBK)
Right?