Feature: platforms/blackpill-f4: only setup LED_BOOTLOADER once

[lenvm]

Detailed description

This pull request slightly updates the code for the blackpill-f4 platforms:

• it adds an #ifdef around the line of the code where the LED_BOOTLOADER pin is set up using gpio_mode_setup() for the second time in the same file, in case BMP_BOOTLOADER is not defined. This change is made as it is redundant to setup the LED_BOOTLOADER pin twice.

Your checklist for this pull request

• I've read the Code of Conduct
• I've read the guidelines for contributing to this repository
• It builds for hardware native (make PROBE_HOST=native)
• It builds as BMDA (make PROBE_HOST=hosted)
• I've tested it to the best of my ability
• My commit messages provide a useful short description of what the commits do

[lenvm]

@dragonmux, now that v1.10 has been released, can this pull request be merged?

[dragonmux]

It's on our list to review as we need to properly spend the time on this to understand what it's doing and if it's taking things in the direction we want the code base going or not. We'll try and get a review done in the next few days.

[ALTracer]

@lenvm I took the liberty of reviewing this as I'm coming back to clean up and post platform-specific changes from my branches.

• gpio_set() & gpio_clear() are pure libopencm3 API functions, while gpio_set_val() is a BMD-specific shim over these. All of their usage can and does get inlined, but I would not Just migrate to it without some other added benefit (for example of honoring LED active polarity level). It would also break status-quo consistency with other platforms (yes they all also have a thing in SET_x_STATE() but it's a macro)
• gpio_mode_setup() walks the bitset of 16 gpio pins in a for-loop, so in fact doing more calls takes more time (and flash space, but it's less relevant for this platform) than aggregating all pins of a port and paying for a single call.
• LED_BOOTLOADER is not set up "by the bootloader section above" as far as BMF runtime payload is concerned. See that it ends in a scb_reset_core() into ST MaskROM, and is hidden away for BMP_BOOTLOADER=1 build variant (which I am pushing for).

If you have issues with platform LED placement -- I suggest you remap them to normal GPIOs, not the power-switch source-limited ones in PC13-15 range. I implemented an enhanced systick led show and it works just fine thanks to you inverting SET_IDLE_STATE() polarity -- PR pending. PC15 cannot be used to drive an LED at active-high level, it can only sink current under an LED already connected to board 3.3v power rail (and current-limiting resistor), which is the reason WeAct followed wiring it that way.

Additionally, I propose dropping the LED_BOOTLOADER entity altogether for the platform, because its sole purpose is to be constantly lit when in BMPbootloader (and we can't do anything to force ST MaskROM to blink it) via a gpio_clear() of PC15 if and only if there is an LED wired that way (similarly to PC13 one). That's how it was used in f4discovery we inherit code from. It can be obsoleted by a DFU blinky / LED show unless you can prove a sinking PC15 pin stays that way in ST MaskROM.

By the way, which bootloader option of this platform are you using -- ST or BMP?

TL,DR: I fail to see the problem that this PR solves.

[dragonmux]

Having given this a review finally (sorry it took so long!) we agree with ALTracer that we're not sure that switching all the set/clear calls to gpio_set_val() gains anything, other than having to then hope that the compiler does DCE (Dead Code Elimination) to the same underlying calls as were there before. We do like the improvement for the bootloader LED though so if you could rebase this onto main with just that change, we'll approve it for merge and get it in.

[lenvm]

@dragonmux, thanks for the review! I have updated the branch according to your request. It should be ready to merge now :)

[dragonmux]

This looks good to us. Please rebase it on main and then we'll get this merged. Thank you for the contribution!

[lenvm]

I have rebased on main. Please merge!