On Thu, Jan 17, 2019 at 12:10:47PM -0800, Sowjanya Komatineni wrote:
> This patch adds I2C interface timing registers support for
> proper bus rate configuration along with meeting the i2c spec

s/i2c/I2C/, s/spec/specification/

> setup and hold times based on the tuning performed on Tegra210,
> Tegra186 and Tegra194 platforms.
> 
> I2C_INTERFACE_TIMING_0 register contains TLOW and THIGH field
> and Tegra I2C controller design uses them as a part of internal
> clock divisor.
> 
> I2C_INTERFACE_TIMING_1 register contains the setup and hold times
> for start and stop conditions.
> 
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
>  [V2]: Updated patch to also configure timings to platform tuned
>        settings
>  [V1]: Uses fixed I2C interface timings which could violate bus
>        timing spec
>   
>  drivers/i2c/busses/i2c-tegra.c | 227 
> ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 201 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index e417ebf7628c..4cbf453fbd3b 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -118,6 +118,20 @@
>  #define I2C_MST_FIFO_STATUS_TX_MASK          0xff0000
>  #define I2C_MST_FIFO_STATUS_TX_SHIFT         16
>  
> +#define I2C_INTERFACE_TIMING_0                  0x94
> +#define I2C_TLOW_MASK                           0x3F
> +#define I2C_THIGH_SHIFT                         8
> +#define I2C_THIGH_MASK                          (0x3F << I2C_THIGH_SHIFT)

These look inconsistently indented. Use tabs to indent.

> +#define I2C_TLOW_NEW_MASK                    0xFF
> +#define I2C_THIGH_NEW_MASK                   (0xFF << I2C_THIGH_SHIFT)
> +
> +#define I2C_INTERFACE_TIMING_1                  0x98

Same here.

