Skip to content

Commit

Permalink
fix(clock): handle HSE TCXO when available
Browse files Browse the repository at this point in the history 
Signed-off-by: Frederic Pillon <[email protected]>
  • Loading branch information
@fpistm
fpistm committed Sep 8, 2023
1 parent 0c69f37 commit 0ffbf3e
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions libraries/SrcWrapper/src/stm32/clock.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,15 @@ void enableClock(sourceClock_t source)
}
break;
case HSE_CLOCK:
__HAL_RCC_HSE_CONFIG(RCC_HSE_ON);
#if defined(RCC_HSE_BYPASS_PWR) && defined(LORAWAN_BOARD_HAS_TCXO) && (LORAWAN_BOARD_HAS_TCXO == 1)
uint32_t HSEState = RCC_HSE_BYPASS_PWR;
#else
uint32_t HSEState = RCC_HSE_ON;
#endif
__HAL_RCC_HSE_CONFIG(HSEState);
if (__HAL_RCC_GET_FLAG(RCC_FLAG_HSERDY) == RESET) {
RCC_OscInitStruct.OscillatorType = RCC_OSCILLATORTYPE_HSE;
RCC_OscInitStruct.HSEState = RCC_HSE_ON;
RCC_OscInitStruct.HSEState = HSEState;
}
break;
default:
Expand Down

8 comments on commit 0ffbf3e

@ellensp
Copy link

@ellensp ellensp commented on 0ffbf3e Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fpistm

This breaks things

.platformio/packages/framework-arduinoststm32/libraries/SrcWrapper/src/stm32/clock.c:135:7: error: a label can only be part of a statement and a declaration is not a statement
  135 |       uint32_t HSEState = RCC_HSE_ON;
      |       ^~~~~~~~

Using braces fixes it.

    case HSE_CLOCK: {
#if defined(RCC_HSE_BYPASS_PWR) && defined(LORAWAN_BOARD_HAS_TCXO) && (LORAWAN_BOARD_HAS_TCXO == 1)
      uint32_t HSEState = RCC_HSE_BYPASS_PWR;
#else
      uint32_t HSEState = RCC_HSE_ON;
#endif
      __HAL_RCC_HSE_CONFIG(HSEState);
      if (__HAL_RCC_GET_FLAG(RCC_FLAG_HSERDY) == RESET) {
        RCC_OscInitStruct.OscillatorType =  RCC_OSCILLATORTYPE_HSE;
        RCC_OscInitStruct.HSEState = HSEState;
      }
      break;
    }
    default:

@fpistm
Copy link
Member Author

@fpistm fpistm commented on 0ffbf3e Sep 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems strange as ci does not have error and I've tested with my nucleo WL55JC to use HSE for RTC.
Seems linked to pio. Maybe gcc version?

@ellensp
Copy link

@ellensp ellensp commented on 0ffbf3e Sep 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what is going on, It may be related to C standard being used... but has been confirmed by several Marlin firmware users

