I2C & LPC1347 - Does the i2c.c::I2CStart() routine need a delay inserted (LPCXpresso board)?

[lpcware]

Content originally posted in LPCWare by mathseng on Wed Oct 09 16:39:58 MST 2013
Hi,
Does anyone have a better 'fix' for an issue I am facing, where to get reliable communications, I have inserted a delay in the I2Start() routine?

Description of the Problem:
On an LPCXpresso LPC1347 testbed, and using a SE95 temperature sensor wired exactly as per SE95 Section 14 Application Information, Fig 18 Typical application circuit, I am finding that the first READ of the SE95 succeeds, but all subsequent reads fail.

A solution which appears to solve the Problem:
I added line which re-uses the now discarded timeout variable:
  for (timeout = 0; timeout < STARTDELAY; ++timeout);
My Revised routine is below.

I2C communication is apparently reliable with a STARTDELAY value of 1000, for the simple testbed case and where the clock is not changed from default start value of 12MHz. I have set STARTDELAY to be 5000 in my code.

Why did I pick this particular solution?
Using the Microchip Pickit Serial Analzyer, I noted that the START lasts 300uS, while the LPC1347 uses timing which looks like the I2C timing diagram in the datasheet, Fig 24. I2C-bus pins clock timing (actually showing as 5.6uS).
I was surprised that this change gave desired results, since the START length is determined elsewhere.

Note: There is no NO visible difference in the timing display from applying a STARTDELAY - values as large as 500000 appearing to have no other effect than to ensure consistent I2C operation. Attached is a copy of the timing display, with STARTDELAY at 500000, and a timing display when STARTDELAY is 1 and the read fails on the second and subsequent reads.

Note #2: {unchecked and untested opinion :) }
I2CSlaveBuffer could possibly over-run in state 50.  i2c.c::void I2C_IRQHandler(void)
Data is read from register BEFORE checking whether number of bytes has been exceeded.


<code>
uint32_t I2CStart( void )
{
  uint32_t timeout = 0;
  uint32_t retVal = FALSE;

  /*--- Issue a start condition ---*/
  LPC_I2C->CONSET = I2CONSET_STA;/* Set Start flag */
   
  /*--- Wait until START transmitted ---*/
  while( 1 )
  {
if ( I2CMasterState == I2C_STARTED )
{
  retVal = TRUE;
  // found issue reading SE95 - this delay seemed to allow it to work
  for (timeout = 0; timeout < startDelay; ++timeout);
  break;
}
if ( timeout >= MAX_TIMEOUT )
{
  retVal = FALSE;
  break;
}
timeout++;
  }
  return( retVal );
}

</code>

[lpcware]

Content originally posted in LPCWare by Rob65 on Thu Oct 17 23:26:07 MST 2013
Bill,

thanks for sharing - I'll look into it this weekend.
Meanwhile, the test routines I use are placed here: http://bikealive.nl/img-for-others/LPCXpresso-forum/i2c-test.c
Not yet in the repository - I really need to look into these things a bit more.One of the nice things on Mercurial is that you can have local repositories where newer versions of the archive reside. One of the bad things on Mercurial is that you can have local repositories where newer versions of the archive reside.
I am afraid that my main archive, containing all code that belongs to this, is on a laptop at my parents place.

I am having a hard time understanding my own code - nothing new here I guess  ;-)  but this does mean that I need to write some proper documentation to go with this.

I started with this driver in 2010 when I got my LPCXpresso board. The original 'demo program' from NXP contained too much issues so I started writing this device driver, testing as much as possible with debugger and logic analyzer.

This driver is actually used by me and some of my customers  but I still cannot state that it will always work. One thing is that it is very hard to test improper bus sequences or arbitration problems when multiple bus masters are being used (I never actually was able to test this since I have no multi-master configuration).

Ah - taskYIELD() - watch out (it does pay of to buy a set of FreeRTOS manuals)
I used vTaskDelay(1) and not taskYIELD() in my spin loop. Reason for this is that a taskYIELD() will release the current task and the scheduler then switches to the task with the highest priority which is ready to run.
So if the task you are calling your I2CEngine() from is the highest priority task, this will result in a tight loop in that task.

The vTaskDelay() does add some delay but at least now the power is put to some useful tasks instead.

Rob

[lpcware]

