Clean up I2C module

No functional changes, just cleanup

BUG=chrome-os-partner:15579
BRANCH=none
TEST=i2cscan; should find all expected devices

Change-Id: I8e11d3fa460236e80a0ce1ee923e4413b3202c1e
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/36569
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

2 files changed, 149 insertions, 150 deletions

diff --git a/chip/lm4/i2c.c b/chip/lm4/i2c.c
index de04bcf727..0fdefc15d9 100644
--- a/chip/lm4/i2c.c
+++ b/chip/lm4/i2c.c
@@ -37,7 +37,12 @@ static task_id_t task_waiting_on_port[NUM_PORTS];
 static struct mutex port_mutex[NUM_PORTS];
 extern const struct i2c_port_t i2c_ports[I2C_PORTS_USED];
 
-
+/**
+ * Wait for port to go idle
+ *
+ * @param port		Port to check
+ * @return EC_SUCCESS if port is idle; non-zero if error.
+ */
 static int wait_idle(int port)
 {
 	int i;
@@ -48,7 +53,8 @@ static int wait_idle(int port)
 		/* Port is busy, so wait for the interrupt */
 		task_waiting_on_port[port] = task_get_current();
 		LM4_I2C_MIMR(port) = 0x03;
-		/* We want to wait here quietly until the I2C interrupt comes
+		/*
+		 * We want to wait here quietly until the I2C interrupt comes
 		 * along, but we don't want to lose any pending events that
 		 * will be needed by the task that started the I2C transaction
 		 * in the first place. So we save them up and restore them when
@@ -68,7 +74,8 @@ static int wait_idle(int port)
 		i = LM4_I2C_MCS(port);
 	}
 
-	/* Restore any events that we saw while waiting. TASK_EVENT_TIMER isn't
+	/*
+	 * Restore any events that we saw while waiting. TASK_EVENT_TIMER isn't
 	 * one, because we've handled it above.
 	 */
 	task_set_event(task_get_current(), event, 0);
@@ -80,28 +87,35 @@ static int wait_idle(int port)
 	return EC_SUCCESS;
 }
 
-
-/* Transmit one block of raw data, then receive one block of raw data.
- * <start> flag indicates this smbus session start from idle state.
- * <stop>  flag means this session can be termicate with smbus stop bit
+/**
+ * Transmit one block of raw data, then receive one block of raw data.
+ *
+ * @param port		Port to access
+ * @param slave_addr	Slave device address
+ * @param out		Data to send
+ * @param out_size	Number of bytes to send
+ * @param in		Destination buffer for received data
+ * @param in_size	Number of bytes to receive
+ * @param start		Did smbus session started from idle state?
+ * @param stop		Can session be terminated with smbus stop bit?
+ * @return EC_SUCCESS, or non-zero if error.
  */