Here is how to reproduce it using Marlin firmware on vscode/platformio (sorry I don't have a small eg to trigger it)

git clone https://github.com/MarlinFirmware/Marlin.git
in vscode open up the directory with platformio.ini file in it so vscode knows this is a platformio project

Edit Configuration.h
Find and set the following:
#define MOTHERBOARD BOARD_BTT_SKR_MINI_E3_V3_0
#define SERIAL_PORT 1

Edit platformio.ini
Find and set the following:
set default_envs = STM32G0B1RE_btt

Delete the .platformio folder in your home directory and restart vscode (forces a full platformio and framework reinstall to ensure you get the latest Arduino_Core_STM32)

Allow it to install and reload vscode as prompted, give it time to get all it needs.
Click Plafromio: Build button on bottom launch bar. (or use pio run)

I have attached a full log of a build
buildlog.txt

@fpistm
Copy link
Member Author

@fpistm fpistm commented on 0ffbf3e Sep 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately I don't use pio and will not test this. Weird thing is I could not reproduce on my side with Arduino IDE. Will try on monday.

@fpistm
Copy link
Member Author

@fpistm fpistm commented on 0ffbf3e Sep 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ellensp I think it is linked to std 11. With arduino we use 17. If you can try to change it to confirm?

@ellensp
Copy link

@ellensp ellensp commented on 0ffbf3e Sep 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly using -std=gnu17 made no difference

arm-none-eabi-gcc -o .pio/build/STM32G0B1RE_btt/SrcWrapper/src/stm32/clock.c.o -c -std=gnu17 -g3 -fmax-errors=5 -mcpu=cortex-m0plus -mthumb -Os -ffunction-sections -fdata-sections -nostdlib --param max-inline-insns-single=500 -DPLATFORMIO=60111 -DSTM32G0xx -DSTM32G0B1xx -D__MARLIN_FIRMWARE__ -DNDEBUG -DHAL_STM32 -DPLATFORM_M997_SUPPORT -DUSBCON -DUSBD_USE_CDC -DTIM_IRQ_PRIO=13 -DADC_RESOLUTION=12 -DPIN_SERIAL4_RX=PC_11 -DPIN_SERIAL4_TX=PC_10 -DSERIAL_RX_BUFFER_SIZE=1024 -DSERIAL_TX_BUFFER_SIZE=1024 -DTIMER_SERVO=TIM3 -DTIMER_TONE=TIM4 -DSTEP_TIMER_IRQ_PRIO=0 -DBOARD_F_CPU=64000000L -DUSART_RX_BUF_SIZE=1024 -DUSART_TX_BUF_SIZE=1024 -DSTM32G0xx -DARDUINO=10808 -DARDUINO_ARCH_STM32 -DARDUINO_MARLIN_STM32G0B1RE -DBOARD_NAME=\"MARLIN_STM32G0B1RE\" -DHAL_UART_MODULE_ENABLED -DUSE_FULL_LL_DRIVER -DVARIANT_H=\"variant_MARLIN_STM32G0B1RE.h\" -D__CORTEX_SC=0 -DHAL_PCD_MODULE_ENABLED -DVECT_TAB_OFFSET=0x2000 -DSTM32_FLASH_SIZE=512 -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino/avr -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino/stm32 -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino/stm32/LL -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino/stm32/usb -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino/stm32/OpenAMP -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino/stm32/usb/hid -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino/stm32/usb/cdc -I/home/username/.platformio/packages/framework-arduinoststm32/system/Drivers/STM32G0xx_HAL_Driver/Inc -I/home/username/.platformio/packages/framework-arduinoststm32/system/Drivers/STM32G0xx_HAL_Driver/Src -I/home/username/.platformio/packages/framework-arduinoststm32/system/STM32G0xx -I/home/username/.platformio/packages/framework-arduinoststm32/system/Middlewares/ST/STM32_USB_Device_Library/Core/Inc -I/home/username/.platformio/packages/framework-arduinoststm32/system/Middlewares/ST/STM32_USB_Device_Library/Core/Src -I/home/username/.platformio/packages/framework-arduinoststm32/system/Middlewares/OpenAMP -I/home/username/.platformio/packages/framework-arduinoststm32/system/Middlewares/OpenAMP/open-amp/lib/include -I/home/username/.platformio/packages/framework-arduinoststm32/system/Middlewares/OpenAMP/libmetal/lib/include -I/home/username/.platformio/packages/framework-arduinoststm32/system/Middlewares/OpenAMP/virtual_driver -I/home/username/.platformio/packages/framework-cmsis/CMSIS/Core/Include -I/home/username/.platformio/packages/framework-arduinoststm32/system/Drivers/CMSIS/Device/ST/STM32G0xx/Include -I/home/username/.platformio/packages/framework-arduinoststm32/system/Drivers/CMSIS/Device/ST/STM32G0xx/Source/Templates/gcc -I/home/username/.platformio/packages/framework-cmsis/CMSIS/DSP/Include -I/home/username/.platformio/packages/framework-cmsis/CMSIS/DSP/PrivateInclude -I/home/username/.platformio/packages/framework-arduinoststm32/cores/arduino -I/home/username/.platformio/packages/framework-arduinoststm32/variants/MARLIN_G0B1RE /home/username/.platformio/packages/framework-arduinoststm32/libraries/SrcWrapper/src/stm32/clock.c
/home/username/.platformio/packages/framework-arduinoststm32/libraries/SrcWrapper/src/stm32/clock.c: In function 'enableClock':
/home/username/.platformio/packages/framework-arduinoststm32/libraries/SrcWrapper/src/stm32/clock.c:135:7: error: a label can only be part of a statement and a declaration is not a statement
  135 |       uint32_t HSEState = RCC_HSE_ON;
      |       ^~~~~~~~

@fpistm
Copy link
Member Author

@fpistm fpistm commented on 0ffbf3e Sep 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum... weird... I will try to understand.

@fpistm
Copy link
Member Author

@fpistm fpistm commented on 0ffbf3e Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I've reproduced by downgrading my arm-none-eab-gcc version.
You got the issue because PIO uses arm-non-eabi-gcc 10.3.1 while in Arduino we used 12.2.1.
I saw that a request was made to update it:
platformio/platform-ststm32#720
Seems it brings some issues with gdb anyway it seems you can update it manually.

Please sign in to comment.