This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

MSP430F5524: UCBBUSY being continuously set

Part Number: MSP430F5524

I am using I2C Module 0 (UCB0) to communicate with two slaves - EEPROM(24LC64) and a temperature sensor(Si7060). I2C Write and Reads from the temperature sensor are being done continuously. Occasional I2C reads are failing as they do not respond in time, but it is okay by me as the next reads work.

But After 10-15 min(one in 10000 or more reads), the UCBBUSY bit gets set. The SDA line is continuously held low. My code is waiting in a while loop for the bit to get cleared which is not happening and thus the code gets stuck. Writes are working fine. I have used the same code which was being used for I2C Module 1(UCB1) which is having no problem.

(1) Why is this happening? I have observed on a CRO that the CLK signal is looking like a sine wave. Is the insufficient hold time leading to the slave pulling the data line low to signal an ACK and never releasing it? In this case changing I2C clock frequency solve the issue, right? Current clock frequency is about 133KHz(16Mhz/120)

(2)When the UCBBUSY is getting set I tried resetting the I2C Module by doing 

UCB0CTL1 = UCSWRST;

__delay_cycles(1000);

UCB0CTL1 &= ~UCSWRST;

Though this reset the module and the SDA line back to the high voltage level, this is leading to unsuccessful I2C Reads continuously in certain cases. Is what I have attempted to correct? If not, how else can I solve this problem?

We are in a critical stage during production. Any help would be dearly appreciated. Please find the code attached.

Thanks

Abhishek

i2cCode.c
uint08 i2cUcb0ByteRead(uint08 device_addr, uint08* write_data, uint08 num_bytes_write, uint08* read_data, uint08* bytes_written, uint08* bytes_read, uint08 timeout)
{
      /*
       * function to do random reads from EEPROM, Temperature Sensor, Digital Audio Amp
       * It involves first sending a write request followed by a read
       *
       */
      uint08 status;
      uint08* dataPtr = write_data;
      uint08 i;
      uint16 currentSr;
      uint08 intDisabled = 0;

      // check for argument validity
      if ( timeout > 500 )    //65, increased for testing
        return I2C_INVALID_TIMEOUT;

      // since this version is polled, we don't want an interrupt firing
      //  and the ISR executing
      currentSr = __get_SR_register();
      if ( currentSr & GIE ) {
        intDisabled = 1;
        __disable_interrupt();
      }

      // turn on the timer
      //TA1CTL |= MC_2;
      i2c0BaselineTimer = TA0R;

      // write the slave address and configure i2c hardware
      UCB0CTL1 = UCSWRST;
      UCB0CTL1 = UCSSEL_2 + UCSWRST;
      UCB0I2CSA = device_addr>>1;
      UCB0CTL1 &= ~UCSWRST;

      // enable module interrupts and start the transfer
      UCB0IE |= UCNACKIE;
      UCB0IE |= UCTXIE;
      //IE2 |= UCB1TXIE;
      UCB0CTL1 |= UCTR + UCTXSTT;  //ideally, it should be done before clearing UCSWRST


      if(1) //device_addr == SI7060_SLAV_ADDR
      {
          __delay_cycles(2000); //delay increased for testing
      }

      //sendUserMessage("Read function, ST sent\n\r");
      *bytes_written = 0;

      //check for NAK?
      status = i2cUcb0CheckNakTimeout(0,1);
      if(status != 0)
      {

          i2cUcb0Cleanup();
          //sendUserMessage("NAK after sending ST\n\r");
          if (intDisabled)
              __enable_interrupt();
          return status;
      }
      //Write Address High and Address Low Bytes

      for (i = 0; i < num_bytes_write; i++)
      {    //can count down
             // spin until transmit buffer needs to be reloaded
            while ((UCB0IFG & UCTXIFG) == 0)   //is this checking for ACK?
            {
                status = i2cUcb0CheckNakTimeout(timeout, 1);
                if (status != 0)
                {
                    i2cUcb0Cleanup();
                    if (intDisabled)
                        __enable_interrupt();
                    return status;
                }
            }

            //UCB1IV &=~USCI_I2C_UCTXIFG;
            // load another data byte
            UCB0TXBUF = *dataPtr;

            // increment actual byte counter
            *bytes_written = *bytes_written + 1;

            // increment the pointer address
            dataPtr++;

            // take a new baseline timer value
            i2cBaselineTimer = TA0R;

            //sendUserMessage("wrote to txbuf\n\r");
      }

      //Check for ACK ?
      while ((UCB0IFG & UCTXIFG) == 0) {
          status = i2cUcb0CheckNakTimeout(timeout, 1);

          if ( status != 0 ) {
            //sendUserMessage("NAK after writing to txbuf\n\r");
            i2cUcb0Cleanup();
            if ( intDisabled )
              __enable_interrupt();
            return status;
          }
        }

      //Send START followed by ControlByte for Receiver Mode
      UCB0CTL1 &= ~(UCTR);  //Clear the last bit -- is this required?
      UCB0I2CSA = device_addr>>1;
      UCB0CTL1 |= UCTXSTT;  //Check if indeed receiver Mode

      //No. of bytes to be read is 1

      //check NACK?
      status = i2cUcb0CheckNakTimeout(0,1);
      if (status != 0)
      {
          //sendUserMessage("NAK after sending repeated start\n\r");
          i2cUcb0Cleanup();
          if (intDisabled)
              __enable_interrupt();
          return status;
      }


      // spin until start condition cleared
    while (UCB0CTL1 & UCTXSTT)
    {
        status = i2cUcb0CheckNakTimeout(timeout, 1);

        if (status != 0)
        {
            i2cUcb0Cleanup();
            if (intDisabled)
                __enable_interrupt();
            return status;
        }
    }

    //wait for data to be received
    while ((UCB0IFG & UCRXIFG) == 0)
    {
        status = i2cUcb0CheckNakTimeout(timeout, 1);

        if (status != 0)
        {
            i2cUcb0Cleanup();
            if (intDisabled)
                __enable_interrupt();
            return status;
        }
    }

    UCB0CTL1 |= UCTXNACK;       //send NACK after receiving a byte
    UCB0CTL1 |= UCTXSTP;        //finally send STOP

    *read_data = UCB0RXBUF;
    *bytes_read = 1;

    // turn off the timer to save power
    while (UCB0STAT & UCBBUSY){};
    //TA1CTL &= ~MC_2;
    UCB0IFG &= ~UCTXIFG; //to cleasr TXIFG
    UCB0IE &= ~UCNACKIE;
    UCB0IE &= ~UCRXIE;
    //UCB0I2CIE &= ~UCNACKIE;
    //IE2 &= ~UCB0RXIE;

    //sendUserMessage("Read successful");
    // no errors, all bytes received
    if (intDisabled)
        __enable_interrupt();
    return 0;

}

