From 68049a082b8aedf09e769e61885e000e598bb516 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Wed, 25 Jun 2014 10:57:27 -0600 Subject: [PATCH 1/3] i2c: tegra: use repeated start for reads I2C read transactions are typically implemented as follows: START(write) address REPEATED_START(read) data... STOP However, Tegra's I2C driver currently implements reads as follows: START(write) address STOP START(read) data... STOP This sequence confuses at least the AS3722 PMIC on the Jetson TK1 board, leading to corrupted read data in some cases. Fix the driver to chain the transactions together using repeated starts to solve this. Signed-off-by: Stephen Warren Reviewed-by: Yen Lin --- arch/arm/include/asm/arch-tegra/tegra_i2c.h | 2 ++ drivers/i2c/tegra_i2c.c | 24 ++++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/arch/arm/include/asm/arch-tegra/tegra_i2c.h b/arch/arm/include/asm/arch-tegra/tegra_i2c.h index 853e59bb6e..7ca690700c 100644 --- a/arch/arm/include/asm/arch-tegra/tegra_i2c.h +++ b/arch/arm/include/asm/arch-tegra/tegra_i2c.h @@ -124,6 +124,8 @@ struct i2c_ctlr { /* bit fields definitions for IO Packet Header 3 format */ #define PKT_HDR3_READ_MODE_SHIFT 19 #define PKT_HDR3_READ_MODE_MASK (1 << PKT_HDR3_READ_MODE_SHIFT) +#define PKT_HDR3_REPEAT_START_SHIFT 16 +#define PKT_HDR3_REPEAT_START_MASK (1 << PKT_HDR3_REPEAT_START_SHIFT) #define PKT_HDR3_SLAVE_ADDR_SHIFT 0 #define PKT_HDR3_SLAVE_ADDR_MASK (0x3ff << PKT_HDR3_SLAVE_ADDR_SHIFT) diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c index 594e5ddeb4..97f0ca44c5 100644 --- a/drivers/i2c/tegra_i2c.c +++ b/drivers/i2c/tegra_i2c.c @@ -110,7 +110,8 @@ static void i2c_init_controller(struct i2c_bus *i2c_bus) static void send_packet_headers( struct i2c_bus *i2c_bus, struct i2c_trans_info *trans, - u32 packet_id) + u32 packet_id, + bool end_with_repeated_start) { u32 data; @@ -132,6 +133,8 @@ static void send_packet_headers( /* Enable Read if it is not a write transaction */ if (!(trans->flags & I2C_IS_WRITE)) data |= PKT_HDR3_READ_MODE_MASK; + if (end_with_repeated_start) + data |= PKT_HDR3_REPEAT_START_MASK; /* Write I2C specific header */ writel(data, &i2c_bus->control->tx_fifo); @@ -209,7 +212,8 @@ static int send_recv_packets(struct i2c_bus *i2c_bus, int_status = readl(&control->int_status); writel(int_status, &control->int_status); - send_packet_headers(i2c_bus, trans, 1); + send_packet_headers(i2c_bus, trans, 1, + trans->flags & I2C_USE_REPEATED_START); words = DIV_ROUND_UP(trans->num_bytes, 4); last_bytes = trans->num_bytes & 3; @@ -267,7 +271,7 @@ exit: } static int tegra_i2c_write_data(struct i2c_bus *bus, u32 addr, u8 *data, - u32 len) + u32 len, bool end_with_repeated_start) { int error; struct i2c_trans_info trans_info; @@ -275,6 +279,8 @@ static int tegra_i2c_write_data(struct i2c_bus *bus, u32 addr, u8 *data, trans_info.address = addr; trans_info.buf = data; trans_info.flags = I2C_IS_WRITE; + if (end_with_repeated_start) + trans_info.flags |= I2C_USE_REPEATED_START; trans_info.num_bytes = len; trans_info.is_10bit_address = 0; @@ -463,7 +469,8 @@ static void tegra_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr) } /* i2c write version without the register address */ -int i2c_write_data(struct i2c_bus *bus, uchar chip, uchar *buffer, int len) +int i2c_write_data(struct i2c_bus *bus, uchar chip, uchar *buffer, int len, + bool end_with_repeated_start) { int rc; @@ -475,7 +482,8 @@ int i2c_write_data(struct i2c_bus *bus, uchar chip, uchar *buffer, int len) debug("\n"); /* Shift 7-bit address over for lower-level i2c functions */ - rc = tegra_i2c_write_data(bus, chip << 1, buffer, len); + rc = tegra_i2c_write_data(bus, chip << 1, buffer, len, + end_with_repeated_start); if (rc) debug("i2c_write_data(): rc=%d\n", rc); @@ -516,7 +524,7 @@ static int tegra_i2c_probe(struct i2c_adapter *adap, uchar chip) if (!bus) return 1; reg = 0; - rc = i2c_write_data(bus, chip, ®, 1); + rc = i2c_write_data(bus, chip, ®, 1, false); if (rc) { debug("Error probing 0x%x.\n", chip); return 1; @@ -554,7 +562,7 @@ static int tegra_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, data[alen - i - 1] = (addr + offset) >> (8 * i); } - if (i2c_write_data(bus, chip, data, alen)) { + if (i2c_write_data(bus, chip, data, alen, true)) { debug("i2c_read: error sending (0x%x)\n", addr); return 1; @@ -591,7 +599,7 @@ static int tegra_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, for (i = 0; i < alen; i++) data[alen - i - 1] = (addr + offset) >> (8 * i); data[alen] = buffer[offset]; - if (i2c_write_data(bus, chip, data, alen + 1)) { + if (i2c_write_data(bus, chip, data, alen + 1, false)) { debug("i2c_write: error sending (0x%x)\n", addr); return 1; } From 981b14f01ae79f85eae3dc6873456abd08de2d86 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Wed, 25 Jun 2014 10:57:28 -0600 Subject: [PATCH 2/3] i2c: tegra: write clean data to TX FIFO The Tegra I2C controller's TX FIFO contains 32-bit words. If the final FIFO entry of a transaction contains fewer than 4 bytes, the driver currently fills the unused FIFO bytes with uninitialized data. This can be confusing when reading back the FIFO content for debugging purposes. Solve this by explicitly initializing the variable containing FIFO data before filling it (partially) with data. With this change, send_recv_packets()'s loop's if (is_write) code mirrors the else (i.e. read) branch. Signed-off-by: Stephen Warren Reviewed-by: Yen Lin --- drivers/i2c/tegra_i2c.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c index 97f0ca44c5..a62f30e66c 100644 --- a/drivers/i2c/tegra_i2c.c +++ b/drivers/i2c/tegra_i2c.c @@ -224,14 +224,16 @@ static int send_recv_packets(struct i2c_bus *i2c_bus, if (is_write) { /* deal with word alignment */ - if ((unsigned)dptr & 3) { + if ((words == 1) && last_bytes) { + local = 0; + memcpy(&local, dptr, last_bytes); + } else if ((unsigned)dptr & 3) { memcpy(&local, dptr, sizeof(u32)); - writel(local, &control->tx_fifo); - debug("pkt data sent (0x%x)\n", local); } else { - writel(*wptr, &control->tx_fifo); - debug("pkt data sent (0x%x)\n", *wptr); + local = *wptr; } + writel(local, &control->tx_fifo); + debug("pkt data sent (0x%x)\n", local); if (!wait_for_tx_fifo_empty(control)) { error = -1; goto exit; From ad3091ad03e39f88c0bcc566c7a691c4475e2c40 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Wed, 25 Jun 2014 10:57:29 -0600 Subject: [PATCH 3/3] i2c: tegra: dump alen in debug statements Since tegra_i2c_{read,write}'s debug() call dumps the chip address, dump the address length (alen) too, so the address value can be correctly interpreted. Signed-off-by: Stephen Warren Reviewed-by: Yen Lin --- drivers/i2c/tegra_i2c.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c index a62f30e66c..257b72f0f7 100644 --- a/drivers/i2c/tegra_i2c.c +++ b/drivers/i2c/tegra_i2c.c @@ -548,8 +548,8 @@ static int tegra_i2c_read(struct i2c_adapter *adap, uchar chip, uint addr, uint offset; int i; - debug("i2c_read: chip=0x%x, addr=0x%x, len=0x%x\n", - chip, addr, len); + debug("i2c_read: chip=0x%x, addr=0x%x, alen=0x%x len=0x%x\n", + chip, addr, alen, len); bus = tegra_i2c_get_bus(adap); if (!bus) return 1; @@ -587,8 +587,8 @@ static int tegra_i2c_write(struct i2c_adapter *adap, uchar chip, uint addr, uint offset; int i; - debug("i2c_write: chip=0x%x, addr=0x%x, len=0x%x\n", - chip, addr, len); + debug("i2c_write: chip=0x%x, addr=0x%x, alen=0x%x len=0x%x\n", + chip, addr, alen, len); bus = tegra_i2c_get_bus(adap); if (!bus) return 1;