lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e901986b-572c-427d-9141-dafd14db4fad@nvidia.com>
Date: Mon, 17 Nov 2025 09:51:55 +0000
From: Jon Hunter <jonathanh@...dia.com>
To: Akhil R <akhilrajeev@...dia.com>, andi.shyti@...nel.org,
 digetx@...il.com, linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-tegra@...r.kernel.org, thierry.reding@...il.com,
 wsa+renesas@...g-engineering.com, wsa@...nel.org
Cc: kkartik@...dia.com, ldewangan@...dia.com, smangipudi@...dia.com
Subject: Re: [PATCH v12 2/6] i2c: tegra: Use separate variables for fast and
 fastplus



On 15/11/2025 04:26, Akhil R wrote:
> The current implementation uses a single value of THIGH, TLOW and setup
> hold time for both fast and fastplus. But these values can be different
> for each speed mode and should be using separate variables. Split the
> variables used for fast and fast plus mode.
> 
> Signed-off-by: Akhil R <akhilrajeev@...dia.com>
> ---
>   drivers/i2c/busses/i2c-tegra.c | 119 ++++++++++++++++++++-------------
>   1 file changed, 73 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index bd26b232ffb3..c0382c9a0430 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -196,12 +196,16 @@ enum msg_end_type {
>    * @has_apb_dma: Support of APBDMA on corresponding Tegra chip.
>    * @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.
> + * @tlow_fast_mode: Low period of the clock in fast mode.
> + * @thigh_fast_mode: High period of the clock in fast mode.
> + * @tlow_fastplus_mode: Low period of the clock in fast-plus mode.
> + * @thigh_fastplus_mode: High period of the clock in fast-plus 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
> - *		conditions in fast/fast-plus modes.
> + * @setup_hold_time_fast_mode: Setup and hold time for start and stop
> + *		conditions in fast mode.
> + * @setup_hold_time_fastplus_mode: Setup and hold time for start and stop
> + *		conditions in fast-plus mode.
>    * @setup_hold_time_hs_mode: Setup and hold time for start and stop conditions
>    *		in HS mode.
>    * @has_interface_timing_reg: Has interface timing register to program the tuned
> @@ -224,10 +228,13 @@ struct tegra_i2c_hw_feature {
>   	bool has_apb_dma;
>   	u32 tlow_std_mode;
>   	u32 thigh_std_mode;
> -	u32 tlow_fast_fastplus_mode;
> -	u32 thigh_fast_fastplus_mode;
> +	u32 tlow_fast_mode;
> +	u32 thigh_fast_mode;
> +	u32 tlow_fastplus_mode;
> +	u32 thigh_fastplus_mode;
>   	u32 setup_hold_time_std_mode;
> -	u32 setup_hold_time_fast_fast_plus_mode;
> +	u32 setup_hold_time_fast_mode;
> +	u32 setup_hold_time_fastplus_mode;
>   	u32 setup_hold_time_hs_mode;
>   	bool has_interface_timing_reg;
>   };
> @@ -677,25 +684,21 @@ 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:
> -		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
> -			non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode;
> -		break;
> -
> -	case 0 ... I2C_MAX_STANDARD_MODE_FREQ:
> +	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;
> -		break;
> +	} else if (t->bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) {
> +		tlow = i2c_dev->hw->tlow_fast_mode;
> +		thigh = i2c_dev->hw->thigh_fast_mode;
> +		tsu_thd = i2c_dev->hw->setup_hold_time_fast_mode;
> +		non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode;
> +	} else {
> +		tlow = i2c_dev->hw->tlow_fastplus_mode;
> +		thigh = i2c_dev->hw->thigh_fastplus_mode;
> +		tsu_thd = i2c_dev->hw->setup_hold_time_fastplus_mode;
> +		non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode;
>   	}
>   
>   	/* make sure clock divisor programmed correctly */
> @@ -1496,10 +1499,13 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
>   	.has_apb_dma = true,
>   	.tlow_std_mode = 0x4,
>   	.thigh_std_mode = 0x2,
> -	.tlow_fast_fastplus_mode = 0x4,
> -	.thigh_fast_fastplus_mode = 0x2,
> +	.tlow_fast_mode = 0x4,
> +	.thigh_fast_mode = 0x2,
> +	.tlow_fastplus_mode = 0x4,
> +	.thigh_fastplus_mode = 0x2,
>   	.setup_hold_time_std_mode = 0x0,
> -	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_fast_mode = 0x0,
> +	.setup_hold_time_fastplus_mode = 0x0,
>   	.setup_hold_time_hs_mode = 0x0,
>   	.has_interface_timing_reg = false,
>   };
> @@ -1521,10 +1527,13 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
>   	.has_apb_dma = true,
>   	.tlow_std_mode = 0x4,
>   	.thigh_std_mode = 0x2,
> -	.tlow_fast_fastplus_mode = 0x4,
> -	.thigh_fast_fastplus_mode = 0x2,
> +	.tlow_fast_mode = 0x4,
> +	.thigh_fast_mode = 0x2,
> +	.tlow_fastplus_mode = 0x4,
> +	.thigh_fastplus_mode = 0x2,
>   	.setup_hold_time_std_mode = 0x0,
> -	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_fast_mode = 0x0,
> +	.setup_hold_time_fastplus_mode = 0x0,
>   	.setup_hold_time_hs_mode = 0x0,
>   	.has_interface_timing_reg = false,
>   };
> @@ -1546,10 +1555,13 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
>   	.has_apb_dma = true,
>   	.tlow_std_mode = 0x4,
>   	.thigh_std_mode = 0x2,
> -	.tlow_fast_fastplus_mode = 0x4,
> -	.thigh_fast_fastplus_mode = 0x2,
> +	.tlow_fast_mode = 0x4,
> +	.thigh_fast_mode = 0x2,
> +	.tlow_fastplus_mode = 0x4,
> +	.thigh_fastplus_mode = 0x2,
>   	.setup_hold_time_std_mode = 0x0,
> -	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_fast_mode = 0x0,
> +	.setup_hold_time_fastplus_mode = 0x0,
>   	.setup_hold_time_hs_mode = 0x0,
>   	.has_interface_timing_reg = false,
>   };
> @@ -1571,10 +1583,13 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
>   	.has_apb_dma = true,
>   	.tlow_std_mode = 0x4,
>   	.thigh_std_mode = 0x2,
> -	.tlow_fast_fastplus_mode = 0x4,
> -	.thigh_fast_fastplus_mode = 0x2,
> +	.tlow_fast_mode = 0x4,
> +	.thigh_fast_mode = 0x2,
> +	.tlow_fastplus_mode = 0x4,
> +	.thigh_fastplus_mode = 0x2,
>   	.setup_hold_time_std_mode = 0x0,
> -	.setup_hold_time_fast_fast_plus_mode = 0x0,
> +	.setup_hold_time_fast_mode = 0x0,
> +	.setup_hold_time_fastplus_mode = 0x0,
>   	.setup_hold_time_hs_mode = 0x0,
>   	.has_interface_timing_reg = true,
>   };
> @@ -1596,10 +1611,13 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
>   	.has_apb_dma = true,
>   	.tlow_std_mode = 0x4,
>   	.thigh_std_mode = 0x2,
> -	.tlow_fast_fastplus_mode = 0x4,
> -	.thigh_fast_fastplus_mode = 0x2,
> +	.tlow_fast_mode = 0x4,
> +	.thigh_fast_mode = 0x2,
> +	.tlow_fastplus_mode = 0x4,
> +	.thigh_fastplus_mode = 0x2,
>   	.setup_hold_time_std_mode = 0,
> -	.setup_hold_time_fast_fast_plus_mode = 0,
> +	.setup_hold_time_fast_mode = 0,
> +	.setup_hold_time_fastplus_mode = 0,
>   	.setup_hold_time_hs_mode = 0,
>   	.has_interface_timing_reg = true,
>   };
> @@ -1621,10 +1639,13 @@ static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
>   	.has_apb_dma = false,
>   	.tlow_std_mode = 0x4,
>   	.thigh_std_mode = 0x3,
> -	.tlow_fast_fastplus_mode = 0x4,
> -	.thigh_fast_fastplus_mode = 0x2,
> +	.tlow_fast_mode = 0x4,
> +	.thigh_fast_mode = 0x2,
> +	.tlow_fastplus_mode = 0x4,
> +	.thigh_fastplus_mode = 0x2,
>   	.setup_hold_time_std_mode = 0,
> -	.setup_hold_time_fast_fast_plus_mode = 0,
> +	.setup_hold_time_fast_mode = 0,
> +	.setup_hold_time_fastplus_mode = 0,
>   	.setup_hold_time_hs_mode = 0,
>   	.has_interface_timing_reg = true,
>   };
> @@ -1646,10 +1667,13 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
>   	.has_apb_dma = false,
>   	.tlow_std_mode = 0x8,
>   	.thigh_std_mode = 0x7,
> -	.tlow_fast_fastplus_mode = 0x2,
> -	.thigh_fast_fastplus_mode = 0x2,
> +	.tlow_fast_mode = 0x2,
> +	.thigh_fast_mode = 0x2,
> +	.tlow_fastplus_mode = 0x2,
> +	.thigh_fastplus_mode = 0x2,
>   	.setup_hold_time_std_mode = 0x08080808,
> -	.setup_hold_time_fast_fast_plus_mode = 0x02020202,
> +	.setup_hold_time_fast_mode = 0x02020202,
> +	.setup_hold_time_fastplus_mode = 0x02020202,
>   	.setup_hold_time_hs_mode = 0x090909,
>   	.has_interface_timing_reg = true,
>   };
> @@ -1671,10 +1695,13 @@ static const struct tegra_i2c_hw_feature tegra256_i2c_hw = {
>   	.has_apb_dma = false,
>   	.tlow_std_mode = 0x8,
>   	.thigh_std_mode = 0x7,
> -	.tlow_fast_fastplus_mode = 0x3,
> -	.thigh_fast_fastplus_mode = 0x3,
> +	.tlow_fast_mode = 0x3,
> +	.thigh_fast_mode = 0x3,
> +	.tlow_fastplus_mode = 0x3,
> +	.thigh_fastplus_mode = 0x3,
>   	.setup_hold_time_std_mode = 0x08080808,
> -	.setup_hold_time_fast_fast_plus_mode = 0x02020202,
> +	.setup_hold_time_fast_mode = 0x02020202,
> +	.setup_hold_time_fastplus_mode = 0x02020202,
>   	.setup_hold_time_hs_mode = 0x090909,
>   	.has_interface_timing_reg = true,
>   };


Reviewed-by: Jon Hunter <jonathanh@...dia.com>

Thanks
Jon

-- 
nvpublic


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