snow: re-factor i2c init

This re-factors i2c initialization to simplify it and make it follow
the correct order. This is intended to fix a bug where the I2C lines
could be driven low for no good reason on EC startup, potentially
causing issues with other devices.

The ordering should be:
1. Setup pins as inputs on EC startup.
2. Initialize I2C module(s)
3. Re-configure pins as alternate function.

(Thanks to dianders for pointing out this bug)

Signed-off-by: David Hendricks <dhendrix@chromium.org>
BRANCH=snow
BUG=chrome-os-partner:13443
TEST=Tested by examining scope traces during EC reboot

Change-Id: Ibb845f3fd538da387132b1c822929f8613de077d
Reviewed-on: https://gerrit.chromium.org/gerrit/31966
Reviewed-by: David Hendricks <dhendrix@chromium.org>
Tested-by: David Hendricks <dhendrix@chromium.org>

3 files changed, 43 insertions, 15 deletions

diff --git a/board/snow/board.c b/board/snow/board.c
index 672df1002b..c2e29e879c 100644
--- a/board/snow/board.c
+++ b/board/snow/board.c
@@ -60,7 +60,14 @@ const struct gpio_info gpio_list[GPIO_COUNT] = {
 	/* Other inputs */
 	{"AC_PWRBTN_L", GPIO_A, (1<<0), GPIO_INT_BOTH, NULL},
 	{"SPI1_NSS",    GPIO_A, (1<<4), GPIO_DEFAULT, NULL},
-
+	/*
+	 * I2C pins should be configured as inputs until I2C module is
+	 * initialized. This will avoid driving the lines unintentionally.
+	 */
+	{"I2C1_SCL",    GPIO_B, (1<<6),  GPIO_INPUT, NULL},
+	{"I2C1_SDA",    GPIO_B, (1<<7),  GPIO_INPUT, NULL},
+	{"I2C2_SCL",    GPIO_B, (1<<10), GPIO_INPUT, NULL},
+	{"I2C2_SDA",    GPIO_B, (1<<11), GPIO_INPUT, NULL},
 	/* Outputs */
 	{"AC_STATUS",   GPIO_A, (1<<5), GPIO_DEFAULT, NULL},
 	{"SPI1_MISO",   GPIO_A, (1<<6), GPIO_DEFAULT, NULL},
@@ -115,20 +122,6 @@ void configure_board(void)
 			       | (1 << 8);
 
 	/*
-	 * I2C SCL/SDA on PB10-11 and PB6-7, bi-directional, no pull-up/down,
-	 * initialized as hi-Z until alt. function is set
-	 */
-	val = STM32_GPIO_CRH_OFF(GPIO_B) & ~0x0000ff00;
-	val |= 0x0000dd00;
-	STM32_GPIO_CRH_OFF(GPIO_B) = val;
-
-	val = STM32_GPIO_CRL_OFF(GPIO_B) & ~0xff000000;
-	val |= 0xdd000000;
-	STM32_GPIO_CRL_OFF(GPIO_B) = val;
-
-	STM32_GPIO_BSRR_OFF(GPIO_B) |= (1<<11) | (1<<10) | (1<<7) | (1<<6);
-
-	/*
 	 * Set alternate function for USART1. For alt. function input
 	 * the port is configured in either floating or pull-up/down
 	 * input mode (ref. section 7.1.4 in datasheet RM0041):
@@ -149,6 +142,25 @@ void configure_board(void)
 	gpio_set_level(GPIO_EC_INT, 1);
 }
 
+/* GPIO configuration to be done after I2C module init */
+void board_i2c_post_init(int port)
+{
+	uint32_t val;
+
+	/* enable alt. function (open-drain) */
+	if (port == STM32_I2C1_PORT) {
+		/* I2C1 is on PB6-7 */
+		val = STM32_GPIO_CRL_OFF(GPIO_B) & ~0xff000000;
+		val |= 0xdd000000;
+		STM32_GPIO_CRL_OFF(GPIO_B) = val;
+	} else if (port == STM32_I2C2_PORT) {
+		/* I2C2 is on PB10-11 */
+		val = STM32_GPIO_CRH_OFF(GPIO_B) & ~0x0000ff00;
+		val |= 0x0000dd00;
+		STM32_GPIO_CRH_OFF(GPIO_B) = val;
+	}
+}
+
 void configure_board_late(void)
 {
 #ifdef CONFIG_AC_POWER_STATUS
diff --git a/board/snow/board.h b/board/snow/board.h
index 79d56ecb5b..a98d3802cb 100644
--- a/board/snow/board.h
+++ b/board/snow/board.h
@@ -84,6 +84,10 @@ enum gpio_signal {
 	/* Other inputs */
 	GPIO_AC_PWRBTN_L,
 	GPIO_SPI1_NSS,
+	GPIO_I2C1_SCL,
+	GPIO_I2C1_SDA,
+	GPIO_I2C2_SCL,
+	GPIO_I2C2_SDA,
 	/* Outputs */
 	GPIO_AC_STATUS,
 	GPIO_SPI1_MISO,
diff --git a/chip/stm32/i2c.c b/chip/stm32/i2c.c
index 42b6da1be4..7068b9bce5 100644
--- a/chip/stm32/i2c.c
+++ b/chip/stm32/i2c.c
@@ -352,6 +352,14 @@ static void i2c_error_handler(int port)
 static void i2c2_error_interrupt(void) { i2c_error_handler(I2C2); }
 DECLARE_IRQ(STM32_IRQ_I2C2_ER, i2c2_error_interrupt, 2);
 
+/* board-specific setup for post-I2C module init */
+void __board_i2c_post_init(int port)
+{
+}
+
+void board_i2c_post_init(int port)
+		__attribute__((weak, alias("__board_i2c_post_init")));
+
 static int i2c_init2(void)
 {
 	/* enable I2C2 clock */
@@ -381,6 +389,8 @@ static int i2c_init2(void)
 	task_enable_irq(STM32_IRQ_I2C2_EV);
 	task_enable_irq(STM32_IRQ_I2C2_ER);
 
+	board_i2c_post_init(I2C2);
+
 	CPUTS("done\n");
 	return EC_SUCCESS;
 }
@@ -411,6 +421,8 @@ static int i2c_init1(void)
 	task_enable_irq(STM32_IRQ_I2C1_EV);
 	task_enable_irq(STM32_IRQ_I2C1_ER);
 
+	board_i2c_post_init(I2C1);
+
 	return EC_SUCCESS;
 
 }