Attention is currently required from: Tarun Tuli, Ian Feng, Dtrain Hsu.
Jamie Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71706 )
Change subject: mb/google/brya/var/omnigul: Update GPIO settings ......................................................................
Patch Set 14:
(27 comments)
File src/mainboard/google/brya/variants/omnigul/gpio.c:
https://review.coreboot.org/c/coreboot/+/71706/comment/536512b5_bcd2f99f PS9, Line 72: PAD_NC(GPP_B23, NONE),
Baseboard is already PAD_NC. […]
NO，I take off that
https://review.coreboot.org/c/coreboot/+/71706/comment/1ee0ba1b_a5748ca3 PS9, Line 148: /* E21 : DDP2_CTRLDATA ==> USB_C1_LSX_SOC_RX_STRAP */ : /* E22 : DDPA_CTRLCLK ==> USB_C0_AUX_DC_STRAP_P */ : PAD_CFG_GPO(GPP_E22, 1, DEEP), : /* E23 : DDPA_CTRLDATA ==> USB_C0_AUX_DC_N */ : PAD_CFG_GPO(GPP_E23, 1, DEEP),
Could you check GPP_E22 and GPP_E23 the same with baseboard or not?
Not same with baseboard , According to the soc gpio table GPP_E22 & GPP_E23 is to fill in the GPO.
https://review.coreboot.org/c/coreboot/+/71706/comment/5cd68b76_6af927f1 PS9, Line 194: /* H6 : I2C1_SDA ==> PCH_I2C_TPM_SDA */ : PAD_CFG_NF_LOCK(GPP_H6, NONE, NF1, LOCK_CONFIG), : /* H7 : I2C1_SCL ==> PCH_I2C_TPM_SCL */ : PAD_CFG_NF_LOCK(GPP_H7, NONE, NF1, LOCK_CONFIG),
Could you check GPP_H6 and GPP_H7 the same with baseboard or not?
yes，I check done, it is same ith baseboard. I take off.
https://review.coreboot.org/c/coreboot/+/71706/comment/44ea5a67_841fdf37 PS9, Line 231: PAD_CFG_NF_LOCK(GPP_R7, NONE, NF3, LOCK_CONFIG),
And GPP_R6 setting?
GPP_R6 & GPP_R7 EE have modfiy with google,they will change the gpios to DMIC_CLK0_R & DMIC_DATA0_R set Function3
https://review.coreboot.org/c/coreboot/+/71706/comment/297094dd_31c1ba04 PS9, Line 233: /* S0 : SNDW0_CLK ==> I2S_SPKR_SCLK_R */ : PAD_CFG_NF_LOCK(GPP_S0, NONE, NF4, LOCK_CONFIG), : /* S1 : SNDW0_DATA ==> I2S_SPKR_SFRM_R */ : PAD_CFG_NF_LOCK(GPP_S1, NONE, NF4, LOCK_CONFIG),
Could you check GPP_S0 and GPP_S1 settings?
GPP_S0 & GPP_S1 EE have modfiy with google,they will change the gpios to I2S_SPKR_SCLK_R & I2S_SPKR_SFRM_R set Function4
https://review.coreboot.org/c/coreboot/+/71706/comment/1231a96e_20cfff35 PS9, Line 240: PAD_NC(GPP_S3, NONE),
And GPP_S2 setting?
GPP_S2 & GPP_S3 EE have modfiy with google,they will change the gpios to I2S_PCH_TX_SPKR_RX_R & I2S_PCH_RX_SPKR_TXset Function4 and dissable
https://review.coreboot.org/c/coreboot/+/71706/comment/ef84d36e_f52ef67d PS9, Line 250: /* GPD2 : LAN_WAKE# ==> EC_PCH_WAKE_ODL */ : PAD_CFG_GPO(GPD2, 1, DEEP),
I think it is the same with baseboard. […]
Not same with baseboard , According to the soc gpio table GPD2 is to fill in the GPO.
File src/mainboard/google/brya/variants/omnigul/gpio.c:
https://review.coreboot.org/c/coreboot/+/71706/comment/1fdd3e13_c852d78a PS10, Line 17: PAD_NC(GPP_A5, NONE),
Baseboard is already PAD_NC. […]
NO，I take off that.
https://review.coreboot.org/c/coreboot/+/71706/comment/6ad65fbe_2c7ace7a PS10, Line 18: /* A6 : ESPI_ALERT1# ==> ESPI_ALERT1 */
/* A6 : ESPI_ALERT1# ==> NC */
done
https://review.coreboot.org/c/coreboot/+/71706/comment/54f34464_b209e22c PS10, Line 20: /* A7 : SRCCLK_OE7# ==> GPP_A7 */
Should PAD_NC(GPP_A7, NONE), or PAD_CFG_GPI(GPP_A7, NONE, DEEP),?
SOC GPIO table fill GPI. PAD_CFG_GPI(GPP_A7, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/462a6ebd_11e6dc6a PS10, Line 49: /* B3 : PROC_GP2 ==> GPP_B3 */
Should PAD_NC_LOCK(GPP_B3, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_B3, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_B3, NONE, LOCK_CONFIG)
https://review.coreboot.org/c/coreboot/+/71706/comment/67526b9d_06ae292b PS10, Line 92: PAD_CFG_GPI_LOCK(GPP_D5, NONE, LEVEL, NONE, LOCK_CONFIG),
Should PAD_NC(GPP_D5, NONE), or PAD_CFG_GPI(GPP_D5, NONE, DEEP),?
PAD_CFG_GPI(GPP_D5, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/1c921a6d_55cdbd6c PS10, Line 96: PAD_CFG_GPI(GPP_D8, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC(GPP_D8, NONE), or PAD_CFG_GPI(GPP_D8, NONE, DEEP),?
PAD_CFG_GPI(GPP_D8, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/324345ad_65ee75c7 PS10, Line 104: PAD_CFG_GPI(GPP_D13, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC_LOCK(GPP_D13, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_D13, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_D13, NONE, LOCK_CONFIG)
https://review.coreboot.org/c/coreboot/+/71706/comment/e700ac14_87406de2 PS10, Line 105: /* D14 : ISH_UART0_TXD ==> NC */ : PAD_NC_LOCK(GPP_D14, NONE, LOCK_CONFIG),
Could you check GPP_D14 the same with baseboard or not?
No,SOC GPIO table fill disable.
https://review.coreboot.org/c/coreboot/+/71706/comment/1db2fb51_34febdfa PS10, Line 108: PAD_CFG_GPI(GPP_D15, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC_LOCK(GPP_D15, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_D15, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_D15, NONE, LOCK_CONFIG)
https://review.coreboot.org/c/coreboot/+/71706/comment/994c44e5_ea36175e PS10, Line 110: PAD_CFG_GPI(GPP_D16, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC_LOCK(GPP_D16, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_D16, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_D16, NONE, LOCK_CONFIG)
https://review.coreboot.org/c/coreboot/+/71706/comment/05ba0fdc_f7edd49b PS10, Line 112: PAD_CFG_GPI(GPP_D17, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC(GPP_D17, NONE), or PAD_CFG_GPI(GPP_D17, NONE, DEEP),?
PAD_CFG_GPI(GPP_D17, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/854f8b02_0700ba89 PS10, Line 122: PAD_CFG_GPI(GPP_E3, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC(GPP_E3, NONE), or PAD_CFG_GPI(GPP_E3, NONE, DEEP),?
PAD_CFG_GPI(GPP_E3, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/c515b6bb_668948c6 PS10, Line 128: PAD_CFG_GPI(GPP_E7, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC(GPP_E7, NONE), or PAD_CFG_GPI(GPP_E7, NONE, DEEP),?
PAD_CFG_GPI(GPP_E7, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/64e78aa9_c0ed75b3 PS10, Line 132: PAD_CFG_GPI_LOCK(GPP_E10, NONE, LEVEL, NONE, LOCK_CONFIG),
Should PAD_NC_LOCK(GPP_E10, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_E10, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_E10, NONE, LOCK_CONFIG)
https://review.coreboot.org/c/coreboot/+/71706/comment/201b5a56_4ae29e80 PS10, Line 141: PAD_CFG_GPI_LOCK(GPP_E17, NONE, LEVEL, NONE, LOCK_CONFIG),
Should PAD_NC_LOCK(GPP_E17, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_E17, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_E17, NONE, LOCK_CONFIG)
https://review.coreboot.org/c/coreboot/+/71706/comment/89282edf_f1cf68d9 PS10, Line 145: PAD_CFG_GPI(GPP_E19, NONE, PLTRST, LEVEL, INVERT),
Should PAD_NC(GPP_E19, NONE), or PAD_CFG_GPI(GPP_E19, NONE, DEEP),?
PAD_CFG_GPI(GPP_E19, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/5a854dfc_6dd50333 PS10, Line 180: PAD_CFG_GPI_LOCK(GPP_F21, NONE, LEVEL, NONE, LOCK_CONFIG),
Should PAD_NC(GPP_F21, NONE), or PAD_CFG_GPI(GPP_F21, NONE, DEEP),?
PAD_CFG_GPI(GPP_F21, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/472202e4_335ccd6a PS10, Line 205: PAD_CFG_GPI_LOCK(GPP_H12, NONE, LOCK_CONFIG),
Should PAD_NC_LOCK(GPP_H12, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_H12, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_H12, NONE, LOCK_CONFIG)
https://review.coreboot.org/c/coreboot/+/71706/comment/7c6fc6ad_3170519e PS10, Line 213: /* H19 : SRCCLKREQ4# ==> GPP_H19 */
Should PAD_NC(GPP_H19, NONE),or PAD_CFG_GPI(GPP_H19, NONE, DEEP),?
PAD_CFG_GPI(GPP_H19, NONE, DEEP)
https://review.coreboot.org/c/coreboot/+/71706/comment/bf0746a6_64f2aa30 PS10, Line 220: PAD_CFG_GPI_LOCK(GPP_H23, NONE, LOCK_CONFIG),
Should PAD_NC_LOCK(GPP_H23, NONE, LOCK_CONFIG), or PAD_CFG_GPI_LOCK(GPP_H23, NONE, LOCK_CONFIG),?
PAD_CFG_GPI_LOCK(GPP_H23, NONE, LOCK_CONFIG)