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

[Jon Hunter]

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@xxxxxxxxxx>
> > > Acked-by: Thierry Reding <treding@xxxxxxxxxx>
> > >
> > > ---
> > >    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