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

[Wolfram Sang]

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.

> 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%?

> 
> 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.

>  	[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
>

Attachment: signature.asc
Description: Digital signature