search

SpenceKonde / megaTinyCore

Arduino core for the tinyAVR 0/1/2-series - Ones's digit 2,4,5,7 (pincount, 8,14,20,24), tens digit 0, 1, or 2 (featureset), preceded by flash in kb. Library maintainers: porting help available!
Other
557 stars 145 forks source link

Reset cause logic could be further improved #213

Closed SpenceKonde closed 3 years ago

SpenceKonde commented 4 years ago

I'm not convinced it's correct, currently...

Right now, it gets the flags, clears them all, and then jumps to app if it was a reset cause that shouldn't start bootloader... but that means that when the dog bites and it does the watchdog reset, it's coming into it with only WDRF. Well, that looks the same as a WDRF from the app! I think we should only stash-n-smath the reset cause register when we start the app

There are 6 reset cause flags, PORF, BORF, EXTRF, WDRF, SWRF, and UPDIRF.

If we reset only right before jumping to app, and did it every time - but not if we didn't jump to app and went down that path that leads to the WDR, then we would know that: If WDRF is set, we know it was reset by the watchdog, and if only WDRF, there because app WDT reset it. If WDRF is set and any others are, that's from Optiboot doing it's WD reset on top of the flag that brought us into the bootloader.., so of course we should jump to app. Other than that, I';m pretty sure (if Optiboot clears it before app start every time) you should never have more than 1 bit set... so if anything other than BORF is set, we enter bootloader, unless it's for use with UPDI as reset, in which case we don't start on just POR either.

SpenceKonde commented 4 years ago

This needs some testing...

WestfW commented 4 years ago

Yeah; I punted on the bootloader reset cause logic in the optiboot implementation, since it's substantially different than the prior ATmegas.

SpenceKonde commented 4 years ago

@WestfW - I actually already did some work on it, but it's still not very good. I don't know if we can get acceptable results passing the reset cause to application in the reset cause register; I may end up just doing what I did with micronucleus over on ATTinyCore: Copy the reset cause to GPIOR0 and then clear it (and clearly document this in the core documentation). BTW, I'm nearly ready to get a version of Optiboot for the Dx-series sent back to you. Just gotta lose 15 instruction words from the version for the 128k parts (but I've got a plan for that, one which I'm quite excited to implement too ;-) - hoping it will make enough space to bring back EEPROM write.

Also, uhm, did you know that there were several issues with the hook to let application write flash? Like, it was clearly never tested, because I've so far fixed two issues that would guarantee t could never have worked. I still haven't tested it, as I don't have a sketch to do that ready (I'm going to include a library for that). What I'm most concerned about is whether you can even fill the page buffer from non-bootloader space! Though even if you have to pass it a pointer to the start of a buffer in memory, I'm optimistic about fitting it. Maybe unjustifiably so, but still....

WestfW commented 4 years ago

No page buffer fills from the application. I checked. https://www.avrfreaks.net/forum/mega0xtiny-self-programming-flash#comment-3002231 (and I have working flash write code! Moving the buffer fills into optiboot as well fits fine (after all, that's what the old avrs did anyway; just "differently.")) I should have looked at what you'd already done, first... Sigh.

Note that the optiboot_x compile reports slightly bogus results from avr-size, since it includes the dummy "application section" (about 40 bytes?) as well as the bootloader itself.

I haven't looked at the Dx chips yet, other than to notice and dislike that the nvmctrl is significantly different than the mega0/xTiny. Sigh.

SpenceKonde commented 4 years ago

Thanks for the update.

Do you want to send me a PR, or should I just pull your changes in? They're in optiboot repo now?

I don't suppose you could share a demo of using that feature from a sketch? That would be really helpful for me as I try to put a wrapper around it to make it a little bit clearer to the average Arduino user how to actually use this

SpenceKonde commented 4 years ago

Reminder to self: As part of this, should also build "ersatz-reset" bootloaders for tinyAVR 0/1-series parts, which would run on POR (maybe there's even a way to signal to bootloader that a rescue attempt

SpenceKonde commented 3 years ago

It was not correct, see #259.

Now it is.

Final outstanding issue was the ersatz reset having a separate bootloader, now it does: ebfb0bd013c8ed3237a9aff7a38b3cd38ebeecf1