olliw42 / mLRS

2.4 GHz & 915/868 MHz & 433 MHz/70 cm LoRa based telemetry and radio link for remote controlled vehicles
GNU General Public License v3.0
295 stars 67 forks source link

System Bootloader Issue w/ STM32F1 Tx? #96

Closed jlpoltrack closed 1 year ago

jlpoltrack commented 1 year ago

Using STM32 F1 Dual E28 Tx, can't seem to connect to STM32CubeProgrammer w/ system bootloader using latest main. Steps to reproduce:

Use system bootloader on my RXs no problem using jumper at boot - something with calling 'BootLoaderInit()' later?

olliw42 commented 1 year ago

thx for the report,

the RXs you mentioned also are F1? With the jumper you also invoke calling the F1 jump-to-bootloader code, or it's shortening the BOOT pin? Just to figure out if it's a F1 jump-to-bootloader code issue, or a tx code issue.

jlpoltrack commented 1 year ago

the RXs you mentioned also are F1? With the jumper you also invoke calling the F1 jump-to-bootloader code, or it's shortening the BOOT pin? Just to figure out if it's a F1 jump-to-bootloader code issue, or a tx code issue.

RXs use the jump to bootloader code and are F1 (both Tx and Rx are STM32F1 Dual E28 boards).

Was able to get it working by commenting the HAL_DeInit(), LL_GPIO and LL_USART lines (these were the ones I didn't see mentioned online even though HAL_DeInit() is marked as 'is important':

image

olliw42 commented 1 year ago

thx hm... this is strange ... what you say is that the original code works on the rx but not the tx, but the modified code works for both

the modified code has some puzzles to me. The gpio & uart deinit are generally considered important, and I had to introduce them more than a decade ago in other projects of mine to make it working. I think it is even said in one of ST's documents. The HAL_Deinit I found to be important in the early days of mLRS, as I found it to not work at the time then.

There is something very fishy going on here.

Since it's only failing on the tx I would think the issue is rather that something isn't going correctly with the initialization of all the periphs in that code.

hmhmhm...

jlpoltrack commented 1 year ago

Also works with LL_GPIO and LL_USART enabled. This is a positive change as previously I had to click Connect twice as I got an error on the GETID command. Now will connect on the first try.

Going to try with STM32G4 later.

olliw42 commented 1 year ago

Also works with LL_GPIO and LL_USART enabled. This is a positive change as previously I had to click Connect twice as I got an error on the GETID command. Now will connect on the first try.

I'm a bit puzzled now as to what the statement is

so, you are talking about the original code but with only the HAL_DeInit() outcommented?

you are saying this is a positive change on the tx only or also on the rx?

jlpoltrack commented 1 year ago

so, you are talking about the original code but with only the HAL_DeInit() outcommented?

Yes, only HAL_DeInit and LL_RCC_SetSysClkSource commented out in latest update.

you are saying this is a positive change on the tx only or also on the rx?

Have only tried on Tx.

When LL_GPIO and LL_USART were commented out on first connect saw this warning in programmer and required clicking connect again. image

This went away after enabling LL_GPIO and LL_USART.

jlpoltrack commented 1 year ago

Checked with STM32F1 Rx, works fine with HAL_DeInit() commented out (same behavior as previously observed).

Edit - works with no HAL_DeInit on STM32WLE5 too.

olliw42 commented 1 year ago

@jlpoltrack just tried to reproduce the issue, but for me it works I did use my E28 dual F103 board (scheme as in hardware repo) and flashed it with current tx-diy-e28dual-board02-f103cb target, without any modifications in the code/hal. Connected via usb-ttl adapter to the com port, ran systemboot;, dicsonnected terminal, started cubeprogrammer, and it just worked fine ... not sure that's relevant, but cubeprogrammer says Device ID 0x410, Rev ID RevX, FlashSize 128kB, CPU CortexM3, Bootloader version - all user config option bits are checked

???

jlpoltrack commented 1 year ago

Very odd, with latest master I still need to comment out the HAL_DeInit line (no other changes).

My hardware is E28 Dual (your design), no diversity. I am not using official 128 kB hardware:

image

olliw42 commented 1 year ago

I am not using official 128 kB hardware:

you mean, you're using a F103C8? this should be fine

but it's a genuine STM32, right? not a GD or such?

jlpoltrack commented 1 year ago

Correct, STM32F103C8 (genuine ST, 64kB)

olliw42 commented 1 year ago

hm ... this really should work exactly the same ... strange

it's a bit mirroring the situation for the G4, where I find it to work on mine

very very strange

jlpoltrack commented 1 year ago

Agreed, very strange. Tried a different USB<>UART and same issue.

olliw42 commented 1 year ago

as mentioned elsewhere, I'm using this exact piece of code in the STorM32 project, which uses a set of different F103 mcus (RC,RE,T8,TB,C8,CB) whithout issues ...

I wouldn't mind removing the HAL_DeInit() - if there wouldn't be the comment "is important". I'm sure I've added this for a reason at some time, but can't recall (I should have added the reason or issue to the comment)

jlpoltrack commented 1 year ago

Sharing some findings.

First, HAL_DeInit() resets peripherals by writing a 1 and then a 0 to these 2 registers (force writes a 1, reset writes a 0): image

Of these registers, only APB2 is of interest because it resets USART1 (which system bootloader uses):

image

After some fiddling with the bits for each peripheral I found that resetting the SPI1 bit seems to be what is causing the system bootloader to not work as expected for me.

System Bootloader works (bit 12 not set):

image

System Bootloader doesn't work (bit 12 set):

image

mLRS does use SPI1 but not sure why this would impact this.

olliw42 commented 1 year ago

very interesting what happens if all are zero except bit 12 is set? what happens if you don't call HAL_Deinit() but LL_SPI_DeInit() instead in addition (should do the same, not work, right)

I have to say though the main question to me remains why you see the problems at all, and I don't. Is this happening just for this one F103 device, or would/do you see it for others with F103 too?

olliw42 commented 1 year ago

btw, on the F0/FRM303 I found that instead of __set_PRIMASK(1); one should clear the NVIC but keep isr enabled:

    // disable interrupts
    //__set_PRIMASK(1);
    // this is what works for F0 to enter DFU!
    __disable_irq();
    for (uint8_t i = 0; i < (sizeof(NVIC->ICER) / sizeof(*NVIC->ICER)); i++) {
        NVIC->ICER[i] = 0xFFFFFFFF;
        NVIC->ICPR[i] = 0xFFFFFFFF;
    }
    __enable_irq();

maybe worth a test for your F1 too...

(at the end of the day I would want to have one code which works for them all LOL)

jlpoltrack commented 1 year ago

very interesting what happens if all are zero except bit 12 is set?

Surprisingly, this allows system bootloader to work

what happens if you don't call HAL_Deinit() but LL_SPI_DeInit() instead in addition (should do the same, not work, right)>

This allows system bootloader to work (added after LL_USART_DeInit)

maybe worth a test for your F1 too...

Tried this too, seems to make no difference.

(at the end of the day I would want to have one code which works for them all LOL)

Agreed, not sure why my hardware is being uncooperative

olliw42 commented 1 year ago

very interesting what happens if all are zero except bit 12 is set?

Surprisingly, this allows system bootloader to work

what happens if you don't call HAL_Deinit() but LL_SPI_DeInit() instead in addition (should do the same, not work, right)>

This allows system bootloader to work (added after LL_USART_DeInit)

this makes it even more weired, doesn't it, right?

Agreed, not sure why my hardware is being uncooperative

you have more F103 to test, or is it just this one?

jlpoltrack commented 1 year ago

Going to close as seems specific to me