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

2023-07-24 Thread Kinsey Moore
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

2023-07-24 Thread 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 
  
  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

2023-07-24 Thread Kinsey Moore
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

2023-07-25 Thread Karel Gardas

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

2023-07-25 Thread Kinsey Moore
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