PORTC Problem (Read/Modify/Write)

[JackBurn8]

Hi ,

I am currently doing my final year project using PIC16f877A. I am using TPA81 ,a temperature sensor that uses i2c communication.

The i2c code that I am using is supplied by the PICC-Lite 9.5, which configures (PORTC) RC3 and RC4 as SDA and SCL.

The problem that I am facing is when I try to display the results on 3 LEDs using the same port (PORTC, RC7,RC6,RC5), it crashes the i2c communication as well. I know this because I use LCD to display the temperature, all the values become 0 and I have to restart the system.

I think it may be caused by the Read/Modify/Write issue in the PIC as stated on the i2c.h. Then I found this website RMW and solutions for it that describe the problem and how to solve it. I follow it , (using variable to store then change the value of PORTC at only 1 time and using Delay) , but to no avail.

I could change the i2c to use other ports as I need to use usart that uses same port as the i2c. Does it crash when using i2c and usart at the same port? (Like the problem caused when I tried to change to value on PORTC for the LEDs)

Thanks and any help would be much appreciated.

Regards

Jacky

 

[Nigel Goodwin]

I can't help you on C, but I would imagine any such potential problems are taken care of by the compiler?.

I would have thought you've probably got a bug somewhere?.

 

[JackBurn8]

I tried to change to other ports like PORTB or PORTE, it works great . Too bad by using i2c i lost the function of the entire port.

 

[Nigel Goodwin]

I tried to change to other ports like PORTB or PORTE, it works great . Too bad by using i2c i lost the function of the entire port.

It sounds more like you're accessing the port incorrectly in some way?.

 

[3v0]

If the fix did not work it is possible that you are looking at the wrong cause.

It is impossible to tell what is going on without seeing the code.

 

[JackBurn8]

I tried to write like this, using a variable first store the PORTC. Then clear the last 3 bit that i wanted to use. In the switch statement i modify the last 3 bit for the LEDs and in the end i put it back to PORTC. However this does work.

temp_port=PORTC;
            temp_port&=0b11111000;
            switch(heat_pos)
            {
                //case 0:    PORTC=temp_port;
                    //RC2=0; RC1=0; RC0=0;
                //    break;
                case 1:    temp_port|=0b00000001;
                    //RC2=0; RC1=0; RC0=1;
                    break;
                case 2:    temp_port|=0b00000010;
                    //RC2=0; RC1=1; RC0=0;
                    break;
                case 3:    temp_port|=0b00000011;
                    //RC2=0; RC1=1; RC0=1;
                    break;
                case 4:    temp_port|=0b00000100;
                    //RC2=1; RC1=0; RC0=0;
                    break;
                case 5:    temp_port|=0b00000101;
                    //RC2=1; RC1=0; RC0=1;
                    break;
                case 6:    temp_port|=0b00000110;
                    //RC2=1; RC1=1; RC0=0;
                    break;
                case 7:    temp_port|=0b00000111;
                    //RC2=1; RC1=1; RC0=1;
                    break;
                default:    
                    break;   
                
            }
            PORTC=temp_port;

 

[Nigel Goodwin]

Perhaps it's just me not understanding C?, but aren't you just zeroing the I2C pins?.

But the entire scheme reinforces my belief that programmers should understand assembler before attempting to use a high level language, as it's uses non-PIC type access to port pins. Three BCF or BSF lines would remove any problems witht he rest of the port.

 

[CafeLogic]

