I2C MAsync with SAMG55 working strangely (solved)

Go To Last Post
6 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

<PS...Note: I think I solved this mystery.  Looks to me like a race condition in the logic due to mis-ordering of operations.  The race is always lost.>

 

Dear reader,

Thanks in advance for any help with this.

Quick description of the problem;

Basically I am able to get io_writes and i2c_m_async_cmd_read() to work BUT I seem to have to use dummy io_write() calls in some places in my code to get I2C access working properly.  The dummy io_write is needed after changing the slave address and after using the repeated start mode to read back a register.  If I don't use the dummy io_write, it puts out the wrong slave adr or it hangs on the second read.

Hardware:

ATSAMG55 XPlained board; using EXT3 port (FLEXCOM4) in I2C/TWI master async mode at 50KHz SCL.

Custom PCB with Kionix KXCJ9 accelerometer and 2K ohm pullups on TWI pins.

Firmware/IDE: Atmel AS7 using START and ASF4 libraries

 

Details:

I have the boards connected to an oscilloscope so I can observe SCL and SDA.  I first initialize the I2C port and try to do a basic io_write transfers in master async mode.  Since, m_async mode does not block operation and uses call backs, I can later make a state machine that can keep checking and go and do other things while waiting for buffered reads.  Also, I will have two different devices sharing this I2C port so I want to change the slave address sometimes.  For now, I'm only working with one device at adr 0x0E.  The other device responds to 0x00 and 0x5a but isn't being used in the code.

Here is my relevant code.  < The forum code editor isn't working for me so I'm just pasting in the code here with different formatting>

The callbacks are simple and just set boolean flags true.  Bool flags are declared as volatile so the tests on them don't get optimized out.  When run, the TX and RX callbacks do happen and sort of when expected.  I never see the error callback happening (a good thing I suppose).  Also, I have disabled optimization just in case it was doing something I don't expect.

static struct io_descriptor *pio_TWIAFE;   // for I2C comms to AFE devices
static bool bTWIAFEWorking = false;
static bool volatile bTWITXCallbackFlag = false;
static bool volatile bTWIRXCallbackFlag = false;
static bool volatile bTWIErrorCallbackFlag = false;
#define TWI_START_TIMEOUT 10000
static int iTimeOutCtr;

/** ============================================================================
 * \brief cb_Default_AFE_TWI_tx_complete()
 *  DO NOT CALL THIS DIRECTLY!  This is registered to an event and is executed by that event.
 */
static void cb_Default_AFE_TWI_tx_complete(struct i2c_m_async_desc *const i2c)
{
     bTWITXCallbackFlag = true;
} // cb_Default_AFE_TWI_tx_complete

/** ============================================================================
 * \brief cb_Default_AFE_TWI_rx_complete()
 *  DO NOT CALL THIS DIRECTLY!  This is registered to an event and is executed by that event.
 */
static void cb_Default_AFE_TWI_rx_complete(struct i2c_m_async_desc *const i2c)
{
     bTWIRXCallbackFlag = true;
} // cb_Default_AFE_TWI_rx_complete

/** ============================================================================
 * \brief cb_Default_AFE_TWI_error()
 *  DO NOT CALL THIS DIRECTLY!  This is registered to an event and is executed by that event.
 */
static void cb_Default_AFE_TWI_error(struct i2c_m_async_desc *const i2c)
{
     bTWIErrorCallbackFlag = true;
} // cb_Default_AFE_TWI_error

 

Here is my startup function. This should initialize the I2C port and then do a few test write transfers and checks that it gets the callbacks. The code below writes to a control register on the accelerometer.  However, the mystery is why it needs a dummy io_write (in red).  This line causes a TX call back but there is no SCL generated as seen on a scope since I put the count to 0.  Without this line, the first io_write will be sent to address 0 instead of 0x0e.  I tried adding time delays instead of the dummy io_write.  So far only the dummy io_write seems to get this to work.  If I replace the dummy io_write with a valid write (count not equal to 0) then it will write out to slave address 0 instead of 0x0e.  With this code I see 6 TX callbacks and 5 I2C bus cycles.

/** ============================================================================
 * \brief iTWI3_Startup()
 * \param none
 * \return int 0 if success; other if fail
 *  Initialize the I2C port for communications with the AFE.
 *  On the ATSAMG55 XP board this is FLEXCOM4 using PB08 for Data and PB09 for CLK.
 *
 *  Clock rate is controlled in lower level config and controlled by START menus.
 *  For comms to the AFE; use TWI clock of 100KHz or lower.  Registers a generic
 *  callback for transfer completed.  This should be changed as needed depending on
 *  the device we are connecting to.
 *
 *  We are using the master async mode of the driver because we do not want it to
 *  wait and block during execution.
 */

