
Hi Stephen,
On 25 June 2014 10:57, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
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 swarren@nvidia.com
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 853e59bb6e65..7ca690700cb4 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 594e5ddeb43e..97f0ca44c59a 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);
I'm not sure if it is safe/advisable to pass this value to a bool type. Perhaps the function parameter should be int? My understanding of bool is that it is supposed to be 0 or 1, but I'm happy to be corrected.
words = DIV_ROUND_UP(trans->num_bytes, 4); last_bytes = trans->num_bytes & 3;
Regards, Simon