hartmannathan commented on code in PR #8945:
URL: https://github.com/apache/nuttx/pull/8945#discussion_r1155374988


##########
arch/arm/src/stm32h7/stm32_otgdev.c:
##########
@@ -60,6 +60,16 @@
          enabled. Enable STM32H7_HSI48
 #endif
 
+#if defined(CONFIG_STM32H7_OTGHS) && !defined(CONFIG_STM32H7_OTGHS_FS) && \
+    defined(CONFIG_STM32H7_OTGHS_NO_ULPI)
+#  error OTG HS selcted but no ULPI enabled

Review Comment:
   ```suggestion
   #  error OTG HS selected but no ULPI enabled
   ```



##########
arch/arm/src/stm32h7/stm32h7x3xx_rcc.c:
##########
@@ -866,30 +885,25 @@ void stm32_stdclockconfig(void)
         {
         }
 
-      /* Over-drive is needed if
-       *  - Voltage output scale 1 mode is selected and SYSCLK frequency is
-       *    over 400 MHz.
-       */
+#if STM32_VOS_OVERDRIVE && (STM32_PWR_VOS_SCALE == PWR_D3CR_VOS_SCALE_1)
+      /* Over-drive support for VOS1 */
 
-      if ((STM32_PWR_VOS_SCALE == PWR_D3CR_VOS_SCALE_1) &&
-           STM32_SYSCLK_FREQUENCY > 400000000)
-        {
-          /* Enable System configuration controller clock to Enable ODEN */
+      /* Enable System configuration controller clock to Enable ODEN */

Review Comment:
   Note that silicon revision Y of STM32H7x3 has an erratum that prevents being 
able to turn on overdrive, so those microcontrollers are limited to 400 MHz. 
This is fixed in all subsequent revisions and all new chips do correctly 
support overdrive.
   
   I am not sure if that is the reason why we had `if ((STM32_PWR_VOS_SCALE == 
PWR_D3CR_VOS_SCALE_1) && STM32_SYSCLK_FREQUENCY > 400000000)` here.



##########
arch/arm/src/stm32h7/stm32h7x3xx_rcc.c:
##########
@@ -102,9 +103,27 @@
 #error When HSI is used, you have to define STM32_BOARD_HSIDIV in 
board/include/board.h
 #endif
 
-/****************************************************************************
- * Private Data
- ****************************************************************************/
+/* Over-drive is supported only for Voltage output scale 1 mode.
+ * It is requred when:

Review Comment:
   ```suggestion
    * It is required when:
   ```



##########
arch/arm/src/stm32h7/stm32h7x3xx_rcc.c:
##########
@@ -866,30 +885,25 @@ void stm32_stdclockconfig(void)
         {
         }
 
-      /* Over-drive is needed if
-       *  - Voltage output scale 1 mode is selected and SYSCLK frequency is
-       *    over 400 MHz.
-       */
+#if STM32_VOS_OVERDRIVE && (STM32_PWR_VOS_SCALE == PWR_D3CR_VOS_SCALE_1)
+      /* Over-drive support for VOS1 */
 
-      if ((STM32_PWR_VOS_SCALE == PWR_D3CR_VOS_SCALE_1) &&
-           STM32_SYSCLK_FREQUENCY > 400000000)
-        {
-          /* Enable System configuration controller clock to Enable ODEN */
+      /* Enable System configuration controller clock to Enable ODEN */
 
-          regval = getreg32(STM32_RCC_APB4ENR);
-          regval |= RCC_APB4ENR_SYSCFGEN;
-          putreg32(regval, STM32_RCC_APB4ENR);
+      regval = getreg32(STM32_RCC_APB4ENR);
+      regval |= RCC_APB4ENR_SYSCFGEN;
+      putreg32(regval, STM32_RCC_APB4ENR);
 
-          /* Enable Overdrive to extend the clock frequency up to 480 MHz. */
+      /* Enable Overdrive */

Review Comment:
   Out of curiosity, why remove the "to extend the clock frequency up to 480 
MHz" portion of this comment?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to