Multiple I2C reads with no restart

[kaslewis]

With help from the Freescale team I was able to get a single register read working perfectly, my code is below. I was wondering what would need to be changed/added to make it a multiregister read. Think of the MPL3115A pressure registers, looking to read 3 or 4 registers without needing to start a new read cycle for each register. I have been able to get two registers but no stop bit at the end. and help would be much appreciated.

Kas

Sorry for the lack of formatting, but when I try use the C code insert it messes up the code completely

/***********************SINGLE REGISTER READ*****/

int main(void)

{

  long int delay;

  unsigned char i;

  unsigned char result;

  char string[] = "My name is Bob C. Marley \r\n\r\n\0";

  Clk_Init();

  UART_Init();

  SIM_SCGC |= SIM_SCGC_I2C_MASK;

  SIM_PINSEL |= SIM_PINSEL_I2C0PS_MASK;

  I2C0_F = 0x11;

  I2C0_C1 |= I2C_C1_MST_MASK | I2C_C1_TX_MASK | I2C_C1_IICEN_MASK;

/**/ //Start

  I2C0_C1 |= I2C_C1_MST_MASK | I2C_C1_TX_MASK;

/**/ //Write device to read from

  I2C0_D = 0x96;//0xC0;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

/**/ //Write register to read from

  I2C0_D = 0x0B;//0x0C;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

/**/ //Restart

  I2C0_C1 |= I2C_C1_RSTA_MASK;

/**/ //Read from device

  I2C0_D = 0x97;//0xC1;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

/**/ //RX mode

    I2C0_C1 &= ~I2C_C1_TX_MASK;

/**/ //Turn off its ACK

    I2C0_C1 |= I2C_C1_TXAK_MASK;

//    for (i = 0; i < 5000; i++){}

//   

    result = I2C0_D;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

  I2C0_C1 &= ~I2C_C1_MST_MASK;

    result = I2C0_D;

  for(i = 0; i < 40; i++){

  asm("nop");

  }

  }

/********Multiple Register Read with no stop bit*************************/

int main(void)

{

  long int delay;

  unsigned char i;

  unsigned char result;

  char string[] = "My name is Bob C. Marley \r\n\r\n\0";

  Clk_Init();

  UART_Init();

  SIM_SCGC |= SIM_SCGC_I2C_MASK;

  SIM_PINSEL |= SIM_PINSEL_I2C0PS_MASK;

  I2C0_F = 0x11;

  I2C0_C1 |= I2C_C1_MST_MASK | I2C_C1_TX_MASK | I2C_C1_IICEN_MASK;

/**/ //Start

  I2C0_C1 |= I2C_C1_MST_MASK | I2C_C1_TX_MASK;

/**/ //Write device to read from

  I2C0_D = 0x96;//0xC0;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

/**/ //Write register to read from

  I2C0_D = 0x01;//0x0C;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

/**/ //Restart

  I2C0_C1 |= I2C_C1_RSTA_MASK;

/**/ //Read from device

  I2C0_D = 0x97;//0xC1;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

/**/ //RX mode

    I2C0_C1 &= ~I2C_C1_TX_MASK;

    I2C0_C1 &= ~I2C_C1_TXAK_MASK;

    result = I2C0_D;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

/**/ //Turn off its ACK

  I2C0_C1 |= I2C_C1_TXAK_MASK;I2C0_C1 |= I2C_C1_TXAK_MASK;

  result = I2C0_D;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

  I2C0_C1 &= ~I2C_C1_MST_MASK;

    result = I2C0_D;

  for(i = 0; i < 40; i++){

  asm("nop");

  }

  return 0;

}

[mjbcswitzerland]

Kas

The user's manual show you how to do this. Just copy the flow chart that it gives:

Since you are not using interrupts just spin waiting for the IICIF - then enter the flow diagram and either loop back to waiting on the flag, or quit when all is done.

Generally however consider using interrupts so that the processor can do other things in parallel (unless doing this in a low priority task under a pre-emptive operating system).

Regards

Mark

[kaslewis]

Thank You Mark,

But again as with my first issue here stating the obvious does not help. The above flow diagram is pretty much useless as accurate order and timing are missing. Example of such iris when is the TXACK set, before or after receiving the second to last byte same with the MST = 0. Also there is no mention of timing. After all is said and done all I was missing was a time delay after receiving the final bit before setting MST = 0, there is NO mention of this anywhere in the chapter on I2C. Anyways all said and done I have it working now but this is one BIG issue I have with Freescale is their appalling documentation due to its ambiguity.

Just to be sure Mark, this rant is not against you as in fact your comment made me think of how many ways can a chart be interpreted and one of those was the answer, but rather against Freescale and their terrible documentation and costumer service.

Kas

[mjbcswitzerland]

Kas

There shouldn't be anything that is timing specific and I also believe that the ordering of setting TXACK and clearing MST is clear in the flow diagram - (TXACK/MST first then read the rx data byte).

I have used the I2C quite intensively in the Coldfires and Kinetis (they are about the same) for 8 years or so and never used any delays. It may be that you have missed clearing the status flag (I2C0_S |= I2C_S_IICIF_MASK) between starting  the last data byte read and then sending the stop condition and so the delay is compensating for the fact that the final wait (while ((I2C0_S & I2C_S_IICIF_MASK) == 0){}) is not working since the flag wasn't cleared.

Otherwise I think that the code I use (uinterrupt driven) follows the recommendation.

There are two details that are possibly missed in the present documentation which trips people up (in standard master mode):

1. The fact that the bus can lock up when the processor is reset during a slave ACK

2. The fact that one needs to be careful about starting a new transmission when the stop bit of a previous transmission hasn't completed

There are a number of discussions about these in the forum:

Re: How can you detect hang / recover on Kinetis i2c?

https://community.freescale.com/message/398627#398627

Re: Why is there a pause() in I2C routines?

Regards

Mark

[kaslewis]

Thank you for the info Mark,

I have attached my code so you can see what I am referring too as well as for others who may look here for a full answer. The delay I am referring to is on line 218 and as far as i can tell that was the only real change I made from what I had and what I got from someone in Freescale.

Kas

Cant seem to attach a file so I'll just add it here :smileysad:

/*

* main implementation: use this 'C' sample to create your own application

*

*/

#include "derivative.h" /* include peripheral declarations */

#define MWSR                   0x00  /* Master write  */

#define MRSW                   0x01  /* Master read */

#define i2c_Start()            I2C0_C1     |= 0x10;\

                               I2C0_C1     |= I2C_C1_MST_MASK

#define i2c_write_byte(data)   I2C0_D = data

#define i2c_Wait()             while((I2C0_S & I2C_S_IICIF_MASK)==0) {} \

                                 I2C0_S |= I2C_S_IICIF_MASK;

#define i2c_Stop()             I2C0_C1  &= ~I2C_C1_MST_MASK;\

                               I2C0_C1  &= ~I2C_C1_TX_MASK

unsigned char MasterTransmission;

/*******************************************************************/

/*!

* Pause Routine

*/

void Pause(void){

    int n;

    for(n=1;n<50;n++) {

      asm("nop");

    }

}

/***********************************************************************************************

*

* @brief    Uart_SendChar - Send a single byte on Uart1

* @param    byte to send

* @return   none

*

************************************************************************************************/ 

void Uart_SendChar(uint8_t send)

{

  while((UART2_S1&UART_S1_TDRE_MASK)==0);

  (void)UART2_S1;

  UART2_D=send;

}

unsigned char readRegister(unsigned char slaveID, unsigned char registerAddress)

{

  unsigned char result;

  unsigned int j;

  MasterTransmission = MRSW;

  slaveID = slaveID << 1;

  slaveID |= MasterTransmission;

  i2c_Start();

  i2c_write_byte(slaveID);

  i2c_Wait();

  I2C0_D = registerAddress;

  i2c_Wait();

  /* Do a repeated start */

  I2C0_C1 |= I2C_C1_RSTA_MASK;

  /* Send Slave Address */

  I2C0_D = (slaveID << 1) | 0x01; //read address

  i2c_Wait();

  /* Put in Rx Mode */

  I2C0_C1 &= (~I2C_C1_TX_MASK);

  /* Turn off ACK */

  I2C0_C1 |= I2C_C1_TXAK_MASK;

  /* Dummy read */

  result = I2C0_D ;

  for (j=0; j<5000; j++){};

  i2c_Wait();

  /* Send stop */

  i2c_Stop();

  result = I2C0_D ;

  Pause();

  return result;

}

void writeRegister(unsigned char slaveID, unsigned char registerAddress, unsigned char data)

{

  MasterTransmission = MWSR;

  slaveID = slaveID << 1;

  slaveID |= MasterTransmission;

  i2c_Start();

  i2c_write_byte(slaveID);

  i2c_Wait();

  I2C0_D = registerAddress;

  i2c_Wait();

  I2C0_D = data;

  i2c_Wait();

  i2c_Stop();

}

//void clkInit(){

// //Clock setup

// SIM_BUSDIV = 0; //Divide bus clock by 1

// ICS_C1 |= ICS_C1_IRCLKEN_MASK; //Use internal reference clock

