Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35175 )
Change subject: mb/google/drallion: Update gpio config for drallion ......................................................................
Patch Set 17:
(14 comments)
In the future please resolve comments as they are fixed. There is one left that was not done can you address it?
https://review.coreboot.org/c/coreboot/+/35175/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35175/4//COMMIT_MSG@8 PS4, Line 8:
Please add B:139370304 here.
Done
https://review.coreboot.org/c/coreboot/+/35175/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35175/11//COMMIT_MSG@8 PS11, Line 8:
From what source (datasheet, …)?
Done
https://review.coreboot.org/c/coreboot/+/35175/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35175/13//COMMIT_MSG@11 PS13, Line 11: BUG=b:139370304
BUG should go before the signed-of-by
Done
https://review.coreboot.org/c/coreboot/+/35175/13//COMMIT_MSG@12 PS13, Line 12: Source: Pin Schematics
This should be after the first line
Done
https://review.coreboot.org/c/coreboot/+/35175/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35175/4/src/mainboard/google/dralli... PS4, Line 358: device pci 19.2 off end # UART #2
I think we should align this.
Done
https://review.coreboot.org/c/coreboot/+/35175/4/src/mainboard/google/dralli... PS4, Line 377: device pci 1e.0 on end # UART #0
same above.
Done
https://review.coreboot.org/c/coreboot/+/35175/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35175/4/src/mainboard/google/dralli... PS4, Line 90: /* UART0_TXD */ PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), /* SERVOTX_UART */
We did this in early table, should we init again?
Please fix or resolve with a comment
https://review.coreboot.org/c/coreboot/+/35175/6/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35175/6/src/mainboard/google/dralli... PS6, Line 91: /* UART0_RTS# */ PAD_CFG_GPI(GPP_C10, NONE, PLTRST),
This should be GPO, WWAN_FULL_PWR_EN default high.
Done
https://review.coreboot.org/c/coreboot/+/35175/6/src/mainboard/google/dralli... PS6, Line 103: /* UART2_RTS# */ PAD_CFG_GPI(GPP_C22, NONE, PLTRST),
/* SPK detect */
Done
https://review.coreboot.org/c/coreboot/+/35175/6/src/mainboard/google/dralli... PS6, Line 157: /* DDPE_HPD3 */ PAD_NC(GPP_E16, NONE),
HDMI_PD#, GPO default high
Done
https://review.coreboot.org/c/coreboot/+/35175/9/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35175/9/src/mainboard/google/dralli... PS9, Line 166: /* GPP_F1 */ PAD_CFG_GPI(GPP_F1, NONE, DEEP), /* DDR_CHA_EN_1P8 */
We should put this into early_gpio_table. F1/F2 based on Duncan's comment.https://review.coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/35175/9/src/mainboard/google/dralli... PS9, Line 177: /* EMMC_DATA0 */ PAD_CFG_GPI(GPP_F12, NONE, DEEP), /* MEM_CONFIGO_1P8 */
We should put this into early_gpio_table. See here:https://review.coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/35175/11/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35175/11/src/mainboard/google/drall... PS11, Line 156: /* DDPE_HPD3 */ PAD_CFG_GPO(GPP_E16, 0, PLTRST), /* HDMI_PD# */
HW suggest default high.
Done
https://review.coreboot.org/c/coreboot/+/35175/13/src/mainboard/google/drall... File src/mainboard/google/drallion/variants/drallion/gpio.c:
https://review.coreboot.org/c/coreboot/+/35175/13/src/mainboard/google/drall... PS13, Line 196: /* I2C2_SCL */ PAD_CFG_GPI(GPP_H5, NONE, PLTRST), /* 360_SENSOR_DET# */
We may should put this in early table as well.
Done