next topic »

[on:]

Hi,

I was hacking through the core libraries and found the pinMode declaration:

void pinMode(uint8_t pin, uint8_t mode)
{
   uint8_t bit = digitalPinToBitMask(pin);
   uint8_t port = digitalPinToPort(pin);
   volatile uint8_t *reg;

   if (port == NOT_A_PIN) return;

   // JWS: can I let the optimizer do this?
   reg = portModeRegister(port);

   if (mode == INPUT) {
      uint8_t oldSREG = SREG;
                cli();
      *reg &= ~bit;
      SREG = oldSREG;
   } else {
      uint8_t oldSREG = SREG;
                cli();
      *reg |= bit;
      SREG = oldSREG;
   }
}

I don´t understand why is the cli() used?
Why do you need to save SREG and disconnect interrupts to change de port mode?

Best regards

[Reply #1 on:]

Because the operation may not be atomic, i.e. an interrupt could cause a change to the value of the register between reading, and writing back the new value

[Reply #2 on:]

Is there an automatic sei() added in the compile? Well, it seems to me there must be something.

[Reply #3 on:]

SREG = oldSREG;

This turns the interrupts back on.

[Reply #4 on:]

SREG = oldSREG;

This turns the interrupts back on.

If they were turned on to begin with.

...
Why do you need to save SREG...

An sei() at the end of the function would turn on the interrupts whether they were initially enabled or not.  This would result in a pinMode call enabling interrupts even if initially disabled.  SREG contains a Global Interrupt Enable bit(cli() and sei() clear and set this bit)  By saving the value of SREG and restoring it at the end of the function you only enable the interrupts if they were enabled to begin with.

[Reply #5 on:]

So it's probably much better to save SREG before cli() and restore SREG later than to just throw cli() and sei() in as needed?

3 steps instead of 2, for better code.

[Reply #6 on:]

Code:

void setup() {
  Serial.println("This is my favorite sketch");
}

void loop() {
  Serial.println("I'm entering a time-sensitive part of the program!");
  noInterrupts();
  digitalWrite(4, HIGH);
  delayMicroseconds(1000);
  Interrupts();
}

Good thing digitalWrite doesn't enable interrupts, or this wouldn't work*

*assuming it did anything

[Reply #7 on:]

So it's probably much better to save SREG before cli() and restore SREG later than to just throw cli() and sei() in as needed?

3 steps instead of 2, for better code.

You sarcasm isn't helpful.

You are not understanding that interrupts are not always enabled and so you can't simply blindly mask
them and blindly turn them on.
If you were to do that then interrupts would be turned on when they were disabled before this function were
called.

ISR routines are allowed to call functions like pinMode() and digitalWrite().
Libraries like the servo library, and IRremote call these functions from their ISR.

The issue is that given the way the core code is currently implemented, it uses a |= operation using non compile
time constants, which means it has to mask interrupts to protect the updates that an ISR may do to a port bit
from the foreground port bit updates.
It also has to save and restore the interrupt mask in the status register to ensure that it
doesn't accidentally re-enable interrupts when they are disabled.

Given the way the core code is implemented, there is no way around this.

--- bill

[Reply #8 on:]

So it's probably much better to save SREG before cli() and restore SREG later than to just throw cli() and sei() in as needed?

3 steps instead of 2, for better code.

You sarcasm isn't helpful.

There's absolutely Zero Sarcasm there. That last line is what I believe to be a statement of fact.

Fact is that 3 steps to do the right thing is better than 2 steps that leave a hole for error.

 

[Reply #9 on:]

So it's probably much better to save SREG before cli() and restore SREG later than to just throw cli() and sei() in as needed?

3 steps instead of 2, for better code.

You sarcasm isn't helpful.

There's absolutely Zero Sarcasm there. That last line is what I believe to be a statement of fact.

Fact is that 3 steps to do the right thing is better than 2 steps that leave a hole for error.

 

Ok, apology offered. Sorry about that.
I totally misinterpreted the comment.

--- bill