blackpill-f4: bootloader LED issues

[lenvm]

I am encountering two issues with the LED_BOOTLOADER on the blackpill-f4 platforms.

1. Bootloader LED only works on PC13

Description

The bootloader LED only works when it is assigned to PC13. If LED_BOOTLOADER is set to for instance PC15, as in the most recent commit (df014ff), the bootloader LED does not work.

Details

• I have tried assigning LED_BOOTLOADER to PC14, PC15 and PA1, but all result in the same: the LED of the respective pin does not turn on. Simultaneously, the LED_IDLE_RUN stops working if LED_BOOTLOADER is not assigned to PC13.
• I have also tried to remove all references to LED_BOOTLOADER from the code. In this case LED_IDLE_RUN works correctly on PC14, PC15 and PA1. LED_IDLE_RUN does not work when it is assigned to PC13.

The easy solution would be to swap the pin assignment of LED_BOOTLOADER and LED_IDLE_RUN, as then the LEDs are working. I would however prefer not to swap these pins, and understand what is the root cause of this issue.

2. Bootloader LED works inverted

Description

The bootloader LED works inverted to my expectation.

Details

• I would expect the bootloader LED to be on when the boot loader is active, and to be off when the boot loader is inactive. Instead, the bootloader LED is off when the boot loader is active, and the LED is on when the bootloader is not active.

I would appreciate your input to find out what could be the cause of and a fix for both of these issues.

[ALTracer]

Platforms like stlink and swlink toggle their respective led_bootloader

blackmagic/src/platforms/stlink/usbdfu.c

