On Thu, Jan 03, 2019 at 04:23:38PM -0800, Sowjanya Komatineni wrote:
> Bus Clear feature of tegra i2c controller helps to recover from
> bus hang when i2c master loses the bus arbitration due to the
> slave device holding SDA LOW continuously for some unknown reasons.
> 
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 65 
> ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 437294ea2f0a..89a0c9fdc99e 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -63,6 +63,7 @@
>  #define I2C_FIFO_STATUS_RX_SHIFT             0
>  #define I2C_INT_MASK                         0x064
>  #define I2C_INT_STATUS                               0x068
> +#define I2C_INT_BUS_CLR_DONE                 BIT(11)
>  #define I2C_INT_PACKET_XFER_COMPLETE         BIT(7)
>  #define I2C_INT_ALL_PACKETS_XFER_COMPLETE    BIT(6)
>  #define I2C_INT_TX_FIFO_OVERFLOW             BIT(5)
> @@ -105,6 +106,15 @@
>  #define I2C_HEADER_MASTER_ADDR_SHIFT         12
>  #define I2C_HEADER_SLAVE_ADDR_SHIFT          1
>  
> +#define I2C_BUS_CLEAR_CNFG                   0x084
> +#define I2C_BC_SCLK_THRESHOLD                        9
> +#define I2C_BC_SCLK_THRESHOLD_SHIFT          16
> +#define I2C_BC_STOP_COND                     BIT(2)
> +#define I2C_BC_TERMINATE                     BIT(1)
> +#define I2C_BC_ENABLE                                BIT(0)
> +#define I2C_BUS_CLEAR_STATUS                 0x088
> +#define I2C_BC_STATUS                                BIT(0)
> +
>  #define I2C_CONFIG_LOAD                              0x08C
>  #define I2C_MSTR_CONFIG_LOAD                 BIT(0)
>  #define I2C_SLV_CONFIG_LOAD                  BIT(1)
> @@ -154,6 +164,8 @@ enum msg_end_type {
>   * @clk_divisor_std_fast_mode: Clock divisor in standard/fast mode. It is
>   *           applicable if there is no fast clock source i.e. single clock
>   *           source.
> + * @has_bus_clr_support: Bus Clear support to recover from bus hang during
> + *           SDA stuck low from device for some unknown reasons.

Might as well spell out clr as clear in the name. Or maybe to make it
shorter, call it supports_bus_clear.

>   */
>  
>  struct tegra_i2c_hw_feature {
> @@ -167,6 +179,7 @@ struct tegra_i2c_hw_feature {
>       bool has_multi_master_mode;
>       bool has_slcg_override_reg;
>       bool has_mst_fifo;
> +     bool has_bus_clr_support;
>  };
>  
>  /**
> @@ -636,6 +649,9 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
>               goto err;
>       }
>  
> +     if (i2c_dev->hw->has_bus_clr_support && (status & I2C_INT_BUS_CLR_DONE))
> +             goto err;
> +

This looks odd. If we support bus clear and the BUS_CLR_DONE interrupt
is set, shouldn't we still process all other potential interrupts? Or
was this meant to be inversed:

        if (!i2c_dev->hw->supports_bus_clear && (status & I2C_INT_BUS_CLR_DONE)

?

>       if (i2c_dev->msg_read && (status & I2C_INT_RX_FIFO_DATA_REQ)) {
>               if (i2c_dev->msg_buf_remaining)
>                       tegra_i2c_empty_rx_fifo(i2c_dev);
> @@ -664,6 +680,8 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
>       tegra_i2c_mask_irq(i2c_dev, I2C_INT_NO_ACK | I2C_INT_ARBITRATION_LOST |
>               I2C_INT_PACKET_XFER_COMPLETE | I2C_INT_TX_FIFO_DATA_REQ |
>               I2C_INT_RX_FIFO_DATA_REQ);
> +     if (i2c_dev->hw->has_bus_clr_support)
> +             tegra_i2c_mask_irq(i2c_dev, I2C_INT_BUS_CLR_DONE);
>       i2c_writel(i2c_dev, status, I2C_INT_STATUS);
>       if (i2c_dev->is_dvc)
>               dvc_writel(i2c_dev, DVC_STATUS_I2C_DONE_INTR, DVC_STATUS);
> @@ -674,6 +692,41 @@ static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
>       return IRQ_HANDLED;
>  }
>  
> +static int tegra_i2c_issue_bus_clear(struct tegra_i2c_dev *i2c_dev)
> +{
> +     int time_left, err;
> +     u32 reg;
> +
> +     if (i2c_dev->hw->has_bus_clr_support) {
> +             reinit_completion(&i2c_dev->msg_complete);
> +             reg = (I2C_BC_SCLK_THRESHOLD << I2C_BC_SCLK_THRESHOLD_SHIFT) |
> +                   I2C_BC_STOP_COND | I2C_BC_TERMINATE;
> +             i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG);
> +             if (i2c_dev->hw->has_config_load_reg) {
> +                     err = tegra_i2c_wait_for_config_load(i2c_dev);
> +                     if (err)
> +                             return err;
> +             }
> +             reg |= I2C_BC_ENABLE;
> +             i2c_writel(i2c_dev, reg, I2C_BUS_CLEAR_CNFG);
> +             tegra_i2c_unmask_irq(i2c_dev, I2C_INT_BUS_CLR_DONE);
> +
> +             time_left = wait_for_completion_timeout(&i2c_dev->msg_complete,
> +                                                     TEGRA_I2C_TIMEOUT);

wait_for_completion_timeout() returns an unsigned long, so the type of
time_left should reflect that.

Thierry

Attachment: signature.asc
Description: PGP signature

Reply via email to