// ICS_C3 = 0x50; //Multiply the internal reference clock by 64, 16 = 39.0625 KHz

// //Wait for clock to lock (running at 40 MHz (1024 * 39.0625Khz)

// while(!(ICS_S & ICS_S_LOCK_MASK ));

// ICS_C2 |= ICS_C2_BDIV(1); //Divide the bus clock by 2

// ICS_S |= ICS_S_LOCK_MASK; //Clear Loss of lock sticky bit

//}

void Clk_Init()

{

  ICS_C1|=ICS_C1_IRCLKEN_MASK; /* Enable the internal reference clock*/

  ICS_C3= 0x50; /* Reference clock frequency = 39.0625 KHz*/

  while(!(ICS_S & ICS_S_LOCK_MASK));   /* Wait for PLL lock, now running at 40 MHz (1024 * 39.0625Khz) */

    ICS_C2|=ICS_C2_BDIV(1)  ; /*BDIV=2, Bus clock = 20 MHz*/

  ICS_S |= ICS_S_LOCK_MASK ; /* Clear Loss of lock sticky bit */

}

//void uartInit(){

// //UART setup

// SIM_SCGC |= SIM_SCGC_UART2_MASK;

// UART2_BDH = 0x00;

// UART2_BDL = 128;

// UART2_C1 = 0x00;

// UART2_C2 |= UART_C2_TE_MASK | UART_C2_RE_MASK;// | UART_C2_RIE_MASK | UART_C2_TIE_MASK;

//}

void UART_Init()

{

  SIM_SCGC |=  SIM_SCGC_UART2_MASK;

  UART2_BDL= 128;

  UART2_C1 = 0;

  UART2_C2 |= UART_C2_TE_MASK;

  UART2_C2 |= UART_C2_RE_MASK;

  UART2_C2 |= UART_C2_RIE_MASK;

}

//void i2cInit(){

// //I2C setup

// SIM_SCGC |= SIM_SCGC_I2C_MASK;

// SIM_PINSEL |= SIM_PINSEL_I2C0PS_MASK;

//    I2C0_F = 0x00;

//    I2C0_C1 = I2C_C1_IICEN_MASK | I2C_C1_MST_MASK | I2C_C1_IICIE_MASK;

//}

int main(void)

{

  long int delay;

  unsigned char i;

  unsigned char result;

  char string[] = "My name is Bob C. Marley \r\n\r\n\0";

  Clk_Init();

  UART_Init();

  SIM_SCGC |= SIM_SCGC_I2C_MASK;

  SIM_PINSEL |= SIM_PINSEL_I2C0PS_MASK;

  I2C0_F = 0x11;

  I2C0_C1 |= I2C_C1_MST_MASK | I2C_C1_TX_MASK | I2C_C1_IICEN_MASK;

  while(1){

  /**/ //Start

  I2C0_C1 |= I2C_C1_MST_MASK | I2C_C1_TX_MASK;

  I2C0_C1 &= ~I2C_C1_TXAK_MASK;

  /**/ //Write device to read from

  I2C0_D = 0x96;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

  /**/ //Write register to read from

  I2C0_D = 0x00;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

  /**/ //Restart

  I2C0_C1 |= I2C_C1_RSTA_MASK;

  /********************************************************Read*************/

  /**/ //Read from device

  I2C0_D = 0x97;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_S |= I2C_S_IICIF_MASK;

  /**/ //RX mode

  I2C0_C1 &= ~I2C_C1_TX_MASK;

  /**/ //Turn off its ACK

  result = I2C0_D;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  I2C0_C1 |= I2C_C1_TXAK_MASK;

  result = I2C0_D;

  while ((I2C0_S & I2C_S_IICIF_MASK) == 0){

  //Wait for data to transmit

  }

  for(i = 0; i < 40; i++){

  asm("nop");

  }

  I2C0_S |= I2C_S_IICIF_MASK;

  I2C0_C1 &= ~I2C_C1_MST_MASK;

  result = I2C0_D;

  // while(1) {  

  //   for (i = 0; string[i] != '\0'; i++){

  //   Uart_SendChar(string[i]);

  //   }

  // 

  // 

    for(delay = 0; delay < 99999; delay++);

  // }

  }

// Uart_SetCallback(Uart_Interrupt); /* Set the callback function that the UART driver will call when receiving a char */

// Enable_Interrupt(INT_UART2);   /* Enable UART2 interrupt */

  return 0;

}

[bobpaddock]

Such code as this:

"#define i2c_Stop() I2C0_C1 &= ~I2C_C1_MST_MASK;\
I2C0_C1 &= ~I2C_C1_TX_MASK;"

