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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250918171247.72087-1-akhilrajeev@nvidia.com>
Date: Thu, 18 Sep 2025 22:42:46 +0530
From: Akhil R <akhilrajeev@...dia.com>
To: <jonathanh@...dia.com>
CC: <akhilrajeev@...dia.com>, <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>, <smangipudi@...dia.com>, <thierry.reding@...il.com>
Subject: Re: [PATCH v8 2/4] i2c: tegra: Add HS mode support

On Thu, 18 Sep 2025 13:55:48 +0100 Jon Hunter wrote:  
> 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.

Yes. That makes sense. If you agree, we can remove the else part because
we weren't doing this before when HS mode support was not there. It is not 
directly related to the HS mode support as well. We can add this at a later
point in a separate patch if found required.

Regards,
Akhil