> +
> +#define I2C_STANDARD_MODE                    100000
> +#define I2C_FAST_MODE                                400000
> +#define I2C_FAST_PLUS_MODE                   1000000
> +#define I2C_HS_MODE                          3500000
> +
>  /*
>   * msg_end_type: The bus control which need to be send at end of transfer.
>   * @MSG_END_STOP: Send stop pulse at end of transfer.
> @@ -142,7 +156,10 @@ enum msg_end_type {
>   * @has_config_load_reg: Has the config load register to load the new
>   *           configuration.
>   * @clk_divisor_hs_mode: Clock divisor in HS mode.
> - * @clk_divisor_std_fast_mode: Clock divisor in standard/fast mode. It is
> + * @clk_divisor_std_mode: Clock divisor in standard mode. It is
> + *           applicable if there is no fast clock source i.e. single clock
> + *           source.
> + * @clk_divisor_fast_mode: Clock divisor in fast mode. It is
>   *           applicable if there is no fast clock source i.e. single clock
>   *           source.
>   * @clk_divisor_fast_plus_mode: Clock divisor in fast mode plus. It is
> @@ -155,6 +172,19 @@ enum msg_end_type {
>   * @has_mst_fifo: The I2C controller contains the new MST FIFO interface that
>   *           provides additional features and allows for longer messages to
>   *           be transferred in one go.
> + * @interface_timing_enhancement: Interface Timing bit fields increased in
> + *           Tegra194 compared to Prior Tegra chips for more tuning to meet
> + *           I2C Timing requirements for various data rates.
> + * @tlow_std_mode: Low period of the clock in standard mode.
> + * @thigh_std_mode: High period of the clock in standard mode.
> + * @tlow_fast_fastplus_mode: Low period of the clock in fast/fast-plus modes.
> + * @thigh_fast_fastplus_mode: High period of the clock in fast/fast-plus 
> modes.
> + * @setup_hold_time_std_mode: Setup and hold time for start and stop 
> conditions
> +             in standard mode.

It looks like you're breaking the block style here. Every line should
start with a *.

> + * @setup_hold_time_fast_fast_plus_mode: Setup and hold time for start and 
> stop
> +             conditions in fast/fast-plus modes.
> + * @setup_hold_time_hs_mode: Setup and hold time for start and stop 
> conditions
> +             in HS mode.
>   */
>  struct tegra_i2c_hw_feature {
>       bool has_continue_xfer_support;
> @@ -162,11 +192,20 @@ struct tegra_i2c_hw_feature {
>       bool has_single_clk_source;
>       bool has_config_load_reg;
>       int clk_divisor_hs_mode;
> -     int clk_divisor_std_fast_mode;
> +     int clk_divisor_std_mode;
> +     int clk_divisor_fast_mode;
>       u16 clk_divisor_fast_plus_mode;
>       bool has_multi_master_mode;
>       bool has_slcg_override_reg;
>       bool has_mst_fifo;
> +     bool interface_timing_enhancement;
> +     u8 tlow_std_mode;
> +     u8 thigh_std_mode;
> +     u8 tlow_fast_fastplus_mode;
> +     u8 thigh_fast_fastplus_mode;
> +     u32 setup_hold_time_std_mode;
> +     u32 setup_hold_time_fast_fast_plus_mode;
> +     u32 setup_hold_time_hs_mode;
>  };
>  
>  /**
> @@ -515,7 +554,62 @@ static int tegra_i2c_wait_for_config_load(struct 
> tegra_i2c_dev *i2c_dev)
>       return 0;
>  }
>  
> -static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
> +static void tegra_i2c_set_timing(struct tegra_i2c_dev *i2c_dev)
> +{
> +     u32 val;
> +     u32 tsu_thd = 0;
> +     u8 tlow = 0;
> +     u8 thigh = 0;
> +
> +     if ((i2c_dev->bus_clk_rate > I2C_STANDARD_MODE) &&
> +                     (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE)) {
> +             tlow = i2c_dev->hw->tlow_fast_fastplus_mode;
> +             thigh = i2c_dev->hw->thigh_fast_fastplus_mode;
> +             tsu_thd = i2c_dev->hw->setup_hold_time_fast_fast_plus_mode;
> +     } else {
> +             tlow = i2c_dev->hw->tlow_std_mode;
> +             thigh = i2c_dev->hw->thigh_std_mode;
> +             tsu_thd = i2c_dev->hw->setup_hold_time_std_mode;
> +     }
> +
> +     val = (thigh << I2C_THIGH_SHIFT) | tlow;
> +     if (val)
> +             i2c_writel(i2c_dev, val, I2C_INTERFACE_TIMING_0);
> +
> +     if (tsu_thd)
> +             i2c_writel(i2c_dev, tsu_thd, I2C_INTERFACE_TIMING_1);
> +}
> +
> +static int tegra_i2c_set_clk_rate(struct tegra_i2c_dev *i2c_dev)
> +{
> +     u32 clk_multiplier = I2C_CLK_MULTIPLIER_STD_FAST_MODE;
> +     u32 val;
> +     u8 low_clk_period, high_clk_period;
> +     int ret = 0;
> +
> +     val = i2c_readl(i2c_dev, I2C_INTERFACE_TIMING_0);
> +
> +     if (i2c_dev->hw->interface_timing_enhancement) {
> +             low_clk_period = val & I2C_TLOW_NEW_MASK;
> +             high_clk_period = (val & I2C_THIGH_NEW_MASK) >>
> +                                                     I2C_THIGH_SHIFT;
> +     } else {
> +             low_clk_period = val & I2C_TLOW_MASK;
> +             high_clk_period = (val & I2C_THIGH_MASK) >> I2C_THIGH_SHIFT;
> +     }
> +
> +     clk_multiplier = (low_clk_period + high_clk_period + 2);
> +     clk_multiplier *= (i2c_dev->clk_divisor_non_hs_mode + 1);
> +
> +     ret = clk_set_rate(i2c_dev->div_clk,
> +                        i2c_dev->bus_clk_rate * clk_multiplier);
> +     if (ret)
> +             dev_err(i2c_dev->dev, "Clock rate change failed %d\n", ret);
> +
> +     return ret;
> +}
> +
> +static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev, bool is_reinit)
>  {
>       u32 val;
>       int err;
> @@ -534,6 +628,8 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>       if (i2c_dev->is_dvc)
>               tegra_dvc_init(i2c_dev);
>  
> +     tegra_i2c_set_timing(i2c_dev);
> +
>       val = I2C_CNFG_NEW_MASTER_FSM | I2C_CNFG_PACKET_MODE_EN |
>               (0x2 << I2C_CNFG_DEBOUNCE_CNT_SHIFT);
>  
> @@ -549,6 +645,12 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>                                       I2C_CLK_DIVISOR_STD_FAST_MODE_SHIFT;
>       i2c_writel(i2c_dev, clk_divisor, I2C_CLK_DIVISOR);
>  
> +     if (!is_reinit) {
> +             err = tegra_i2c_set_clk_rate(i2c_dev);
> +             if (err < 0)
> +                     goto err;
> +     }
> +
>       if (!i2c_dev->is_dvc) {
>               u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG);
>  
> @@ -747,7 +849,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev 
> *i2c_dev,
>       if (time_left == 0) {
>               dev_err(i2c_dev->dev, "i2c transfer timed out\n");
>  
> -             tegra_i2c_init(i2c_dev);
> +             tegra_i2c_init(i2c_dev, true);
>               return -ETIMEDOUT;
>       }
>  
> @@ -758,7 +860,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev 
> *i2c_dev,
>       if (likely(i2c_dev->msg_err == I2C_ERR_NONE))
>               return 0;
>  
> -     tegra_i2c_init(i2c_dev);
> +     tegra_i2c_init(i2c_dev, true);
>       if (i2c_dev->msg_err == I2C_ERR_NO_ACK) {
>               if (msg->flags & I2C_M_IGNORE_NAK)
>                       return 0;
> @@ -842,12 +944,21 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw 
> = {
>       .has_per_pkt_xfer_complete_irq = false,
>       .has_single_clk_source = false,
>       .clk_divisor_hs_mode = 3,
> -     .clk_divisor_std_fast_mode = 0,
> +     .clk_divisor_std_mode = 0,
> +     .clk_divisor_fast_mode = 0,
>       .clk_divisor_fast_plus_mode = 0,
>       .has_config_load_reg = false,
>       .has_multi_master_mode = false,
>       .has_slcg_override_reg = false,
>       .has_mst_fifo = false,
> +     .interface_timing_enhancement = false,
> +     .tlow_std_mode = 0x4,
> +     .thigh_std_mode = 0x2,
> +     .tlow_fast_fastplus_mode = 0x4,
> +     .thigh_fast_fastplus_mode = 0x2,
> +     .setup_hold_time_std_mode = 0x0,
> +     .setup_hold_time_fast_fast_plus_mode = 0x0,
> +     .setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
> @@ -855,12 +966,21 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw 
> = {
>       .has_per_pkt_xfer_complete_irq = false,
>       .has_single_clk_source = false,
>       .clk_divisor_hs_mode = 3,
> -     .clk_divisor_std_fast_mode = 0,
> +     .clk_divisor_std_mode = 0,
> +     .clk_divisor_fast_mode = 0,
>       .clk_divisor_fast_plus_mode = 0,
>       .has_config_load_reg = false,
>       .has_multi_master_mode = false,
>       .has_slcg_override_reg = false,
>       .has_mst_fifo = false,
> +     .interface_timing_enhancement = false,
> +     .tlow_std_mode = 0x4,
> +     .thigh_std_mode = 0x2,
> +     .tlow_fast_fastplus_mode = 0x4,
> +     .thigh_fast_fastplus_mode = 0x2,
> +     .setup_hold_time_std_mode = 0x0,
> +     .setup_hold_time_fast_fast_plus_mode = 0x0,
> +     .setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
> @@ -868,12 +988,21 @@ static const struct tegra_i2c_hw_feature 
> tegra114_i2c_hw = {
>       .has_per_pkt_xfer_complete_irq = true,
>       .has_single_clk_source = true,
>       .clk_divisor_hs_mode = 1,
> -     .clk_divisor_std_fast_mode = 0x19,
> +     .clk_divisor_std_mode = 0x19,
> +     .clk_divisor_fast_mode = 0x19,
>       .clk_divisor_fast_plus_mode = 0x10,
>       .has_config_load_reg = false,
>       .has_multi_master_mode = false,
>       .has_slcg_override_reg = false,
>       .has_mst_fifo = false,
> +     .interface_timing_enhancement = false,
> +     .tlow_std_mode = 0x4,
> +     .thigh_std_mode = 0x2,
> +     .tlow_fast_fastplus_mode = 0x4,
> +     .thigh_fast_fastplus_mode = 0x2,
> +     .setup_hold_time_std_mode = 0x0,
> +     .setup_hold_time_fast_fast_plus_mode = 0x0,
> +     .setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
> @@ -881,12 +1010,21 @@ static const struct tegra_i2c_hw_feature 
> tegra124_i2c_hw = {
>       .has_per_pkt_xfer_complete_irq = true,
>       .has_single_clk_source = true,
>       .clk_divisor_hs_mode = 1,
> -     .clk_divisor_std_fast_mode = 0x19,
> +     .clk_divisor_std_mode = 0x19,
> +     .clk_divisor_fast_mode = 0x19,
>       .clk_divisor_fast_plus_mode = 0x10,
>       .has_config_load_reg = true,
>       .has_multi_master_mode = false,
>       .has_slcg_override_reg = true,
>       .has_mst_fifo = false,
> +     .interface_timing_enhancement = false,
> +     .tlow_std_mode = 0x4,
> +     .thigh_std_mode = 0x2,
> +     .tlow_fast_fastplus_mode = 0x4,
> +     .thigh_fast_fastplus_mode = 0x2,
> +     .setup_hold_time_std_mode = 0x0,
> +     .setup_hold_time_fast_fast_plus_mode = 0x0,
> +     .setup_hold_time_hs_mode = 0x0,
>  };
>  
>  static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
> @@ -894,30 +1032,71 @@ static const struct tegra_i2c_hw_feature 
> tegra210_i2c_hw = {
>       .has_per_pkt_xfer_complete_irq = true,
>       .has_single_clk_source = true,
>       .clk_divisor_hs_mode = 1,
> -     .clk_divisor_std_fast_mode = 0x19,
> +     .clk_divisor_std_mode = 0x19,
> +     .clk_divisor_fast_mode = 0x19,
>       .clk_divisor_fast_plus_mode = 0x10,
>       .has_config_load_reg = true,
>       .has_multi_master_mode = true,
>       .has_slcg_override_reg = true,
>       .has_mst_fifo = false,
> +     .interface_timing_enhancement = false,
> +     .tlow_std_mode = 0x4,
> +     .thigh_std_mode = 0x2,
> +     .tlow_fast_fastplus_mode = 0x4,
> +     .thigh_fast_fastplus_mode = 0x2,
> +     .setup_hold_time_std_mode = 0,
> +     .setup_hold_time_fast_fast_plus_mode = 0,
> +     .setup_hold_time_hs_mode = 0,
>  };
>  
> -static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
> +static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
>       .has_continue_xfer_support = true,
>       .has_per_pkt_xfer_complete_irq = true,
>       .has_single_clk_source = true,
>       .clk_divisor_hs_mode = 1,
> -     .clk_divisor_std_fast_mode = 0x19,
> +     .clk_divisor_std_mode = 0x16,
> +     .clk_divisor_fast_mode = 0x19,
>       .clk_divisor_fast_plus_mode = 0x10,
>       .has_config_load_reg = true,
>       .has_multi_master_mode = true,
>       .has_slcg_override_reg = true,
> +     .has_mst_fifo = false,
> +     .interface_timing_enhancement = false,
> +     .tlow_std_mode = 0x4,
> +     .thigh_std_mode = 0x3,
> +     .tlow_fast_fastplus_mode = 0x4,
> +     .thigh_fast_fastplus_mode = 0x2,
> +     .setup_hold_time_std_mode = 0,
> +     .setup_hold_time_fast_fast_plus_mode = 0,
> +     .setup_hold_time_hs_mode = 0,
> +};
> +
> +static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
> +     .has_continue_xfer_support = true,
> +     .has_per_pkt_xfer_complete_irq = true,
> +     .has_single_clk_source = true,
> +     .clk_divisor_hs_mode = 2,
> +     .clk_divisor_std_mode = 0x4f,
> +     .clk_divisor_fast_mode = 0x3c,
> +     .clk_divisor_fast_plus_mode = 0x16,
> +     .has_config_load_reg = true,
> +     .has_multi_master_mode = true,
> +     .has_slcg_override_reg = true,
>       .has_mst_fifo = true,
> +     .interface_timing_enhancement = true,
> +     .tlow_std_mode = 0x8,
> +     .thigh_std_mode = 0x7,
> +     .tlow_fast_fastplus_mode = 0x2,
> +     .thigh_fast_fastplus_mode = 0x2,
> +     .setup_hold_time_std_mode = 0x08080808,
> +     .setup_hold_time_fast_fast_plus_mode = 0x02020202,
> +     .setup_hold_time_hs_mode = 0x090909,
>  };
>  
>  /* Match table for of_platform binding */
>  static const struct of_device_id tegra_i2c_of_match[] = {
>       { .compatible = "nvidia,tegra194-i2c", .data = &tegra194_i2c_hw, },
> +     { .compatible = "nvidia,tegra186-i2c", .data = &tegra186_i2c_hw, },
>       { .compatible = "nvidia,tegra210-i2c", .data = &tegra210_i2c_hw, },
>       { .compatible = "nvidia,tegra124-i2c", .data = &tegra124_i2c_hw, },
>       { .compatible = "nvidia,tegra114-i2c", .data = &tegra114_i2c_hw, },
> @@ -937,7 +1116,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>       void __iomem *base;
>       int irq;
>       int ret = 0;
> -     int clk_multiplier = I2C_CLK_MULTIPLIER_STD_FAST_MODE;
>  
>       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>       base = devm_ioremap_resource(&pdev->dev, res);
> @@ -1002,20 +1180,17 @@ static int tegra_i2c_probe(struct platform_device 
> *pdev)
>               }
>       }
>  
> -     i2c_dev->clk_divisor_non_hs_mode =
> -                     i2c_dev->hw->clk_divisor_std_fast_mode;
> -     if (i2c_dev->hw->clk_divisor_fast_plus_mode &&
> -             (i2c_dev->bus_clk_rate == 1000000))
> +     if ((i2c_dev->bus_clk_rate > I2C_FAST_MODE) &&
> +                     (i2c_dev->bus_clk_rate <= I2C_FAST_PLUS_MODE))

