Remove GPIO_BRR_* defines

[maribu]

The GPIO peripheral of the F4 series has no BRR register, only a BSRR register. Hence, delete all misleading defines.

[TOUNSTM]

Hello @maribu,

Thank you for this report. We will get back to you as soon as we analyze it further. This may take some time. Thank you for your comprehension.

With regards,

[TOUNSTM]

Hello @maribu,

Thank you for your report. Unfortunately, the GPIO_BRR register was defined in previous versions and to avoid any regression in our users' code, our development team decided to keep it in the cmsis device drivers as a legacy definition.

Please allow me to close this thread. Thank you for your comprehension.

With Regards,

[maribu]

This fixes a bug. I'm pretty sure you haven't even read the PR description...

[maribu]

In case my impression is wrong that this is closed with a copy pasted "customer compatibility" without every looking into this: Let me explain why this is a bug and will break code, but cannot help legacy code written for other family run.

Customer code written for STM32 MCU families with a BRR register will look like this:

    gpio->BRR = GPIO_BRR_BR0 | GPIO_BRR_BR1;

There now is a compatibility GPIO_BRR_BR1 define, but there still is no BRR member in the GPIO_TypeDef, as there is no BRR register. It won't compile. With the compatibility defines one could now use:

    gpio->BSRR = GPIO_BRR_BR0 | GPIO_BRR_BR1;

However, when having to touch that file anyways one could also update the macro names. Second, with mismatching register name and macro name everyone ever looking into this code would directly assume to have spotted a bug: Constants defined for use in the GPIO's BRR register are written into the BSRR register. And one would directly assume that pins are set, rather than reset.

Now let's assume we have code that is written for multiple STM32 MCU families (taking STM32F4 out of the picture here). One may have code looking like this:

void clear_pins(GPIO_TypeDef *gpio, uint32_t pins)
{
#ifdef GPIO_BRR_BR0
    gpio->BRR = pins;
#else
    gpio->BSRR = pins << 16;
#endif
}

This would now compile and run correctly with only BSRR or with both BSRR and BRR being present - except for the STM32F4: There it will fail due to the compatibility flag, as incorrectly the code would assume there is a BRR register despite no such register being present.

This is not a compatibility define easing the live of customers, but a bug that will cause issues.

[TOUNSTM]

Dear @maribu,

Thank you for having pointed out this aspect. The solution you suggested will be adopted and made available in a future release.

With regards

[TOUNSTM]

Hello @maribu,

I hope you are well. Your request has been submitted to our development team who has concluded that it is more appropriate to keep these legacy definitions. Indeed, some users of STM32F4 series may be using these definitions in their applications.

Even though, this decision may affect portability across series, these definitions must be retained for backwards compatibility in STM32CubeF4 firmware, which is given higher priority in our strategy.

Unfortunately, your request could not be processed this time. I appreciate your understanding and thank you again for having pointed this out.

Please allow me to close this issue now.

With Regards,

[maribu]

Hi,

thanks for looking into this. We have workarounds for the bug implemented and this isn't worth fighting over. But just one final remark:

    gpio->BSRR = GPIO_BRR_BR0 | GPIO_BRR_BR1;

is the only code this defined actually can be helpful. If this would be compiled for any other STM32 family that does have a BRR register, GPIO_BRR_BR0 would have the value 0x1 instead of (0x1 << 16). And there the same code would set the register rather than clear it.

This is a pretty obvious footgun...

Kind regards, Marian