stm32: Fix machine.bootloader() for stm32 f7 & h7

[andrewleech]

On my stm32f765 based board, the machine.bootloader() function doesn't work.
The board simply reboots back into micropython.
I'm not sure if this is the same on other stm32f7 boards or not, I don't have any others to check on.

I've seen the discussion on st forums from @dhylands regarding the current implementation: https://community.st.com/s/question/0D50X00009XkfN7SAJ/would-like-to-enter-dfu-mode-programmatically-on-stm32f7
Clearly it does/did work for some stm32f7 boards, but not for mine for some reason.
Possibly due to changed ram / mpu settings, or some other runtime change that's incompatible with the DFU startup code.

This PR resolves the issue for me, based on suggestions here and other places: https://community.st.com/s/question/0D50X00009XkXKUSA3/jump-to-internal-bootloader-with-stm32f7

Many posts on the topic refer to needing the chip to be in as close to reset conditions as possible before jumping to the DFU code.
This is achieved by writing a known "magic" pattern into a ram location and rebooting, then checking for said pattern at the very start of the assembly Reset_Handler.

[andrewleech]

In future I'll be migrating to the micropython bootloader implementation, however I don't have enough spare flash currently to include it. As such we'll be sticking with the stm32 rom bootloader for now.

[dhylands]

I like this approach much better than the original, because it uses NVIC_SystemReset which will reset all of the peripherals and other interrupt sources.

[andrewleech]

It's just annoying that it requires a magic number referenced separately in asm and c files.
At least the address can can be referenced to a linker variable to keep that in sync and board agnostic.

I believe a similar approach can be used for the F4 too, but there's always the "if it aint broke..."

[dpgeorge]

I can see how this is a comprehensive solution, but it makes me uneasy putting rarely used code in the critical path of reset like this. In particular, what if by accident there was the magic constant 0xdeadbeef in the right memory location, which inadvertently made the device go into DFU mode forever (eg out in the field)?

[dpgeorge]

r0 will be clobbered by the above code!

[dpgeorge]

Would it not be more efficient to use _estack, which is already loaded into sp below. That would save one instruction.

[andrewleech]

I definitely understand your reservations, I'm not particularly happy with it myself - it feels pretty hacky.

I guess there's no reason I can't just make a global variable and then use that for the marker, for the cost of one byte it avoids the possibility of overlap.

I agree with your other comments above too, I'll clean up the register / location usages.

[andrewleech]

Ok, for now I've changed it to work with a declared variable, rather than just re-using any particular location. This at least looks a lot cleaner to my mind, but yes it does add some extra code at reset for a arguably rarely used function.

I'm also ok if you decide to not merge this due to the required changes to reset code, in that case this PR can serve as reference for others that need this to work, or a starting point for a safer implementation.

As mentioned, we've got a medium term plan to migrate to the micropython bootloader anyway... is there a way to jump to that from micropython application code?

[dpgeorge]

is there a way to jump to [mboot] from micropython application code?

Yes:

    {
        #if __DCACHE_PRESENT == 1
        SCB_DisableICache();
        SCB_DisableDCache();
        #endif
        __set_MSP(*(volatile uint32_t*)0x08000000);
        ((void (*)(uint32_t)) *((volatile uint32_t*)(0x08000000 + 4)))(0x70ad0000);
    }

