[U-Boot] ColdFire I2C implementing I2C idle [PATCH] -- resent in git format

[Heiko Schocher <hs@denx.de>]

Hello Wolfgang,

Wolfgang Wegner wrote:
> On Thu, Jan 21, 2010 at 09:04:29AM +0100, Heiko Schocher wrote:
>> Hello Joakim,
>>
>> Joakim Tjernlund wrote:
>>>> Hello Michael,
>>>>
>>>> Thanks for posting your patches in plain text.
>>>>
>>>> Michael Durrant wrote:
>>>>> drivers_i2c_fsl_i2c.patch
>>>>>    - need to set I2C to be idle according to the MCF5282 user's manual
>>>>>
>>>>>    If I2SR[IBB] is set when the I2C bus module is enabled,
>>>>>    execute the following code sequence before proceeding with
>>>>>    normal initialization code. This issues a STOP command to the
>>>>>    slave device, placing it in idle state as if it were just
>>>>>    power-cycled on.
>>>>>
>>>>>    I2CR = 0x0
>>>>>    I2CR = 0xA
>>>>>    dummy read of I2DR
>>>>>    I2SR = 0x0
>>>>>    I2CR = 0x0
>>>>>
>>>>> Signed-off-by: David Wu <davidwu@arcturusnetworks.com>
>>>>> Signed-off-by: Michael Durrant <mdurrant@arcturusnetworks.com>
>>>>> ---
>>>>>  drivers/i2c/fsl_i2c.c |   13 +++++++++++++
>>>>>  1 files changed, 13 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/i2c/fsl_i2c.c b/drivers/i2c/fsl_i2c.c
>>>>> index 2241990..bce750c 100644
>>>>> --- a/drivers/i2c/fsl_i2c.c
>>>>> +++ b/drivers/i2c/fsl_i2c.c
>>>>> @@ -251,12 +251,25 @@ i2c_init(int speed, int slaveadd)
>>>>>  #endif
>>>>>  }
>>>>>
>>>>> +static __inline__ int i2c_set_idle(void)
>>>>> +{
>>>>> +    writeb(0, &i2c_dev[i2c_bus_num]->cr);
>>>>> +    writeb(I2C_CR_MEN | I2C_CR_MSTA, &i2c_dev[i2c_bus_num]->cr);
>>>>> +    readb(&i2c_dev[i2c_bus_num]->dr);
>>>>> +    writeb(0, &i2c_dev[i2c_bus_num]->sr);
>>>>> +    writeb(0, &i2c_dev[i2c_bus_num]->cr);
>>>>> +    writeb(I2C_CR_MEN, &i2c_dev[i2c_bus_num]->cr);
>>>>> +}
>>>>> +
>>>>>  static int
>>>>>  i2c_wait4bus(void)
>>>>>  {
>>>>>      unsigned long long timeval = get_ticks();
>>>>>      const unsigned long long timeout = usec2ticks(CONFIG_I2C_MBB_TIMEOUT);
>>>>>
>>>>> +    if (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB)
>>>>> +        i2c_set_idle();
>>>>> +
>>>>>      while (readb(&i2c_dev[i2c_bus_num]->sr) & I2C_SR_MBB) {
>>>>>          if ((get_ticks() - timeval) > timeout)
>>>>>              return -1;
>>>> Hmm.. I can;t find this for example in the MPC8360ERM.pdf, which
>>>> uses also this driver ... So I vote for activating this only,
>>>> if this driver is used for  __M68K__ ...
>>>>
>>>> Also, you write, that this sequence is necessary before normal
>>>> initialization code, but i2c_wait4bus() is called from i2c_read()
>>>> and i2c_write(), so the I2C bus is long ago initialized ...
>>>> or what do you mean with "normal initialization code" ?
>>>>
>>>> Also, whats with multimaster systems? Your code maybe cuts a running
>>>> data transmission. The MPC8360ERM.pdf manual says "check the MBB bit,
>>>> if the bus is free, before switching to master mode", thats what
>>>> actual code did ... so, I want this only activated, if __M68K__
>>>> is defined ...
>>> All true. This cannot go in as is. What you need is a I2C reset sequence
>>> as the above isn't enough in the general case. Michael, have a look at the
>>> kernel driver, it has some fixup code you could borrow.
>> Michael: Maybe you take a look in u-boot:board/keymile/common/common.c
>> i2c_init_board(), there is a I2C reset sequence for 83xx based boards
>> from keymile, which use this i2c driver.
>>
>> Maybe its time to move this code in the i2c driver?
> 
> this code looks good except I do not see how the special case of
> multi-master systems you mentioned is handled.

I have no experience with multimaster systems, and I think, this
special case is not yet factored in code.

> I think for multi-master a timeout would have to be implemented to
> wait for a possible other master transfer to finish before initiating
> the abort sequence, or is this too much time spent during init?

Or, it could be detected? If BB Bit is set and the SCL pin doesn;t
alter, the I2C bus must be blocked ... just an idea.

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany