Merge branch 'master' of git://git.denx.de/u-boot-i2c

- Fix for mxc_i2c driver in DM mode, thanks to Trent!

3 files changed, 116 insertions, 37 deletions

diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c
index 69fbc8b690..9d7a94ff9d 100644
--- a/board/wandboard/wandboard.c
+++ b/board/wandboard/wandboard.c
@@ -46,6 +46,15 @@ DECLARE_GLOBAL_DATA_PTR;
 #define ETH_PHY_AR8035_POWER	IMX_GPIO_NR(7, 13)
 #define REV_DETECTION		IMX_GPIO_NR(2, 28)
 
+/* Speed defined in Kconfig is only applicable when not using DM_I2C.  */
+#ifdef CONFIG_DM_I2C
+#define I2C1_SPEED_NON_DM	0
+#define I2C2_SPEED_NON_DM	0
+#else
+#define I2C1_SPEED_NON_DM	CONFIG_SYS_MXC_I2C1_SPEED
+#define I2C2_SPEED_NON_DM	CONFIG_SYS_MXC_I2C2_SPEED
+#endif
+
 static bool with_pmic;
 
 int dram_init(void)
@@ -463,13 +472,13 @@ int board_init(void)
 	gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
 
 #if defined(CONFIG_VIDEO_IPUV3)
-	setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6dl_i2c2_pad_info);
+	setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info);
 	if (is_mx6dq() || is_mx6dqp()) {
-		setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6q_i2c2_pad_info);
-		setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6q_i2c3_pad_info);
+		setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6q_i2c2_pad_info);
+		setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6q_i2c3_pad_info);
 	} else {
-		setup_i2c(1, CONFIG_SYS_MXC_I2C1_SPEED, 0x7f, &mx6dl_i2c2_pad_info);
-		setup_i2c(2, CONFIG_SYS_MXC_I2C2_SPEED, 0x7f, &mx6dl_i2c3_pad_info);
+		setup_i2c(1, I2C1_SPEED_NON_DM, 0x7f, &mx6dl_i2c2_pad_info);
+		setup_i2c(2, I2C2_SPEED_NON_DM, 0x7f, &mx6dl_i2c3_pad_info);
 	}
 
 	setup_display();
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index 215624020f..095a9bc6a4 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -161,7 +161,10 @@ config SYS_I2C_MXC
 	  channels and operating on standard mode up to 100 kbits/s and fast
 	  mode up to 400 kbits/s.
 
-if SYS_I2C_MXC
+# These settings are not used with DM_I2C, however SPL doesn't use
+# DM_I2C even if DM_I2C is enabled, and so might use these settings even
+# when main u-boot does not!
+if SYS_I2C_MXC && (!DM_I2C || SPL)
 config SYS_I2C_MXC_I2C1
 	bool "NXP MXC I2C1"
 	help
diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 5420afbc8e..23119cce65 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -482,8 +482,13 @@ static int i2c_write_data(struct mxc_i2c_bus *i2c_bus, u8 chip, const u8 *buf,
 	return ret;
 }
 