-static int i2c_transmit_receive(int port, int slave_addr,
-		uint8_t *transmit_data, int transmit_size,
-		uint8_t *receive_data, int receive_size,
-		int start, int stop)
+static int i2c_xfer(int port, int slave_addr, const uint8_t *out, int out_size,
+		    uint8_t *in, int in_size, int start, int stop)
 {
 	int rv, i;
 	int started = start ? 0 : 1;
 	uint32_t reg_mcs;
 
-	if (transmit_size == 0 && receive_size == 0)
+	if (out_size == 0 && in_size == 0)
 		return EC_SUCCESS;
 
-	if (transmit_data) {
+	if (out) {
 		LM4_I2C_MSA(port) = slave_addr & 0xff;
-		for (i = 0; i < transmit_size; i++) {
-			LM4_I2C_MDR(port) = transmit_data[i];
-			/* Setup master control/status register
+		for (i = 0; i < out_size; i++) {
+			LM4_I2C_MDR(port) = out[i];
+			/*
+			 * Set up master control/status register
 			 * MCS sequence on multi-byte write:
 			 *     0x3 0x1 0x1 ... 0x1 0x5
 			 * Single byte write:
@@ -113,12 +127,11 @@ static int i2c_transmit_receive(int port, int slave_addr,
 				started = 1;
 				reg_mcs |= LM4_I2C_MCS_START;
 			}
-			/* Send stop bit if the stop flag is on,
-			 * and caller doesn't expect to receive
-			 * data.
+			/*
+			 * Send stop bit if the stop flag is on, and caller
+			 * doesn't expect to receive data.
 			 */
-			if (stop && receive_size == 0 && i ==
-					(transmit_size - 1))
+			if (stop && in_size == 0 && i == (out_size - 1))
 				reg_mcs |= LM4_I2C_MCS_STOP;
 
 			LM4_I2C_MCS(port) = reg_mcs;
@@ -129,15 +142,16 @@ static int i2c_transmit_receive(int port, int slave_addr,
 		}
 	}
 
-	if (receive_size) {
-		if (transmit_size)
+	if (in_size) {
+		if (out_size)
 			/* resend start bit when change direction */
 			started = 0;
 
 		LM4_I2C_MSA(port) = (slave_addr & 0xff) | 0x01;
-		for (i = 0; i < receive_size; i++) {
-			LM4_I2C_MDR(port) = receive_data[i];
-			/* MCS receive sequence on multi-byte read:
+		for (i = 0; i < in_size; i++) {
+			LM4_I2C_MDR(port) = in[i];
+			/*
+			 * MCS receive sequence on multi-byte read:
 			 *     0xb 0x9 0x9 ... 0x9 0x5
 			 * Single byte read:
 			 *     0x7
@@ -148,7 +162,7 @@ static int i2c_transmit_receive(int port, int slave_addr,
 				reg_mcs |= LM4_I2C_MCS_START;
 			}
 			/* ACK all bytes except the last one */
-			if (stop && i == (receive_size - 1))
+			if (stop && i == (in_size - 1))
 				reg_mcs |= LM4_I2C_MCS_STOP;
 			else
 				reg_mcs |= LM4_I2C_MCS_ACK;
@@ -157,26 +171,22 @@ static int i2c_transmit_receive(int port, int slave_addr,
 			rv = wait_idle(port);
 			if (rv)
 				return rv;
-			receive_data[i] = LM4_I2C_MDR(port) & 0xff;
+			in[i] = LM4_I2C_MDR(port) & 0xff;
 		}
 	}
 
 	return EC_SUCCESS;
 }
 
-
 int i2c_read16(int port, int slave_addr, int offset, int *data)
 {
 	int rv;
 	uint8_t reg, buf[2];
 
 	reg = offset & 0xff;
-	/* I2C read 16-bit word:
-	 * Transmit 8-bit offset, and read 16bits
-	 */
+	/* I2C read 16-bit word: transmit 8-bit offset, and read 16bits */
 	mutex_lock(port_mutex + port);
-	rv = i2c_transmit_receive(port, slave_addr, &reg, 1, buf, 2,
-					START, STOP);
+	rv = i2c_xfer(port, slave_addr, &reg, 1, buf, 2, START, STOP);
 	mutex_unlock(port_mutex + port);
 
 	if (rv)
@@ -190,7 +200,6 @@ int i2c_read16(int port, int slave_addr, int offset, int *data)
 	return EC_SUCCESS;
 }
 
-
 int i2c_write16(int port, int slave_addr, int offset, int data)
 {
 	int rv;
@@ -207,14 +216,12 @@ int i2c_write16(int port, int slave_addr, int offset, int data)
 	}
 
 	mutex_lock(port_mutex + port);
-	rv = i2c_transmit_receive(port, slave_addr, buf, 3, 0, 0,
-					START, STOP);
+	rv = i2c_xfer(port, slave_addr, buf, 3, 0, 0, START, STOP);
 	mutex_unlock(port_mutex + port);
 
 	return rv;
 }
 
-
 int i2c_read8(int port, int slave_addr, int offset, int* data)
 {
 	int rv;
@@ -223,8 +230,7 @@ int i2c_read8(int port, int slave_addr, int offset, int* data)
 	reg = offset;
 
 	mutex_lock(port_mutex + port);
-	rv = i2c_transmit_receive(port, slave_addr, &reg, 1, &val, 1,
-					START, STOP);
+	rv = i2c_xfer(port, slave_addr, &reg, 1, &val, 1, START, STOP);
 	mutex_unlock(port_mutex + port);
 
 	if (!rv)
@@ -233,7 +239,6 @@ int i2c_read8(int port, int slave_addr, int offset, int* data)
 	return rv;
 }
 
-
 int i2c_write8(int port, int slave_addr, int offset, int data)
 {
 	int rv;
@@ -243,16 +248,14 @@ int i2c_write8(int port, int slave_addr, int offset, int data)
 	buf[1] = data;
 
 	mutex_lock(port_mutex + port);
-	rv = i2c_transmit_receive(port, slave_addr, buf, 2, 0, 0,
-					START, STOP);
+	rv = i2c_xfer(port, slave_addr, buf, 2, 0, 0, START, STOP);
 	mutex_unlock(port_mutex + port);
 
 	return rv;
 }
 
-
 int i2c_read_string(int port, int slave_addr, int offset, uint8_t *data,
-	int len)
+		    int len)
 {
 	int rv;
 	uint8_t reg, block_length;
@@ -260,18 +263,20 @@ int i2c_read_string(int port, int slave_addr, int offset, uint8_t *data,
 	mutex_lock(port_mutex + port);
 
 	reg = offset;
-	/* Send device reg space offset, and read back block length.
-	 * Keep this session open without a stop */
-	rv = i2c_transmit_receive(port, slave_addr, &reg, 1, &block_length, 1,
-				  START, NO_STOP);
+	/*
+	 * Send device reg space offset, and read back block length.  Keep this
+	 * session open without a stop.
+	 */
+	rv = i2c_xfer(port, slave_addr, &reg, 1, &block_length, 1,
+		      START, NO_STOP);
 	if (rv)
 		goto exit;
 
 	if (len && block_length > (len - 1))
 		block_length = len - 1;
 
-	rv = i2c_transmit_receive(port, slave_addr, 0, 0, data, block_length,
-				  NO_START, STOP);
+	rv = i2c_xfer(port, slave_addr, 0, 0, data, block_length,
+		      NO_START, STOP);
 	data[block_length] = 0;
 
 exit:
@@ -279,13 +284,41 @@ exit:
 	return rv;
 }
 
+/**
+ * Configure GPIOs for the module.
+ */
+static void configure_gpio(void)
+{
+#ifdef BOARD_link
+	/* PA6:7 = I2C1 SCL/SDA; PB2:3 = I2C0 SCL/SDA; PB6:7 = I2C5 SCL/SDA */
+	gpio_set_alternate_function(LM4_GPIO_A, 0xc0, 3);
+	gpio_set_alternate_function(LM4_GPIO_B, 0xcc, 3);
+
+	/* Configure SDA as open-drain.  SCL should not be open-drain,
+	 * since it has an internal pull-up. */
+	LM4_GPIO_ODR(LM4_GPIO_A) |= 0x80;
+	LM4_GPIO_ODR(LM4_GPIO_B) |= 0x88;
+#else
+	/* PG6:7 = I2C5 SCL/SDA */
+	gpio_set_alternate_function(LM4_GPIO_G, 0xc0, 3);
+
+	/* Configure SDA as open-drain.  SCL should not be open-drain,
+	 * since it has an internal pull-up. */
+	LM4_GPIO_ODR(LM4_GPIO_G) |= 0x80;
+#endif
+}
+
+/*****************************************************************************/
+/* Hooks */
+
 static void i2c_freq_changed(void)
 {
 	int freq = clock_get_freq();
 	int i;
 
 	for (i = 0; i < I2C_PORTS_USED; i++) {
-		/* From datasheet:
+		/*
+		 * From datasheet:
 		 *     SCL_PRD = 2 * (1 + TPR) * (SCL_LP + SCL_HP) * CLK_PRD
 		 *
 		 * so:
@@ -310,10 +343,47 @@ static void i2c_freq_changed(void)
 }
 DECLARE_HOOK(HOOK_FREQ_CHANGE, i2c_freq_changed, HOOK_PRIO_DEFAULT + 1);
 
-/*****************************************************************************/
-/* Interrupt handlers */
+static void i2c_init(void)
+{
+	uint32_t mask = 0;
+	int i;
+
+	/* Enable I2C modules and delay a few clocks */
+	for (i = 0; i < I2C_PORTS_USED; i++)
+		mask |= 1 << i2c_ports[i].port;
+
+	LM4_SYSTEM_RCGCI2C |= mask;
+	clock_wait_cycles(3);
+
+	/* Configure GPIOs */
+	configure_gpio();
+
+	/* No tasks are waiting on ports */
+	for (i = 0; i < NUM_PORTS; i++)
+		task_waiting_on_port[i] = TASK_ID_INVALID;
+
+	/* Initialize ports as master, with interrupts enabled */
+	for (i = 0; i < I2C_PORTS_USED; i++)
+		LM4_I2C_MCR(i2c_ports[i].port) = 0x10;
+
+	/* Set initial clock frequency */
+	i2c_freq_changed();
+
+	/* Enable irqs */
+	task_enable_irq(LM4_IRQ_I2C0);
+	task_enable_irq(LM4_IRQ_I2C1);
+	task_enable_irq(LM4_IRQ_I2C2);
+	task_enable_irq(LM4_IRQ_I2C3);
+	task_enable_irq(LM4_IRQ_I2C4);
+	task_enable_irq(LM4_IRQ_I2C5);
+}
+DECLARE_HOOK(HOOK_INIT, i2c_init, HOOK_PRIO_DEFAULT);
 
-/* Handles an interrupt on the specified port. */
+/**
+ * Handle an interrupt on the specified port.
+ *
+ * @param port		I2C port generating interrupt
+ */
 static void handle_interrupt(int port)
 {
 	int id = task_waiting_on_port[port];
@@ -326,7 +396,6 @@ static void handle_interrupt(int port)
 		task_set_event(id, TASK_EVENT_I2C_IDLE, 0);
 }
 
-
 static void i2c0_interrupt(void) { handle_interrupt(0); }
 static void i2c1_interrupt(void) { handle_interrupt(1); }
 static void i2c2_interrupt(void) { handle_interrupt(2); }
@@ -377,7 +446,6 @@ static void scan_bus(int port, const char *desc)
 	ccputs("\n");
 }
 
-
 static int command_i2cread(int argc, char **argv)
 {
 	int port, addr, count = 1;
@@ -432,7 +500,6 @@ DECLARE_CONSOLE_COMMAND(i2cread, command_i2cread,
 			"Read from I2C",
 			NULL);
 
-
 static int command_scan(int argc, char **argv)
 {
 	int i;
@@ -445,66 +512,3 @@ DECLARE_CONSOLE_COMMAND(i2cscan, command_scan,
 			NULL,
 			"Scan I2C ports for devices",
 			NULL);
-
-
-/*****************************************************************************/
-/* Initialization */
-
-/* Configures GPIOs for the module. */
-static void configure_gpio(void)
-{
-#ifdef BOARD_link
-	/* PA6:7 = I2C1 SCL/SDA; PB2:3 = I2C0 SCL/SDA; PB6:7 = I2C5 SCL/SDA */
-	gpio_set_alternate_function(LM4_GPIO_A, 0xc0, 3);
-	gpio_set_alternate_function(LM4_GPIO_B, 0xcc, 3);
-
-	/* Configure SDA as open-drain.  SCL should not be open-drain,
-	 * since it has an internal pull-up. */
-	LM4_GPIO_ODR(LM4_GPIO_A) |= 0x80;
-	LM4_GPIO_ODR(LM4_GPIO_B) |= 0x88;
-#else
-	/* PG6:7 = I2C5 SCL/SDA */
-	gpio_set_alternate_function(LM4_GPIO_G, 0xc0, 3);
-
-	/* Configure SDA as open-drain.  SCL should not be open-drain,
-	 * since it has an internal pull-up. */
-	LM4_GPIO_ODR(LM4_GPIO_G) |= 0x80;
-#endif
-}
-
-static void i2c_init(void)
-{
-	volatile uint32_t scratch  __attribute__((unused));
-	uint32_t mask = 0;
-	int i;
-
-	/* Enable I2C modules and delay a few clocks */
-	for (i = 0; i < I2C_PORTS_USED; i++)
-		mask |= 1 << i2c_ports[i].port;
-
-	LM4_SYSTEM_RCGCI2C |= mask;
-	scratch = LM4_SYSTEM_RCGCI2C;
-
-	/* Configure GPIOs */
-	configure_gpio();
-
-	/* No tasks are waiting on ports */
-	for (i = 0; i < NUM_PORTS; i++)
-		task_waiting_on_port[i] = TASK_ID_INVALID;
-
-	/* Initialize ports as master, with interrupts enabled */
-	for (i = 0; i < I2C_PORTS_USED; i++)
-		LM4_I2C_MCR(i2c_ports[i].port) = 0x10;
-
-	/* Set initial clock frequency */
-	i2c_freq_changed();
-
-	/* Enable irqs */
-	task_enable_irq(LM4_IRQ_I2C0);
-	task_enable_irq(LM4_IRQ_I2C1);
-	task_enable_irq(LM4_IRQ_I2C2);
-	task_enable_irq(LM4_IRQ_I2C3);
-	task_enable_irq(LM4_IRQ_I2C4);
-	task_enable_irq(LM4_IRQ_I2C5);
-}
-DECLARE_HOOK(HOOK_INIT, i2c_init, HOOK_PRIO_DEFAULT);
diff --git a/chip/stm32/i2c.c b/chip/stm32/i2c.c
index fe8655968c..4c83ad9678 100644
--- a/chip/stm32/i2c.c
+++ b/chip/stm32/i2c.c
@@ -57,7 +57,7 @@
 #define I2C1      STM32_I2C1_PORT
 #define I2C2      STM32_I2C2_PORT
 
-/* select the DMA channels matching the board configuration */
+/* Select the DMA channels matching the board configuration */
 #define DMAC_SLAVE_TX ((I2C_PORT_SLAVE) ? DMAC_I2C2_TX : DMAC_I2C1_TX)
 #define DMAC_SLAVE_RX ((I2C_PORT_SLAVE) ? DMAC_I2C2_RX : DMAC_I2C1_RX)
 #define DMAC_HOST_TX  ((I2C_PORT_HOST) ? DMAC_I2C2_TX : DMAC_I2C1_TX)
@@ -68,8 +68,8 @@ enum {
 	 * A stop condition should take 2 clocks, but the process may need more
 	 * time to notice if it is preempted, so we poll repeatedly for 8
 	 * clocks, before backing off and only check once every
-	 * STOP_SENT_RETRY_US for up to TIMEOUT_STOP_SENT clocks before
-	 * giving up.
+	 * STOP_SENT_RETRY_US for up to TIMEOUT_STOP_SENT clocks before giving
+	 * up.
 	 */
 	SLOW_STOP_SENT_US	= I2C_PERIOD_US * 8,
 	TIMEOUT_STOP_SENT_US	= I2C_PERIOD_US * 200,
@@ -93,7 +93,7 @@ static const struct dma_option dma_rx_option[NUM_PORTS] = {
 static uint16_t i2c_sr1[NUM_PORTS];
 static struct mutex i2c_mutex;
 
-/* buffer for host commands (including version, error code and checksum) */
+/* Buffer for host commands (including version, error code and checksum) */
 static uint8_t host_buffer[EC_HOST_PARAM_SIZE + 4];
 static struct host_cmd_handler_args host_cmd_args;
 
@@ -136,7 +136,7 @@ void __board_i2c_release(int port)
 void board_i2c_release(int port)
 	__attribute__((weak, alias("__board_i2c_release")));
 
-static int i2c_init_port(unsigned int port);
+static void i2c_init_port(unsigned int port);
 
 static int i2c_write_raw_slave(int port, void *buf, int len)
 {
@@ -452,7 +452,7 @@ static void unwedge_i2c_bus(int port)
 	gpio_set_level(sda, 1);
 }
 
-static int i2c_init_port(unsigned int port)
+static void i2c_init_port(unsigned int port)
 {
 	const int i2c_clock_bit[] = {21, 22};
 
@@ -490,29 +490,22 @@ static int i2c_init_port(unsigned int port)
 	STM32_I2C_SR1(port) = 0;
 
 	board_i2c_post_init(port);
-
-	return EC_SUCCESS;
 }
 
 static void i2c_init(void)
 {
-	int rc = 0;
-
-	/* FIXME: Add #defines to determine which channels to init */
-	rc |= i2c_init_port(I2C1);
-	rc |= i2c_init_port(I2C2);
-
-	/* enable event and error interrupts */
-	if (!rc) {
-		task_enable_irq(STM32_IRQ_I2C1_EV);
-		task_enable_irq(STM32_IRQ_I2C1_ER);
-		task_enable_irq(STM32_IRQ_I2C2_EV);
-		task_enable_irq(STM32_IRQ_I2C2_ER);
-	}
+	/* TODO: Add #defines to determine which channels to init */
+	i2c_init_port(I2C1);
+	i2c_init_port(I2C2);
+
+	/* Enable event and error interrupts */
+	task_enable_irq(STM32_IRQ_I2C1_EV);
+	task_enable_irq(STM32_IRQ_I2C1_ER);
+	task_enable_irq(STM32_IRQ_I2C2_EV);
+	task_enable_irq(STM32_IRQ_I2C2_ER);
 }
 DECLARE_HOOK(HOOK_INIT, i2c_init, HOOK_PRIO_DEFAULT);
 
-
 /*****************************************************************************/
 /* STM32 Host I2C */
 
@@ -532,7 +525,6 @@ DECLARE_HOOK(HOOK_INIT, i2c_init, HOOK_PRIO_DEFAULT);
 #define CR2_DMAEN	(1 << 11)	/* DMA enable */
 #define CR2_LAST	(1 << 12)	/* Next EOT is last EOT */
 
-
 static inline void dump_i2c_reg(int port)
 {
 #ifdef CONFIG_DEBUG_I2C
@@ -615,12 +607,14 @@ static int master_start(int port, int slave_addr)
 	rv = wait_status(port, SR1_SB, WAIT_MASTER_START);
 	if (rv)
 		return rv;
+
 	/* Send address */
 	STM32_I2C_DR(port) = slave_addr;
 	/* Wait for addr ready */
 	rv = wait_status(port, SR1_ADDR, WAIT_ADDR_READY);
 	if (rv)
 		return rv;
+
 	read_clear_status(port);
 
 	return EC_SUCCESS;
@@ -670,7 +664,7 @@ static void handle_i2c_error(int port, int rv)
 	timestamp_t t1, t2;
 	uint32_t r;
 
-	/* we have not used the bus, just exit */
+	/* We have not used the bus, just exit */
 	if (rv == EC_ERROR_BUSY)
 		return;
 
@@ -726,16 +720,17 @@ static void handle_i2c_error(int port, int rv)
 		usleep(1000);
 		r = STM32_I2C_SR2(port);
 	}
+
 cr_cleanup:
-	/**
-	 * reset control register to the default state :
+	/*
+	 * Reset control register to the default state :
 	 * I2C mode / Periphal enabled, ACK enabled
 	 */
 	STM32_I2C_CR1(port) = (1 << 10) | (1 << 0);
 }
 
 static int i2c_master_transmit(int port, int slave_addr, uint8_t *data,
-	int size, int stop)
+			       int size, int stop)
 {
 	int rv, rv_start;
 
@@ -780,7 +775,7 @@ static int i2c_master_transmit(int port, int slave_addr, uint8_t *data,
 }
 
 static int i2c_master_receive(int port, int slave_addr, uint8_t *data,
-	int size)
+			      int size)
 {
 	int rv, rv_start;
 
@@ -840,7 +835,7 @@ static int i2c_master_receive(int port, int slave_addr, uint8_t *data,
  * @return 0 if ok, else ER_ERROR...
  */
 static int i2c_xfer(int port, int slave_addr, uint8_t *out, int out_bytes,
-	     uint8_t *in, int in_bytes)
+		    uint8_t *in, int in_bytes)
 {
 	int rv;