[coreboot-gerrit] Change in coreboot[master]: google/gru: Stop mucking with unused I2S0 pins in codec config

[Julius Werner (Code Review)]

Hello David Schneider, Philip Chen,

I'd like you to do a code review. Please visit

    https://review.coreboot.org/22791

to review the following change.


Change subject: google/gru: Stop mucking with unused I2S0 pins in codec config
......................................................................

google/gru: Stop mucking with unused I2S0 pins in codec config

Due to a schematic error, our code was written to configure more I2S0
pins than are actually used. We're also pinmuxing the whole bank of pins
over to the I2S controller even though we don't need them all. Restrict
the GPIO initialization and pinmuxing to the pins we really need so the
other ones can be correctly used as SKU ID pins on Scarlet.

Also, move the "audio" IO voltage domain selection to the other such
selections in the bootblock, since that covers two whole banks of GPIOs
and there's no guarantee that they're all used for audio (and thus not
needed before ramstage).

BUG=b:69373077
TEST=Booted Scarlet, confirmed correct SKU ID (7) was detected on rev2.

Change-Id: I9314617e725fe83d254984529f269d4442e736f1
Signed-off-by: Julius Werner <jwerner at chromium.org>
---
M src/mainboard/google/gru/bootblock.c
M src/mainboard/google/gru/mainboard.c
M src/soc/rockchip/rk3399/include/soc/grf.h
3 files changed, 7 insertions(+), 23 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/22791/1

diff --git a/src/mainboard/google/gru/bootblock.c b/src/mainboard/google/gru/bootblock.c
index a18a61c..0013414 100644
--- a/src/mainboard/google/gru/bootblock.c
+++ b/src/mainboard/google/gru/bootblock.c
@@ -31,20 +31,10 @@
 
 void bootblock_mainboard_early_init(void)
 {
-	/* Let gpio2ab io domains works at 1.8V.
-	 *
-	 * If io_vsel[0] == 0(default value), gpio2ab io domains is 3.0V
-	 * powerd by APIO2_VDD, otherwise, 1.8V supplied by APIO2_VDDPST.
-	 * But from the schematic of kevin rev0, the APIO2_VDD and
-	 * APIO2_VDDPST both are 1.8V(intentionally?).
-	 *
-	 * So, by default, CPU1_SDIO_PWREN(GPIO2_A2) can't output 3.0V
-	 * because the supply is 1.8V.
-	 * Let ask GPIO2_A2 output 1.8V to make GPIO interal logic happy.
-	 */
-	write32(&rk3399_grf->io_vsel, RK_SETBITS(1 << 0));
-
-	/* Scarlet-based gpio4cd iodomain is 1.8V */
+	/* Configure all programmable IO voltage domains (3D/4A and 2A/2B) early
+	   so that we know we can use our GPIOs reliably in following code. */
+	write32(&rk3399_grf->io_vsel, RK_SETBITS(1 << 1 | 1 << 0));
+	/* On Scarlet-based boards, the 4C/4D domain is 1.8V (on others 3.0V) */
 	if (IS_ENABLED(CONFIG_GRU_BASEBOARD_SCARLET))
 		write32(&rk3399_grf->io_vsel, RK_SETBITS(1 << 3));
 
diff --git a/src/mainboard/google/gru/mainboard.c b/src/mainboard/google/gru/mainboard.c
index 9a241f5..1a24862 100644
--- a/src/mainboard/google/gru/mainboard.c
+++ b/src/mainboard/google/gru/mainboard.c
@@ -218,17 +218,12 @@
 	gpio_input(GPIO(3, D, 1));	/* I2S0_RX remove pull-up */
 	gpio_input(GPIO(3, D, 2));	/* I2S0_TX remove pull-up */
 	gpio_input(GPIO(3, D, 3));	/* I2S0_SDI0 remove pull-up */
-	gpio_input(GPIO(3, D, 4));	/* I2S0_SDI1 remove pull-up */
-	/* GPIO3_D5 (I2S0_SDI2SDO2) not connected */
-	gpio_input(GPIO(3, D, 6));	/* I2S0_SDO1 remove pull-up */
+	/* GPIOs 3_D4 - 3_D6 not used for I2S and are SKU ID pins on Scarlet. */
 	gpio_input(GPIO(3, D, 7));	/* I2S0_SDO0 remove pull-up */
 	gpio_input(GPIO(4, A, 0));	/* I2S0_MCLK remove pull-up */
 
-	write32(&rk3399_grf->iomux_i2s0, IOMUX_I2S0);
+	write32(&rk3399_grf->iomux_i2s0, IOMUX_I2S0_SD0);
 	write32(&rk3399_grf->iomux_i2sclk, IOMUX_I2SCLK);
-
-	/* AUDIO IO domain 1.8V voltage selection */
-	write32(&rk3399_grf->io_vsel, RK_SETBITS(1 << 1));
 
 	if (!IS_ENABLED(CONFIG_GRU_BASEBOARD_SCARLET))
 		gpio_output(GPIO_P18V_AUDIO_PWREN, 1);
diff --git a/src/soc/rockchip/rk3399/include/soc/grf.h b/src/soc/rockchip/rk3399/include/soc/grf.h
index 8e12007..9bda967 100644
--- a/src/soc/rockchip/rk3399/include/soc/grf.h
+++ b/src/soc/rockchip/rk3399/include/soc/grf.h
@@ -356,8 +356,7 @@
 #define IOMUX_I2C0_SCL	RK_CLRSETBITS(3 << 0, 2 << 0)
 #define IOMUX_I2C0_SDA	RK_CLRSETBITS(3 << 14, 2 << 14)
 
-#define IOMUX_I2S0	RK_SETBITS(1 << 14 | 1 << 12 | 1 << 10 | 1 << 8 |\
-				   1 << 6 | 1 << 4 | 1 << 2 | 1 << 0)
+#define IOMUX_I2S0_SD0	RK_SETBITS(1 << 14 | 1 << 6 | 1 << 4 | 1 << 2 | 1 << 0)
 #define IOMUX_I2SCLK	RK_SETBITS(1 << 0)
 
 #define IOMUX_PWM_0	RK_SETBITS(1 << 4)

-- 
To view, visit https://review.coreboot.org/22791
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9314617e725fe83d254984529f269d4442e736f1
Gerrit-Change-Number: 22791
Gerrit-PatchSet: 1
Gerrit-Owner: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: David Schneider <dnschneid at chromium.org>
Gerrit-Reviewer: Philip Chen <philipchen at chromium.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20171209/11ff1f57/attachment-0001.html>