[PATCH v1 2/2] bsps/stm32h7: Configure UART clocks when enabled
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 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, -- 2.39.2 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1 2/2] bsps/stm32h7: Configure UART clocks when enabled
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 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, ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1 2/2] bsps/stm32h7: Configure UART clocks when enabled
On Mon, Jul 24, 2023 at 3:30 PM Karel Gardas wrote: > > 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... > Given that, I now understand the confusion. I have a board in hand that will never see an upstream BSP and I was hoping to be able to configure the base STM32H7 BSP for it, but what I interpreted as the "base" STM32H7 BSP is not actually a generic base BSP. I was also contemplating moving more of the peripheral configuration into shared files since the enable/disable configuration items are already there and it would be convenient for the various BSPs to exist as some custom code and then a selection of presets for those configuration items. I'll have to think about the best way forward for the project I'm working with. Kinsey ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1 2/2] bsps/stm32h7: Configure UART clocks when enabled
On 7/24/23 23:17, Kinsey Moore wrote: [...] 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... Given that, I now understand the confusion. I have a board in hand that will never see an upstream BSP and I was hoping to be able to configure the base STM32H7 BSP for it, but what I interpreted as the "base" STM32H7 BSP is not actually a generic base BSP. I was also contemplating moving more of the peripheral configuration into shared files since the enable/disable configuration items are already there and it would be convenient for the various BSPs to exist as some custom code and then a selection of presets for those configuration items. I'll have to think about the best way forward for the project I'm working with. I guess the philosophy in stm32h7 is to avoid cpp directives even in case of a bit more code duplication. So if I may ask, just copy board directory structure, name it after your board and tweak the copied files there. No need to #ifdefing in different board which would not use that. And btw, I'm using the same for Precidata SL-3011 (board on market, but BSP and firmware not ready for RTEMS 6, will be submitted for RTEMS 7 probably) and for Arduino Portenta H7 (ditto but even less mature codewise). Thanks! Karel ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH v1 2/2] bsps/stm32h7: Configure UART clocks when enabled
Unfortunately, this board is not publicly advertised or available and so a dedicated BSP in RTEMS is not really appropriate. It seems that the current "stm32h7" BSP should be renamed to stm32h743i-eval for clarity, but I'm sure that would affect existing users. I'll just have to keep the BSP changes private and upstream what I can. Thanks, Kinsey On Tue, Jul 25, 2023 at 8:27 AM Karel Gardas wrote: > On 7/24/23 23:17, Kinsey Moore wrote: > > [...] > > > 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... > > > > > > Given that, I now understand the confusion. I have a board in hand that > > will never see an upstream BSP and I was hoping to be able to configure > > the base STM32H7 BSP for it, but what I interpreted as the "base" > > STM32H7 BSP is not actually a generic base BSP. I was also contemplating > > moving more of the peripheral configuration into shared files since the > > enable/disable configuration items are already there and it would be > > convenient for the various BSPs to exist as some custom code and then a > > selection of presets for those configuration items. I'll have to think > > about the best way forward for the project I'm working with. > > I guess the philosophy in stm32h7 is to avoid cpp directives even in > case of a bit more code duplication. > > So if I may ask, just copy board directory structure, name it after your > board and tweak the copied files there. No need to #ifdefing in > different board which would not use that. > > And btw, I'm using the same for Precidata SL-3011 (board on market, but > BSP and firmware not ready for RTEMS 6, will be submitted for RTEMS 7 > probably) and for Arduino Portenta H7 (ditto but even less mature > codewise). > > Thanks! > Karel > ___ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel > ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel