[U-Boot] [PATCH 3/3] mxc_i2c: remove setting speed at each start

[Stefano Babic]

On 06/05/2012 01:06, Marek Vasut wrote:
> Dear Troy Kisky,
> 
>> On 5/5/2012 2:36 PM, Marek Vasut wrote:
>>> Dear Troy Kisky,
>>>
>>>> On 5/5/2012 6:08 AM, Marek Vasut wrote:
>>>>> Dear Troy Kisky,
>>>>>
>>>>>> On 4/24/2012 8:33 PM, Troy Kisky wrote:
>>>>>>> Other then being very weird, this code was also wrong.
>>>>>>> For example, say I set speed to 100K. I'll read back the speed
>>>>>>> as 85937. But the speed is really 85937.5, so we I reset
>>>>>>> the speed to 85937, I'll get 73660.7. After a couple of transactions
>>>>>>> my speed is now exactly 68750 so it will remain there.
>>>>>>>
>>>>>>> Signed-off-by: Troy Kisky<troy.kisky at boundarydevices.com>
>>>>>>> ---
>>>>>>>
>>>>>>>     drivers/i2c/mxc_i2c.c |    6 ------
>>>>>>>     1 files changed, 0 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
>>>>>>> index 416ffee..fc68062 100644
>>>>>>> --- a/drivers/i2c/mxc_i2c.c
>>>>>>> +++ b/drivers/i2c/mxc_i2c.c
>>>>>>> @@ -231,12 +231,6 @@ int i2c_imx_start(void)
>>>>>>>
>>>>>>>     	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
>>>>>>>     	unsigned int temp = 0;
>>>>>>>     	int result;
>>>>>>>
>>>>>>> -	int speed = i2c_get_bus_speed();
>>>>>>> -	u8 clk_idx = i2c_imx_get_clk(speed);
>>>>>>> -	u8 idx = i2c_clk_div[clk_idx][1];
>>>>>>> -
>>>>>>> -	/* Store divider value */
>>>>>>> -	writeb(idx,&i2c_regs->ifdr);
>>>>>>>
>>>>>>>     	/* Enable I2C controller */
>>>>>>>     	writeb(0,&i2c_regs->i2sr);
>>>>>>
>>>>>> Marek would you care to ack/nak this? It is deleting code that you
>>>>>> added.
>>>>>
>>>>> Ok, who will set the controller speed if you remove this?
>>>>
>>>> i2c_init is the only function that writes the ifdr register after this
>>>> patch.
>>>
>>> And i2c_init() is called on every boot. Correct?
>>>
>>>> This is fine because this register is not affected by a software reset.
>>>
>>> I take it you verified this or that you're sure here :)
>>
>> I haven't looked at every reference manual, but if some oddball chip
>> needs it reinitialized after
>> a software reset, then the fix should go into the i2c_reset function,
>> not i2c_imx_start.
>> And this patch would not be introducing a regression for the oddball
>> chip anyway.

Ok, everything was already clear, and I can also add my:

Acked-by: Stefano Babic <sbabic at denx.de>

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================