+/* Will generate a STOP after the last byte if "last" is true, i.e. this is the
+ * final message of a transaction.  If not, it switches the bus back to TX mode
+ * and does not send a STOP, leaving the bus in a state where a repeated start
+ * and address can be sent for another message.
+ */
 static int i2c_read_data(struct mxc_i2c_bus *i2c_bus, uchar chip, uchar *buf,
-			 int len)
+			 int len, bool last)
 {
 	int ret;
 	unsigned int temp;
@@ -513,17 +518,31 @@ static int i2c_read_data(struct mxc_i2c_bus *i2c_bus, uchar chip, uchar *buf,
 			return ret;
 		}
 
-		/*
-		 * It must generate STOP before read I2DR to prevent
-		 * controller from generating another clock cycle
-		 */
 		if (i == (len - 1)) {
-			i2c_imx_stop(i2c_bus);
+			/* Final byte has already been received by master!  When
+			 * we read it from I2DR, the master will start another
+			 * cycle.  We must program it first to send a STOP or
+			 * switch to TX to avoid this.
+			 */
+			if (last) {
+				i2c_imx_stop(i2c_bus);
+			} else {
+				/* Final read, no stop, switch back to tx */
+				temp = readb(base + (I2CR << reg_shift));
+				temp |= I2CR_MTX | I2CR_TX_NO_AK;
+				writeb(temp, base + (I2CR << reg_shift));
+			}
 		} else if (i == (len - 2)) {
+			/* Master has already recevied penultimate byte.  When
+			 * we read it from I2DR, master will start RX of final
+			 * byte.  We must set TX_NO_AK now so it does not ACK
+			 * that final byte.
+			 */
 			temp = readb(base + (I2CR << reg_shift));
 			temp |= I2CR_TX_NO_AK;
 			writeb(temp, base + (I2CR << reg_shift));
 		}
+
 		writeb(I2SR_IIF_CLEAR, base + (I2SR << reg_shift));
 		buf[i] = readb(base + (I2DR << reg_shift));
 	}
@@ -533,13 +552,34 @@ static int i2c_read_data(struct mxc_i2c_bus *i2c_bus, uchar chip, uchar *buf,
 		debug(" 0x%02x", buf[ret]);
 	debug("\n");
 
-	i2c_imx_stop(i2c_bus);
+	/* It is not clear to me that this is necessary */
+	if (last)
+		i2c_imx_stop(i2c_bus);
 	return 0;
 }
 
 #ifndef CONFIG_DM_I2C
 /*
  * Read data from I2C device
+ *
+ * The transactions use the syntax defined in the Linux kernel I2C docs.
+ *
+ * If alen is > 0, then this function will send a transaction of the form:
+ *     S Chip Wr [A] Addr [A] S Chip Rd [A] [data] A ... NA P
+ * This is a normal I2C register read: writing the register address, then doing
+ * a repeated start and reading the data.
+ *
+ * If alen == 0, then we get this transaction:
+ *     S Chip Wr [A] S Chip Rd [A] [data] A ... NA P
+ * This is somewhat unusual, though valid, transaction.  It addresses the chip
+ * in write mode, but doesn't actually write any register address or data, then
+ * does a repeated start and reads data.
+ *
+ * If alen < 0, then we get this transaction:
+ *     S Chip Rd [A] [data] A ... NA P
+ * The chip is addressed in read mode and then data is read.  No register
+ * address is written first.  This is perfectly valid on most devices and
+ * required on some (usually those that don't act like an array of registers).
  */
 static int bus_i2c_read(struct mxc_i2c_bus *i2c_bus, u8 chip, u32 addr,
 			int alen, u8 *buf, int len)
@@ -566,7 +606,7 @@ static int bus_i2c_read(struct mxc_i2c_bus *i2c_bus, u8 chip, u32 addr,
 		return ret;
 	}
 
-	ret = i2c_read_data(i2c_bus, chip, buf, len);
+	ret = i2c_read_data(i2c_bus, chip, buf, len, true);
 
 	i2c_imx_stop(i2c_bus);
 	return ret;
@@ -574,6 +614,20 @@ static int bus_i2c_read(struct mxc_i2c_bus *i2c_bus, u8 chip, u32 addr,
 
 /*
  * Write data to I2C device
+ *
+ * If alen > 0, we get this transaction:
+ *    S Chip Wr [A] addr [A] data [A] ... [A] P
+ * An ordinary write register command.
+ *
+ * If alen == 0, then we get this:
+ *    S Chip Wr [A] data [A] ... [A] P
+ * This is a simple I2C write.
+ *
+ * If alen < 0, then we get this:
+ *    S data [A] ... [A] P
+ * This is most likely NOT something that should be used.  It doesn't send the
+ * chip address first, so in effect, the first byte of data will be used as the
+ * address.
  */
 static int bus_i2c_write(struct mxc_i2c_bus *i2c_bus, u8 chip, u32 addr,
 			 int alen, const u8 *buf, int len)
@@ -881,6 +935,7 @@ static int mxc_i2c_probe(struct udevice *bus)
 	return 0;
 }
 
