Awaken from SLEEP - A "Gotcha"

Go To Last Post
12 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This little note is based on recent experience with a Mega328P. Given Atmel's consistency across the Mega and Tiny families, I would expect this to apply to most, if not all, other Mega and Tiny MCUs.

 

Scenario: I have a Mega328 that is awakened by a Pin Change interrupt from a sensor IC. I discovered, the hard way, that if you enter sleep with the pin change unserviced, Hello Zombie! It will never wake up (until it is reset). The reason is that it awakens when the INTERRUPT FLAG BIT for that interrupt changes from 0 to 1. If it begins sleeping with the interrupt unserviced, it is entering with the interrupt flag bit already set. The external signal can bang on that PCInt pin all it wants, and nothing happens, as far as sleep is concerned, because there is no change in the interrupt flag bit - its already set.

 

Solution: Easy - make sure that the flag bit for any interrupt that you expect a wake-up from to be cleared before you put it to sleep.

 

Application: I have not checked, but I'll bet that this  applies to most wake-up sources.

 

Additional Note: One can, correctly ask: how could that interrupt flag possibly be set and the interrupt NOT be serviced? That, I cannot tell you. Interrupts were specifically enabled before it was put to sleep, so any interrupt SHOULD have fired. Yet, when I did Break-All in the debugger, it shows that the interrupt flag is set, the interrupt enable bits are all set, and the peripheral is banging away, with no wake-up. Clearly, it will not waken if the flag bit does not change from clear to set. I cannot explain HOW it got into this state, but I do know that as soon as I added a flag clear operation before entering sleep, the situation no longer occurred. 

 

Cheers 

 

Jim

 

 

 

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

Last Edited: Thu. Jun 4, 2015 - 04:10 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm a bit skeptical.  Can you show a code snippet leading up to and including the call to sleep?  Also, post the .lss for that section of code.  I would more suspect a race condition, or perhaps an artefact of the debug environment.  The recommended code for dealing with just the scenario you've described would not work if what you suggest turns out to be true.
 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

There is a pretty complex "preamble" prior to the sleep call. I'll try to tease out the details.

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would have thought good practice would be to save pending interrupts and then clear all interrupts before going to sleep.

And then on wake restore the interrupt flags.

 

Paul

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

There is indeed a race there.  There is a very extensive thread (from when I ran into the situation) about it, and the way around it.

 

(Indeed, clearing the flag >>almost<< works--unless the trigger hits after the flag clear and before sleep.)

 

So we need to see the code.  GCC sleep mechanisms should be bulletproof with respect to this, and Codevision recent versions should be as well.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

https://www.avrfreaks.net/forum/m...

 

Not as extensive as I recalled, but I think exactly to the point...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

BTW -- I disable sleep in my awakening ISRs.  So if an event occurs just before sleep and is serviced (during the "getting ready for bed") then that event will look like it occurred when sleeping and serve as a "re-awakening".

 

With that, I'd think the next pin-change edge should indeed be serviced.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just found a mechanism by which this CAN occur.

 

In the M328, etc, spec sheet, in the section on interrupt response time, it points out that if sleep command is immediately preceded by an sei, it will enter sleep before any pending interrupts are serviced. I assume that this is due to interrupt latency.

 

Thus, if you have interrupts globally disabled and have a flag set that would normally awaken from sleep, and you do sei() immediately followed by sleep(), it will go to sleep with that interrupt flag still set. If the awaken-from-sleep process depends on CHANGES in the interrupt flag bit (not changes in the underlying state), then it won't wake up, as I described above.

 

Lee's link, a couple of posts up, describes a situation almost identical to the case I described. Again, I clearly did not do sufficient search - mea culpa!

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here's some test code which should expose your proposed vulnerability:

 

#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/sleep.h>
#include <util/delay.h>

#define LED_BIT     5
#define BUTTON_BIT  1

// For wakeup only
EMPTY_INTERRUPT(PCINT0_vect)

int __attribute__ ((__OS_main__)) main(void) {

  // Enable sleep
  sleep_enable();
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);

  // LED on
  DDRB |= (1<<LED_BIT);

  // Button pull-up
  PORTB |= (1<<BUTTON_BIT);

  // Pin-change interrupt on button
  PCMSK0 = (1<<BUTTON_BIT);
  PCICR = (1<<PCIE0);
  PCIFR = (1<<PCIF0);

  // Loop forever
  while(1) {

    // LED on
    PORTB |= (1<<LED_BIT);

    // Wait for pin-change flag
    while (!(PCIFR & (1<<PCIF0))) {}

    // Turn off LED and pause to indicate the set pin-change flag
    PORTB &= ~(1<<LED_BIT);
    _delay_ms(1000);

    // Enable interrupts and sleep
    sei();
    sleep_cpu();

    // LED on again
    //   If we get this far, the  pin-change interrupt flag which was set prior
    //   to sleeping  successfully woke the  device, despite no change  in that
    //   flag during sleep.
    PORTB |= (1<<LED_BIT);

    // Disable interrupts
    cli();

  }

}

 

