adafruit / uf2-samdx1

MSC bootloader (based on UF2) for SAMD21
Other
213 stars 184 forks source link

Shouldn't check_start_application clear *DBL_TAP_PTR on BOD reset, also? #108

Closed JetForMe closed 4 years ago

JetForMe commented 4 years ago

I originally filed this against the microsoft repo, but maybe it’ll get more attention here.

I've been experiencing an issue where poor power rails cause the bootloader to be entered spontaneously. You can simulate this by applying power to a Feather M0 by holding the ends of two wires together to complete the path to the positive power supply, and then rub them to make a series of interrupted power spikes.[^1]

I think this is triggering the brown-out detection circuitry, and setting the RESET_CONTROLLER->RCAUSE.bit.BODxx bit(s), rather than RESET_CONTROLLER->RCAUSE.bit.POR. The code, as written in main.c interprets this as the user pressing the reset button, and it enters the boot loader.

Ignoring SINGLE_RESET, I think more robust code might look like this. I have not tested this as I currently have no way to load the boot loader:

    if (RESET_CONTROLLER->RCAUSE.bit.POR            //  If power-on reset
        || RESET_CONTROLLER->RCAUSE.bit.BOD12       //  or either brown-out
        || RESET_CONTROLLER->RCAUSE.bit.BOD33 ) {   //  reset, clear *DBL_TAP_PTR
        *DBL_TAP_PTR = 0;
    } else if (RESET_CONTROLLER->RCAUSE.bit.EXT     //  If user pressed reset button
                && *DBL_TAP_PTR == DBL_TAP_MAGIC) { //  AND it’s the second time through
        *DBL_TAP_PTR = 0;
        return; // stay in bootloader
    } else {                                        //  This should probably check RESET_CONTROLLER->RCAUSE.bit.EXT, too
        if (*DBL_TAP_PTR != DBL_TAP_MAGIC_QUICK_BOOT) {
            *DBL_TAP_PTR = DBL_TAP_MAGIC;
            delay(500);
        }
        *DBL_TAP_PTR = 0;
    }

This change should do a couple of things: treat power-on and brown-out-detect resets the same, and only count a /RESET assertion as the second press.

I’m not sure if the SAM51 has different BOD bits.

Note: I took a chance and flashed this bootloader onto a chip, and bricked it. I've ordered an ATMEL-ICE, should be here tomorrow. Not sure if I loaded it incorrectly (I used the resulting .ino file), or if this code doesn't work. A different bug suggests maybe it's the .ino file, and not my changes (especially since my changes should only result in either the bootloader always triggering, or never triggering).


[^1]: In my case, I have an Adafruit solar lipo charger. When the battery is depleted and the sun is just beginning to shine on the panel, the power provided to the load can be spotty.

JetForMe commented 4 years ago

I've managed to program a new boot loader with this fix and it seems to work. I'll make a PR against the microsoft repo.