Re: [PATCH] drivers: i2c: i2c-st: Update i2c timings

[Maxime Coquelin]

Hi Wolfram,

On 06/02/2014 06:31 PM, Wolfram Sang wrote:
> On Fri, May 16, 2014 at 05:32:10PM +0200, Maxime COQUELIN wrote:
> > The i2c timing values specified in the driver are the minimun
> > values defined in the I2C specifications.
> > The I2C specification does not specify any default or maximum values.
> >
> > Some I2C devices are out of spec, and might not work properly with minimum
> > values.
>
> Can you give names here? Would be interesting to know since a few
> drivers implement the minimum timings.

I don't have the name actually.
The request to implement this change came from hw guys.

> > This patch adds a 10% margin on all the timings.
>
> Is there a safety margin or do the devices start to work exactly at 10%?

10% is a safety margin, I don't know what is the limit.

> > Signed-off-by: Maxime Coquelin <maxime.coquelin@xxxxxx>
> > ---
> >   drivers/i2c/busses/i2c-st.c | 24 ++++++++++++------------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-st.c b/drivers/i2c/busses/i2c-st.c
> > index 8720161..09142f1 100644
> > --- a/drivers/i2c/busses/i2c-st.c
> > +++ b/drivers/i2c/busses/i2c-st.c
> > @@ -210,21 +210,21 @@ static inline void st_i2c_clr_bits(void __iomem *reg, u32 mask)
> >   static struct st_i2c_timings i2c_timings[] = {
>
> That needs a comment about the margin, otherwise people will wonder
> where these values come from.

Ok, I will add a comment in the v2.

> >   	[I2C_MODE_STANDARD] = {
> >   		.rate			= 100000,
> > -		.rep_start_hold		= 4000,
> > -		.rep_start_setup	= 4700,
> > -		.start_hold		= 4000,
> > -		.data_setup_time	= 250,
> > -		.stop_setup_time	= 4000,
> > -		.bus_free_time		= 4700,
> > +		.rep_start_hold		= 4400,
> > +		.rep_start_setup	= 5170,
> > +		.start_hold		= 4400,
> > +		.data_setup_time	= 275,
> > +		.stop_setup_time	= 4400,
> > +		.bus_free_time		= 5170,
> >   	},
> >   	[I2C_MODE_FAST] = {
> >   		.rate			= 400000,
> > -		.rep_start_hold		= 600,
> > -		.rep_start_setup	= 600,
> > -		.start_hold		= 600,
> > -		.data_setup_time	= 100,
> > -		.stop_setup_time	= 600,
> > -		.bus_free_time		= 1300,
> > +		.rep_start_hold		= 660,
> > +		.rep_start_setup	= 660,
> > +		.start_hold		= 660,
> > +		.data_setup_time	= 110,
> > +		.stop_setup_time	= 660,
> > +		.bus_free_time		= 1430,
> >   	},
> >   };
> >
> > --
> > 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/