The hi-tech library files you posted are for bit-banging I2C when you have no hardware serial port available. I am not familiar with that particular library (although I don't see why it would effect the other pins) but I can assure you that when using the PIC16 series hardware serial port for I2C, there is no issue with using the remaining pins of the port. The PIC16f877A does have hardware I2C, read the data sheet and see what registers you need to use. It is really very simple and no library is necessary.

 

[Diver300]

To start with, you shouldn't be driving the SCL and SDA lines up, ever. There should be pull-up resistors for that. You should always write to PORTC with the SCL and SDA bits low, and then drive the i2c bus by toggling the TRISC bits.

Secondly, you shouldn't ever read a port unless you actually want data from it. The RMW page showed the code toggling bits in PB_SHADOW (which you called temp_port), and then moving that to PORTB. You seem to have started the process by moving PORTB to PB_SHADOW, which then causes the problem you were trying to get rid of.

You are reading in a random state on bits 3 to 7 of the port, then writing those random bits back to the pins, or at least to the port register, which become the pin voltages when you enable the output.

If there are no other outputs on PORTC I think that you can write this:-

portc = heat_pos;

That automatically writes zeros to the other bits on PORTC, which is what you want.

You could have a shadow register, but as the rest of it would always be zero, there is no point.

You must not have any instances of any of these:-

SCL = 1;
SCL = 0;
SDA = 1;
SDA = 0;

or any other single bit writes to PORTC. That is because the i2c lines are pulled high, and if SDA_DIR (the TRISC bit that controls SDA) is high, when you run :-

SCL = 0;

SDA will be an input at that moment. If it is high, then the 16F877 reads it as high, modifies the value of the 8 bit set by clearing the SCL bit, and immediately the SDA bit is high. Then when you try to lower the pin by clearing the TRIS bit, it doesn't go low.

 

[Diver300]

I see that you didn't get any help on the Hi-Tech forum........
**broken link removed**

 

[CafeLogic]

Perhaps it's just me not understanding C?, but aren't you just zeroing the I2C pins?.

I guess you are reffering to

PORTC &=0b11111000;

with that statement?

Taken literally, it is a read-modify-write, the contents of PORTC are loaded into a register, a bitwise AND with the given constant is performed, the result is written back to PORTC. Hence if his code is correct (can't see how anything is declared), the effect of that would be to set the 3 LSBs to zero and leave the rest unchanged.

However, if bit-manipulation instructions exist (like with the PIC16), the compiler would normally see what you are trying to do and substitute the more appropriate BIT instructions. It is possible that the compiler might get confused because that statement would more traditionally be written like this: PORTC &= ~(0b00000111); but I doubt it.

 

[CafeLogic]

Just to satisfy my own curiosity, I compiled this: PORTC &=0b11111000; it results in this:

   7FD    30F8     MOVLW 0xf8
   7FE    058E     ANDWF 0xe, F

However, if I compile this: PORTC &=0b11111110; we get:

   7FE    100E     BCF 0xe, 0

Good decisions I think. I also checked to see what it would do in the case of equal numbers of instructions (changing 2 bits), it took the first route.

 

[3v0]

It does not take too much time to study the processor and instruction set to the point where you can do what cafeLogic is doing. Reading the asm generated by the compiler. Esp if it is only for a line or two.

 

[JackBurn8]

portc = heat_pos;

I cant use like that because I am also going to use UART to the computer (which uses RC7 and RC6 as well). It might clear the UART values ??

If the simple LEDs doesn't function, I fear that the UART are also affected the same way.

About the Shadow Register, I thought that I would retain the values of PORTC by putting it into a variable, then using bit-wise operation by changing which bit that i only want, then putting it back to the PORTC.(To solve RMW)

 

[Pommie]

You can probably fix it by changing I2C.h to,

#ifdef I2C_MODULE
#define SCL_HIGH() SCL_DIR = I2C_INPUT
#define SCL_LOW()  [COLOR="red"]{SCL=0;[/COLOR]SCL_DIR = I2C_OUTPUT[COLOR="red"]}[/COLOR]
#define SDA_HIGH() SDA_DIR = I2C_INPUT
#define SDA_LOW()  [COLOR="red"]{SDA=0;[/COLOR]SDA_DIR = I2C_OUTPUT[COLOR="red"]}[/COLOR]
#else
#define SCL_HIGH() SCL = 1; SCL_DIR = I2C_OUTPUT
#define SCL_LOW()  SCL = 0; SCL_DIR = I2C_OUTPUT
#define SDA_HIGH() SDA = 1; SDA_DIR = I2C_OUTPUT
#define SDA_LOW()  SDA = 0; SDA_DIR = I2C_OUTPUT
#endif

And make sure you never set the I²C pins to output anywhere else.

The above fixes the fact that when you do a RMW instruction it reads a high from the pullup and writes it back to the latch.

Edit, also note that the I²C bus can get confused if it is interrupted during transfer such as when you compile. If this is a problem then I have a solution somewhere.

Mike.

 

[Diver300]

portc = heat_pos;

I cant use like that because I am also going to use UART to the computer (which uses RC7 and RC6 as well). It might clear the UART values ??

If the simple LEDs doesn't function, I fear that the UART are also affected the same way.

The hardware UART takes over the pin functions and it doesn't get affected if you write to the port.

About the Shadow Register, I thought that I would retain the values of PORTC by putting it into a variable, then using bit-wise operation by changing which bit that i only want, then putting it back to the PORTC.(To solve RMW)

I think that you have understood "retaining the values of PORTC" incorrectly.

The shadow register retains the value that you want the port to be at, not the value that it was recently. In particular, it retains the value of 7 of the bits when you want to be concentrating on only one with a bitwise operation.

The RMW problem happens because many operations on PORTC read the voltages on the pins as bits, modify some of those, and write back to the port C latches. There area several things that can cause the PORTC voltages to be different from the port C latches.

The ones that I know of are:-
The TRISC bits are set high so that the pin is an output.
The output is overloaded by an external load.
The output is still changing from a previous command.

So reading PORTC is a very unreliable way of finding out the contents of the port C latch.

So don't do it.

With a shadow register, you define any convenient register as the port shadow, and define its state, normally by clearing it. During program running, you can change bits on that shadow with no problems, because it is a normal register and there is no RMW problem. Then the only port write you do is:-

PORTC = portc_shadow;

So the port C latches are what you want them to be.

Your code was just a long winded way of replicating the problem because you read the port at the start.

18F and 24F processors have separate latch registers, which you can set bits on because reading them does not read the state of the pins, it just reads the state of the latch. I am fairly sure that was added to get over the RMW problem.

 

[JackBurn8]

Oops, sorry about the shadow register. Just googled it, and enlighten by it.
That happens to a newbie like me. Just learn PIC about 6 months ago. Start to learn by C.

I thank you all for your advices.

I am modifying and testing the coding as we speak.

Suggestion by Pommie yields the same result. It is funny because previously the value is 0, but now it is 255 . @@

If I am to do PORTC = portc_shadow;
Doesn't it affect the i2c as well? Sorry its very confusing ....

EDIT : PORTC = portc_shadow; doesn't work as well. It seems that whenever i change the value of PORTC it crashes.

switch(heat_pos)
{
case 0: PORTC=0;
//RC5=0; RC6=0; RC7=0;
break;
case 1: PORTC=1;
//RC5=0; RC6=0; RC7=1;
break;
case 2: PORTC=2;
//RC5=0; RC6=1; RC7=0;
break;
case 3: PORTC=3;
//RC5=0; RC6=1; RC7=1;
break;
case 4: PORTC=4;
//RC5=1; RC6=0; RC7=0;
break;
case 5: PORTC=5;
//RC5=1; RC6=0; RC7=1;
break;
case 6: PORTC=6;
//RC5=1; RC6=1; RC7=0;
break;
case 7: PORTC=7;
//RC5=1; RC6=1; RC7=1;
break;
default:
break;

 

[Pommie]

If you are interrupting the I²C routines then you will have to use a shadow register and modify the I²C routines appropriately. If you are not interrupting the routines then anding out the two I²C bits should work. So something like,

	temp=PORTC;		//read port
	temp|=b'00110000';	//set bits 4 and 5
	temp&=b'11111100';	//clear I2C bits
	PORTC=temp;		//write it back

I can't understand why my suggestion above doesn't work. Did you power down the circuit between test as once the I²C bus is messed up it rarely fixes itself.

Mike.

 

[Pommie]

Try this way,

Say you want to set PORTC to the value in variable port, and your using bits 0 and 1 for I2C, try

	temp=port;		//Value to write
	temp&=b'11111100';	//clear I2C bits
	PORTC=temp;		//write it back

Setting or clearing individual bits will cause the I2C bits to get set.

To set an individual bit do,

	temp=PORTC;		//read port
	temp|=b'0010000';	//set bits 5
	temp&=b'11111100';	//clear I2C bits
	PORTC=temp;		//write it back

Mike.

 

[JackBurn8]

I have a very troublesome power supplies so I have to power down everytime i change my code.

I did try your example code , keeping value of PORTC using a variable then set the bits that i wanted (carefully not changing the i2c bits) and in the end put it back ,

but Diver300 said that reading PORTC is a very unreliable way of finding out the contents of the port C latch. and it is creating more trouble for it.

temp_port=PORTC;
temp_port&=0b11111000;
switch(heat_pos)
{
//case 0: PORTC=temp_port;
//RC5=0; RC6=0; RC7=0;
// break;
case 1: temp_port|=0b00000001;
//RC5=0; RC6=0; RC7=1;
break;
case 2: temp_port|=0b00000010;
//RC5=0; RC6=1; RC7=0;
break;
case 3: temp_port|=0b00000011;
//RC5=0; RC6=1; RC7=1;
break;
case 4: temp_port|=0b00000100;
//RC5=1; RC6=0; RC7=0;
break;
case 5: temp_port|=0b00000101;
//RC5=1; RC6=0; RC7=1;
break;
case 6: temp_port|=0b00000110;
//RC5=1; RC6=1; RC7=0;
break;
case 7: temp_port|=0b00000111;
//RC5=1; RC6=1; RC7=1;
break;
default:
break;

}
PORTC=temp_port;