Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46437 )
Change subject: mb/google/volteer: New variant for Volteer reworked with Dauntless ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46437/10/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/volteer2/gpio.c:
https://review.coreboot.org/c/coreboot/+/46437/10/src/mainboard/google/volte... PS10, Line 249: UP_2K
That was how it was in the code that I copied from, "reef", I think: […]
Can you please check the schematics to confirm if there are external pulls on these lines? If yes, then we should not be adding the internal pulls here. Let's not copy paste code from old boards without evaluating their impact/applicability on the new boards.
https://review.coreboot.org/c/coreboot/+/46437/10/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/volteer2/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/46437/10/src/mainboard/google/volte... PS10, Line 23: register "common_soc_config.gspi[0].early_init" = "CONFIG(MAINBOARD_HAS_SPI_TPM_CR50)" : register "common_soc_config.i2c[1].early_init" = "CONFIG(MAINBOARD_HAS_I2C_TPM_CR50)"
I had not. Looking at static. […]
Adding the entire common_soc_config to overridetree should be fine. It anyways requires per variant configuration for i2c rise/fall times. Instead of adding code in baseboard/devicetree.cb I think it is better to just make a copy of the common_soc_config in overridetree. That is already being done for eldrid and volteer variant.