needs to be put in do{}while(0) like this:

"#define i2c_Stop() do{ I2C0_C1 &= ~I2C_C1_MST_MASK; I2C0_C1 &= ~I2C_C1_TX_MASK; }while(0).

This prevents a bug should i2c_Stop() et.al. be used after a if() statement that is not using {}.  Always a good idea to use the {} even if not required.

Some compilers with aggressive optimizers will remove code like this:

while(!(ICS_S & ICS_S_LOCK_MASK));

because it has no side effects.

Placing a volatile nop instruction in {} will prevent the code from being optimized away.

static inline void nop( void ) /* For GCC */
{
__asm__ __volatile__ ("nop");
}

while(!(ICS_S & ICS_S_LOCK_MASK))
{
        nop();
}

That is why it appears that adding delays fix things.

[mjbcswitzerland]

Kas

To attach files you need to enable the advanced editor (top right corner) and then there is an attach button (at the bottom right corner).

I can't follow the code that you posted above. There is some reading code in the main loop but other reading code in readRegister (with the new delay) but it is not being called to see what is happening afterwards (which could be affected the the delay before returning). There are also new functions like i2c_Wait() with no code showing what they do. There were originally  also delays with no obvious purpose, such as

  for (j=0; j<5000; j++){};

As noted previously, code with delays and no comments (explaining why they are needed, plus pointing to the detail in the chips data sheet, user's manual or errata) are often not needed and were added "in desparation" by someone who found that it changed things enought to stop an 'unwanted' behaviour. There are some valid delays that are required (eg. the data sheet may point out that some operation is not possible until a peripheral has completed an internal reset or synchronised to a different bus, which can take a maximum number of clock cycles or us) but not usually in interrupt or flag driven peripheral drivers.

Workarounds of this nature can fail when conditions change, or bus clocks or peripheral speeds are adjusted, or a different device type is moved to and so are potential code-bombs that can go off at some time in future and require a proper solution to be worked out.

Below I have attached the I2C interrupt code for the reception case from the uTasker project as reference but I don't see any basic difference to the order of operations (most of the code is for the I2C driver's state and rx circular buffer). The transmission of the first byte is not shown because that is not in the interrupt but it is only sending the start condition and the first byte.

Regards

Mark

I2C0_S = IIC_IIF; // clear the interrupt flag
unsigned char ucFirstRead = (I2C0_C1 & IIC_MTX); 
if (IIC_tx_control[0]->ucPresentLen == 1) { // final byte
    I2C0_C1 = (IIC_IEN | IIC_IIEN | IIC_MSTA | IIC_TXAK); // we don't acknowledge last byte
}
else if (IIC_tx_control[0]->ucPresentLen == 0) { // we have completed the read
    I2C0_C1 = (IIC_IEN | IIC_TXAK); // send end condition and disable interrupts
    IIC_tx_control[0]->ucState &= ~(TX_WAIT | TX_ACTIVE | RX_ACTIVE);
    IIC_rx_control[0]->msgs++;
    if (IIC_rx_control[0]->wake_task != 0) { // wake up the receiver task if desired
        uTaskerStateChange(IIC_rx_control[0]->wake_task, UTASKER_ACTIVATE); // wake up owner task
    }
}
else {
    I2C0_C1 = (IIC_IEN | IIC_IIEN | IIC_MSTA); // ensure we acknowledge multibyte reads
}

if (ucFirstRead != 0) { // have we just sent the slave address?
    (void)I2C0_D; // dummy read
}
else {
    *IIC_rx_control[0]->IIC_queue.put++ = I2C0_D; // read the byte
    IIC_rx_control[0]->IIC_queue.chars++; // and put it into the rx buffer
    if (IIC_rx_control[0]->IIC_queue.put >= IIC_rx_control[0]->IIC_queue.buffer_end) {
        IIC_rx_control[0]->IIC_queue.put = IIC_rx_control[0]->IIC_queue.QUEbuffer;
    }
}

if (IIC_tx_control[0]->ucPresentLen != 0) {
    IIC_tx_control[0]->ucPresentLen--;
}
else { // read sequence complete so continue with next write if something is waiting
    if (IIC_tx_control[0]->IIC_queue.chars != 0) {
        fnTxIIC(IIC_tx_control[0], 0); // we have another message to send so we can send a repeated start condition
    }
}

[kaslewis]

Mark,

I was able to remove all delays from my code with a bit of work thank you for pushing me to do that :smileyhappy:, I had everything working but I now have some odd happenings as I have outlined in a new question thread. I have also posted a full copy of my code there (with comments) if anyone wants to have a look how I finally got my code to mostly work.

Thanks

Kas