Why does digitalWrite disable global interrupts before changing a pin state?

Hi, I was recently doing some casual non-fiction reading through the Arduino core file "wiring_digital.c", when I came across the digitalWrite() command:

void digitalWrite(uint8_t pin, uint8_t val)
{
	uint8_t timer = digitalPinToTimer(pin);
	uint8_t bit = digitalPinToBitMask(pin);
	uint8_t port = digitalPinToPort(pin);
	volatile uint8_t *out;

	if (port == NOT_A_PIN) return;

	// If the pin that support PWM output, we need to turn it off
	// before doing a digital write.
	if (timer != NOT_ON_TIMER) turnOffPWM(timer);

	out = portOutputRegister(port);

	uint8_t oldSREG = SREG;
	cli();

	if (val == LOW) {
		*out &= ~bit; //set pin LOW
	} else {
		*out |= bit; //set pin HIGH
	}

	SREG = oldSREG;
}

I feel like I know a lot about interrupts and timers these days, and I don't see why interrupts are disabled via cli() before setting the pin state. I'm also not sure why the "out" pointer is volatile in this case. Can anyone help me understand this please?

Because

*out &= ~bit;
//and
*out |= bit;

are not atomic operations. Basically the PORT register is loaded, modified using AND or OR, then stored. If some ISR were also modifying the same register, the change could be lost.

Indeed - digitalWrite () can be called from inside an ISR so it will execute
asynchronously while another digitalWrite() is in mid-execution, causing
the output pin state to get garbled if the two pins are in the same port.

If you know you won't be updating pins in a particular port from your
ISRs you could use a simplified version of digitalWrite().

On the Due the chip has a different architecture where there are separate
registers for setting and for clearing pins, which means the operations
are guaranteed single instructions and no special handling is needed.

On the Uno et al if you write in assembler you can often code a pin update
as a single instruction, but in C the compiler cannot guarantee to do this.

The compiler will recognize the idioms, e.g.

		PORTB |= _BV(PORTB5);
		PORTB &= ~_BV(PORTB5);

and generate single sbi or cbi instructions. This does not apply to all ports on the larger MCUs such as the ATmega2560 that have some of their ports above location 0x1F, but will work on the ATmega328P, ATmega1284P, and various ATtinys.

and generate single sbi or cbi instructions.

But digitalWrite() doesn't DO "PORTB |= 8;"
It does "*out |= bit;" where both out and bit are variables, and that's always going to be non-atomic on an AVR.

Arguably digitalWrite could have been done as a macro, then it would allow
the compiler to do this optimization. However sometimes you call digitalWrite
with a variable or non-constant-expression anyway:

  digitalWrite (pins [i], state) ;

Ok, I think I got it. So calling out = ~bit; would be atomic, but doing the and with itself makes it 2 operations, so it's not atomic. Is that right?

I see that they made the pointer volatile, but why didn't they make timer, bit, or port volatile? I know a pointer is 2 bytes, but this point still isn't all clear to me.

panther3001:
Ok, I think I got it. So calling out = ~bit; would be atomic, but doing the and with itself makes it 2 operations, so it's not atomic. Is that right?

I doubt that'd be atomic. The reason that PORTB |= _BV(PORTB5) is atomic is because the compiler is smart enough to boil it down to a single SBI instruction. Note that in this case, the right hand side of the assignment is a known constant at compile time. Make it a variable and it pretty much has to be more than one instruction then.

I'm not sure why out is volatile, nor am I sure that it has to be.

out is pointer to an IO register, which is volatile. It can change other than by program action. If you output to it twice, you want to output to it twice, and not just with the last value. (hopefully the "volatile" is in the right place and means "this is a pointer to a volatile value", rather than "this is a volatile pointer to a value." You have to be careful with that...)

port, bit, and timer are just "constants" retrieved from the flash; not volatile at all.

digitalWrite could have been done as a macro

Yes. And in fact people have done that, yielding "digitalWriteFast" and similar. Teensy (even the AVR teensy) has a highly optimized digitalWrite implementation. The Arduino teams seems to have decided that (1) the current implementation is fast enough. (2) the fast versions obscure the functionality. (3) Having digitalWrite have wildly varying execution speeds depending on arguments isn't really a good idea.

westfw:
out is pointer to an IO register, which is volatile. It can change other than by program action. If you output to it twice, you want to output to it twice, and not just with the last value. (hopefully the "volatile" is in the right place and means "this is a pointer to a volatile value", rather than "this is a volatile pointer to a value." You have to be careful with that...)

Thanks, that makes sense. Found a decent explanation of the various permutations here, so it does appear to be coded correctly. Oy. :fearful:

good enough for me. Thanks for all the responses!