system header deterioration bug ...

[Ozone]

I recently struggled with the SPI peripheral of the stm32c031, the reference manual is somewhat veiled on occasions.

Anyway, taking a closer look at the relevant system header part, I saw this :

  * @file    stm32c031xx.h
  * @author  MCD Application Team
  * Copyright (c) 2022 STMicroelectronics.
...
 
  /**
  * @brief Serial Peripheral Interface
  */
typedef struct
{
  __IO uint32_t CR1;      /*!< SPI Control register 1 (not used in I2S mode),       Address offset: 0x00 */
  __IO uint32_t CR2;      /*!< SPI Control register 2,                              Address offset: 0x04 */
  __IO uint32_t SR;       /*!< SPI Status register,                                 Address offset: 0x08 */
  __IO uint32_t DR;       /*!< SPI data register,                                   Address offset: 0x0C *
  __IO uint32_t CRCPR;    /*!< SPI CRC polynomial register (not used in I2S mode),  Address offset: 0x10 */
  __IO uint32_t RXCRCR;   /*!< SPI Rx CRC register (not used in I2S mode),          Address offset: 0x14 */
  __IO uint32_t TXCRCR;   /*!< SPI Tx CRC register (not used in I2S mode),          Address offset: 0x18 */
  __IO uint32_t I2SCFGR;  /*!< SPI_I2S configuration register,                      Address offset: 0x1C */
  __IO uint32_t I2SPR;    /*!< SPI_I2S prescaler register,                          Address offset: 0x20 */
} SPI_TypeDef;

Especially the line "__IO uint32_t DR;" gave me a headache, which caused my code to trigger two transfers instead of one. I have resolved this issue in the meantime.

The point is, SPI->DR is 16-bit wide, not 32-bit.

This is what the reference manual says:

 

And then I realized almost all other registers are 16-bit as well, but defined as 32-bit.

Just sloppy, I say.

There were times when ST had put more efforts into their system headers, here the same section of the stm32f0xx.h for comparison:

/** 
  * @brief Serial Peripheral Interface
  */
  
typedef struct
{
  __IO uint16_t CR1;      /*!< SPI Control register 1 (not used in I2S mode),       Address offset: 0x00 */
  uint16_t  RESERVED0;    /*!< Reserved, 0x02                                                            */
  __IO uint16_t CR2;      /*!< SPI Control register 2,                              Address offset: 0x04 */
  uint16_t  RESERVED1;    /*!< Reserved, 0x06                                                            */
  __IO uint16_t SR;       /*!< SPI Status register,                                 Address offset: 0x08 */
  uint16_t  RESERVED2;    /*!< Reserved, 0x0A                                                            */
  __IO uint16_t DR;       /*!< SPI data register,                                   Address offset: 0x0C */
  uint16_t  RESERVED3;    /*!< Reserved, 0x0E                                                            */
  __IO uint16_t CRCPR;    /*!< SPI CRC polynomial register (not used in I2S mode),  Address offset: 0x10 */
  uint16_t  RESERVED4;    /*!< Reserved, 0x12                                                            */
  __IO uint16_t RXCRCR;   /*!< SPI Rx CRC register (not used in I2S mode),          Address offset: 0x14 */
  uint16_t  RESERVED5;    /*!< Reserved, 0x16                                                            */
  __IO uint16_t TXCRCR;   /*!< SPI Tx CRC register (not used in I2S mode),          Address offset: 0x18 */
  uint16_t  RESERVED6;    /*!< Reserved, 0x1A                                                            */ 
  __IO uint16_t I2SCFGR;  /*!< SPI_I2S configuration register,                      Address offset: 0x1C */
  uint16_t  RESERVED7;    /*!< Reserved, 0x1E                                                            */
  __IO uint16_t I2SPR;    /*!< SPI_I2S prescaler register,                          Address offset: 0x20 */
  uint16_t  RESERVED8;    /*!< Reserved, 0x22                                                            */    
} SPI_TypeDef;

While it might make no difference in some cases, it will do so in others.

Just saying ...

[Pavel A.]

The SPI data register is special, it is sensitive to the write cycle width vs. the data frame size in bits.

IIRC it can split 32-bit write to 2*16 bit or 4*8 bit entries in the TX FIFO.

To avoid these complications, write to DR the minimal width for the selected frame size: 8 bits if frame size <= 8 bits, else 16 bit.

[Ozone]

> IIRC it can split 32-bit write to 2*16 bit or 4*8 bit entries in the TX FIFO.

I think not.

The register is 16-bit wide (according to the reference manual), and the upper half word surely goes to nirvana.

And declaring the peripheral register as 32-bit causes the toolchain to always emit a 32-bit write access if one uses DR directly.

> To avoid these complications, write to DR the minimal width for the selected frame size: 8 bits if frame size <= 8 bits, else 16 bit.

I know.

But my point is something else - the careless and sloppy header file.

In fact, <ALL> peripheral registers are declared as 32-bit. Which does not reflect reality.

To me, it looks very much like machine-generated. Which is IMHO no excuse for mediocre code.

[Simon.T]

Hello @Ozone  ,

The SPI registers are declared as 32-bit like all other registers because STM32C0 is a 32-bit Cortex M0+ MCU and all registers are 32-bit wide. Furthermore the CMSIS file is subject to rules/recommandation coming from ARM and they recommand to use 32-bit registers in the typedef struct (https://arm-software.github.io/CMSIS_5/Core/html/device_h_pg.html )

Concerning the SPI the bit number from 16 to 31 are always reserved on all SPI registers this is why these bits are not shown in the documentation but the register size is still 32-bit.

The size of the content inside each registers are specified by the reference manual and they must be handle by the masks available on the header file.

Moreover, I checked on the last STM32F0 cube firmware package and it is the same as STM32C0 all registers are declared as 32-bit wide.

Best regards,

Simon

[Ozone]

Sorry, but I beg to disagree.
> The SPI registers are declared as 32-bit like all other registers because STM32C0 is a 32-bit Cortex M0+ MCU and all registers are 32-bit wide.

No, they are not. Just look up the reference manual.
Just because the are aligned to 32-bit word addresses does not make them 32-bit wide.

> Furthermore the CMSIS file is subject to rules/recommandation coming from ARM and they recommand to use 32-bit registers in the typedef struct 

I don't think so.
CMSIS defines the system components common to all Cortex M implementations.
The other peripherals are fully up the the vendor - which would be ST in this case.
And this 32-bit declaration causes the generation of incorrect code.

> Concerning the SPI the bit number from 16 to 31 are always reserved on all SPI registers this is why these bits are not shown in the documentation but the register size is still 32-bit.

> The size of the content inside each registers are specified by the reference manual and they must be handle by the masks available on the header file.

Not sure if you are aware of the type promotion rules in C.
Those rules will cause any access to an lvalue to match the type of said lvalue. Which is 32-bit in this case. Thus the incorrect code.
Forcing users to declare local pointers of different size for this sake is not a smart solution, I would say.

I have corrected my system header file. Which I got through the Segger ES support package, by the way.

> Moreover, I checked on the last STM32F0 cube firmware package ...

I did not bother to check any Cube package, but older SPL packages. 
And then, header definitions matched actual register sizes for quite some while.
It seems Cube has "improved" a lot of things. I have my reasons for not using it.
Just saying.