[U-Boot] [PATCH 1/3] i2c: tegra: use repeated start for reads

[Simon Glass]

Hi Stephen,

On 25 June 2014 10:57, Stephen Warren <swarren at wwwdotorg.org> wrote:
>
> From: Stephen Warren <swarren at 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 at 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