STM32F103xe defines values that don't fit in register

[bmourit]

Describe the bug in file stm32f103xe.h we have:

#define RCC_CFGR_SWS_Pos                     (2U)
#define RCC_CFGR_SWS_Msk                    (0x3UL << RCC_CFGR_SWS_Pos)        /*!< 0x0000000C */
#define RCC_CFGR_SWS                             RCC_CFGR_SWS_Msk                  /*!< SWS[1:0] bits (System Clock Switch Status) */
#define RCC_CFGR_SWS_0                       (0x1UL << RCC_CFGR_SWS_Pos)        /*!< 0x00000004 */
#define RCC_CFGR_SWS_1                       (0x2UL << RCC_CFGR_SWS_Pos)        /*!< 0x00000008 */

#define RCC_CFGR_SWS_HSI                     0x00000000U                       /*!< HSI oscillator used as system clock */
#define RCC_CFGR_SWS_HSE                     0x00000004U                       /*!< HSE oscillator used as system clock */
#define RCC_CFGR_SWS_PLL                     0x00000008U                       /*!< PLL used as system clock */

Where SWS_PLL is assigned a value of 0x00000008U for a register mask of 2 bits. The max possible value is 0x00000003U

The code directly above this is:

#define RCC_CFGR_SW_Pos                      (0U)
#define RCC_CFGR_SW_Msk                      (0x3UL << RCC_CFGR_SW_Pos)         /*!< 0x00000003 */
#define RCC_CFGR_SW                          RCC_CFGR_SW_Msk                   /*!< SW[1:0] bits (System clock Switch) */
#define RCC_CFGR_SW_0                        (0x1UL << RCC_CFGR_SW_Pos)         /*!< 0x00000001 */
#define RCC_CFGR_SW_1                        (0x2UL << RCC_CFGR_SW_Pos)         /*!< 0x00000002 */

#define RCC_CFGR_SW_HSI                      0x00000000U                       /*!< HSI selected as system clock */
#define RCC_CFGR_SW_HSE                      0x00000001U                       /*!< HSE selected as system clock */
#define RCC_CFGR_SW_PLL                      0x00000002U                       /*!< PLL selected as system clock */

This is correct, and the values for the SWS_xxx should be the same as here since both use the same mask size.

This is also an issue in the system_stm32f1xx.c file which in SystemCoreClockUpdate has the following:

  tmp = RCC->CFGR & RCC_CFGR_SWS;

  switch (tmp)
  {
    case 0x00U:  /* HSI used as system clock */
      SystemCoreClock = HSI_VALUE;
      break;
    case 0x04U:  /* HSE used as system clock */
      SystemCoreClock = HSE_VALUE;
      break;
    case 0x08U:  /* PLL used as system clock */

      /* Get PLL clock source and multiplication factor ----------------------*/
      pllmull = RCC->CFGR & RCC_CFGR_PLLMULL;
      pllsource = RCC->CFGR & RCC_CFGR_PLLSRC;

So it reads a register with a 2 bit mask, and then uses a switch to look for values that wont fit, leaving this to always return HSI_VALUE as SystemCoreClock.

I'm unsure if this is something STM32duino would fix, or if it would need reported to STMicroelectronics somehow. This is present in all versions of the SDK.

[fpistm]

Hi @bmourit Thanks for the detailed description.

I'm unsure if this is something STM32duino would fix, or if it would need reported to STMicroelectronics somehow. This is present in all versions of the SDK.

This need to be reported to STMicroelectronics : https://github.com/STMicroelectronics/STM32CubeF1/

We only use it so no change is performed to the Cube part. Anyway, looking deeply to values they are correct. As RCC_CFGR_SWS* position is 2. The register value is masked with 0x0000000C then values are correct. They could be define like this:

#define RCC_CFGR_SWS_HSI                     (0x00000000U << 2)                      /*!< HSI oscillator used as system clock */
#define RCC_CFGR_SWS_HSE                     (0x00000001U << 2)                      /*!< HSE oscillator used as system clock */
#define RCC_CFGR_SWS_PLL                     (0x00000002U << 2)                      /*!< PLL used as system clock */

[bmourit]

Got it

[fpistm]

Where are you getting those defines from? Those are correct, but the ones in stm32f103xe.h are not. Again, your code here is good, but that is not how t is defined for this stm32f103xestm32f103xe At least not the ones in the system folder...

 tmp = RCC->CFGR & RCC_CFGR_SWS;`

with

#define RCC_CFGR_SWS_Pos                     (2U)
#define RCC_CFGR_SWS_Msk                    (0x3UL << RCC_CFGR_SWS_Pos)        /*!< 0x0000000C */
#define RCC_CFGR_SWS                             RCC_CFGR_SWS_Msk  

So tmp contain the 32 bits value of the register masked with 0x0000000C that's all. Then this masked value is compared again the value of the 32 bits register, so it is ok.

[bmourit]

Thanks. Lol I need sleep.

[fpistm]

Anyway, I understand you point that the define does not match the bitfield value but the whole register masked.