Skip to content

What is the purpose of erasing in banzai() #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
keestux opened this issue Jan 4, 2017 · 7 comments
Open

What is the purpose of erasing in banzai() #197

keestux opened this issue Jan 4, 2017 · 7 comments

Comments

@keestux
Copy link

keestux commented Jan 4, 2017

Hi,

In commit dd50d5c the 1200bps touch feature was added. But not only that, the code also erases the Reset vector (and thus it "self destructs") before doing a soft reset.
After the reset it enters the bootloader. The bootloaders sees no program and it will hang in there until a new program is uploaded.

Take a look at Reset.cpp, there is the function banzai. It is called (indirectly) from the the ticks handler. And this is initiated from CDC.cpp function CDC_Setup.

My question: What is the reasoning to "Erase application" before doing a reset?

Let me already give my opinion about it :-)
I find this code unnecessary, because it should be the task of the bootloader to erase (and load a new program)
Even if you want to be able to force the bootloader to stay in "bootloader mode", it should be possible to use the "double tap" trick that the bootloader itself uses, namely set the magic word (0x07738135) at address 0x20007FFC. I did some experimenting and it works perfectly well.

I find the current behaviour unwanted and maybe even a bit dangerous. Suppose there is a run-away pointer that changes the variable static int ticks in Reset.cpp, as soon as ticks is back to zero your program will be erased.

@facchinm
Copy link
Member

facchinm commented Jan 5, 2017

Calling banzai() was needed on DUE (since the SAM3X core only started in ROM bootloader mode if the flash content was previously erased). On Zero, it makes it easier to avoid Leonardo-like bugs (see arduino/Arduino#2474).
BTW, I like your proposal; since NVIC_SystemReset is not IRQ based (different from AVR soft reset, watchdog based), the memory location can be safely written atomically just before NVIC_SystemReset with interrupts disabled. I'll test this today and report back!

@Michael-Dutchband
Copy link

@facchinm did you ever get around to this? I ran into this today - looking for a hardware solution where a controller board needs updating remotely, i.e. the reset button could be an hour drive away from me.

I have a raspberry connect (and close) the connection to trigger a reset, and inadvertently hit banzai(). Interestingly, reconnecting the device to my Windows installation of Arduino refuses to flash, even though I see the LED pulse of being 'in' the bootloader. It resolves itself only if I physically hit the reset button or if I'm already trying to program while I connect the USB cable.

It does allow me to reprogram at any time (just doing the Blink example) if it hasn't been banzai'd. So this timeout for programming seems to be in the bootloader idle-loop only. It gives me the impression the serial-reset opens up a critical window from which no recovery is possible.

'Genuino Zero' - Arduino 1.8.7 (Win10 x64 installer), SAMD 1.6.19 board package, Atmel Studio 7.0.1931, EDBG Firmware 3.25, and programming over the USB, not the EDBG - production won't have that port.

Happy to assist in resolving this but new to the environment, let me know if there's alternatives I should try. Thanks.

@facchinm
Copy link
Member

@Michael-Dutchband the serial port open/close should not trigger a reset nor a banzai() unless the baud rate is set to 1200bps; this can happen during normal open() on certain platforms, thus banzai() does not occur immediately but after a timeout and can be stopped if the 1200bps open was not intentional; all the other symptoms make me think about some problems with signal integrity (once in bootloader - led fading - the board should be reprogrammable even if you don't press the reset button).
About this issue, I almost forgot about this, but it would be nice to integrate it in the near future. @cmaglie @sandeepmistry what do you think about it?

@Michael-Dutchband
Copy link

I was intentionally opening at 1200bps as I was looking at piupdue's approach to automatically upload new firmware from our application. Actually, I wasn't, that tool was, which instantly seemed to 'brick' the device (having to switch back to windows for a clean reflash) as it did a basic connection, noted it wasn't compatible, and closed out. But I'm considering doing a similar approach.

Currently trying to get the bootloader loaded through atmel studio with the blink application, but having some trouble getting things properly linked together. Hopefully I can then put some breakpoints on the "wait loop" to see why it doesn't want to program anymore after the initial boot.

@sandeepmistry
Copy link
Contributor

@facchinm yes the behaviour seems reasonable to me:

Even if you want to be able to force the bootloader to stay in "bootloader mode", it should be possible to use the "double tap" trick that the bootloader itself uses, namely set the magic word (0x07738135) at address 0x20007FFC. I did some experimenting and it works perfectly well.

@Michael-Dutchband would you be able to open a pull request for the changes you are suggesting?

@Michael-Dutchband
Copy link

@sandeepmistry I would like to, unless this is a courtesy question. In which case go right ahead.

I'm still having trouble getting the project set up properly in Atmel Studio. And with that I mean in a setting where I can debug my own application with the bootloader properly linked in. That seemed a bit off-topic here so I opened a topic on Arduino.cc but haven't gotten any replies yet.

I'd like to get that sorted first as I suspect there might be a second bug in the wait-loop being exposed once after banzai() is hit, but no new firmware is immediately presented. For some reason, it stops accepting new images then. Almost as if banzai() can't be called twice?

@sslupsky
Copy link

@facchinm @sandeepmistry @Michael-Dutchband @keestux Gentlemen, I came across the "banzai" code today and I agree with @keestux ... this is kinda scary code. Has there been any further progress toward resolution of this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants