Dualoptiboot Preserving MCUSR [discussion]

[teddylambert]

I know that the Moteino uses its own version of optiboot--however I've noticed this version hasn't been updated in a while. Since then the standard optiboot has included support for retaining the value of the MCUSR register—is this something that will be integrated/supported into Dualoptiboot at some point? Is the suggested method of saving the MCUSR register still just commenting out the

Code: [Select]

MCUSR = 0;

line (line 668)? I'm looking at the bootloader code in this area:

Code: [Select]

asm volatile ("clr __zero_reg__");
#if defined(__AVR_ATmega8__) || defined (__AVR_ATmega32__)
  SP=RAMEND;  // This is done by hardware reset
#endif

  // Adaboot no-wait mod
  ch = MCUSR;
  MCUSR = 0;
    
#ifdef DEBUG_ON  
    putch('S');
#endif

  if (!(ch & _BV(EXTRF))) //if not external reset
  {
    if (ch & _BV(WDRF)) //if reset by watchdog
      CheckFlashImage();
#ifdef DEBUG_ON
    putch('A');
#endif
    SPCR &= ~_BV(SPE) & ~_BV(MSTR);
    appStart(ch);
  }

[Felix]

I guess the answer is to do it however the latest optiboot does it.

[teddylambert]

Looks like the v7.0 version was implemented like this:

Code: [Select]

#if !defined(__AVR_ATmega16__)
  ch = MCUSR;
#else
  ch = MCUCSR;
#endif
  // Skip all logic and run bootloader if MCUSR is cleared (application request)
  if (ch != 0) {
      /*
       * To run the boot loader, External Reset Flag must be set.
       * If not, we could make shortcut and jump directly to application code.
       * Also WDRF set with EXTRF is a result of Optiboot timeout, so we
       * shouldn't run bootloader in loop :-) That's why:
       *  1. application is running if WDRF is cleared
       *  2. we clear WDRF if it's set with EXTRF to avoid loops
       * One problematic scenario: broken application code sets watchdog timer 
       * without clearing MCUSR before and triggers it quickly. But it's
       * recoverable by power-on with pushed reset button.
       */
      if ((ch & (_BV(WDRF) | _BV(EXTRF))) != _BV(EXTRF)) { 
	  if (ch & _BV(EXTRF)) {
	      /*
	       * Clear WDRF because it was most probably set by wdr in bootloader.
	       * It's also needed to avoid loop by broken application which could
	       * prevent entering bootloader.
	       * '&' operation is skipped to spare few bytes as bits in MCUSR
	       * can only be cleared.
	       */
#if !defined(__AVR_ATmega16__)
	      MCUSR = ~(_BV(WDRF));  
#else
	      MCUCSR = ~(_BV(WDRF));  
#endif
	  }
	  appStart(ch);
      }
  }

Since then there's been some more changes in V8 to save some bytes instead of the appStart function, but I don't think that is really necessary here. Will test out putting this change into Dualoptiboot and see what happens

[teddylambert]

Looking back at the code for optiboot and dualoptiboot, and it seems like the logic path is slightly different for the two so I'm not sure where to put the dualboot functionality (like CheckFlashImage).

Optiboot:

Code: [Select]

if (ch != 0) {
      /*
       * To run the boot loader, External Reset Flag must be set.
       * If not, we could make shortcut and jump directly to application code.
       * Also WDRF set with EXTRF is a result of Optiboot timeout, so we
       * shouldn't run bootloader in loop :-) That's why:
       *  1. application is running if WDRF is cleared
       *  2. we clear WDRF if it's set with EXTRF to avoid loops
       * One problematic scenario: broken application code sets watchdog timer 
       * without clearing MCUSR before and triggers it quickly. But it's
       * recoverable by power-on with pushed reset button.
       */
      if ((ch & (_BV(WDRF) | _BV(EXTRF))) != _BV(EXTRF)) { 
	  if (ch & _BV(EXTRF)) {
	      /*
	       * Clear WDRF because it was most probably set by wdr in bootloader.
	       * It's also needed to avoid loop by broken application which could
	       * prevent entering bootloader.
	       * '&' operation is skipped to spare few bytes as bits in MCUSR
	       * can only be cleared.
	       */
	      MCUSR = ~(_BV(WDRF));  
	  }
	  appStart(ch);
      }
  }

Dualopti:

Code: [Select]

if (!(ch & _BV(EXTRF))) //if not external reset
  {
    if (ch & _BV(WDRF)) //if reset by watchdog
    {
      CheckFlashImage();
    }
    SPCR &= ~_BV(SPE) & ~_BV(MSTR);
    appStart(ch);
  }

The dualboot code seems to check if it's not an external reset, but is a watchdog reset to clear flash. However, the optiboot version seems to check whether it was a watchdog not set by external, then if it was external. Do you have any suggestions on how to refactor dualboot into a similar logic flow?

[Felix]

Not without diving into the code again, its not something fresh in my mind.
What is the issue with MCUSR?

[teddylambert]

I want to retain the type of reset that occurred, which is stored in MCUSR. However right now, MCUSR is cleared in the boot loader so I'm unable to read the value into my program.

[Felix]

The problem is the bootloader itself uses the WDT to reset the MCU, after a reflash.
So you'd have to differentiate between a WDT reset from the main firmware vs a reset from the bootloader itself.

[teddylambert]

Yeah, that's the general consensus I've seen—detecting a WDT reset isn't really feasible given the way the bootloader operates. However, if I can retain MCUSR I should still be able to check for other reset types though right? For my application being able to tell whether it was brown out, external reset, or power-on/watchdog is good enough.

[teddylambert]

Going to try attacking it from a slightly different angle. Looks like since optiboot v4.6 MCUSR has been saved to r2, and this functionality is included in your dualoptiboot as well. Going off some optiboot example code I might be able to pull that value from r2 without having to touch the bootloader or actual MCUSR

[Felix]

Allright, let me know your findings. And maybe a status should be added to the top of this thread. (FWIW I don't think its a bug.)

[teddylambert]

By following the example code from optiboot here I was able to save a copy of MCUSR to use in my program. A couple of things I noticed:
- As you mentioned earlier, the bootloader uses the watchdog during reset. So an external reset (like with the reset pushbutton) is appearing as a WDT reset
- The flags are not mutually exclusive. On a power on reset, both the power flag and brown out flag get set (I'm assuming just based on the voltage rise time during start up). However, it seems like the case where both are set really just means that its a power on.

For my purposes this level of distinction is good enough, I think this is the closest I am able to get without going back to editing the bootloader.

And maybe a status should be added to the top of this thread. (FWIW I don't think its a bug.)

I agree with this--the code/bootloader is operating exactly as it's defined to. It's more just a consequence of the bootloaders operation. Not exactly sure what you mean by adding a status though...I'm happy to add one if you let me know what to do

[Felix]

I appreciate your follow up, thank you.
I marked the thread as [discussion] since this is not really a bug but just a talk through of what the MCUSR bits are doing during bootloader/reboots.