Content originally posted in LPCWare by mathseng on Thu Oct 17 12:59:23 MST 2013
Hi Rob,
Thanks. After studying your code, I remembered that I had not put a taskYIELD() into my timeout loop. (I meant to, but forgot, truly :( )
I didn't feel the need for a semaphore since I am only calling the I2C code from a FreeRTOS task - so conflict is averted by only looping in that task and then delaying to allow other tasks to run if there is nothing pending.

In I2CStart(), I have a hardcoded delay, now reduced to 0xffff - if the state machine has not started in that time, there is definitely a failure, so I bug out.

The ONLY other spin loop, or timeout is in i2CEngine. My I2C code is not yet ready for publication, but here is the code for Engine.

Note: in case 0x50 I chose to test RdIndex+1 against I2CReadLength rather than test RdIndex against I2CReadLength-1, because if for some unknown reason I2CReadLength is 0, the system can die horribly.

Regards,
Bill

<code>
//-----------------------------------------------------------------------------
// External callbacks
// This code uses external callbacks, to allow for unchanged code use under
// FreeRTOS or stand-alone.
// Requirements:
//          TimerReset - call with number of milliSeconds before timer times out
//  TimerIsTimedOut - returns non-Zero if the timer has timed out
//  Yield - IF and ONLY IF there are other tasks which need to be serviced
//          then yield to them, else continue in spin loop.
//         This prevents this task from starving the other tasks.
//-----------------------------------------------------------------------------
extern void TimerReset(uint32_t timeLeft);
extern int  TimerIsTimedOut(void);
extern void Yield(void);


uint32_t I2CEngine(void)
{
    I2CMASTERSTATE currentState = I2CMasterState;
    I2CMasterState = I2C_IDLE;
    RdIndex = 0;
    WrIndex = 0;
    uint32_t result = FALSE;
    uint32_t done   = FALSE;

    TimerReset(2);

    if (I2CStart())
    {
        while (!done && !TimerIsTimedOut())
        {
            switch (I2CMasterState)
            {
            case I2C_RX_COMPLETE:
            case I2C_TX_COMPLETE:
                result = TRUE;
                done = TRUE;
                break;
            case I2C_FAIL:
            case I2C_ARBITRATION_LOST:
                done = TRUE;
                break;
            default:
                break;
            }

            if (currentState != I2CMasterState)
            {
                currentState = I2CMasterState;
                TimerReset(2);
            }
            Yield();
       }
   }

   I2CStop();

   return (result);
}

</code>

[lpcware]

Content originally posted in LPCWare by Rob65 on Thu Oct 17 07:00:45 MST 2013
You are not the only one banging your head on NXP's I2C code.
Is it not strange that the inventors of I2C do not give us a proper example of an I2C driver for their own microcontroller  :~

Have a look at: http://hg.bikealive.nl/Platform1754/file/ae0834ece8c9/Drivers/lpc17xx

The I2C driver given there has been tested with different I2C peripherals.
Maybe this one is more to your liking than the NXP example.

Ehm... example ... the repository currently only contains the i2c driver, not a matching example.
The interface differs a bit from the NXP one: no fixed buffers is the biggest difference.

I do still have some application code using an EEPROM - I'l dig this up this evening and see that it's placed in the repository as well.

Rob

[lpcware]

Content originally posted in LPCWare by mathseng on Mon Oct 14 15:55:01 MST 2013
Further info.
The proper answer to this question is :The example code is full of stuff which is misleading and IMHO incorrect, NXP, please CORRECT the EXAMPLE!!!!
so NO, there is NO need for delays in the I2C.c code from the LPCXpresso examples examples.lpc13xx.new.zip in v5.2 to 6.0.4

The primary reason the code fails is that the interrupt handler has a number of states, one of which DATA_NACK is used by the routine I2CEngine() to determine when the I2C communication is complete. The I2CEngine() tests fail early, which is why the inserted delay 'appeared' to work - it didn't affect the START time, but it prevented early loop termination for some conditions. My delay also had the side-effect of stretching the STOP time between I2C packets. The while loop in I2CEngine() also allows for infinite hangs when bus errors occur - yuk! See also the "while (1)" fatal error in i2ctest.c - this can never execute (why not - there is only 1 return from I2CInit()), and what is the reason for programmed hangs in an embedded device???? Yuk! Yuk! Yuk!

Modifying the states allows removal of timing issues and then allows timers to be used for invalid bus conditions and proper recovery. How to know the states - the doco on the I2C states is displayed in the LPC1347 UM - I found that (unlike experts) that I needed to really study the diagrams (14.10) and Software example (14.11).
- I now have my modified version of this I2C example running under FreeRTOS and getting reads/writes with 62uS between each packet in continuous comms - this 62uS time being the setup time for the operation. (I am still testing for use with other peripherals operating concurrently under FreeRTOS...)

Having appropriate handling of the I2C comms then allows verifiable handling of EEProm writing - the I2CTEST.C program puts in an empty for loop delaying for 200000 cycles. A better solution would be to use the Delay(uint32_t dlyTicks) from the LPCXpresso1347_systick_twinkle code, and reading the datasheet for the PCF8594 tells the delay time (eg 32mS) which needs to be programmed to allow the internal write to complete. (No more need for magic :) )

Regards,
Bill