This was wrong before, but might as well fix the alignment of the
conditions while at it.

>               i2c_dev->clk_divisor_non_hs_mode =
>                       i2c_dev->hw->clk_divisor_fast_plus_mode;
> -
> -     clk_multiplier *= (i2c_dev->clk_divisor_non_hs_mode + 1);
> -     ret = clk_set_rate(i2c_dev->div_clk,
> -                        i2c_dev->bus_clk_rate * clk_multiplier);
> -     if (ret) {
> -             dev_err(i2c_dev->dev, "Clock rate change failed %d\n", ret);
> -             goto unprepare_fast_clk;
> -     }
> +     else if ((i2c_dev->bus_clk_rate > I2C_STANDARD_MODE) &&
> +                     (i2c_dev->bus_clk_rate <= I2C_FAST_MODE))

Same here.

Thierry

> +             i2c_dev->clk_divisor_non_hs_mode =
> +                     i2c_dev->hw->clk_divisor_fast_mode;
> +     else
> +             i2c_dev->clk_divisor_non_hs_mode =
> +                     i2c_dev->hw->clk_divisor_std_mode;
>  
>       ret = clk_prepare(i2c_dev->div_clk);
>       if (ret < 0) {
> @@ -1041,7 +1216,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>               }
>       }
>  
> -     ret = tegra_i2c_init(i2c_dev);
> +     ret = tegra_i2c_init(i2c_dev, false);
>       if (ret) {
>               dev_err(&pdev->dev, "Failed to initialize i2c controller\n");
>               goto disable_div_clk;
> -- 
> 2.7.4
> 

Attachment: signature.asc
Description: PGP signature

Reply via email to