static int iTWI3_Startup(void)
{
#define TWI_STARTUP_TEST_CYCLES 5
#define TWI_DUMMY_SLAVE 0x0e
    int ixI;
    bool bError = false;
    uint8_t caChar[2];

    caChar[0] = 0x1b;  // is a valid reg adr in the accelerometer
    caChar[1] = 0;
    i2c_m_async_get_io_descriptor(&AFE_TWI3, &pio_TWIAFE); // gets io_write and io_read method ptrs
    i2c_m_async_enable(&AFE_TWI3);
    i2c_m_async_register_callback(&AFE_TWI3, I2C_M_ASYNC_TX_COMPLETE, (FUNC_PTR)cb_Default_AFE_TWI_tx_complete);
    i2c_m_async_register_callback(&AFE_TWI3, I2C_M_ASYNC_RX_COMPLETE, (FUNC_PTR)cb_Default_AFE_TWI_rx_complete);
    i2c_m_async_register_callback(&AFE_TWI3, I2C_M_ASYNC_ERROR, (FUNC_PTR)cb_Default_AFE_TWI_error);

    // Run some dummy transfers on startup; this tests basic operation including whether
    // the pull-ups are working, a target board is attached, etc.
    i2c_m_async_set_slaveaddr(&AFE_TWI3, TWI_DUMMY_SLAVE, I2C_M_SEVEN);
    // documentation doesn't mention needing a dummy write after slaveadr change but testing shows
    // this prevents some problem in a delayed addr update (the old adr is sent 1st time after change

    // but is the correct adr after that)
   
io_write(pio_TWIAFE,&caChar,0); // dummy cycle after change of slave adr req'd?
    bTWIAFEWorking = true;  // optimistic assumption
    bTWIErrorCallbackFlag = false;
    for (ixI=0; ixI < TWI_STARTUP_TEST_CYCLES; ixI++) {
        if (true == bError) {
            break;
        }
        bTWITXCallbackFlag = false;
        io_write(pio_TWIAFE,&caChar,2);
        // check the call back to verify we got it
        iTimeOutCtr = 0;
        while ((false == bTWITXCallbackFlag) && (false == bError)) {
            if ((TWI_START_TIMEOUT <= ++iTimeOutCtr) || (true == bTWIErrorCallbackFlag)) {
                bTWIAFEWorking = false;
                bError = true;
            }
        } // while()

    } // for()

    if (false == bError) {
        return(0);
    } else {
        return(-1);
    }
 } // TWI3_Startup

 

So again, the mystery is why the dummy io_write() with a count of 0 is needed after a change to the slave address.  If I don't do it, then the prior slave address comes out again for the first access to the new address.  After that the correct address comes out.  

 

An additional problem; after getting the I2C bus verified, I attempt to configure the actual device on it.  Before that, I attempt to read back the an ID code in its register address 0x0c just so it tests that the device is working.  The code for that is just below.  The first read works but will hang on the second one if it doesn't have another dummy io_write between them and after the bus stop call.  The function i2c_m_async_cmd_read() is in hal_i2c_m_async.c and is generated by Atmel START.  Here is its prototype

 int32_t i2c_m_async_cmd_read(struct i2c_m_async_desc *const i2c, uint8_t reg, uint8_t *value);

