HSE Bypass setup attempt causes problems to the XTAL setup

[LordGuilly]

Description

• Type: Bug
• Priority: Minor

---

Bug

Target
BLUEPILL_F103C8
although similar F1 platforms can be affected

Toolchain:
GCC_ARM

Toolchain version:

mbed-cli version:

mbed-os sha:
mbed_lib_145

DAPLink version:

Expected behavior
the SetSysClock function attempts to initialise the the bypass oscillator config first, then the Xtal oscillator, and finally the internal one. For this, it sets the HSEState variable of an RCC_OscInitTypeDef struct.
For the BLUEPILL, the system should fail the bypass and succeed with the Xtal, generating a SystemCoreClock of 72MHz

Actual behavior
The __HAL_RCC_HSE_CONFIG (of stm32f1xx_hal_rcc.c) sets the bits of the RCC_CR registry according to that state, but if it's HSE_ON, it does not clear the BYPASS flag. Because the BYPASS bit is set when trying to start in bypass mode, the attempt to use the xtal is not done correctly
The system then always falls to the HSI scenario, and ends up working at 64MHz

Steps to reproduce
start the system, and at main.c, print the system clock

---

Suggested enhancement
a proper fix would be to change the __HAL_RCC_HSE_CONFIG macro, with the follwoing code:

#define __HAL_RCC_HSE_CONFIG(__STATE__)                                     \
                    do{                                                     \
                      if ((__STATE__) == RCC_HSE_ON)                        \
                      {                                                     \
                        CLEAR_BIT(RCC->CR, RCC_CR_HSEBYP);                  \
                        SET_BIT(RCC->CR, RCC_CR_HSEON);                     \
                      }                                                     \
                      else if ((__STATE__) == RCC_HSE_OFF)                  \
                      {                                                     \
                        CLEAR_BIT(RCC->CR, RCC_CR_HSEON);                   \
                        CLEAR_BIT(RCC->CR, RCC_CR_HSEBYP);                  \
                      }                                                     \
                      else if ((__STATE__) == RCC_HSE_BYPASS)               \
                      {                                                     \
                        SET_BIT(RCC->CR, RCC_CR_HSEBYP);                    \
                        SET_BIT(RCC->CR, RCC_CR_HSEON);                     \
                      }                                                     \
                      else                                                  \
                      {                                                     \
                        CLEAR_BIT(RCC->CR, RCC_CR_HSEON);                   \
                        CLEAR_BIT(RCC->CR, RCC_CR_HSEBYP);                  \
                      }                                                     \
                    }while(0U)

another alternative would be to change the follwoing define at the BLUEPILL startup code (system_stm32f1xx.c)

#define USE_PLL_HSE_EXTC (0) /* Use external clock */

both solutions are not exclusive, and can be applied simultaneosly

Pros

Cons

---

Question

happy to implement the patch and generate a pull request, not sure if there are tests for this?

[theotherjimmy]

Hi @LordGuilly. Thanks for this issue report. I updated your comment to syntax highlight your code. For future reference, you can put ```C before and ``` after a C code block on their own lines to do the same thing. Generally, PRs get more attention on this repo than issues. We are working to change that. that is especially true if you want changes to be reviewed. I think that the only downside to submitting a PR would be that you would have to create the branch to hold the changes. You already have the description down!

tl;dr: If you have time please submit a PR.

[LordGuilly]

sure, no problem, I already submitted a simple one for the build system, so I have the repo, hopefully won't take much

[theotherjimmy]

@LMESTM FYI

[jeromecoutant]

Hi @LordGuilly

• Sure, you can submit a PR. I am going to address this point to our driver team in parallel. Thx

• about clock configuration, you should check STM32 : Clock source selection in json config file (PR2) #4740
  For NUCLEO_F103RB target, scenario (first bypass, then Xtal oscillator, and finally the internal one) is still present. But you can update it by changing clock_source json config value.
  I didn't update BLUEPILL_F103C8 scenario as I couldn't validate it.

[jeromecoutant]

ST_INTERNAL_REF 35827

[LordGuilly]

the bluepill file has the clock config hardcoded

#define USE_PLL_HSE_EXTC (1) /* Use external clock */
#define USE_PLL_HSE_XTAL (1) /* Use external xtal */

will these be overrided by the clock_source json values?
anyway, I haven't tested the nucleo board, but I would think that not clearing the bypass flag can still cause the problem. Maybe not if there is POR event, but if doing remote debugging (like I am doing), the flag seems to be persistent

[jeromecoutant]

Hi
I think we can close this issue?
Thx