[PATCH v1 2/2] bsps/stm32h7: Configure UART clocks when enabled

[Karel Gardas]

  Hello Kinsey,

honestly I don't know what to do about this patch. Let me explain a bit 
history behind STM32h7. It was originally submitted by embedded brains 
guys (Sebastian main, Christian added few things later) supporting the 
only eval board of that time stm32h743-eval(2). Sebastian also added 
support for nucleo with h743zi and Robin Mueller fixed several things on 
that. So this was 2 boards supported. Then I came and added support for 
3 more boards where two was with 2 BSPs variants each (one for M7 and 
one for M4). Due to amount of code in #ifdef for various boards we (my 
customer and me) have decided to de-cpp code a bit and divide it into 
different supported board directories. Basically we have also followed a 
lead by embedded brains guys and their imxrt bsps family (and boards 
directory). This was accepted and now you have this boards directory 
with different boards providing just as lean as possible configuration 
in C files sharing some common generic in bspstarthooks.c from start 
subdirectory.

So basically the idea was to avoid complex #ifdefs and rather copy clean 
C files around. (as much as possible, realistically).

Now, your patch seems to be going a bit in reverse direction and I do 
not see clearly the motivation behind it, why do you do that this way?

I mean, you edit peripherals clock for stm32h743-eval(2) board BSP. 
AFAIK this file is not reused on any other BSP variant (different board) 
at all.
The board in question (stm32h743-eval) supports only UART1 on its 
ST-Link or DB-9 canon connection. The UART1 is shared between those two 
mechanisms, which one is used, you define by jumper placed on the board. 
There is no other UART1 connector provided on the board. So I do not see 
reason why you add all other UARTs into #ifdefs for this particular 
board/bsp variant? And hence my question about your motivation and where 
you are heading...

Thanks!
Karel



On 7/24/23 20:28, Kinsey Moore wrote:
> This change allows configuration of all enabled UART clocks without
> direct modificaton of the exiting BSP.
> ---
>   .../stm/stm32h743i-eval/stm32h7-config-per.c  | 41 +++++++++++++++++--
>   1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/bsps/arm/stm32h7/boards/stm/stm32h743i-eval/stm32h7-config-per.c b/bsps/arm/stm32h7/boards/stm/stm32h743i-eval/stm32h7-config-per.c
> index 8ca665915f..dcce247a51 100644
> --- a/bsps/arm/stm32h7/boards/stm/stm32h743i-eval/stm32h7-config-per.c
> +++ b/bsps/arm/stm32h7/boards/stm/stm32h743i-eval/stm32h7-config-per.c
> @@ -32,9 +32,34 @@
>   #include <stm32h7/hal.h>
>   
>   const RCC_PeriphCLKInitTypeDef stm32h7_config_peripheral_clocks = {
> -  .PeriphClockSelection = RCC_PERIPHCLK_RTC | RCC_PERIPHCLK_USART3
> -    | RCC_PERIPHCLK_FDCAN | RCC_PERIPHCLK_USART1 | RCC_PERIPHCLK_I2C1
> -    | RCC_PERIPHCLK_USB | RCC_PERIPHCLK_FMC | RCC_PERIPHCLK_RNG,
> +  .PeriphClockSelection = RCC_PERIPHCLK_RTC
> +    | RCC_PERIPHCLK_FDCAN | RCC_PERIPHCLK_I2C1
> +    | RCC_PERIPHCLK_USB | RCC_PERIPHCLK_FMC | RCC_PERIPHCLK_RNG
> +#ifdef STM32H7_CONSOLE_ENABLE_USART1
> +    | RCC_PERIPHCLK_USART1
> +#endif
> +#ifdef STM32H7_CONSOLE_ENABLE_USART2
> +    | RCC_PERIPHCLK_USART2
> +#endif
> +#ifdef STM32H7_CONSOLE_ENABLE_USART3
> +    | RCC_PERIPHCLK_USART3
> +#endif
> +#ifdef STM32H7_CONSOLE_ENABLE_UART4
> +    | RCC_PERIPHCLK_UART4
> +#endif
> +#ifdef STM32H7_CONSOLE_ENABLE_UART5
> +    | RCC_PERIPHCLK_UART5
> +#endif
> +#ifdef STM32H7_CONSOLE_ENABLE_USART6
> +    | RCC_PERIPHCLK_USART6
> +#endif
> +#ifdef STM32H7_CONSOLE_ENABLE_UART7
> +    | RCC_PERIPHCLK_UART7
> +#endif
> +#ifdef STM32H7_CONSOLE_ENABLE_UART8
> +    | RCC_PERIPHCLK_UART8
> +#endif
> +  ,
>     .PLL2.PLL2M = 3,
>     .PLL2.PLL2N = 48,
>     .PLL2.PLL2P = 1,
> @@ -53,8 +78,18 @@ const RCC_PeriphCLKInitTypeDef stm32h7_config_peripheral_clocks = {
>     .PLL3.PLL3FRACN = 0,
>     .FmcClockSelection = RCC_FMCCLKSOURCE_PLL2,
>     .FdcanClockSelection = RCC_FDCANCLKSOURCE_PLL,
> +#if defined(STM32H7_CONSOLE_ENABLE_USART2) \
> +  || defined(STM32H7_CONSOLE_ENABLE_USART3) \
> +  || defined(STM32H7_CONSOLE_ENABLE_UART4) \
> +  || defined(STM32H7_CONSOLE_ENABLE_UART5) \
> +  || defined(STM32H7_CONSOLE_ENABLE_UART7) \
> +  || defined(STM32H7_CONSOLE_ENABLE_UART8)
>     .Usart234578ClockSelection = RCC_USART234578CLKSOURCE_D2PCLK1,
> +#endif
> +#if defined(STM32H7_CONSOLE_ENABLE_USART1) \
> +  || defined(STM32H7_CONSOLE_ENABLE_USART6)
>     .Usart16ClockSelection = RCC_USART16CLKSOURCE_D2PCLK2,
> +#endif
>     .I2c123ClockSelection = RCC_I2C123CLKSOURCE_D2PCLK1,
>     .UsbClockSelection = RCC_USBCLKSOURCE_PLL3,
>     .RTCClockSelection = RCC_RTCCLKSOURCE_LSE,