antmerlino commented on a change in pull request #3520:
URL: https://github.com/apache/incubator-nuttx/pull/3520#discussion_r612513389



##########
File path: arch/arm/src/stm32h7/Kconfig
##########
@@ -493,12 +501,14 @@ config STM32H7_OTGFS
        bool "OTG FS"
        default n
        select USBHOST_HAVE_ASYNCH if USBHOST
+       select STM32H7_HSI48

Review comment:
       @davids5 Okay, so now I can't pass CI because of the following problem:
   
   If the platform board.h defines STM32_RCC_D2CCIP2R_USBSRC == 
RCC_D2CCIP2R_USBSEL_HSI48, then the STM32H7_HSI48 option has to be enabled for 
every configuration of that board, regardless of whether or not OTGHS is 
enabled, or it will fail with the error I added.
   
   1. Add CONFIG_STM32H7_HSI48 to all config files in H7 board folder using 
HSI48. 
   (I don't like this because it seems like we are forcing an unnecessary 
burden on board configurations.)
   
   2. Add an additional `&& CONFIG_STM32H7_OTGHS` condition to the enable logic.
   (This one is better, but no other RCC logic uses the peripheral enable flag, 
only whether RCC_XXXSRC is defined)
   
   3. Change the board.h files to only define  STM32_RCC_D2CCIP2R_USBSRC when 
CONFIG_STM32H7_OTGHS is selected.
   (This one might be the cleanest, but I'd argue, forcing this logic into the 
board file is a good way for the purpose to get lost)
   
   4. Change STM32H7_HSI48 to default to y
   (I don't like this because it really shouldn't be the default.)
   
   Let me know if you have a preference, or different suggestion.




-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to