Skip to content

Commit

Permalink
chore(uart): set LSE as source only if LSE is ready
Browse files Browse the repository at this point in the history
Fixes #2642.

Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
  • Loading branch information
fpistm committed Feb 4, 2025
1 parent ee9c0dd commit eb243d5
Showing 1 changed file with 17 additions and 14 deletions.
31 changes: 17 additions & 14 deletions libraries/SrcWrapper/src/stm32/uart.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "uart.h"
#include "Arduino.h"
#include "PinAF_STM32F1.h"
#include "stm32yyxx_ll_rcc.h"

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -457,28 +458,30 @@ void uart_init(serial_t *obj, uint32_t baudrate, uint32_t databits, uint32_t par
if (baudrate <= 9600) {
/* Enable the clock if not already set by user */
enableClock(LSE_CLOCK);
if (obj->uart == LPUART1) {
__HAL_RCC_LPUART1_CONFIG(RCC_LPUART1CLKSOURCE_LSE);
}
if (LL_RCC_LSE_IsReady()) {
if (obj->uart == LPUART1) {
__HAL_RCC_LPUART1_CONFIG(RCC_LPUART1CLKSOURCE_LSE);
}
#if defined(LPUART2_BASE)
if (obj->uart == LPUART2) {
__HAL_RCC_LPUART2_CONFIG(RCC_LPUART2CLKSOURCE_LSE);
}
if (obj->uart == LPUART2) {
__HAL_RCC_LPUART2_CONFIG(RCC_LPUART2CLKSOURCE_LSE);
}
#endif
#if defined(LPUART3_BASE)
if (obj->uart == LPUART3) {
__HAL_RCC_LPUART3_CONFIG(RCC_LPUART3CLKSOURCE_LSE);
}
if (obj->uart == LPUART3) {
__HAL_RCC_LPUART3_CONFIG(RCC_LPUART3CLKSOURCE_LSE);
}
#endif
if ((uart_rx == NP) && (uart_rx_swap == NP)) {
if (HAL_HalfDuplex_Init(huart) == HAL_OK) {
if ((uart_rx == NP) && (uart_rx_swap == NP)) {
if (HAL_HalfDuplex_Init(huart) == HAL_OK) {
return;
}
} else if (HAL_UART_Init(huart) == HAL_OK) {
return;
}
} else if (HAL_UART_Init(huart) == HAL_OK) {
return;
}
}
if (__HAL_RCC_GET_FLAG(RCC_FLAG_HSIRDY)) {
if (LL_RCC_HSI_IsReady()) {
if (obj->uart == LPUART1) {
__HAL_RCC_LPUART1_CONFIG(RCC_LPUART1CLKSOURCE_HSI);
}
Expand Down

4 comments on commit eb243d5

@MaJerle
Copy link

@MaJerle MaJerle commented on eb243d5 Feb 4, 2025

Choose a reason for hiding this comment

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

I think that if statement for LSE_IS_Ready should be closed before we initialize the HAL UART.

if (LL_RCC_LSE_IsReady()) {
if (obj->uart == LPUART1) {
__HAL_RCC_LPUART1_CONFIG(RCC_LPUART1CLKSOURCE_LSE);
}
#if defined(LPUART2_BASE)
if (obj->uart == LPUART2) {
__HAL_RCC_LPUART2_CONFIG(RCC_LPUART2CLKSOURCE_LSE);
}
#endif
#if defined(LPUART3_BASE)
if (obj->uart == LPUART3) {
__HAL_RCC_LPUART3_CONFIG(RCC_LPUART3CLKSOURCE_LSE);
}
#endif
if ((uart_rx == NP) && (uart_rx_swap == NP)) {
if (HAL_HalfDuplex_Init(huart) == HAL_OK) {
return;
}
} else if (HAL_UART_Init(huart) == HAL_OK) {
return;
}
}

Instead of:

      if (LL_RCC_LSE_IsReady()) {
        if (obj->uart == LPUART1) {
          __HAL_RCC_LPUART1_CONFIG(RCC_LPUART1CLKSOURCE_LSE);
        }
#if defined(LPUART2_BASE)
        if (obj->uart == LPUART2) {
          __HAL_RCC_LPUART2_CONFIG(RCC_LPUART2CLKSOURCE_LSE);
        }
#endif
#if defined(LPUART3_BASE)
        if (obj->uart == LPUART3) {
          __HAL_RCC_LPUART3_CONFIG(RCC_LPUART3CLKSOURCE_LSE);
        }
#endif
        if ((uart_rx == NP) && (uart_rx_swap == NP)) {
          if (HAL_HalfDuplex_Init(huart) == HAL_OK) {
            return;
          }
        } else if (HAL_UART_Init(huart) == HAL_OK) {
          return;
        }
      }

I think it should be:

      if (LL_RCC_LSE_IsReady()) {
        if (obj->uart == LPUART1) {
          __HAL_RCC_LPUART1_CONFIG(RCC_LPUART1CLKSOURCE_LSE);
        }
#if defined(LPUART2_BASE)
        if (obj->uart == LPUART2) {
          __HAL_RCC_LPUART2_CONFIG(RCC_LPUART2CLKSOURCE_LSE);
        }
#endif
#if defined(LPUART3_BASE)
        if (obj->uart == LPUART3) {
          __HAL_RCC_LPUART3_CONFIG(RCC_LPUART3CLKSOURCE_LSE);
        }
#endif
      }
      if ((uart_rx == NP) && (uart_rx_swap == NP)) {
        if (HAL_HalfDuplex_Init(huart) == HAL_OK) {
          return;
        }
      } else if (HAL_UART_Init(huart) == HAL_OK) {
        return;
      }
  • We always want to check for LSE, but only enabled it if LSE is enabled
  • We always need to enable UART, even if LSE isn't enabled

@fpistm
Copy link
Member Author

@fpistm fpistm commented on eb243d5 Feb 4, 2025

Choose a reason for hiding this comment

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

If LSE is not available, then the HSI is tested then other clock source.

@MaJerle
Copy link

@MaJerle MaJerle commented on eb243d5 Feb 4, 2025

Choose a reason for hiding this comment

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

Sure. But if LSE isnt valid, then when do you init the UART? I am missing this part :)

@fpistm
Copy link
Member Author

@fpistm fpistm commented on eb243d5 Feb 4, 2025

Choose a reason for hiding this comment

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

See the code after.

Please sign in to comment.