Re: [PATCH 1/5] I2C driver for MXC

[Darius Augulis <augulis.darius-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Guennadi Liakhovetski wrote:
> On Mon, 30 Mar 2009, Sascha Hauer wrote:
>
>   
>> On Mon, Mar 30, 2009 at 11:26:31AM +0200, Guennadi Liakhovetski wrote:
>>     
>>> Well, I have been able to get your driver to at least pass the 
>>> initialisation with mt9t031 (other parts are missing yet for a complete 
>>> test). For that I used this silly patch:
>>>
>>> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
>>> index 3296380..46e1033 100644
>>> --- a/drivers/i2c/busses/i2c-imx.c
>>> +++ b/drivers/i2c/busses/i2c-imx.c
>>> @@ -371,6 +371,8 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>>>  	if (result)
>>>  		goto fail0;
>>>  
>>> +	msleep(2);
>>> +
>>>  	/* Start I2C transfer */
>>>  	i2c_imx_start(i2c_imx);
>>>
>>> As you understand, this cannot be the final fix. We have to understand why 
>>> a delay is needed there and how long it actually has to be...
>>>  
>>>       
>> Have you checked the value of disable_delay in Darius' driver or tried to
>> increase it?
>>     
>
> Why? That variable is addreccing a problem on a completely different 
> hardware:
>
> 	/*
> 	 * There dummy delay is calculated.
> 	 * It should be about one I2C clock period long.
> 	 * This delay is used in I2C bus disable function
> 	 * to fix chip hardware bug.
> 	 */
>
> and is used on a different path - during controller disable.
>   

Freescale datasheet don't specifies any timing requirement when 
disabling I2C module.
If I disable it immediately after generating STOP signal, it's not 
generated.
When one clock delay is used, problem is solved. So I guess it is 
hardware bug.
Strange, why your driver works without that.

>   
>> BTW I just realized that the handling of disable_delay in Darius' driver
>> is wrong. This should be a per device variable.
>>     
>
> Indeed.
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
>
>