Re: [PATCH RESEND 2/2] i2c: tegra: Add Tegra256 support

[Jon Hunter <jonathanh@nvidia.com>]

Hi Akhil,

On 08/10/2025 06:35, Akhil R wrote:
> Hi Jon,
> 
> On Tue, 7 Oct 2025 15:50:56 +0100, Jon Hunter wrote:
>> On 18/08/2025 05:33, Akhil R wrote:
>>> Add compatible and the hardware struct for Tegra256. Tegra256 controllers
>>> use a different parent clock. Hence the timing parameters are different
>>> from the previous generations to meet the expected frequencies.
>>>
>>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>
>>> ---
>>>    drivers/i2c/busses/i2c-tegra.c | 26 ++++++++++++++++++++++++++
>>>    1 file changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
>>> index 4eb31b913c1a..e533460bccc3 100644
>>> --- a/drivers/i2c/busses/i2c-tegra.c
>>> +++ b/drivers/i2c/busses/i2c-tegra.c
>>> @@ -1649,7 +1649,33 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
>>>         .has_interface_timing_reg = true,
>>>    };
>>>   
>>> +static const struct tegra_i2c_hw_feature tegra256_i2c_hw = {
>>> +     .has_continue_xfer_support = true,
>>> +     .has_per_pkt_xfer_complete_irq = true,
>>> +     .clk_divisor_hs_mode = 7,
>>> +     .clk_divisor_std_mode = 0x7a,
>>> +     .clk_divisor_fast_mode = 0x40,
>>> +     .clk_divisor_fast_plus_mode = 0x19,
>>
>>
>> Can you check this divisor value? I see we have been using a value of
>> 0x14 for this which does not align with what we have here. Can you
>> confirm if this should be 0x19 or 0x14?
> 
> If you happen to notice, we are using a different tlow, thigh and hold
> time values as well internally. We are also using separate variables
> (tlow, thigh) for fast and fastplus modes, whereas this driver currently
> uses the same variable (and value) for both fast and fastplus mode. With
> that limitation, these are the closest timing values we can use now to
> get the required frequency.

Yes I did see that we have been re-working these variables and separated 
some of the variables. However, this parameter itself has not changed 
and now we have a different value in upstream. So regardless of the 
changes being planned, I don't see why we are not using the same value 
for this variable everywhere.

Jon

-- 
nvpublic