uint08 i2cUcb0CheckNakTimeout(uint08 timeout, uint08 check_nak)
/
 * return   0 - Completed successfully
 *          1 - Slave NAck'ed
 *          2 - Slave did not respond before timeout period expired
 *
 */
{
  uint16 currentTimerVal;
  uint16 timerDiff;

  // check for NAK
  if ( check_nak ) {
    if ( UCB0IFG & UCNACKIFG ) {
      // send a stop and clear the interrupt flag
        UCB0CTL1 |= UCTXSTP;
         while (UCB0STAT & UCBBUSY){};
        //UCB1IV &= ~USCI_I2C_UCNACKIFG;
         UCB0IFG &= ~UCNACKIFG;
        //TA1CTL &= ~MC_2;
        UCB0CTL1 = UCSWRST;
        return I2C_NO_ACK;
    }
  }

  // do we need to check for timeout?
  if ( timeout != 0 ) {
    currentTimerVal = TA0R;

    // check for TA1R rollover since we baselined
    if ( currentTimerVal > i2c0BaselineTimer )
      timerDiff = currentTimerVal - i2c0BaselineTimer;
    else
      timerDiff = (65535 - i2c0BaselineTimer) + currentTimerVal;

    // Current TimerA0 frequency is 32768Hz (ACLK with ID_0)
    // A timer count of 33 corresponds to 1ms

    if((timerDiff >>10)  >= (timeout)) {
      // generate a stop and return the error flag
      //TA1CTL &= ~MC_2;
      UCB0CTL1 |= UCTXSTP;

      //while(UCB0STAT & UCBBUSY);   OLD CODE
 
      //My Change to solve the issue
      if(UCB0STAT & UCBBUSY)
      {
          testBusy++;
          UCB0CTL1 = UCSWRST;
          __delay_cycles(1000);
          UCB0CTL1 &= ~UCSWRST;
      }


      //while (UCB0STAT & UCBBUSY){};



      return I2C_TIMEOUT;
    }
  }

  // nak or timeout did not occur
  return 0;
}

