Nick Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44632 )
Change subject: mb/google/volteer/variants/eldrid: add memory.c for ddr4 support ......................................................................
Patch Set 9:
(23 comments)
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/gpio.c:
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 22: PAD_NC(GPP_A18, NONE),
This is not needed, it is already defined as a PAD_NC(GPP_A18, NONE) in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 30: PAD_NC(GPP_A22, NONE),
Not needed, already defined as PAD_NC in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 37: PAD_NC(GPP_B3, NONE), : /* B5 : ISH_I2C0_CVF_SDA ==> NC */ : PAD_NC(GPP_B5, NONE), : /* B6 : ISH_I2C0_CVF_SCL ==> NC */ : PAD_NC(GPP_B6, NONE), :
Not needed, already defined as PAD_NC in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 61: /* C7 : SML1DATA ==> NC */ : PAD_NC(GPP_C7, NONE),
Not needed, already defined as PAD_NC in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 65: /* C13 : UART1_TXD ==> NC */ : PAD_NC(GPP_C13, NONE),
Not needed, already defined as PAD_NC in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 83: /* D1 : ISH_GP1 ==> NC */ : PAD_NC(GPP_D1, NONE), : /* D2 : ISH_GP2 ==> NC */ : PAD_NC(GPP_D2, NONE), : /* D3 : ISH_GP3 ==> NC */ : PAD_NC(GPP_D3, NONE), : /* D4 : IMGCLKOUT0 ==> NC */ : PAD_NC(GPP_D4, NONE),
Not needed, already defined as PAD_NC in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 99: /* D14 : ISH_UART0_TXD ==> NC */ : PAD_NC(GPP_D14, NONE)
Not needed, already defined as PAD_NC in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 105: /* D18 : ISH_GP5 ==> NC */ : PAD_NC(GPP_D18, NONE),
Not needed, already defined as PAD_NC in baseboard's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 109: PAD_CFG_GPO(GPP_E0, 1, DEEP),
Not needed - this is already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 110: /* E1 : SPI1_IO2 ==> NC */ : PAD_NC(GPP_E1, NONE),
Not needed - this is already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 125: PAD_CFG_GPO(GPP_E10, 1, DEEP)
Do you want PAD_CFG_NF(GPP_E10, NONE, DEEP, NF6) here?
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 129: PAD_CFG_GPO(GPP_E13, 1, DEEP),
Do you want PAD_CFG_NF(GPP_E10, NONE, DEEP, NF6) here?
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 132: /* E16 : ISH_GP7 ==> NC */ : PAD_NC(GPP_E16, NONE), : /* E17 : THC0_SPI1_INT# ==> NC */ : PAD_NC(GPP_E17, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 145: /* F6 : CNV_PA_BLANKING ==> NC */ : PAD_NC(GPP_F6, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 159: /* F14 : GSXDIN ==> NC */ : PAD_NC(GPP_F14, NONE), : /* F15 : GSXSRESET# ==> NC */ : PAD_NC(GPP_F15, NONE), : /* F16 : GSXCLK ==> NC */ : PAD_NC(GPP_F16, NONE), : /* F17 : THC1_SPI2_RST# ==> NC */ : PAD_NC(GPP_F17, NONE), : /* F18 : THC1_SPI2_INT# ==> NC */ : PAD_NC(GPP_F18, NONE), : /* F19 : SRCCLKREQ6# ==> NC */ : PAD_NC(GPP_F19, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 184: /* H6 : I2C3_SDA ==> NC */ : PAD_NC(GPP_H6, NONE), : /* H7 : I2C3_SCL ==> NC */ : PAD_NC(GPP_H7, NONE), : /* H8 : I2C4_SDA ==> NC */ : PAD_NC(GPP_H8, NONE), : /* H9 : I2C4_SCL ==> NC */ : PAD_NC(GPP_H9, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 196: /* H12 : M2_SKT2_CFG0 ==> NC */ : PAD_NC(GPP_H12, NONE), : /* H13 : M2_SKT2_CFG1 # ==> NC */ : PAD_NC(GPP_H13, NONE), : /* H14 : M2_SKT2_CFG2 # ==> NC */ : PAD_NC(GPP_H14, NONE), : /* H15 : M2_SKT2_CFG3 # ==> NC */ : PAD_NC(GPP_H15, NONE), : /* H16 : DDPB_CTRLCLK ==> NC */ : PAD_NC(GPP_H16, NONE), : /* H17 : DDPB_CTRLDATA ==> NC */ : PAD_NC(GPP_H17, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 208: /* H19 : TIME_SYNC0 ==> NC */ : PAD_NC(GPP_H19, NONE), : /* H20 : IMGCLKOUT1 ==> NC */ : PAD_NC(GPP_H20, NONE), : /* H21 : IMGCLKOUT2 ==> NC */ : PAD_NC(GPP_H21, NONE), : /* H22 : IMGCLKOUT3 ==> NC */ : PAD_NC(GPP_H22, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 225: /* R5 : HDA_SDI1 ==> NC */ : PAD_NC(GPP_R5, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 238: /* S3 : SNDW1_DATA ==> NC */ : PAD_NC(GPP_S3, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/3/src/mainboard/google/voltee... PS3, Line 244: /* S6 : SNDW3_CLK ==> NC */ : PAD_NC(GPP_S6, NONE), : /* S7 : SNDW3_DATA ==> NC */ : PAD_NC(GPP_S7, NONE),
Not needed, already defined this way in baseboard gpio.c's gpio_table[].
Done
https://review.coreboot.org/c/coreboot/+/44632/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/memory.c:
https://review.coreboot.org/c/coreboot/+/44632/4/src/mainboard/google/voltee... PS4, Line 7: static const struct mb_ddr4_cfg eldrid_memcfg = {
Please add a comment here that states "This mb_ddr4_cfg structure is intentially left empty so that […]
Done
https://review.coreboot.org/c/coreboot/+/44632/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/eldrid/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/44632/4/src/mainboard/google/voltee... PS4, Line 69: max98373
What is the speaker Amp on Eldrid? […]
remove at latest patch