It is actually supposed to do a bus stop at the end of the read but I'm not sure that is working either so I put in one explicitly.

 

     if (true == bTWIAFEWorking) {
          i2c_m_async_set_slaveaddr(&AFE_TWI3, 0x0e, I2C_M_SEVEN);
          caStringVar[0] = 0x1b;
          caStringVar[1] = 0;
          io_write(pio_TWIAFE,caStringVar,0); // dummy cycle after change of slave adr req'd
          bError = false;
          iTimeOutCtr = 0;
          while ((false == bTWITXCallbackFlag) && (false == bError)) {
              if ((TWI_START_TIMEOUT <= ++iTimeOutCtr) || (true == bTWIErrorCallbackFlag)) {
                  bError = true;
              }
          } // while()

          // read the device id from reg 0x0c; try it 10 times in a row

          if (!bError) {

              for (ixI = 0; ixI < 10; ixI++) { // doing multiple reads as test
                  bTWIRXCallbackFlag = false;
                  i2c_m_async_cmd_read(&AFE_TWI3,0x0c,caStringVar);
                  // check the rx call back to verify we got it
                  iTimeOutCtr = 0;
                  while (false == bTWIRXCallbackFlag) {
                      iTimeOutCtr++;
                      if ((TWI_START_TIMEOUT <= iTimeOutCtr) || (true == bTWIErrorCallbackFlag)) {
                          bError = true;
                          break; // while
                      }
                  } // while()
                  i2c_m_async_send_stop(&AFE_TWI3);   // terminate the bus communications
                 
// dummy write required here for some reason; else it hangs
                  io_write(pio_TWIAFE,caStringVar,0);

                  if (bError) {
                      break;  // for
                  }
              }  // for
          } // if (!bError)

 

I'm hoping someone has suggestions or found similar issues.  I'll probably have to start the process of reviewing what the generated code that controls the I2C transfers is doing.

Cheers.

Last Edited: Thu. Oct 12, 2017 - 06:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

I'm posting scope pics of what I see.  On the left is the first i2c write if I don't put in a dummy io_write after setting the initial slave address; the slave address sent is 0 but the next two bytes look OK.  On the right is what happens with the dummy_io_write.  All the values look good.  Top trace is SDA and bottom is SCL.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tracked this down to the file hpl_twi.c and function _i2c_m_irq_handler.  At the end of this it is looking for the TXCOMP bit to be set with a message length of 0.  Sending a dummy write would satisfy this condition.

 

static void _i2c_m_irq_handler(struct _i2c_m_async_device *dev)
{
    uint32_t sr = hri_twi_read_SR_reg(dev->hw) & hri_twi_read_IMR_reg(dev->hw);

    ASSERT(dev);

    if ((dev->service.msg.flags & I2C_M_BUSY) == 0) {
        return;
    }
    if (sr & TWI_SR_TXRDY) {
        _i2c_m_async_write(dev, &(dev->service.msg));
    } else if (sr & TWI_SR_RXRDY) {
        _i2c_m_async_read(dev, &(dev->service.msg));
    }

    /*
     * Any error occurs
     * NACK: A data or address byte has not been acknowledged by the slave
     * component. Set at the same time as TXCOMP
     * OVER, UNRE, ARBLST, TOUT, PECERR, LOCK,
     */
    if (dev->cb.error) {
        if (sr & TWI_SR_NACK) {
            dev->service.msg.flags &= ~I2C_M_BUSY;
            hri_twi_clear_IMR_reg(dev->hw, TWI_IDR_TXRDY | TWI_IDR_TXCOMP | TWI_IDR_RXRDY);
            dev->cb.error(dev, I2C_NACK);
        } else if (sr & TWI_SR_ARBLST) {
            dev->service.msg.flags &= ~I2C_M_BUSY;
            hri_twi_clear_IMR_reg(dev->hw, TWI_IDR_TXRDY | TWI_IDR_TXCOMP | TWI_IDR_RXRDY);
            dev->cb.error(dev, I2C_ERR_ARBLOST);
        } else if (sr & (TWI_SR_OVRE | TWI_SR_UNRE | TWI_SR_TOUT | TWI_SR_PECERR)) {
            dev->service.msg.flags &= ~I2C_M_BUSY;
            hri_twi_clear_IMR_reg(dev->hw, TWI_IDR_TXRDY | TWI_IDR_TXCOMP | TWI_IDR_RXRDY);
            dev->cb.error(dev, I2C_ERR_ARBLOST);
        }
    }

    /* Read/Write completed when TXCOMP set */
    
if ((sr & TWI_SR_TXCOMP) && (dev->service.msg.len == 0)) {
        dev->service.msg.flags &= ~I2C_M_BUSY;
        hri_twi_clear_IMR_reg(dev->hw, TWI_IDR_TXRDY | TWI_IDR_TXCOMP | TWI_IDR_RXRDY);
        if ((dev->service.msg.flags & I2C_M_RD) && dev->cb.rx_complete) {
            dev->cb.rx_complete(dev);
        } else if (dev->cb.tx_complete) {
            dev->cb.tx_complete(dev);
        }
    }
}

This is auto-generated code so I'm wondering if this is a bug or was intentional.  I see the TWI_SR_TXCOMP is set but the length isn't zero because I'm sending an address or reading data.  If I modify the code to only check the TWI_SR_TXCOMP bit and not the count then it will clear the busy flag too early in a multi-byte transfer.  This is also probably why I need to add a dummy write on the reads.  <Side note: if you go to the implementation of the TWI_SR_TXCOMP value it says "\deprecated Old style mask definition for 1 bit bitfield. Use TWI_SR_TXCOMP_Msk instead */".  Guess what deprecated means?  I thought the ASF4 code was all new?>

 

I see that the end of the function is what clears the I2C_M_BUSY flag and generates the TX or RX callbacks. So, this condition is generated by the intentional dummy write of count 0.

 

Now on to see

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have found that the auto-generated code from START/ASF4 has a problem where the function _i2c_m_async_transfer generates an interrupt as soon as the interrupt mask is enabled and before the slave address gets programmed.  The simple way to fix this would be to rearrange the order of the instructions.  Here is what I found in the generated code.

 

If the IMR reg is set AFTER the MMR by moving line 255 after the IF {.. MMR_reg...}.. then it works as expected.  I'm tempted to say this is a bug in the auto generated code.

 

I don't think the read portion of the function has the same issue because its interrupt shouldn't happen until the RX buffer is full which would only happen if there was old unread stuff there or until after the transfer.  Still, to be consistent its IMR reg configuration should probably also be moved to after the MMR and CR reg setups.

 

 

 

 

 

Last Edited: Fri. Oct 13, 2017 - 02:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'd like to point out that this is now fixed in the ASF4 code, very similarly to how you did it.

I'm working on another nasty ASF4 TWI issue right now where if there is a delay in sending the repeated start condition, the driver stops clocking the slave and a byte is never received.  I'll probably make a thread about it once I'm a bit more certain.  

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I just posted a new topic - I'm having a problem with SAME54 and master async ASF4 I2C driver.

 

https://www.avrfreaks.net/forum/asf4-atmel-start-i2c-masync-problem-works-12mhz-not-120mhz

 

Any chance you think I'm seeing a problem like the one in this post? My auto-generated code looks a little different than what you're showing.

 

Any tips appreciated!

pondzone