i2c register read back problem

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

i2c register read back problem

2,292 Views
love_hate_and_repeat
Contributor III

I2C_Status is structure and defined as volatile keyword.

 

while(I2C_Status->RXAK != ACK_SIG);

 

if ( I2C_Status->RXAK != ACK_SIG)

{

     printf("we got an ack from slave because "while loop" is broken, it is weird because now it's like we didnt get an ack from slave");

}

 

if i put a delay before if condition, printf in if condition is not printed and this means everyting is okay but if i dont put a delay before if condition, how come i dont know  printf in if condition is printed. it's weird.

 

why i should put a delay to read again same register ? i guess this problem is about reading consecutively same register but i dont know the technique reason  ? can you explain for me ?

 

thanks in advance.

Tags (1)
0 Kudos
19 Replies

1,680 Views
Ray_V
Contributor V

Can you put your declaration of  I2C_Status?

The status register is an 8 bit register and RXAK is just one bit in that register. So how did you declare the status register as a structure?

Also what is your declaration of ACK_SIG?

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

I defined it as bitfield and bit order is same with t1040 ref manual.

0 Kudos

1,680 Views
Ray_V
Contributor V

did you define the bitfield structure as "packed"? in many compilers you need to.

If you don't share the actual code it's impossible to help you beyond some generic tips.

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

i dont need to use packed attribute because its size is already power of two or it's not out of aligment and no need to add pad by compiler.. the structure is defined as the following way.

 

struct statusRegisterStc 

{

   unsigned char MCF : 1;

   unsigned char MAAS : 1;

   unsigned char  MBB : 1;

   unsigned char  MAL : 1;

   unsigned char  BCSTM : 1;

   unsigned char  SRW : 1;

   unsigned char  MIF : 1;

   unsigned char RXAK : 1;

};

and ACK_SIG is defined as following way.

 

#define ACK_SIG 0

0 Kudos

1,680 Views
Ray_V
Contributor V

i dont need to use packed attribute because its size is already power of two or it's not out of aligment and no need to add pad by compiler

It doesn't matter, I have found when porting code between processors+compilers that in some you still have to use "packed" or it doesn't work.

I would just avoid it and use mask, it's more portable. Also try waiting until the bus is idle, that way you know RXAK is valid.

#define RXAK_MASK 0x80
#define MBB_MASK 0x04

while (*I2C_I2CSR & MBB_MASK) { }

while (*I2C_I2CSR & RXAK_MASK) { }

if (*I2C_I2CSR & RXAK_MASK)
{
 ... // some code
}
‍‍‍‍‍‍‍‍‍‍‍

And I would also add timeouts to the while loops to avoid hanging up if the condition never becomes false.

By the way, the problem is very likely caused by checking for the ACK before the frame is sent by the hardware and the ACK you see initially is for the previous frame.

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

i resolved this problem with delay but i don't know the reason why it's working when putting a delay before accessing registers.

0 Kudos

1,680 Views
Ray_V
Contributor V

I have not used nor do I have this processor, so I have no hands on experience with it.

It's possible that the bus idle bit can only be used to detect if other masters are controlling the bus, and not if this i2c is transmitting. Also I could not find information on when the RXAK bit gets set, it could be that it is set after the byte has been transmitted and cleared when the ACK is received.

You can try looking at the MCF bit (byte transfer complete) before looking at RXAK

#define RXAK_MASK 0x80
#define MCF_MASK 0x01

/* 1 = transfer is completed */
while ((*I2C_I2CSR & MCF_MASK) == 0) { }

/* 0 = ACK received */
while (*I2C_I2CSR & RXAK_MASK) { }

if (*I2C_I2CSR & RXAK_MASK)
{
     // Handle Failure
}‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍
else
{
    // Handle success
}

Hope this helps.

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

thanks for your effort and reply.

i use counter beside of checking bit with mask  like following way.


*I2C_I2CSR & MCF_MASK != MCF_MASK)
{
    // THERE IS A PROBLEM
}

the above code is not working. it's always entering to if condition but the following code is working. 

char data = readByte(I2C_DATA);
udelay(some_time);

if (*I2C_I2CSR & MCF_MASK!= MCF_MASK)
{
    // THERE IS A PROBLEM
}

the above code is alway working and  it's not entering to if condition. 

0 Kudos

1,680 Views
Ray_V
Contributor V

MCF bit = 0 -> transfer ongoing

MCF bit = 1 -> Transfer done, so your code should be

char data = readByte(I2C_DATA);
while (!(*I2C_I2CSR & MCF_MASK) &&  (counter++ < 100000)) { }
if (*I2C_I2CSR & MCF_MASK != MCF_MASK)
{
    // THERE IS A PROBLEM
}

you also have to keep in mind your CPU speed, for example if you are running at 200MHz and the "while" loop takes 8 cpu cycles(just an example, I don't know for this mcu) counting  form 0 to 100000 the wait is only 80 us, which may not be enough.

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

Sorry i wrote the code as wrong. Yes my code is like your example.

i changed timeout value as 1000000 and i also tried it in big value to make sure the problem is not about timeout value and the problem is still same.

0 Kudos

1,680 Views
Ray_V
Contributor V

How long is the delay (udelay(some_time);) when it works?

Also, what happens if you remove the counter?

char data = readByte(I2C_DATA);
while (!(*I2C_I2CSR & MCF_MASK)) { }
if (*I2C_I2CSR & MCF_MASK != MCF_MASK)
{
    // THERE IS A PROBLEM
}
0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

delay time is 10000 microseconds and when i remove counter the code is frozen because of infinite loop.

0 Kudos

1,680 Views
Ray_V
Contributor V

How about you try this

char data = readByte(I2C_DATA);
while (!(*(volatile unsigned char *)I2C_I2CSR & MCF_MASK)) { }
if (*(volatile unsigned char *)I2C_I2CSR & MCF_MASK != MCF_MASK)
{
    // THERE IS A PROBLEM
}

By the way, this really should be handled inside the readByte() function so you know the data value is valid, the function should not return until the byte has been read, else the return value is not valid.

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

by the way, the topic is assumed as answered.  can you correct it as not answered again ?

0 Kudos

1,680 Views
ufedor
NXP Employee
NXP Employee

Which processor is in question?

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

thanks, i specifed the processor.

0 Kudos

1,680 Views
ufedor
NXP Employee
NXP Employee

Please refer to the U-Boot I2C driver source code available at:

u-boot/fsl_i2c.c at b3f98d438eefd1b355efdec0b50af5813ff8d0e1 · qoriq-open-source/u-boot · GitHub 

0 Kudos

1,680 Views
love_hate_and_repeat
Contributor III

Primarily, thanks to your answer.

I actually want technique explanation because i also handle this issue by placing a delay before checking if condition but i dont know why it's working when i put a delay. I want to understand what i do.

0 Kudos

1,680 Views
ufedor
NXP Employee
NXP Employee

I believe that the issue is caused by an instruction before the "while".

Please consider the QorIQ T1040 Reference Manual, 2.6.1 Accessing CCSR Memory from the Local Processor.

0 Kudos