abcminiuser / lufa

LUFA - the Lightweight USB Framework for AVRs.
http://www.lufa-lib.org
1.03k stars 321 forks source link

[DFU] BootloaderDFU doesn't allow entering bootloader from software when using BOOTRST #150

Open Duckle29 opened 5 years ago

Duckle29 commented 5 years ago

Hey there, I'm working on getting BOOTRST and reset source detection working for entering bootloader on a qmk based keyboard https://github.com/qmk/lufa/pull/5

In that process, I noticed that the example given here: http://www.fourwalledcubicle.com/files/LUFA/Doc/120730/html/_page__software_bootloader_start.html and the check in BootloaderDFU.c seem to be opposit of each other: https://github.com/abcminiuser/lufa/blob/6d9077370b613e9bf26d5d5e03481258873efa02/Bootloaders/DFU/BootloaderDFU.c#L136-L154

It seems the example uses the magic key to enter the bootloader, while the bootloader uses it to exit the bootloader.

Additionally, it seems with this check, if using BOOTRST fuse, that the only way to enter bootloader is through an external reset, and not reachable via a watchdog timer reset.

Am I correct in assuming that this is not the intended behavior?

As I read the code DFU bootloader, the magic key is used to exit the code, which is fine, however in that case I think the mentioned check should be changed to:

if(MCUSR & (1 << PORF) || MCUSR & (1 << BORF) || (MagicBootKey == MAGIC_BOOT_KEY))
  JumpToApplication = true;

MCUSR &= ~((1 << PORF) & (1 << BORF));

I have opened a PR with a suggestion on this solution. Here: https://github.com/abcminiuser/lufa/pull/151

abcminiuser commented 4 years ago

Sorry, I meant to look at this weeks ago, but completely forgot about the Github notification. Looks like there is indeed an issue here.

The logic is supposed to be:

1) If BOOTRST is set, we always start running the bootloader after reset. If we weren't triggered by an external reset or if the magic bypass key isn't set, we run the user application. If the bootloader runs, and needs to start the user application, it'll set the magic key and trigger a watchdog reset, so that the user application starts from a fresh boot.

2) If BOOTRST isn't set, we got to the bootloader either by accident (regular application watchdog triggered while HWB pin pulled low, with HWBE fuse set) or on purpose (jump from application after watchdog reset to ensure bootloader starts from fresh boot.

In both cases, the magic key is only supposed to be internal to the bootloader - the user application needs the same mechanism to ensure that it can jump to the bootloader after a fresh start from a watchdog reset, in case BOOTRST isn't set.

I think we need to use a different magic key in the user application jump example to indicate it should enter the bootloader after a watchdog reset, so it doesn't conflict with the bootloader's magic escape key. That would solve the immediate issue I think and not require existing units in the field to be upgraded with an external programmer.

Duckle29 commented 4 years ago

Okay, I think I understood that, and have updated my PR, however, I'm unsure of how I'd manage to ensure that the variable access in the application, lives in the same RAM address as the one checked in the bootloader. Any clues on how I'd go about that?