ST, please maintain style in the CMSIS-mandated headers

[waclawek.jan]

In [CubeH7]\Drivers\CMSIS\Device\ST\STM32H7xx\Include\stm32h743xx.h I see:

#define RCC_PLLCFGR_PLL1RGE_0                  (0x0UL << RCC_PLLCFGR_PLL1RGE_Pos) /*!< 0x00000000 */
#define RCC_PLLCFGR_PLL1RGE_1                  (0x1UL << RCC_PLLCFGR_PLL1RGE_Pos) /*!< 0x00000004 */
#define RCC_PLLCFGR_PLL1RGE_2                  (0x2UL << RCC_PLLCFGR_PLL1RGE_Pos) /*!< 0x00000008 */
#define RCC_PLLCFGR_PLL1RGE_3                  (0x3UL << RCC_PLLCFGR_PLL1RGE_Pos) /*!< 0x0000000C */

The _0, _1 etc. suffix is used across all headers and all registers to denote *individual bits* of a non-single-bit bitfield in the SFRs. Here, it confusingly denotes *values*.

ST, please stop being innovative and stick to established style.

Please, make up clear policies regarding the CMSIS-mandated headers, and STICK TO THEM rigorously.

Also, whle I applaud adding values for non-single-bit bitfields, please do this for ALL such bitfields in headers for ALL STM32 devices. Please don't shift them to the position as it prevents the reuse for bitfields of the same meaning at other positions; and please do it in a uniform style, making clear the intention (there's no point making numeral symbols for plainly enumerated values, the symbol has to be explanatory); and, as said above, use a style clearly distinct from the style used to denote individual bits.

Please note, that symbols defined haphazardly in the "libraries" (SPL, Cuben) are of no relevance for developers who don't intend to use these "libraries".

JW

[Stanislav Husár]

Also, I have already seen differences in these #defines between F0 and F1 (example), that do not result from architecture differences. Like, GPIO->BSRR vs. GPIO->BSRRH,GPIO->BSRRL.

That is the worst thing I can imagine, having to remember everything once for F0, second time for F1,...

Or having to look into the .h file each time I want to code something.

So, do uniformly, what can be done uniformly.

[Stanislav Husár]

Last idea, not so strictly neccesary than previous post.

I like #defines like RCC_CFGR_SW_PLL1. When you see this, you immediately know what it does. When you see RCC_CFGR_SW_1, you need to look at register definition, to understand what it does.

[AVI-crak]

There is a line above

#define RCC_PLLCFGR_PLL1RGE RCC_PLLCFGR_PLL1RGE_Msk

The mask is used to reset the data, as well as to set the new value.

RCC-> PLLCFGR & = ~ RCC_PLLCFGR_PLL1RGE;

RCC-> PLLCFGR | = _VAL2FLD (RCC_PLLCFGR_PLL1RGE, new_value);

Separate bits are reserved for backward compatibility. However, this compatibility has already become the throat, and in every way interfered. At the moment, this compatibility brings confusion with real single bits.

Probably it makes sense to divide the main file stm32h743xx.h into two parts - in the first working data, in the second - additional compatibility. Those who will be few - just connect an additional file.

But what is there exactly superfluous is the sheet of the description of the CAN interface masks. It should be cut out completely.