Optiboot / optiboot

Small and Fast Bootloader for Arduino and other Atmel AVR chips
Other
1.08k stars 396 forks source link

optiboot.h if(read_character != 0 && read_character != 255) #299

Open SpenceKonde opened 3 years ago

SpenceKonde commented 3 years ago

In the functions that read a page of flash into an array, they treat a 0 (which would be a value that must have been written) and a 255 (0xFF, blank flash) the same way.... Though there's no documentation, and it's unclear how it's intended to be used, my best guess is that the intent is that someone would read a page of flash into the storage array, modify that array, and then write it back to flash? But in that case, it will turn all 255's into zeros, won't it? I can't really imagine a situation where that would be correct behavior....

WestfW commented 3 years ago

optiboot.h in this case is from the demo_dospm example. "read_flashPage()" seems to be more of a support command for the "read page of flash memory" command, rather than a general purpose function. The conversion of 255 and 0 to the "blank character" looks more display/print oriented than useful (the default "blankCharacter" is '.')

I was noticing as I wrote test_nvmctrl.ino for testing the xmega chips that I didn't "like" the dospm.ino demo very much. But on the other hand, "test_nvmctrl" is too complex to be a "demo" - it's more for the more experienced folks to poke at things.

SpenceKonde commented 3 years ago

Yeah - I honestly got kind of lost with test_nvmctrl - there's so much stuff around the interactions with optiboot and calls to optiboot.h functions that I couldn't see how to use it.... And optiboot.h just says to go look at the examples to see how to use the API... This is actually why I never added an example of how to do this to megaTinyCore... it's because I don't know how to myself, and I'd have to really study it to figure out how to use optiboot.h....

SpenceKonde commented 3 years ago

Well, I haven't tested it (optiboot.h would need modification - and as noted above, I don't really know how to use the basic version, much less adapt it to different hardware) - but after my latest work optimizing optiboot for DxCore, I can just barely fit the write-to-flash-from-app code for the Dx-series parts... except for parts with 128k of flash and the serial port on alternate pins.... But who would ever use that? What's that? You say the default pins for USART0 are used for external HF crystal on the DB-series? So the answer to my question is "almost everyone"?

Oh Hmmph... well, at least it's only 2 bytes away now.

Had to get aggressive optimizing on DxCore. Once I scrounge up 1 more instruction word (I'd love to get a couple more - realized there's a corner case that the modern-AVR versions weren't catching even with my mid-november fixes), and have a chance to figure out how to test the write-from-app stuff, I'll let you know. I'd say I was going to give you a PR, but I am confident you wouldn't merge it without reformatting my code with a flamethrower and disinfecting the ashes :-P

The adapted optiboot.h will need to be smart enough to point the flash mapping to the target section, among other things

SpenceKonde commented 3 years ago

Ooo! Actually, this may work as is; realized I could save a 2 words of flash by calling the existing function to write the nvm command... I need to stop doing this and work on the more urgent things for megaTinyCore and DxCore for now though...

WestfW commented 3 years ago

There is an improved demo of the do_spm() code:

https://github.com/Optiboot/optiboot/blob/master/optiboot/examples/demo_flashwrite/demo_flashwrite.ino

SpenceKonde commented 3 years ago

Hm... Thanks - what has happened on this front, actually, was that MCUDude implemented a wrapper and wrote it in a way that I could follow, and told me I should port it to megaTinyCore too. So that's what megaTinyCore did for white to clash from app code. So that sorted out megaTinyCore. Meanwhile, for DxCore I was never able to quite shoehorn it into 512b of flash if I did the entry conditions without leaving loopholes (though I realize now that I may have made an unjustifiedly pessimistic assumption about the way the word write system works) without jettisoning the do_spm() method. Instead, what I did (and which obviously reflects both my personal "system aesthetics" as well as the very different behavior of NVM controller for the Dx-series parts) was to realize that because the only thing that needs to execute from NVM on Dx... is the SPM instruction itself. You can change NVMCTRL.CTRLA in app code no problem - all it cares about is where the SPM (or ST) instruction that writes to the flash is located. Okay, no problem. I just put, right before the optiboot version number, two instructions, SPM Z+ and RET. Then in my flash from app code I copy the same assembly I wrote to write while in the bootloader, and just replace the spm z+ there, with call 0x1fa, after setting everything up the same way. 4 bytes in the bootloader section; also had the side benefit that it required no code changes to adapt to two other use cases where there's no optiboot bootloader involved: When you want to write to the APPDATA section from APPCODE, and when there's no actual bootloader, and you want to write to APPCODE from APPCODE. Obviously that's prohibited, so you have a dummy bootloader section configured (selfprogramming is disabled if BOOTSIZE == 0 anyway, always, so just be sure that you poke the IVSEL bit in w/e register of the interrupt controller it is before you enable interrupts).... and then expoanding the logic from just the normal optiboot case to the no-real-bootloader cases is easy-peasy: just #define the incantation that does the magic conditionally based on whether defined(USING_BOOTLOADER) ==1 ( call 0x1fa) or CODESIZE != 0 ( then section permissions are configured so just spm z+) - and if neither of those is true, but there is a define indicating it's enabled from the tools submenu, I call... a label in a 3-instruction tidbit of inline assembly in main.cpp in some weird section to guarantee it shows up in the first 512b (progmem gets positioned at lower addresses than almost everything) so I know it will be in the first 512b; it's rjmp .+4 to skip next 2 instructions, so whatever part of program flow I'm inserting it into, it moves out of the way :-P

There's one cute bit of "safety" from that method of achieving SPM: You can guarantee that it cannot be called by any jump other than a direct one to that exact location - that is to say - a mangled ijmp can't get you there, nor can a jump to any other location. You can jump there with ijmp, but it can't write anything, because you could only end up if the Z pointer was pointing to it. In which case, Z is pointing to a location in the bootloader section, which is not writable! (though IMO once you start writing to APPCODE from APPCODE via one of these tricks , yes, they can destroy the running app, my library docs make very clear that I don't put any guard rails up

My point being that at this point, both of my cores have a library written and working to write to flash, so I think I'm all set at this point - MCUDude's one for megaTinyCore that uses do_spm() (I don't have the same examples you do, but it looks like that's just a matter of which subsert of MCUDude examples we're using. also successfully communicated how to use the flasher. they're code he wrote. and mine for DxCore that uses the magic number (Oh, did I not mention that? I get it into the binary by declaring a ((used)) unsigned long with the appropriate value so it works out to be spm z+ ret , just like the optiboot version (and of course APP_NO_SPM replaces the first one with a NOOP or something else harmless like a second RET.