Re: [PATCH v11 2/4] i2c: tegra: Add HS mode support

[Jon Hunter <jonathanh@nvidia.com>]

On 11/11/2025 09:16, Kartik Rajput wrote:
> From: Akhil R <akhilrajeev@nvidia.com>
> 
> Add support for HS (High Speed) mode transfers, which is supported by
> Tegra194 onwards. Also adjust the bus frequency such that it uses the
> fast plus mode when HS mode is not supported.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
> ---
> v10 -> v11:
> 	* Update the if condition as per the comments received on:
> 	  https://lore.kernel.org/linux-tegra/20251110080502.865953-1-kkartik@nvidia.com/T/#t
> v9 -> v10:
>          * Change switch block to an if-else block.
> v5 -> v9:
>          * In the switch block, handle the case when hs mode is not
>            supported. Also update it to use Fast mode for master code
>            byte as per the I2C spec for HS mode.
> v3 -> v5:
>          * Set has_hs_mode_support to false for unsupported SoCs.
> v2 -> v3:
>          * Document tlow_hs_mode and thigh_hs_mode.
> v1 -> v2:
>          * Document has_hs_mode_support.
>          * Add a check to set the frequency to fastmode+ if the device
>            does not support HS mode but the requested frequency is more
>            than fastmode+.
> ---
>   drivers/i2c/busses/i2c-tegra.c | 62 ++++++++++++++++++++++++++--------
>   1 file changed, 48 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index bd26b232ffb3..2b18ceb837da 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -91,6 +91,7 @@
>   #define I2C_HEADER_IE_ENABLE			BIT(17)
>   #define I2C_HEADER_REPEAT_START			BIT(16)
>   #define I2C_HEADER_CONTINUE_XFER		BIT(15)
> +#define I2C_HEADER_HS_MODE			BIT(22)
>   #define I2C_HEADER_SLAVE_ADDR_SHIFT		1
>   
>   #define I2C_BUS_CLEAR_CNFG			0x084
> @@ -198,6 +199,8 @@ enum msg_end_type {
>    * @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.
> + * @tlow_hs_mode: Low period of the clock in HS mode.
> + * @thigh_hs_mode: High period of the clock in HS mode.
>    * @setup_hold_time_std_mode: Setup and hold time for start and stop conditions
>    *		in standard mode.
>    * @setup_hold_time_fast_fast_plus_mode: Setup and hold time for start and stop
> @@ -206,6 +209,7 @@ enum msg_end_type {
>    *		in HS mode.
>    * @has_interface_timing_reg: Has interface timing register to program the tuned
>    *		timing settings.
> + * @has_hs_mode_support: Has support for high speed (HS) mode transfers.
>    */
>   struct tegra_i2c_hw_feature {
>   	bool has_continue_xfer_support;
> @@ -226,10 +230,13 @@ struct tegra_i2c_hw_feature {
>   	u32 thigh_std_mode;
>   	u32 tlow_fast_fastplus_mode;
>   	u32 thigh_fast_fastplus_mode;
> +	u32 tlow_hs_mode;
> +	u32 thigh_hs_mode;
>   	u32 setup_hold_time_std_mode;
>   	u32 setup_hold_time_fast_fast_plus_mode;
>   	u32 setup_hold_time_hs_mode;
>   	bool has_interface_timing_reg;
> +	bool has_hs_mode_support;
>   };
>   
>   /**
> @@ -677,25 +684,28 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
>   	if (IS_VI(i2c_dev))
>   		tegra_i2c_vi_init(i2c_dev);
>   
> -	switch (t->bus_freq_hz) {
> -	case I2C_MAX_STANDARD_MODE_FREQ + 1 ... I2C_MAX_FAST_MODE_PLUS_FREQ:
> -	default:
> +	if (t->bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) {
> +		tlow = i2c_dev->hw->tlow_std_mode;
> +		thigh = i2c_dev->hw->thigh_std_mode;
> +		tsu_thd = i2c_dev->hw->setup_hold_time_std_mode;
> +		non_hs_mode = i2c_dev->hw->clk_divisor_std_mode;
> +	} else {
>   		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;
>   
> -		if (t->bus_freq_hz > I2C_MAX_FAST_MODE_FREQ)
> -			non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode;
> -		else
> +		/*
> +		 * When HS mode is supported, the non-hs timing registers will be used for the
> +		 * master code byte for transition to HS mode. As per the spec, the 8 bit master
> +		 * code should be sent at max 400kHz. Therefore, limit the bus speed to fast mode.
> +		 * Whereas when HS mode is not supported, allow the highest speed mode capable.
> +		 */
> +		if (t->bus_freq_hz < I2C_MAX_FAST_MODE_PLUS_FREQ ||
> +		    (t->bus_freq_hz > I2C_MAX_FAST_MODE_PLUS_FREQ &&
> +			i2c_dev->hw->has_hs_mode_support))
>   			non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode;

I am still not sure this is correct. Before we had ...

  if (t->bus_freq_hz > I2C_MAX_FAST_MODE_FREQ)
      non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode;

Now we have ...

  if (t->bus_freq_hz >= I2C_MAX_FAST_MODE_PLUS_FREQ)
      non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode;

Don't we want ...

  if (t->bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ)
      non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode;
  else if (i2c_dev->hw->has_hs_mode_support)
      non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode;
  else
      non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode;

Or

  if (!i2c_dev->hw->has_hs_mode_support && t->bus_freq_hz > 
I2C_MAX_FAST_MODE_FREQ)
      non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode;
  else
      non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode;

Jon

-- 
nvpublic