Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37427 )
Change subject: soc/intel/tigerlake: Update GPIO config ......................................................................
Patch Set 10:
(6 comments)
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/ac... File src/soc/intel/tigerlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/ac... PS10, Line 123: Field (PREG, AnyAcc, NoLock, Preserve) : { : VAL0, 32 : } : And (GPIOTXSTATE_MASK, VAL0, Local0) : Return (Local0) Use tap
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/ac... PS10, Line 137: Field (PREG, AnyAcc, NoLock, Preserve) : { : VAL0, 32 : } : Or Use tap
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/ac... PS10, Line 150: Field (PREG, AnyAcc, NoLock, Preserve) : { : VAL0, 32 : } : And (Not (GPIOTXSTATE_MASK), VAL0, VAL0) Use tap
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/gp... File src/soc/intel/tigerlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/gp... PS10, Line 19: - Intentionally added?
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/in... PS10, Line 33: #define GPP_B_DW0_1 0 : #define GPP_T_DW0_1 1 : #define GPP_A_DW0_1 2 : #define GPP_R_DW0_1 3 : #define GPD_DW0_1 4 : #define GPP_S_DW0_1 5 : #define GPP_H_DW0_1 6 : #define GPP_D_DW0_1 7 : #define GPP_U_DW0_1 8 : #define GPP_F_DW0_1 0xA : #define GPP_C_DW0_1 0xB : #define GPP_E_DW0_1 0xC : /* For DW2*/ : #define GPP_G_DW2 0 : #define GPP_B_DW2 1 : #define GPP_A_DW2 2 : #define GPP_R_DW2 3 : #define GPP_S_DW2 4 : #define GPD_DW2 5 : #define GPP_H_DW2 6 : #define GPP_D_DW2 7 : #define GPP_F_DW2 8 : #define GPP_C_DW2 0xA : #define GPP_E_DW2 0xB
All hex or all decimal.
it's better use hex.
https://review.coreboot.org/c/coreboot/+/37427/10/src/soc/intel/tigerlake/in... PS10, Line 380: /* TGL alternative native function pin mux */
Need a bit more explanation why we need these macros.
This is needed for setting alternative pin mux for UART0, I2C4, DMIC0, DMIC1, some of CNVi pins. And the pin mux setting will be done by FSPs and FSPm parameter setting along with each IP enable.
-
Wonkyu Kim (Code Review)