void i2cUcb0Cleanup(void)
/**
 * Cleanup after a failed transaction. Turn off the timer, disable interrupts,
 * and put the I2C hardware into reset.
 *
 */
{
  // turn off the timer to save power
  //TA1CTL &= ~MC_2;

  // clear interrupt flags
  UCB0IFG &= ~UCTXIFG;
  //IFG2 &= ~UCB0TXIFG;

  // no errors, all bytes transmitted
  UCB0IE &= ~UCNACKIE;
  UCB0IE &= ~UCTXIE;
  //UCB0I2CIE &= ~UCNACKIE;
  //IE2 &= ~UCB0TXIE;

  // put i2c hardware into reset
  UCB0CTL1 = UCSWRST;
}

  • You say that the clock signal looks like a sine. The rise time is controlled by your resistor pullup. The fall time depends on the ability of the MSP430 output to pull it low. If both rise and fall times are long, then you have excessive capacitance on the line.

    Missing from your code is initialization for the I/O pins. You might want to double check that.

  • Hi David

    Do you think the hold time is more of a problem rather than the rise or fall time? Any idea why the hold time is less?

    This is the initialisation I am doing in main.c:

    P3SEL  |=  (MCU_UCB0_SDA|MCU_UCB0_SCL);   // where MCU_UCB0_SDA = 0x1, MCU_UCB0_SCL=0x2

    Given that we cannot change the hardware design now, can you think of any other firmware hack through which I could solve the problem? Failed I2C reads are okay. I just don't want the SDA line to be continuously held low.

    Thanks

    Abhishek

  • I suggest that you read TI's slva689 and verify that you are meeting the I2C signal rise and fall time specifications. If not, you will have to change something. (Pullup resistors and/or F5524 output drive strength.)

    Perhaps a slower clock will be enough.

  • Hi David

    I shall try reading that. What do you mean by changing FF24 output drive strength?

    I want to report something that I observed today. I tried outputting a series of log statements onto PuTTY through UART  in the read,write functions. Eg: Start sent, ACK received, write successful etc. On doing this I was able to successfully run the code for 2-3 hours at a stretch. Now this leads me to conclude that adding delay through __delay_cycles( ) might well be  a suitable workaround to the issue I am facing. The CPU clock speed is 16MHz. This could be too fast for the slave to respond to, right? The I2C clock speed is about 133KHz , so that should be fine. Could I get your opinion on this? In your comment on a slower clock doing the trick, were you referring to the CPU clock or the I2C clock?

    Thanks

    Abhishek

  • See the description in the manual on the drive strength register. (PxDS)

    I cannot follow that middle part very well. It sounds as though you are pausing in the middle of the I2C transaction to send data over a separate serial port. This would alter the inter-character timing but not do anything to the timing within each byte.

    I was of course referring to the I2C clock. If your signal rise and fall times really are as bad as described, you need to slow the clock down.  With a 133Khz clock you should be meeting the fast mode specifications.

  • Hi, Abhishek, 

    I agree with David to slow down the I2C clock speed to relax the timing. This is the software workaround you can try. Do you have tried this and if it works well? 

    If you think David recommendation help to resolve your questions, please click on the This resolved my issue  button to close this thread.  

    Best Regards, 

    Lixin 

  • Hi David, Lixin

    Firstly, Apologies for the delay in replying. I have been very busy at work. 

    The pausing in the middle of the I2C transaction to send data over a separate serial port somehow helped in solving the problem following which I have left it like that. But I am facing another issue with respect to the I2C code. We are in production mode right now and for certain MSP430s, I am unable to pause the code using the programmer after 2-3 hours. The code stops running and upon pausing I get the message ""Break at address "0xd00c4" with no debug information available, or outside of program code". For certain other ICs, the code runs fine for over 8 hours. For some, on the other hand, it fails within a minute.

    Could you please look at my code to see if there is something wrong that I am doing? Maybe there is an issue in the order of writing registers or something that is acceptable but not perfect because the code is giving inconsistent behaviour.

    Thanks and Regards

    1513.i2cCode.c
    uint08 i2cUcb1PolledWrite(uint08 device_addr, uint08* write_data, uint08 num_bytes, uint08* bytes_written, uint08 timeout)
    /**
     * Writes data to the specified device address
     *
     * @param   device_addr   - I - 7 Bit device Address
     * @param   write_data    - I - Pointer to data buffer to be written to slave
     * @param   num_bytes     - I - Number of bytes to be written
     * @param   bytes_written - O - Actual number of bytes written to slave
     * @param   timeout       - I - Timeout max (in msec) between each transmitted byte
     *                              Timeout=0, permits an infinite timeout (not recommended). Max value = 65
     * @return  0 - Completed successfully 
     *          I2C_NO_ACK - Slave NAck'ed            
     *          I2C_WRITE_TIMEOUT - Slave did not respond before timeout period expired
     *          I2C_INVALID_TIMEOUT - timeout parameter is greater than 65
     *
     */
    {
      uint08 status;
      uint08* dataPtr = write_data;
      uint08 i;
      uint16 currentSr;
      uint08 intDisabled = 0;
      
      // check for argument validity
      if ( timeout > 65 )
        return I2C_INVALID_TIMEOUT;
        
      // since this version is polled, we don't want an interrupt firing
      //  and the ISR executing
      currentSr = __get_SR_register();
      if ( currentSr & GIE ) {
        intDisabled = 1;
        __disable_interrupt();
      }
      
      // turn on the timer
      TBCTL |= MC_2;
      i2cBaselineTimer = TBR;
      
      // write the slave address and configure i2c hardware
      UCB1CTL1 = UCSWRST;
      UCB1CTL1 = UCSSEL_2 + UCSWRST;
      UCB1I2CSA = device_addr>>1;
      UCB1CTL1 &= ~UCSWRST;
    
      // enable module interrupts and start the transfer
      UCB1IE |= UCNACKIE;
      UCB1IE |= UCTXIE;
      //IE2 |= UCB1TXIE;
      UCB1CTL1 |= UCTR + UCTXSTT;
      
      *bytes_written = 0;
      
      // special handling of 1 byte case
      if ( num_bytes == 1 ) {
        while ( (UCB1IFG & UCTXIFG) == 0 ) {
          status = i2cUcb1CheckNakTimeout(timeout, 1);
          
          if ( status != 0 ) {
            i2cUcb1Cleanup();
            if ( intDisabled )
              __enable_interrupt();
            return status;
          }
        }
        
        UCB1TXBUF = *dataPtr;
        while ( (UCB1IFG & UCTXIFG) == 0 ) {
          status = i2cUcb1CheckNakTimeout(timeout, 1);
          
          if ( status != 0 ) {
            i2cUcb1Cleanup();
            if ( intDisabled )
              __enable_interrupt();
            return status;
          }
        }
        *bytes_written = 1;
      }
      else {
        for ( i=0; i<num_bytes; i++ ) {    //can count down
          // spin until transmit buffer needs to be reloaded
          while ( (UCB1IFG & UCTXIFG) == 0 ) {
            status = i2cUcb1CheckNakTimeout(timeout, 1);
            
            if ( status != 0 ) {
              i2cUcb1Cleanup();
              if ( intDisabled )
                __enable_interrupt();
              return status;
            }
          }
          
          //UCB1IV &=~USCI_I2C_UCTXIFG;
          // load another data byte
          UCB1TXBUF = *dataPtr;
          
          // increment actual byte counter
          *bytes_written = *bytes_written + 1;
          
          // increment the pointer address
          dataPtr++;
          
          // take a new baseline timer value
          i2cBaselineTimer = TBR;
        }
      }
      
      // wait until the last byte has been transferred
      while ( (UCB1IFG & UCTXIFG) == 0 ) {
        status = i2cUcb1CheckNakTimeout(timeout, 1);
        
        if ( status != 0 ) {
          i2cUcb1Cleanup();
          if ( intDisabled )
            __enable_interrupt();
          return status;
        }
      }
      // generate a stop
      UCB1CTL1 |= UCTXSTP;
      //while (UCB1STAT & UCBBUSY){}
      //Trying to solve I2C 1 stuck issue
    
      if(UCB1STAT & UCBBUSY)
      {
    
          __delay_cycles(10000);  //Wait
          if (UCB1STAT & UCBBUSY)
          {
              testI2c1Busy++;
              UCB1CTL1 = UCSWRST;
              __delay_cycles(1000);
              UCB1CTL1 &= ~UCSWRST;
    
          }
      }
    
    
      // turn off the timer to save power
      TBCTL &= ~MC_2;
    
      UCB1IFG &= ~UCTXIFG;
    
      // no errors, all bytes transmitted
      UCB1IE &= ~UCNACKIE;
      UCB1IE &= ~UCTXIE;
      if ( intDisabled )
        __enable_interrupt();
      return 0;
    }
    
    uint08 i2cUcb0PolledWrite(uint08 device_addr, uint08* write_data, uint08 num_bytes, uint08* bytes_written, uint08 timeout)
    /**
     * Writes data to the specified device address
     *
     * @param   device_addr   - I - 7 Bit device Address
     * @param   write_data    - I - Pointer to data buffer to be written to slave
     * @param   num_bytes     - I - Number of bytes to be written
     * @param   bytes_written - O - Actual number of bytes written to slave
     * @param   timeout       - I - Timeout max (in msec) between each transmitted byte
     *                              Timeout=0, permits an infinite timeout (not recommended). Max value = 65
     * @return  0 - Completed successfully
     *          I2C_NO_ACK - Slave NAck'ed
     *          I2C_WRITE_TIMEOUT - Slave did not respond before timeout period expired
     *          I2C_INVALID_TIMEOUT - timeout parameter is greater than 65
     *
     */
    {
        uint08 status;
        uint08* dataPtr = write_data;
        uint08 i;
        uint16 currentSr;
        uint08 intDisabled = 0;
    
        // check for argument validity
        if (timeout > 1000)    //65 increased for testing
            return I2C_INVALID_TIMEOUT;
    
        // since this version is polled, we don't want an interrupt firing
        //  and the ISR executing
        currentSr = __get_SR_register();
        if (currentSr & GIE)
        {
            intDisabled = 1;
            __disable_interrupt();
        }
    
        // turn on the timer
        //TA1CTL |= MC_2;           use TA0R
        i2c0BaselineTimer = TA0R;
    
        // write the slave address and configure i2c hardware
        UCB0CTL1 = UCSWRST;
        UCB0CTL1 = UCSSEL_2 + UCSWRST;   //UCSSEL_2 by default
        UCB0I2CSA = (device_addr >> 1);
    
        //uint08 dAddr=device_addr, userAddr;
        ///intToStr((dAddr>>1),&userAddr);
        //sendUserMessage("device addr: ");
        //sendUserMessage(&userAddr);
    
        UCB0CTL1 &= ~UCSWRST;
    
        // enable module interrupts and start the transfer
        UCB0IE |= UCNACKIE;
        UCB0IE |= UCTXIE;
        //IE2 |= UCB1TXIE;
        UCB0CTL1 |= UCTR + UCTXSTT;
        //UCB0TXBUF = *dataPtr;        //for testing
    
        //temp sensor requires some delay to process comands
        if(device_addr == SI7060_SLAV_ADDR)
        {
            __delay_cycles(750);
        }
    
        sendUserMessage("Write Start sent\n\r");
        __delay_cycles(4000);
        //for debugging
        if(UCB0IFG & UCNACKIFG)
        {
            sendUserMessage("NACK after sending start\n\r");
            __delay_cycles(4000);
        }
        else
        {
            sendUserMessage("No NACK after sending start\n\r");
            __delay_cycles(4000);
        }
        *bytes_written = 0;
    
        // special handling of 1 byte case
        if(num_bytes == 1)
        {
            while ((UCB0IFG & UCTXIFG) == 0)
            {
                // when while loop is exited, it means transfer buffer is empty
                status = i2cUcb0CheckNakTimeout(timeout, 1);
    
                if (status != 0)
                {
                    i2cUcb0Cleanup();
                    if (intDisabled)
                        __enable_interrupt();
                    return status;
                }
            }
    
            UCB0TXBUF = *dataPtr;
            while ((UCB0IFG & UCTXIFG) == 0)
            {
                status = i2cUcb0CheckNakTimeout(timeout, 1);
    
                if (status != 0)
                {
                    i2cUcb0Cleanup();
                    if (intDisabled)
                        __enable_interrupt();
                    return status;
                }
            }
            *bytes_written = 1;
        }
        else
        {
            for (i = 0; i < num_bytes; i++)
            {         //can count down
                // spin until transmit buffer needs to be reloaded
                while((UCB0IFG & UCTXIFG) == 0)
                {
                    status = i2cUcb0CheckNakTimeout(timeout, 1);
    
                    if (status != 0)
                    {
    
                        char c;
                        intToStr(status, &c);
                        char arrC[2];
                        arrC[0] = c;
                        arrC[1] = '\0';
                        sendUserMessage("WritE failed, status: \r");
                        __delay_cycles(4000);
                        sendUserMessage(arrC);
                        sendUserMessage("\n\r");
    
                        i2cUcb0Cleanup();
                        if (intDisabled)
                            __enable_interrupt();
                        return status;
                    }
                }
    
                //UCB1IV &=~USCI_I2C_UCTXIFG;
                // load another data byte
                UCB0TXBUF = *dataPtr;
    
                sendUserMessage("wrote to txbuf: \r");
                __delay_cycles(4000);
    
                char d = 0;
                intToStr((*dataPtr), &d);
                char arrD[2];
                arrD[0] = d;
                arrD[1] = '\0';
                sendUserMessage(arrD);
                __delay_cycles(4000);
                sendUserMessage("\n\r");
    
                // increment actual byte counter
                *bytes_written = *bytes_written + 1;
    
                // increment the pointer address
                dataPtr++;
    
                // take a new baseline timer value
                i2c0BaselineTimer = TA0R;
            }
        }
    
        // wait until the last byte has been transferred
        while ((UCB0IFG & UCTXIFG) == 0)
        {
            status = i2cUcb0CheckNakTimeout(timeout, 1);
    
            if (status != 0)
            {
                __delay_cycles(4000);
                sendUserMessage("Failed after sending last byte, status: \n\r");
                char d = 0;
                intToStr((*dataPtr), &d);
                char arrD[2];
                arrD[0] = d;
                arrD[1] = '\0';
                sendUserMessage(arrD);
                i2cUcb0Cleanup();
                if (intDisabled)
                    __enable_interrupt();
                return status;
            }
        }
        // generate a stop
        UCB0CTL1 |= UCTXSTP;
        //while(UCB0STAT & UCBBUSY){};
    
    
        if(UCB0STAT & UCBBUSY)
        {
            __delay_cycles(10000); //Wait
            if(UCB0STAT & UCBBUSY)
            {
                testI2c0Busy++;
                UCB0CTL1 = UCSWRST;
                __delay_cycles(1000);
                UCB0CTL1 &= ~UCSWRST;
                //break;
            }
        }
    
        // turn off the timer to save power
        //TA1CTL &= ~MC_2;
    
        UCB0IFG &= ~UCTXIFG;
    
        // no errors, all bytes transmitted
        UCB0IE &= ~UCNACKIE;
        UCB0IE &= ~UCTXIE;
        if (intDisabled)
            __enable_interrupt();
        sendUserMessage("WRITE SUCCESSFUL\n\r");
        __delay_cycles(4000);
    
        return 0;
    }
    
    uint08 i2cUcb1PolledRead(uint08 device_addr, uint08* read_data, uint08 num_bytes, uint08* bytes_read, uint08 timeout)
    /**
     * Reads data from the specified device address
     *
     * @param   device_addr   - I - 7 Bit device Address
     * @param   read_data     - I - Pointer to buffer to hold received data from slave
     * @param   num_bytes     - I - Number of bytes to be read
     * @param   bytes_read    - O - Actual number of bytes read from slave
     * @param   timeout       - I - Timeout max (in msec) between each received byte
     *                              Timeout=0, permits an infinite timeout (not recommended). Max value = 65
     * @return  0 - Completed successfully 
     *          I2C_NO_ACK - Slave NAck'ed            
     *          I2C_TIMEOUT - Slave did not respond before timeout period expired
     *          I2C_INVALID_TIMEOUT - timeout parameter is greater than 65
     *
     */
    {
      uint08 status;
      uint08* dataPtr = read_data;
      uint08 i;
      uint16 currentSr;
      uint08 intDisabled = 0;
      
      // check for argument validity
      if ( timeout > 65 )
        return I2C_INVALID_TIMEOUT;
        
      // since this version is polled, we don't want an interrupt firing
      //  and the ISR taking over
      currentSr = __get_SR_register();
      if ( currentSr & GIE ) {
        intDisabled = 1;
        __disable_interrupt();
      }
        
      // turn on the timer
      TBCTL |= MC_2;
      i2cBaselineTimer = TBR;
    
      // write the slave address and kick off the transfer
    
      UCB1CTL1 = UCSWRST;
      UCB1CTL1 = UCSSEL_2 + UCSWRST;
      UCB1I2CSA = device_addr>>1;
      UCB1CTL1 &= ~UCSWRST;
      UCB1IE |= UCNACKIE;
      //IE2 |= UCB0RXIE;
    
      UCB1CTL1 |= UCTXSTT;
        
      if ( num_bytes == 1 ) {
        // spin until start condition cleared
        while ( UCB1CTL1 & UCTXSTT ) {
          status = i2cUcb1CheckNakTimeout(timeout, 1);
          
          if ( status != 0 ) {
            i2cUcb1Cleanup();
            if ( intDisabled )
              __enable_interrupt();
            return status;
          }
        }
        UCB1CTL1 |= UCTXSTP;
        while ( (UCB1IFG & UCRXIFG) == 0 ) {
          status = i2cUcb1CheckNakTimeout(timeout, 1);
          
          if ( status != 0 ) {
            i2cUcb1Cleanup();
            if ( intDisabled )
              __enable_interrupt();
            return status;
          }
        }
        *read_data = UCB1RXBUF;
        *bytes_read = 1;
      } else {
        *bytes_read = 0;
        
        for ( i=0; i<num_bytes; i++ ) {
          // spin until data is available
          while ( (UCB1IFG & UCRXIFG) == 0 ) {
            status = i2cUcb1CheckNakTimeout(timeout, 1);
            
            if ( status != 0 ) {
              i2cUcb1Cleanup();
              if ( intDisabled )
                __enable_interrupt();
              return status;
            }
          }
          
          // increment actual byte counter
          *bytes_read = *bytes_read + 1;
          
          // copy received byte
          *dataPtr = UCB1RXBUF;
          
          // increment the pointer address
          dataPtr++;
          
          // take a new baseline timer value
          i2cBaselineTimer = TBR;
      
          // send stop after the next byte
          if ( i == (num_bytes-2) )
        	  UCB1CTL1 |= UCTXSTP;
        }
      }    
      
      // turn off the timer to save power
      //while (UCB1STAT & UCBBUSY){}
    
      if(UCB1STAT & UCBBUSY)
        {
    
            __delay_cycles(10000);  //Wait
            if (UCB1STAT & UCBBUSY)
            {
                testI2c1Busy++;
                UCB1CTL1 = UCSWRST;
                __delay_cycles(1000);
                UCB1CTL1 &= ~UCSWRST;
    
            }
        }
      TBCTL &= ~MC_2;
      UCB1IE &= ~UCNACKIE;
      UCB1IE &= ~UCRXIE;
      //UCB0I2CIE &= ~UCNACKIE;
      //IE2 &= ~UCB0RXIE;
      
      // no errors, all bytes received
      if ( intDisabled )
        __enable_interrupt();
      return 0;
    }
    
    uint08 i2cUcb0ByteRead(uint08 device_addr, uint08* write_data, uint08 num_bytes_write, uint08* read_data, uint08* bytes_written, uint08* bytes_read, uint08 timeout)
    {
          /*
           * function to do random reads from EEPROM, Temperature Sensor, Digital Audio Amp
           * It involves first sending a write request followed by a read
           *
           */
        uint08 status;
        uint08* dataPtr = write_data;
        uint08 i;
        uint16 currentSr;
        uint08 intDisabled = 0;
    
        // check for argument validity
        if (timeout > 500)    //65, increased for testing
            return I2C_INVALID_TIMEOUT;
    
        // since this version is polled, we don't want an interrupt firing
        //  and the ISR executing
        currentSr = __get_SR_register();
        if (currentSr & GIE)
        {
            intDisabled = 1;
            __disable_interrupt();
        }
    
        // turn on the timer
        //TA1CTL |= MC_2;
        i2c0BaselineTimer = TA0R;
    
        // write the slave address and configure i2c hardware
        UCB0CTL1 = UCSWRST;
        UCB0CTL1 = UCSSEL_2 + UCSWRST;
        UCB0I2CSA = device_addr >> 1;
        UCB0CTL1 &= ~UCSWRST;
    
        // enable module interrupts and start the transfer
        UCB0IE |= UCNACKIE;
        UCB0IE |= UCTXIE;
        //IE2 |= UCB1TXIE;
        UCB0CTL1 |= UCTR + UCTXSTT; //ideally, it should be done before clearing UCSWRST
    
        if(1) //device_addr == SI7060_SLAV_ADDR
        {
            __delay_cycles(2000); //delay increased for testing
        }
    
    
        sendUserMessage("Read function, ST sent\n\r");
    
        __delay_cycles(1000);
        *bytes_written = 0;
    
        //check for NAK?
        status = i2cUcb0CheckNakTimeout(0, 1);
        if(status != 0)
        {
    
            i2cUcb0Cleanup();
            sendUserMessage("NAK after sending ST\n\r");
            __delay_cycles(1000);
            if (intDisabled)
                __enable_interrupt();
            return status;
        }
        //Write Address High and Address Low Bytes
    
        for(i = 0;i < num_bytes_write;i++)
        {    //can count down
             // spin until transmit buffer needs to be reloaded
            while ((UCB0IFG & UCTXIFG) == 0)   //is this checking for ACK?
            {
                status = i2cUcb0CheckNakTimeout(timeout, 1);
                if(status != 0)
                {
                    i2cUcb0Cleanup();
                    if (intDisabled)
                        __enable_interrupt();
                    return status;
                }
            }
    
            //UCB1IV &=~USCI_I2C_UCTXIFG;
            // load another data byte
            UCB0TXBUF = *dataPtr;
    
            // increment actual byte counter
            *bytes_written = *bytes_written + 1;
    
            // increment the pointer address
            dataPtr++;
    
            // take a new baseline timer value
            i2cBaselineTimer = TA0R;
    
            //sendUserMessage("wrote to txbuf\n\r");
            __delay_cycles(1000);
        }
    
        //Check for ACK ?
        while((UCB0IFG & UCTXIFG) == 0)
        {
            status = i2cUcb0CheckNakTimeout(timeout, 1);
    
            if(status != 0)
            {
                sendUserMessage("NACK after writing to txbuf\n\r");
                __delay_cycles(1000);
                i2cUcb0Cleanup();
                if (intDisabled)
                    __enable_interrupt();
                return status;
            }
        }
    
        //Send START followed by ControlByte for Receiver Mode
        UCB0CTL1 &= ~(UCTR);  //Clear the last bit -- is this required?
        UCB0I2CSA = device_addr >> 1;
        UCB0CTL1 |= UCTXSTT;  //Check if indeed receiver Mode
    
        //No. of bytes to be read is 1
    
        //check NACK?
    
        status = i2cUcb0CheckNakTimeout(0, 1);
        if(status != 0)
        {
            sendUserMessage("NACK after sending repeated start\n\r");
            __delay_cycles(1000);
            i2cUcb0Cleanup();
            if (intDisabled)
                __enable_interrupt();
            return status;
        }
    
        // spin until start condition cleared
        while(UCB0CTL1 & UCTXSTT)
        {
            status = i2cUcb0CheckNakTimeout(timeout, 1);
    
            if(status != 0)
            {
                sendUserMessage("Read Failed waiting for st clear\n\r");
                __delay_cycles(1000);
                i2cUcb0Cleanup();
                if (intDisabled)
                    __enable_interrupt();
                return status;
            }
        }
    
        //wait for data to be received
        while((UCB0IFG & UCRXIFG) == 0)
        {
            status = i2cUcb0CheckNakTimeout(timeout, 1);
    
            if(status != 0)
            {
                i2cUcb0Cleanup();
                if (intDisabled)
                    __enable_interrupt();
                return status;
            }
        }
    
        UCB0CTL1 |= UCTXNACK;       //send NACK after receiving a byte
        UCB0CTL1 |= UCTXSTP;        //finally send STOP
    
        *read_data = UCB0RXBUF;
        *bytes_read = 1;
    
        // turn off the timer to save power
        //while (UCB0STAT & UCBBUSY){}
    
    
        if(UCB0STAT & UCBBUSY)
        {
            __delay_cycles(10000); //Wait
            if (UCB0STAT & UCBBUSY)
            {
                testI2c0Busy++;
    
                UCB0CTL1 = UCSWRST;
                __delay_cycles(1000);
                UCB0CTL1 &= ~UCSWRST;
                //break;
            }
        }
    
        //TA1CTL &= ~MC_2;
        UCB0IFG &= ~UCTXIFG; //to cleasr TXIFG
        UCB0IE &= ~UCNACKIE;
        UCB0IE &= ~UCRXIE;
        //UCB0I2CIE &= ~UCNACKIE;
        //IE2 &= ~UCB0RXIE;
        t1 = TA0R;
        sendUserMessage("READ SUCCESSFUL\n\r");
        t2 = TA0R - t1;
        //__delay_cycles(1000);
        // no errors, all bytes received
        if (intDisabled)
            __enable_interrupt();
        return 0;
    }
    
    uint08 i2cUcb1CheckNakTimeout(uint08 timeout, uint08 check_nak)
    /**
     * 
     * @param   timeout   - I - Timeout max (in msec)
     *                          Timeout=0, permits an infinite timeout (not recommended). Max value = 65
     * @param   check_nak - I - Interrupt driven routines do not need to check for no acknowledge from
     *                          slave devices. 1=check for nak, 0=do not check for nak
     *
     * @return  0 - Completed successfully 
     *          I2C_NO_ACK - Slave NAck'ed            
     *          I2C_TIMEOUT - Slave did not respond before timeout period expired
     *
     */
    {
      uint16 currentTimerVal;
      uint16 timerDiff;
      
      // check for NAK
      if ( check_nak ) {
        if ( UCB1IFG & UCNACKIFG ) {
          // send a stop and clear the interrupt flag
        	UCB1CTL1 |= UCTXSTP;
        	 //while (UCB1STAT & UCBBUSY){};
            if (UCB1STAT & UCBBUSY)
            {
    
                    __delay_cycles(10000);  //Wait for 1s
    
                if (UCB1STAT & UCBBUSY)
                {
                    testI2c1Busy++;
                    UCB1CTL1 = UCSWRST;
                    __delay_cycles(1000);
                    UCB1CTL1 &= ~UCSWRST;
                    //break;
                }
    
            }
        	//UCB1IV &= ~USCI_I2C_UCNACKIFG;
        	 UCB1IFG &= ~UCNACKIFG;
        	TBCTL &= ~MC_2;
        	UCB1CTL1 = UCSWRST;
        	return I2C_NO_ACK;
        }
      }
    
      // do we need to check for timeout?
      if ( timeout != 0 ) {
        currentTimerVal = TBR;
    
        // check for TBR rollover since we baselined
        if ( currentTimerVal > i2cBaselineTimer )
          timerDiff = currentTimerVal - i2cBaselineTimer;
        else
          timerDiff = (65535 - i2cBaselineTimer) + currentTimerVal;
        
        // a count of 1000 is equal to 1ms, approximate with shift by 10
        // have we reached the timeout for this byte?
        //if ( (timerDiff>>10) > timeout ) {
        if ( (timerDiff >>10) >= (timeout) ) {
          // generate a stop and return the error flag
          TBCTL &= ~MC_2;
          UCB1CTL1 |= UCTXSTP;
          //while (UCB1STAT & UCBBUSY){};
          //trying to solve I2C stuck issue
          if(UCB1STAT & UCBBUSY)
          {
    
              __delay_cycles(10000);  //Wait
    
              if(UCB1STAT & UCBBUSY)
              {
                  testI2c1Busy++;
                  UCB1CTL1 = UCSWRST;
                  __delay_cycles(1000);
                  UCB1CTL1 &= ~UCSWRST;
                        //break;
              }
    
          }
    
          return I2C_TIMEOUT;
        }
      }
      
      // nak or timeout did not occur
      return 0;
    }
    
    uint08 i2cUcb0CheckNakTimeout(uint08 timeout, uint08 check_nak)
    /**
     *
     * @param   timeout   - I - Timeout max (in msec)
     *                          Timeout=0, permits an infinite timeout (not recommended). Max value = 65
     * @param   check_nak - I - Interrupt driven routines do not need to check for no acknowledge from
     *                          slave devices. 1=check for nak, 0=do not check for nak
     *
     * @return  0 - Completed successfully
     *          I2C_NO_ACK - Slave NAck'ed
     *          I2C_TIMEOUT - Slave did not respond before timeout period expired
     *
     */
    {
        uint16 currentTimerVal;
        uint16 timerDiff;
    
        // check for NAK
        if(check_nak)
        {
            if(UCB0IFG & UCNACKIFG)
            {
                // send a stop and clear the interrupt flag
                UCB0CTL1 |= UCTXSTP;
                //while(UCB0STAT & UCBBUSY){}
    
    
                if(UCB0STAT & UCBBUSY)
                {
                    __delay_cycles(10000); //Wait for 1sec
                    if(UCB0STAT & UCBBUSY)
                    {
                        testI2c0Busy++;
    
                        UCB0CTL1 = UCSWRST;
                        __delay_cycles(1000);
                        UCB0CTL1 &= ~UCSWRST;
                        //break;
                    }
                }
    
                //UCB1IV &= ~USCI_I2C_UCNACKIFG;
                UCB0IFG &= ~UCNACKIFG;
                //TA1CTL &= ~MC_2;
                UCB0CTL1 = UCSWRST;
                return I2C_NO_ACK;
            }
        }
    
        // do we need to check for timeout?
        if(timeout != 0)
        {
            currentTimerVal = TA0R;
    
            // check for TA1R rollover since we baselined
            if (currentTimerVal > i2c0BaselineTimer)
                timerDiff = currentTimerVal - i2c0BaselineTimer;
            else
                timerDiff = (65535 - i2c0BaselineTimer) + currentTimerVal;
    
            // Current TimerA0 frequency is 32768Hz (ACLK with ID_0)
            // A timer count of 33 corresponds to 1ms
    
            if((timerDiff >> 10) >= (timeout))
            {
                // generate a stop and return the error flag
                //TA1CTL &= ~MC_2;
                UCB0CTL1 |= UCTXSTP;
    
                if(UCB0STAT & UCBBUSY)           //while(UCB0STAT & UCBBUSY)
                {
    
                    testI2c0Busy++;
                    /*
                     P3SEL &= ~(MCU_UCB0_SDA|MCU_UCB0_SCL);
    
                     uint08 i;
                     for(i=0;i<100;i++)
                     {
                     P3OUT &= ~(MCU_UCB0_SDA|MCU_UCB0_SCL);
                     __delay_cycles(100);
                     P3OUT |= (MCU_UCB0_SDA|MCU_UCB0_SCL);
                     }
    
                     P3SEL |= (MCU_UCB0_SDA|MCU_UCB0_SCL);
                     */
                    UCB0CTL1 = UCSWRST;
                    __delay_cycles(1000);
                    UCB0CTL1 &= ~UCSWRST;
                }
    
                //while (UCB0STAT & UCBBUSY){};
    
                return I2C_TIMEOUT;
            }
        }
    
        // nak or timeout did not occur
        return 0;
    }
    
    void i2cUcb1Cleanup(void)
    /**
     * Cleanup after a failed transaction. Turn off the timer, disable interrupts,
     * and put the I2C hardware into reset.
     *
     */
    {
      // turn off the timer to save power
      TBCTL &= ~MC_2;
      
      // clear interrupt flags
      UCB1IFG &= ~UCTXIFG;
      //IFG2 &= ~UCB0TXIFG;
      
      // no errors, all bytes transmitted
      UCB1IE &= ~UCNACKIE;
      UCB1IE &= ~UCTXIE;
      //UCB0I2CIE &= ~UCNACKIE;
      //IE2 &= ~UCB0TXIE;
    
      // put i2c hardware into reset  
      UCB1CTL1 = UCSWRST;
    }
    
    void i2cUcb0Cleanup(void)
    /**
     * Cleanup after a failed transaction. Turn off the timer, disable interrupts,
     * and put the I2C hardware into reset.
     *
     */
    {
      // turn off the timer to save power
      //TA1CTL &= ~MC_2;
    
      // clear interrupt flags
      UCB0IFG &= ~UCTXIFG;
      //IFG2 &= ~UCB0TXIFG;
    
      // no errors, all bytes transmitted
      UCB0IE &= ~UCNACKIE;
      UCB0IE &= ~UCTXIE;
      //UCB0I2CIE &= ~UCNACKIE;
      //IE2 &= ~UCB0TXIE;
    
      // put i2c hardware into reset
      UCB0CTL1 = UCSWRST;
    }
    
    void i2cUcb1Setup(uint32 rate)
    {
    	i2cUcb1Init();
    	i2cUcb1SetClockrate(rate);
    	i2cUcb1ConfigTimer();
    
    }
    
    void i2cUcb0Setup(uint32 rate)
    {
        i2cUcb0Init();
        i2cUcb0SetClockrate(rate);
        //i2cUcb0SConfigTimer();    use timer of Remote itself
    
    }
    

    Abhishek

  • You disable global interrupts but then enable the module interrupts. Much simpler to just not enable the module interrupts! (The interrupt flags will be set regardless of the state of the IE bits.) If some other device dislikes having its interrupt service delayed this could cause trouble.

    Most of i2cUcb0Cleanup() is useless. If you never enable the module interrupts, you don't need to disable them. Setting UCSWRST clears both the IE and IFG registers automatically.

    I usually go with an interrupt driven method so this polled stuff looks strange to me.

  • Hi David,

    Thanks for going through the code. This was the existing code when I joined my workplace and because it was working, I let it be. I had been thinking of changing it as well. While it can be improved, there is no obvious error with it that should cause this behaviour, right?

    I might have diagnosed the cause of this. The MCLK frequency was 16MHz(through the internal DCO). On increasing or decreasing this frequency, the code is running fine in MCUs that were getting corrupted within seconds. As far as I know, the maximum frequency for MSP430F5524 is 25MHz. So, any idea why this could be happening? Could this simply be an issue with the ICs themselves due to improper handling, ESD events etc.?

    Here is the code where I am setting the MCLK frequency. Could you let me know if this is okay?

    UCSCTL3 |= SELREF_2;               // Set DCO FLL reference = REFO
    UCSCTL4 |= SELA_2;                   // Set ACLK = REFO
    UCSCTL0 = 0x0000;                    // Set lowest possible DCOx, MODx
    // Loop until XT1,XT2 & DCO stabilizes - In this case only DCO has to stabilize
    do
    {
    UCSCTL7 &= ~(XT2OFFG | XT1LFOFFG | DCOFFG);
    // Clear XT2,XT1,DCO fault flags
    SFRIFG1 &= ~OFIFG; // Clear fault flags
    }
    while (SFRIFG1&OFIFG); // Test oscillator fault flag

    __bis_SR_register(SCG0); // Disable the FLL control loop
    UCSCTL1 = DCORSEL_5; // Select DCO range 16MHz operation
    UCSCTL2 = FLLD_0 + 487; // Set DCO Multiplier for 16MHz // (487 + 1) * 32768 = 16MHz
    __bic_SR_register(SCG0); // Enable the FLL control loop

    // Worst-case settling time for the DCO when the DCO range bits have been
    // changed is n x 32 x 32 x f_MCLK / f_FLL_reference. See UCS chapter in 5xx
    // UG for optimization.
    // 32 x 32 x 16 MHz / 32,768 Hz = 500000 = MCLK cycles for DCO to settle

    __delay_cycles(500000);

    Thanks and Regards

    Abhishek

  • Hi, Abhishek, 

    I don't think it is an ESD issue that the code is running fine when increasing or decreasing this frequency in MCUs that were getting corrupted within seconds. This should be related to the frequency setting and the I2C communication speed setting. If the I2C code is removed and the code is running well, the issue can be addressed the I2C code. Then you can debug it step by step. 

    Regarding the MCLK 16MHz setting, I don't see anything incorrect. 

    Best Regards, 

    Lixin 

  • Hi Lixin,

    That was a good point you made. When I reduced the clock speed from 16MHz to 12MHz, the I2C clock speed had also reduced from 133KHz to 100KHz. To dig deeper, I reduced the I2C clock speed to 100KHz while keeping the MCLK speed at 16MHz. It failed immediately. I hit upon the clock speed being an issue when, with the clock speed being 16MHz, I debugged the I2C code line by line and it didn't fail. If the issue doesn't recur on reducing the clock speed, I'm planning to try further reducing it to 8MHz and if this doesn't work, trying to issue a software reset( using watchdog timer), preferably from the ISR .

    Thanks

    Abhishek

  • Hi, Abhishek,

    For the case of " I reduced the I2C clock speed to 100KHz while keeping the MCLK speed at 16MHz. It failed immediately", do you know where is the failed place? Is the code hang up at somewhere or I2C communication failed with no ACK or wrong data receiving/writing? If it can be addressed, we can know if the MCLK has something wrong or the I2C communication has something wrong. You can also try another case: MCLK setting to 16MHz, I2C clock setting to 100KHz, reduce the pull-up resistance for the I2C SCL and SDA lines. This can help to address if the I2C communication is wrong. 

    Regards, 

    Lixin 

**Attention** This is a public forum