[GitHub] [incubator-nuttx] v01d commented on pull request #2177: FLASH waiting cycles are configured based on HCLK.

[GitBox <gi...@apache.org>]

v01d commented on pull request #2177:
URL: https://github.com/apache/incubator-nuttx/pull/2177#issuecomment-720642168


   > Hi @v01d ,
   > 
   > I wasn't aware of this, I thought that these functions were only used by the start-up code.
   > 
   > So, let me present some points. Let's think how we can make this better.
   > 
   >     * I did a search on were `stm32_clockenable()` is used. It is true that it is called only from start-up code. No other code in STM32 arch is using it. The only other use of this function is in `stm3210e-eval/src/stm32_idle.c`, which is a board-level function. Maybe this specific board is to be fixed?
   
   The calls I mention will be from IDLE loop (maybe not directly, maybe via some other function) since that is the point when the board exits sleep and needs to restart clock sources, PLLs, etc. That is the only point where there will be clock changes (besides w.r.t. default clock when booting).
   
   >     * I think clock configuration needs different routines for the start-up code, and for dynamically changing frequency. Starting the system usually makes many assumptions for its state (that do not apply during run-time). Also, usually, some of the initial configuration is not  required to be repeated later.
   
   This is not so much a dynamic frequency change, it is during a very specific moment and the goal is to restore whatever was changed before going to sleep. Usually any waits for clock stability need to be waited again, but this will change a lot on different chips/archs. 
   
   There are some cases (don't remember which one) where the initial clock config was separate from a "reconfig" after sleep. But I think this is not done for all chips. The problem is that there will be much repeated code between these two routines. Maybe there should be checks before doing something with a clock (for STM32L476 I remember I added a check before waiting for stability, instead of blindly doing so). That may be better than repeating everything.
   
   >     * Based on the assumption that `stm32_stdclockconfig()` is called outside `stm32_start()`, then core-overdrive is also broken on STM32F4 targets, I think. It has similar requirements.
   
   No idea about overdrive but it is likely a similar case. There was some discussion about this recently on the list.
   
   >     * If we want to make this perfect (and take the most performance available from these chips), then there should also be a way to state the voltage range that the MCU works. Maybe in `board.h`?
   
   You mean core clock voltage? This is done according to clock frequency as per datasheet recommendations. Again, I did this for STM32L476, but I'm not sure if it is 100% enabled as I didn't trust it while I encountered the issue I mentioned, which turned out to be related to flash instead.
    
   >     * Having multiple `*_rcc.c` files is very very annoying when maintaining code. Maybe we should first try to merge all these files in one?
   
   This was discussed many times. Merging similarly looking code was many times not wanted since it may end up in a very complex code needs to support every variation of the SoC line and every specific chip. And if not considered right, when adding support for a chip, it may break many others. I would welcome simplification to improve maintenance, but be aware this is a very critical part of NuttX and most likely would require testing with various chips if you start modifying a lot of this part of the code.
   This is one of the reasons as why the different STM32 families are separated. I think ST does not make this simple since two chips of different families may be similar in one aspect but very different in another.


----------------------------------------------------------------
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:
users@infra.apache.org