PORTK not keeping output value

Hi,

In the below code I have configured the Arduino Mega’s PORTK pins 0, 1 and 2 and PORTF pins 6 and 7 as outputs. I need to set PORTK1 to either HIGH or LOW and toggle PORTK0 in a timer ISR. My problem is as soon as I start toggling PORTK0, K1 switches itself from HIGH to LOW. It’s not only when I toggle K0, I’ve toggled K2 as well and K1 goes LOW as well. I tried the same setup with PORTF but it keeps its pin HIGH when the other pin is toggled. If I toggle K0 in a while loop instead of an ISR K1 stays HIGH. Am I missing some alternate function I need to turn off?

void setup(){
  DDRK |= (1<<DDK0) | (1<<DDK1) | (1<<DDK2);
  DDRF |= (1<<DDF7) | (1<<PORTF6);

  Serial.begin(9600);
  sei();
}

void loop(){
  if(Serial.available() > 0){
    String in = Serial.readString();
    if(in == String("1")){
      PORTK |= (1<<PORTK1);
      PORTF |= (1<<PORTF6);
    }else if(in == String("-1")){
      PORTK &= ~(1<<PORTK1);
      PORTF &= ~(1<<PORTF6);
    }else if(in == String("on")){
      TCCR3A = 0;
      TCCR3B = 0;
      TCNT3 = 0;
      OCR3A = (16000000 * 0.5) / (2 * 1024) - 1;
      TIMSK3 |= (1<<OCIE3A);
      TCCR3B |= (1<<WGM32) | (1<<CS30) | (1<<CS32);
    }else if(in == String("off")){
      TCCR3A = 0;
      TCCR3B = 0;
      TCNT3 = 0;
      OCR3A = 0;
      TIMSK3 = 0;
      TCCR3B = 0;
    }
  }
}

ISR(TIMER3_COMPA_vect){
  PINF |= (1<<PINF7);
  PINK |= (1<<PINK2);
}

Thanks!

My guess: PINK is out of “sbi” instruction reach?

My guess: PINK is out of “sbi” instruction reach?

Yep. You’ll need:

  PINK |= (1<<PINK2);

to be

  PINK = (1<<PINK2);

instead.
There has always been some grumbling, in the grumpy circles, that “PINx |= bitMask;” doesn’t actually do what the statement says it’s doing. It’ll do what people want and expect (“set” the single bit, causing a pin toggle) for most ports, but once you get to the ports that need to be accessed by IN/OUT or LDS/STS, and it actually reads the full port, sets the one bit in that value, and writes the full port back out, which will turn off (via toggle) any bits that were on…

Thank you both very much, changing "PINxn |=" to "PINxn =" seems to have solved the problem. Just so I learn from this, what does "PINK is out of "sbi" instruction reach?" mean and how do I know which ports need to be accessed with IN/OUT or LDS/STS?

Thanks again!

Since I want to know also...

https://www.microchip.com/webdoc/avrassembler/avrassembler.wb_SBI.html

It looks like the Set Bit in I/O register can only operate on the lower 32 I/O registers.

The AVR has special instructions “SBI” and “CBI” (Set/Clear Bit in IO register) that will do the operation in a single instruction, but only of the first 32 IO registers. (Early AVRs had 32 or fewer IO registers in total. but new, bigger chips have more ports and more peripherals, so not all of the are accessible via these instructions.)

The compiler will helpfully optimize expressions like “PORTB |= 1<<4;” or “PORTB &= ~(1<<4)” to uses these instructions; after all: setting a bit is setting a bit, right? If the port register is NOT located in the first 32 registers, you have to read the whole port into a cpu register, fiddle with the bit, and then write it back out. For memory-like registers, this accomplishes the same thing.

Except the PINx registers don’t behave the way that C expects memory to behave. Writing a one bit doesn’t set the bit to one; it does something that depends on the old contents. Reading PINx on a register set to outputs will normally read whatever you last wrote to the port. Writing the same value back will toggle all the bits that were set, which will clear all the bits that were set. If it happened to use SBI, it only writes the single bit, and does “the expected thing.”

This can also result in problems with any other registers where “writing 1” doesn’t result in a 1. There was a problem on some of the SAM ARM chips with interrupt registers. An expression like:

SERCOM0.INTSTATUS.FERR = 1;

Meant to clear the framing error interrupt status bit (bits are cleared by writing 1 to them, in this case) actually resulted in reading the whole word worth of INTSTATUS bits, and clearing them when it was written back out.

IMO the "SBI feature" is an incorrect behavior of the compiler but AFAIK all compilers do so.

It is hard to tell if it is better to use "PINx = 1" which forces to use of OUT instruction or "PINx |= 1" which uses SBI. SBI is 1 word 2 CK instruction while OUT is 1 word 1 CK - which seems better. The problem with OUT is you need to prepare the value to be written into PINx in some CPU register. This may take another instruction 1 word 1 CK instruction. Even worse is if you do not have any register free for this - such as during an ISR - you need to PUSH and POP something and that adds even more overheat. OTOH it is possible there is the right value in some CPU register already - in that case no overheat is needed and the OUT is truly better. But this may change anytime later with seemingly independent change of the code somewhere else.

In short "PINx |= 1" is more predictable, possibly more effective but surely more dangerous to use.