Lines 85 to 96 in b2156f9

	void sys_tick_handler(void)
	{
	if (rev == 0) {
	gpio_toggle(GPIOA, led_bootloader);
	} else {
	if (led2_state & 1U) {
	gpio_set_mode(GPIOA, GPIO_MODE_OUTPUT_2_MHZ, GPIO_CNF_OUTPUT_PUSHPULL, led_bootloader);
	gpio_clear(GPIOA, led_bootloader);
	} else
	gpio_set_mode(GPIOA, GPIO_MODE_INPUT, GPIO_CNF_INPUT_ANALOG, led_bootloader);
	++led2_state;

(or outright GPIO_C13) in usbdfu.c via systick_handler,

blackmagic/src/platforms/swlink/usbdfu.c

Lines 92 to 99 in b2156f9

	void sys_tick_handler(void)
	{
	switch (rev) {
	case 0:
	gpio_toggle(GPIOA, GPIO8);
	break;
	case 1:
	gpio_toggle(GPIOC, GPIO13);

and I would like to port that behaviour to blackpill-f4 platform as well. Something as simple as

diff --git a/src/platforms/common/blackpill-f4/usbdfu.c b/src/platforms/common/blackpill-f4/usbdfu.c
index a116e41e..0cc6a521 100644
--- a/src/platforms/common/blackpill-f4/usbdfu.c
+++ b/src/platforms/common/blackpill-f4/usbdfu.c
@@ -51,10 +51,16 @@ int main(void)
 
        rcc_clock_setup_pll(&rcc_hse_25mhz_3v3[PLATFORM_CLOCK_FREQ]);
 
+       /* Use SysTick at 10Hz to toggle-blink the blue LED at 5 Hz */
+       systick_set_clocksource(STK_CSR_CLKSOURCE_AHB_DIV8);
+       systick_set_reload(96000000U/8/10);
+       systick_interrupt_enable();
+       systick_counter_enable();
+
        /* Assert blue LED as indicator we are in the bootloader */
        rcc_periph_clock_enable(RCC_GPIOC);
        gpio_mode_setup(LED_PORT, GPIO_MODE_OUTPUT, GPIO_PUPD_NONE, LED_BOOTLOADER);
-       gpio_set(LED_PORT, LED_BOOTLOADER);
+       gpio_clear(LED_PORT, LED_BOOTLOADER);
 
        /* Enable peripherals */
        rcc_periph_clock_enable(RCC_OTGFS);
@@ -76,3 +82,8 @@ int main(void)
 void dfu_event(void)
 {
 }
+
+void sys_tick_handler(void)
+{
+       gpio_toggle(LED_PORT, LED_BOOTLOADER);
+}

The 96e6 could be a rcc_ahb_frequency variable but then it's not subject to constant propagation.
native platform has entire 3 pins mapped to onboard LEDs (PB2 LED_UART, PB10 LED_IDLE_RUN, PB11 LED_ERROR) and sweeps them in usbdfu mode. So there's no single LED_BOOTLOADER.

blackmagic/src/platforms/native/usbdfu.c

Lines 90 to 94 in b2156f9

	/* Run the LED show only if there is no DFU activity. */
	if (dfu_activity_counter != 0) {
	dfu_activity_counter--;
	reset = true;
	} else {

When attaching external LEDs please keep in mind the note from RM0383 on RTC/LSE pins GPIO PC13/PC14/PC15:

Due to the fact that the switch only sinks a limited amount of current (3 mA), the use of PC13 to PC15 GPIOs in output mode is restricted: the speed has to be limited to 2 MHz with a maximum load of 30 pF and these I/Os must not be used as a current source (e.g. to drive an LED).

BMP firmware does not use neither LSE pads/crystals nor the RTC IP block, so I think we could use all three pins as sinking slow GPIOs, active-low, like designed on stm32f103-minimum and blackpill-f4 PCBs.

My thoughts on this are: honestly this entire situation asks for a BSP abstraction, factoring out the led_enable/led_disable and GPIO_ACTIVE_HIGH / GPIO_ACTIVE_LOW, like in STM32Cube_F4 packages, or in Linux device-tree gpio bindings. There's already a lot of code in BMP using gpio_set()/gpio_clear(), sometimes sprinkled with comments on active polarity.

For blackpill-f4 I propose we drop LED_BOOTLOADER altogether and blink the PC13 which is LED_IDLE_RUN and the only user-programmable onboard LED in a sys_tick_handler only in usbdfu.c. Even on 96MHz DIV8 the 24-bit reload allows a 10Hz trigger rate.

I don't know what to do with LED_ERROR or LED_UART.
There is a sys_tick_handler touching the LED_IDLE_RUN in

blackmagic/src/platforms/common/stm32/timing_stm32.c

Lines 57 to 59 in b2156f9

	if (running_status)
	gpio_toggle(LED_PORT, LED_IDLE_RUN);
	SET_ERROR_STATE(morse_update());

but it only ever uses LED_ERROR to blink out the morse error message, I think.
LED_UART is used in aux_serial.c+usb_serial.c to indicate hardware USART activity (both on reception and transmission.

@lenvm Are you using a breadboard with additional LEDs for UART/ERROR/BOOTLOADER? Can you post something like a fritzing or imgur photo of your setup?
On an unrelated note: are you using the main branch with my PRs incorporated and with BMP_BOOTLOADER=1? Does BMP DFU work for your boards? It writes a corrupted binary on mine.

[lenvm]

Hi @ALTracer, thanks for your inputs!

I am using a small PCB that I've prototyped. It's still a work in progress. The PCB has a Black Magic Debug Unified Connector and uses all 4 LEDs: LED_IDLE_RUN, LED_BOOTLOADER, LED_ERROR and LED_UART. It also has the UART TX and RX pins broken out to a 2x1 2.54mm header, to allow easy connection to a UART with jump wires. It would therefore be convenient to me to retain at least the LED_IDLE_RUN (with bootlader functionality), LED_ERROR and LED_UART.

It would be cool if the LED_IDLE_RUN (PC13) blinks when the bootloader is active. As written in the description of the first issue, I cannot get LED_IDLE_RUN to work on PC13, so it would be helpful to first figure out why this is the case. Only when using LED_BOOTLOADER as PC13, the LED on PC13 works properly.

To answer your unrelated question: I use make PROBE_HOST=blackpill-f401cc SHIELD=1 to compile, based on my branch blackpill-f4-add-shield-macro. This branch is based on main (b2156f9) and therefore includes your changes from #1508. The SHIELD macro invokes ALTERNATIVE_PINOUT 1 and PLATFORM_HAS_POWER_SWITCH. I don't use BMP_BOOTLOADER=1. Using this I am able to start the board in bootloader using the steps from the blackpill-f4 readme and upload the firmware using dfu-util. The board is fully functional with this code uploaded via dfu-util.

[ALTracer]

I am using a small PCB that I've prototyped. It's still a work in progress.

@lenvm That's why I'm asking. I need to understand how you've wired the LEDs. WeAct MiniF4 schematic shows two LEDs:
3.3v -> R 5.1k -> Red LED -> GND for Power indicator (always-on) and
3.3v -> R 1.5k -> Blue LED -> PC13 GPIO for user-programmable indicator (set logic low to sink <3mA).

I cannot get LED_IDLE_RUN to work on PC13, so it would be helpful to first figure out why this is the case. Only when using LED_BOOTLOADER as PC13, the LED on PC13 works properly.

Strange. It works on vanilla blackpill-f4 boards for me, as in, the LED toggles sometimes on blackmagic activity.
It could be due to wrong initialization code, but

1. rcc_periph_clock_enable(GPIOC) is always called, RTC is never enabled, RCC is never reconfigured with LSEON bit which would take over the PC14/PC15. These don't have alternative functions.
2. gpio_mode_setup() changes direction from default inputs to outputs, and gpio_set_output_options() is not necessary because GPIO_OTYPE_PP=0 (push-pull as opposed to open-drain) and 2MHz defaults are okay for this purpose. One could argue such active-low onboard LED layouts should use open-drain configuration.

Reading some more through DS and RM I noticed a second remark from ST. RM0383 (and RM0090), paragraph 8.3.13:

The PC14/PC15 GPIO functionality is lost when the 1.2 V domain is powered off (by the device entering the standby mode) or when the backup domain is supplied by VBAT (VDD no more supplied). In this case the I/Os are set in analog input mode.

After watching all 10 GPIOC registers via OOCD+DragonProbe Pico during quite a few different resets (BMP->BMPDFU and back, ST DFU, triggered both by PA0 key + nRST key, or dfu-util --detach) I didn't get a clear picture.
Sure, ST BootROM reconfigures PC10/PC11/PC12 into AF SPI3 slave mode, but nowhere in AN2606 for STM32F401/411 does it state anything regarding PC13/PC14/PC15 or LSE usage. But nevertheless the LED pins become inputs, which is default.

We specifically call scb_reset_system() on the first reset-jump BMP->BMP_Init->ST_DFU (or BMP->BMP_DFU) and scb_reset_core() on the second jump, BMP_Init->ST_DFU (or from dfu_detach_complete() to keep the peripherals' configuration, such as GPIOs, when entering the respective DFU implementation.

My conclusion is that once you hand over control to ST DFU, all bets are off. It sets RCC to 60MHz, drops voltage range to lowest, enables a bunch of peripherals and GPIO AF, and clobbers first 12 KBytes of SRAM. However, it works, and it's reliable. It would help you understand what is happening if you had a disassembled and cross-referenced listing of the dumpable 30k BootROM V9.x for STM32F401 at 0x1FFF0000-0x1FFF77FF.

To be able to blink anything in "bootloader" mode, I need to fix the BMPBootloader for F411. Currently it only works enough to allow readouts and to seamlessly jump into payload on first boot. There's no DFU_DETACH command handler in dfucore.c (but there is one in usb_dfu_stub.c); rather, it detaches after download. I see why this was a problem.

[lenvm]

I have also tested the code on a vanilla board as I have multiple boards. There I obtain the same results. The PC13 LED is not turning on when used as LED_IDLE_RUN, but it is turning on when LED_BOOTLOADER is defined as PC13.

Interesting that you mention “3.3v -> R 1.5k -> Blue LED -> PC13 GPIO for user-programmable indicator“.

The LEDs that I have connected on my PCB (PC14, PC15, PA1) are wired as
PCxx -> LED -> R 330 -> GND. Thus the user-programmable LED uses the GPIO as a sink, whereas the LEDs on the PCB shield use the GPIO as a source. This may be a hint why the bootloader LED is only working on PC13 and not when assigned to any other LEDs, and why LED_IDLE_RUN is not working on PC13, but is working on PC14, PC15 and PA1.

[ALTracer]

The PC13 LED is not turning on when used as LED_IDLE_RUN, but it is turning on when LED_BOOTLOADER is defined as PC13.

Did you also look into, or try to change, any of LED handling logic? Once BMP DFU F4 sector erase bug got fixed, I returned to hacking and added the mentioned systick blinking of LED_BOOTLOADER. Of course, I saw nothing, because I don't have a bunch of free LEDs laying around to plug into a proto board. After adding LED_IDLE_RUN, the onboard PC13 active-low LED started blinking.

You can cherry-pick ALTracer@57e40b0 to reproduce this.

On another note, I pitched the idea of JTAG shield with level-shifters, headers and additional LEDs to the PCB designers on my team. Are your boards/designs open-source hardware? I've found a railgun BMPM-compatible design in PRs with new level-shifters and even an ADUM4160, but I really don't like the F103 on it and lack of SPI headers.

[lenvm]

I did have a look into the LED handling logic. Although I don’t find the defines used for SET_IDLE_STATE and SET_ERROR_STATE particularly nice in terms of code (see blackpill-f4.h#L215-L222), and sometimes gpio_set_val() is used and elsewhere gpio_set() and gpio_clear() which seems inconsistent, I didn’t find any real peculiarities in the code regarding the LEDs.

Regarding your question if my PCB design is open source: Following the discussion on why the BMP design files are not published, I understand @esden’s reasoning that this project is only being properly maintained thanks to the hardware sales, for which I’m very grateful. I would therefore not want to open my PCB design and reduce the sales of native BMPs, unless @esden would give his explicit go ahead. On top of this I would like to guarantee a certain quality of the hardware released and have first managed to get the boards with this shield PCB (including the LEDs) working properly, and spent some more time on testing and improving the design.

[lenvm]

I have done some additional research on the bugs described.

1. LED_IDLE_RUN issue [Resolved]

I have been able to resolve the issue that LED_IDLE_RUN does not work when it is assigned to PC13.

The schematic of PC13 is as follows:
3.3v -> R 1k -> Blue LED -> PC13 (reference). Therefore PC13 needs to be set to low to turn the LED on. The LED can be turned on by updating state to !state in

blackmagic/src/platforms/common/blackpill-f4/blackpill-f4.h

Line 235 in 9fe5889

gpio_set_val(LED_PORT, LED_IDLE_RUN, state); \

This change will be implemented in #1645

2. LED_BOOTLOADER issue [Open]

The issue that LED_BOOTLOADER only works on PC13 is caused by LED_BOOTLOADER always being low. Therefore, only an LED that turns on when being pulled low will work (PC13), whereas other LEDs that need to be pulled high to turn on (PC14, PC15) will not work.

The issue is that LED_BOOTLOADER is always low, whereas one would expect that it is high when the bootloader is active, and low when the bootloader is inactive.

My observations:

• LED_BOOTLOADER is low when LED_BOOTLOADER is setup as a pin, irregardless if it is set up as PUPD_NONE, PUPD_PULLUP, PUPD_PULLDOWN (tested with LED_BOOTLOADER as PC13), in

  blackmagic/src/platforms/common/blackpill-f4/blackpill-f4.c

  Line 97 in 9fe5889

  gpio_mode_setup(LED_PORT, GPIO_MODE_OUTPUT, GPIO_PUPD_NONE, LED_IDLE_RUN | LED_ERROR | LED_BOOTLOADER);

• LED_BOOTLOADER is not low when

  blackmagic/src/platforms/common/blackpill-f4/blackpill-f4.c

  Line 97 in 9fe5889

  gpio_mode_setup(LED_PORT, GPIO_MODE_OUTPUT, GPIO_PUPD_NONE, LED_IDLE_RUN | LED_ERROR | LED_BOOTLOADER);

  is removed from the code. I have tested this by defining LED_BOOTLOADER as PC13 (LED on when low): the LED is on (PC13 low) when this line is present, and it is off (PC13 not low) when this line is removed.
• Changing gpio_set to gpio_clearor vice-versa in

  blackmagic/src/platforms/common/blackpill-f4/blackpill-f4.c

  Line 62 in 9fe5889

  gpio_set(LED_PORT, LED_BOOTLOADER);

  and

  blackmagic/src/platforms/common/blackpill-f4/usbdfu.c

  Line 57 in 9fe5889

  gpio_set(LED_PORT, LED_BOOTLOADER);

  does not change the output of LED_BOOTLOADER: it is always low.
• The LED assigned to LED_BOOTLOADER, either PC15 or PC13 for debug purposes, is off when the bootloader is active. One would expect the LED to be on when the bootloader is active.

@dragonmux, do you perhaps have a clue why LED_BOOTLOADER is always low when it is setup as a pin?

[lenvm]

I have done some additional investigations.

The pin assigned as LED_BOOTLOADER only stays low when the boot loader is activated using the following steps:

• Push NRST and BOOT0 buttons
• Wait a moment
• Release NRST button
• Wait a moment
• Release BOOT0 button

The pin assigned as LED_BOOTLOADER becomes high, and therefore functions as expected, in the following cases:

• the bootloader is activated by running du-util, without pressing any buttons
• the bootloader is activated by the following steps:
  • Push NRST and KEY (PA0) buttons
  • Wait a moment
  • Release NRST button
  • Wait a moment
  • Release KEY (PA0) button

Is the expected behavior for the LED_BOOTLOADER pin that it stays low when the bootloader is activated using the NRST and BOOT0 buttons? Or should the user also expect the bootloader LED to turn on in this case?

[dragonmux]

For consistency with other platforms, we'd expect it to turn on in that case. Unfortunately we don't have particular familiarity with the project's Black Pill bootloader, so can't offer any salient advice, though it seems you're making good progress on analysing the behaviour and trying to find a solution.