Evan Green has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45458 )
Change subject: mb/google/dedede: Enable I2C5 pads ......................................................................
mb/google/dedede: Enable I2C5 pads
I2C5 is now active on Waddledee variants for the WFC. Enable its pin configuration.
BUG=b:162024182 TEST=build and boot
Signed-off-by: Evan Green evgreen@chromium.org Change-Id: I7d28b4d9e4d1cf5b8d7feb44327410288634f229 --- M src/mainboard/google/dedede/variants/baseboard/gpio.c 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/45458/1
diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index d6a2d61..19b5c91 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -185,9 +185,9 @@ /* D21 : WWAN_WLAN_COEX3 */ PAD_CFG_NF(GPP_D21, NONE, DEEP, NF1), /* D22 : AP_I2C_SUB_SDA*/ - PAD_NC(GPP_D22, NONE), + PAD_CFG_NF(GPP_D22, NONE, DEEP, NF1), /* D23 : AP_I2C_SUB_SCL */ - PAD_NC(GPP_D23, NONE), + PAD_CFG_NF(GPP_D23, NONE, DEEP, NF1),
/* E0 : CLK_24M_UCAM */ PAD_CFG_NF(GPP_E0, NONE, DEEP, NF2),
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45458 )
Change subject: mb/google/dedede: Enable I2C5 pads ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45458/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45458/1//COMMIT_MSG@9 PS1, Line 9: Waddledee Nit: Waddledee -> Dedede
https://review.coreboot.org/c/coreboot/+/45458/1//COMMIT_MSG@13 PS1, Line 13: build and boot Nit: Build and boot Waddledee.
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... PS1, Line 187: AP_I2C_SUB_SDA Nit: AP_I2C_SUB_SDA/AP_I2C_CAM_SDA. Same for SCL.
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45458 )
Change subject: mb/google/dedede: Enable I2C5 pads ......................................................................
Patch Set 1:
(2 comments)
I just learned from Will that I2C5 may not be the final plan. Maybe I should just shelve this change until there's a non-reference board that actually needs it.
https://review.coreboot.org/c/coreboot/+/45458/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45458/1//COMMIT_MSG@9 PS1, Line 9: Waddledee
Nit: Waddledee -> Dedede
Wait did Doo change too? The sentence was trying to convey that one specific variant needs it (even though we're turning it on generically).
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... PS1, Line 187: AP_I2C_SUB_SDA
Nit: AP_I2C_SUB_SDA/AP_I2C_CAM_SDA. Same for SCL.
Will told me that in the event they do adopt I2C5 as the final solution, they likely wouldn't rename the nets in the schematic. Do you still want me to rename?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45458 )
Change subject: mb/google/dedede: Enable I2C5 pads ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... PS1, Line 187: AP_I2C_SUB_SDA
Will told me that in the event they do adopt I2C5 as the final solution, they likely wouldn't rename […]
In the event we are not sure about the plan for other variants, please move this configuration to variants/waddledee/gpio.c. The net name can be updated there, so that we know the purpose of these GPIOs.
We can configure the GPIOs on other variants as and when needed.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45458 )
Change subject: mb/google/dedede: Enable I2C5 pads ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45458/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45458/1//COMMIT_MSG@9 PS1, Line 9: Waddledee
Wait did Doo change too? The sentence was trying to convey that one specific variant needs it (even […]
Yes, since you changed in baseboard it applies to all the variants.
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/45458/1/src/mainboard/google/dedede... PS1, Line 187: AP_I2C_SUB_SDA
In the event we are not sure about the plan for other variants, please move this configuration to va […]
Boten wants to use I2C5 for Proximity Sensor - https://review.coreboot.org/c/coreboot/+/43478.
Given that we are using I2C5 in select variants, we can move it to override GPIO config in variants. Also we can add a comment stating the purpose of this GPIO.
Evan Green has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45458 )
Change subject: mb/google/dedede: Enable I2C5 pads ......................................................................
Abandoned
Hardware guys are going a different direction than i2c5.
-
Evan Green (Code Review) -
Karthik Ramasubramanian (Code Review)