I cannot get the device to fail to wake from sleep. 

 

The test code waits for a pin change interrupt flag to become set, then goes to sleep.  The device wakes every time, despite the fact that the pin change flag is already set at the moment sleep is entered.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I used sleep_mode(). You used sleep_cpu(). The avr-libc manual has this:

 

There are several macros provided in this header file to actually put the device into sleep mode. The simplest way is to optionally set the desired sleep mode using set_- sleep_mode() (it usually defaults to idle mode where the CPU is put on sleep but all peripheral clocks are still running), and then call sleep_mode(). Unless it is the purpose to lock the CPU hard (until a hardware reset), interrupts need to be enabled at this point. This macro automatically takes care to enable the sleep mode in the CPU before going to sleep, and disable it again afterwards.

 

As this combined macro might cause race conditions in some situations, the individual steps of manipulating the sleep enable (SE) bit, and actually issuing the SLEEP in- struction are provided in the macros sleep_enable(), sleep_disable(), and sleep_cpu(). This also allows for test-and-sleep scenarios that take care of not missing the interrupt that will awake the device from sleep.

 Perhaps it would not have happened using sleep_cpu(). I'm not in a position to test it right now.

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I used sleep_mode(). You used sleep_cpu().
That's almost certainly it.

 

The simpler sleep_mode() is actually defined as:

#define sleep_mode() \
do {                 \
    sleep_enable();  \
    sleep_cpu();     \
    sleep_disable(); \
} while (0)

... and compiles to this on the atmeg328p:

    sleep_mode();
  ba:	83 b7       	in	r24, 0x33	; 51
  bc:	81 60       	ori	r24, 0x01	; 1
  be:	83 bf       	out	0x33, r24	; 51
  c0:	88 95       	sleep
  c2:	83 b7       	in	r24, 0x33	; 51
  c4:	8e 7f       	andi	r24, 0xFE	; 254
  c6:	83 bf       	out	0x33, r24	; 51

This enables sleep, sleeps, and disables sleep (after wakeup).

 

When combined with sei(), it looks like this:

    sei();
  b8:	78 94       	sei
    sleep_mode();
  ba:	83 b7       	in	r24, 0x33	; 51
  bc:	81 60       	ori	r24, 0x01	; 1
  be:	83 bf       	out	0x33, r24	; 51
  c0:	88 95       	sleep
  c2:	83 b7       	in	r24, 0x33	; 51
  c4:	8e 7f       	andi	r24, 0xFE	; 254
  c6:	83 bf       	out	0x33, r24	; 51

So between the sei and the sleep are 3 instructions, after each of which a pending interrupt has the opportunity to break in and be serviced.

 

However sleep_cpu() compiles to a single instruction:

    sleep_cpu();
  ba:	88 95       	sleep

When combined with sei():

    sei();
  b8:	78 94       	sei
    sleep_cpu();
  ba:	88 95       	sleep

The device is guaranteed to go to sleep prior to the servicing of any pending interrupts.

 

Can I make a guess and ask if your pin-change ISR disables its own interrupt source?  This would explain the behaviour you're seeing.  Once you enable interrupts with sei(), the pending pin change interrupt is serviced after the first 'in' instruction above, the pin-change interrupt is disabled, then the device goes to sleep forever.

 

The manual you quoted goes on to describe how to avoid this kind of race:

 

#include <avr/interrupt.h>
#include <avr/sleep.h>
...
set_sleep_mode(<mode>);
cli();
if (some_condition)
{
sleep_enable();
sei();
sleep_cpu();
sleep_disable();
}
sei();

... and the SLEEP instruction will be scheduled immediately after an SEI instruction. As the intruction(sic) right after the SEI is guaranteed to be executed before an interrupt could trigger, it is sure the device will really be put to sleep.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, I muck with interrupt enable to try to avoid responding to the "wrong" edge of the Data Available signal. There MAY be some abnormal conditions that I need to investigate that lets it go to sleep with the int disabled. I've recoded it a bit to try to avoid this possibility. That is ANOTHER source of problems though every time I broke it, the interrupt flag bit was always set. Maybe, as you suggested, this is an artifact of the break-from-sleep.

 

At this point, I hope that I have it under better control. I need to run it for a week or more to really find out.

 

Thanks for all the incredible input.

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

Last Edited: Fri. Jun 5, 2015 - 04:32 AM