RE: [PATCH V18 6/6] i2c: tegra: add i2c interface timing support

[Sowjanya Komatineni]

> On Fri, Feb 08, 2019 at 10:59:42AM -0800, Sowjanya Komatineni wrote:
> > This patch adds I2C interface timing registers support for proper bus 
> > rate configuration along with meeting the I2C spec setup and hold 
> > times based on the tuning performed on Tegra210,
> > Tegra186 and Tegra194 platforms.
> > 
> > I2C_INTERFACE_TIMING_0 register contains TLOW and THIGH field and 
> > Tegra I2C controller design uses them as a part of internal clock 
> > divisor.
> > 
> > I2C_INTERFACE_TIMING_1 register contains the setup and hold times for 
> > start and stop conditions.
> > 
> > Acked-by: Thierry Reding <treding@xxxxxxxxxx>
> > Reviewed-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> > Tested-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> > Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
>
> Are you aware that we also have DT bindings for various I2C timing values? They are usually board dependent and not so much SoC dependent.
> Please check here:
>
> Documentation/devicetree/bindings/i2c/i2c.txt
>
> (Oops, I noticed 'i2c-sda-hold-time-ns' is missing from the docs. Will fix that ASAP)
>
> Just saying. The patch here is fine for me. DT support could be added later, if you want that.
>
Hi Wolfram,
Yes I looked into those and the reason I added them as part of hw_feature inside driver are
1. property is in-terms of ns for fall time but the value we program in interface timing is not exact ns time
2. Below are the timing parameters need to be programmed for tegra I2C and I don't see equivalent property for each of those
     TSU_STA : Setup time for repeated START condition
     THD_STA : Hold time for (repeated) START condition
     TSU_STO : Setup time for STOP condition
     TBUF: Bus free time b/w STOP and START conditions
3. All above Timing configuration settings are different for different speed modes (STD/FM & FM+/HS Mode) 
4. All tegra platforms of specific Tegra chip has the same timing configuration at-least for now so added this info as part of hw feature inside the driver.

Sowjanya