linux-kernel - Re: [PATCH v8 2/4] i2c: tegra: Add HS mode support

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: <e5c06b4e-3edc-4977-bbcc-2035cf11811e@nvidia.com>
Date: Thu, 18 Sep 2025 13:55:48 +0100
From: Jon Hunter <jonathanh@...dia.com>
To: Akhil R <akhilrajeev@...dia.com>
Cc: andi.shyti@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org,
 digetx@...il.com, kkartik@...dia.com, krzk+dt@...nel.org,
 ldewangan@...dia.com, linux-i2c@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org, robh@...nel.org,
 thierry.reding@...il.com, smangipudi@...dia.com
Subject: Re: [PATCH v8 2/4] i2c: tegra: Add HS mode support



On 18/09/2025 12:16, Akhil R wrote:
> On Thu, 18 Sep 2025 11:21:14 +0100, Jon Hunter wrote:
>> On 18/09/2025 11:04, Akhil R wrote:
>>> On Wed, 17 Sep 2025 14:59:54 +0100, Jon Hunter wrote:
>>>> On 17/09/2025 09:56, Kartik Rajput wrote:
> ...
> ...
> 
>>>> No mention in the changelog about this part. Looks like this is a fallback.
>>>>
>>>> Should all of this be handled in the case statement for t->bus_freq_hz?
>>>>
>>>
>>> HS mode timing parameters are programmed in registers different from the other
>>> speed modes. These registers does not affect the timing in other speed modes.
>>> HS mode registers being used or not is determined by the packet header.
>>>
>>> We may also want to program the regular timing registers, because it will be
>>> used for the master code byte to transition to HS mode.
>>>
>>> So, I guess, even if we move this to the switch statement, we might end up
>>> doing something similar outside it.
>>
>>
>> The 'tlow', 'thigh' and 'tsu_thd' are configured under the case
>> statement and so seems logical to also configure these for HS mode under
>> this too. I see that there are different timing registers for HS mode,
> 
> We are just reusing the variables since the fields are similar. If required,
> we can define separate variables with _hs suffix. Do you suggest it that way?
> 
>> but right now looks like we are programming both the normal ones and HS
>> ones. Do both need to be programmed for HS mode?
> 
> Yes. As mentioned in my previous comment, the normal timing registers will
> be used for the 'master code' byte sent to transition to HS mode. We need
> to program both for HS mode.

OK, I see now. So we need to program the normal timings first and then 
we are re-using the variables to then program the HS timings. And 
because of that we cannot setup the HS timing values in the existing 
case statement?

> So, I am not sure if moving this section to the switch block will add
> any benefit. We might end up making it more complicated that it is now.

Yes that's true. It was really this else part that caught my eye ...

  } else if (t->bus_freq_hz > I2C_MAX_FAST_MODE_PLUS_FREQ) {
   	t->bus_freq_hz = I2C_MAX_FAST_MODE_PLUS_FREQ;
  }

It feels like at least this part should be handled as part of the case 
statement.

Jon

-- 
nvpublic