| 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
| ||
|
Message-ID: <8c6c8c3e-b605-467f-9c6d-1e13e0e5e009@nvidia.com>
Date: Mon, 10 Nov 2025 11:25:13 +0000
From: Jon Hunter <jonathanh@...dia.com>
To: Kartik Rajput <kkartik@...dia.com>, akhilrajeev@...dia.com,
andi.shyti@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, thierry.reding@...il.com, ldewangan@...dia.com,
digetx@...il.com, smangipudi@...dia.com, linux-i2c@...r.kernel.org,
devicetree@...r.kernel.org, linux-tegra@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 2/4] i2c: tegra: Add HS mode support
On 10/11/2025 08:05, Kartik Rajput wrote:
> From: Akhil R <akhilrajeev@...dia.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@...dia.com>
> Signed-off-by: Kartik Rajput <kkartik@...dia.com>
> ---
> 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 | 65 ++++++++++++++++++++++++++--------
> 1 file changed, 51 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index bd26b232ffb3..2038ab2d8095 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,31 @@ 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) {
Is this correct? Before we had ...
case 0 ... I2C_MAX_STANDARD_MODE_FREQ:
So shouldn't this be ...
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 if (t->bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) {
> 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;
Do we need this else-if? It appears that tlow, thigh and tsu_thd are the
same between the else-if and else? Therefore, why not just have an else
and then handle 'non_hs_mode' accordingly?
> + } 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 (i2c_dev->hw->has_hs_mode_support)
> non_hs_mode = i2c_dev->hw->clk_divisor_fast_mode;
> - break;
> + else
> + non_hs_mode = i2c_dev->hw->clk_divisor_fast_plus_mode;
>
> - case 0 ... 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;
> + 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;
> }
Jon
--
nvpublic
Powered by blists - more mailing lists