This code snippet will soon make its way into master, so that machine.bootloader() jumps to mboot (and something like machine.bootloader(1) jumps to ST's ROM loader).

[dhylands]

I can see how this is a comprehensive solution, but it makes me uneasy putting rarely used code in the critical path of reset like this. In particular, what if by accident there was the magic constant 0xdeadbeef in the right memory location, which inadvertently made the device go into DFU mode forever (eg out in the field)?

One solution would be to have the bootloader clear the location containing the DEADBEEF then it would only enter into the bootloader once.

[dpgeorge]

One solution would be to have the bootloader clear the location containing the DEADBEEF then it would only enter into the bootloader once.

Yes, I think the code already does this.

@andrewleech since resethandler.s is a pure assembler file it is passed straight to the assembler (not via gcc) so I don't think C preprocessor directives work in it. It could be changed to a .S file.

But maybe we can fix the root cause of this issue: is it the F765 that is different to other F7's (I know machine.bootloader() works on F722, F733, F767), or is your code enabling some interrupts/peripherals that need to be turned off?

[andrewleech]

765 is basically identical to 767 (they share the same datasheet) so I doubt it's a hardware incompatibility.

Yes I suspect it's a peripheral thing, so it should be possible to continue to shut down things until it works...
The reset script route is the hammer that will always work, but also always adds code that's getting run and is messier in that the functionality is split between two otherwise unrelated places.

I didn't explicitly check if my #if's were working, I assumed they should be when I didn't get built failures.

[dpgeorge]

Yes I suspect it's a peripheral thing, so it should be possible to continue to shut down things until it works...

A simple way to do this is just zero out all the RCC ENR's, eg:

RCC->AHB1ENR = 0
RCC->AHB2ENR = 0
RCC->AHB3ENR = 0
RCC->APB1ENR = 0
RCC->APB2ENR = 0

[andrewleech]

machiine.bootloader() already includes

HAL_RCC_DeInit();
HAL_DeInit();

which appears to basically to that, but to be sure I tried adding

CLEAR_REG(RCC->AHB1ENR);
CLEAR_REG(RCC->AHB2ENR);
CLEAR_REG(RCC->AHB3ENR);
CLEAR_REG(RCC->APB1ENR);
CLEAR_REG(RCC->APB2ENR);

I also built with sdram not turned on.
Also disabled all my startup code (empty main.py)
Still not working with the existing implementation :-(

Tried also copying the jump to bootloader assembly from this pr's resethandler into machine_bootloader() instead of ((void (*)(void)) *((uint32_t*) 0x00000004))(); but that didn't help either.

Maybe DMA? Haven't found a straightforward way to disable all of them to try.
This post refers to dual bank flash issues, though I don't know how that could be the problem if other similar parts have been working: https://stackoverflow.com/questions/42020893/stm32l073rz-rev-z-iap-jump-to-bootloader-system-memory

[hardkrash]

FYI, the existing bootloader() call in 1.9.4 breaks the STM DFU over UART. My application uses the UART at 115200 kbps. When I use bootloader() the UART auto baudrates incorrectly. Send in 115200bps, get out 900bps. I have to use boot0 externally with a reset.

I would love a bootloader() that clears all configurations to POR settings...

[TravisJoe]

I am also seeing sporadic pyb.bootloader issues with the F405, which I am assuming is implemented similar in machine.bootloader() at the moment. They are seemingly random so diagnosing or really observing it well is hard.

Physically putting it into bootloader mode is not an option, and I need to be able to support devices that might not have MBOOT on them.

It seems like this needs to be one of those bullet proof features, in spite of possibly not having an elegant solution.

[andrewleech]

My original solution here was not only inelegant, but somewhat fragile. It would be very easy to break it accidentally in the future.

While I think providing valid but empty interrupt handlers and a working watchdog handler would resolve most of the issues, it won't reset all peripherals to defaults for the stm bootloader.

I think a cleaner version of this PR is possible though, going via reboot and checked at the start of main rather than in assembler. As long as the bootloader just is done before peripheral inits it should be fine; I've been running with a python implementation where the reboot is detected in boot.py and the jump done there and that seems fine. We should be able to have a __no_init __root variable or similar (https://community.st.com/s/question/0D50X00009XkeLySAJ/keep-ram-value-when-nvicsystemreset) for use in bootloader reset requesting in a single easy to maintain location. I might be able to find some time soon to try this out

[dpgeorge]

See #4862 for another version of this that also uses NVIC_SystemReset.

[dpgeorge]

Fixed in a different way in 04c7cdb