+/* Sends: S Addr Wr [A|NA] P */
 static int mxc_i2c_probe_chip(struct udevice *bus, u32 chip_addr,
 			      u32 chip_flags)
 {
@@ -905,42 +960,54 @@ static int mxc_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
 	ulong base = i2c_bus->base;
 	int reg_shift = i2c_bus->driver_data & I2C_QUIRK_FLAG ?
 		VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
+	int read_mode;
 
-	/*
-	 * Here the 3rd parameter addr and the 4th one alen are set to 0,
-	 * because here we only want to send out chip address. The register
-	 * address is wrapped in msg.
+	/* Here address len is set to -1 to not send any address at first.
+	 * Otherwise i2c_init_transfer will send the chip address with write
+	 * mode set.  This is wrong if the 1st message is read.
 	 */
-	ret = i2c_init_transfer(i2c_bus, msg->addr, 0, 0);
+	ret = i2c_init_transfer(i2c_bus, msg->addr, 0, -1);
 	if (ret < 0) {
 		debug("i2c_init_transfer error: %d\n", ret);
 		return ret;
 	}
 
+	read_mode = -1; /* So it's always different on the first message */
 	for (; nmsgs > 0; nmsgs--, msg++) {
-		bool next_is_read = nmsgs > 1 && (msg[1].flags & I2C_M_RD);
-		debug("i2c_xfer: chip=0x%x, len=0x%x\n", msg->addr, msg->len);
-		if (msg->flags & I2C_M_RD)
-			ret = i2c_read_data(i2c_bus, msg->addr, msg->buf,
-					    msg->len);
-		else {
-			ret = i2c_write_data(i2c_bus, msg->addr, msg->buf,
-					     msg->len);
-			if (ret)
-				break;
-			if (next_is_read) {
-				/* Reuse ret */
+		const int msg_is_read = !!(msg->flags & I2C_M_RD);
+
+		debug("i2c_xfer: chip=0x%x, len=0x%x, dir=%c\n", msg->addr,
+		      msg->len, msg_is_read ? 'R' : 'W');
+
+		if (msg_is_read != read_mode) {
+			/* Send repeated start if not 1st message */
+			if (read_mode != -1) {
+				debug("i2c_xfer: [RSTART]\n");
 				ret = readb(base + (I2CR << reg_shift));
 				ret |= I2CR_RSTA;
 				writeb(ret, base + (I2CR << reg_shift));
-
-				ret = tx_byte(i2c_bus, (msg->addr << 1) | 1);
-				if (ret < 0) {
-					i2c_imx_stop(i2c_bus);
-					break;
-				}
 			}
+			debug("i2c_xfer: [ADDR %02x | %c]\n", msg->addr,
+			      msg_is_read ? 'R' : 'W');
+			ret = tx_byte(i2c_bus, (msg->addr << 1) | msg_is_read);
+			if (ret < 0) {
+				debug("i2c_xfer: [STOP]\n");
+				i2c_imx_stop(i2c_bus);
+				break;
+			}
+			read_mode = msg_is_read;
 		}
+
+		if (msg->flags & I2C_M_RD)
+			ret = i2c_read_data(i2c_bus, msg->addr, msg->buf,
+					    msg->len, nmsgs == 1 ||
+						      (msg->flags & I2C_M_STOP));
+		else
+			ret = i2c_write_data(i2c_bus, msg->addr, msg->buf,
+					     msg->len);
+
+		if (ret < 0)
+			break;
 	}
 
 	if (ret)