You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/08/11 09:56:08 UTC
[GitHub] [incubator-nuttx] fjpanag opened a new pull request #1561: Fix in STM32F4 over-drive configuration.
fjpanag opened a new pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561
## Summary
Some MCUs of the STM32F4 family have the ability to operate up to 180MHz, but only if core over-drive is enabled.
The current code indeed enables the core over-drive, but not for all MCUs that need it.
The MCUs in need of over-drive to reach their maximum clock are:
STM32F427
STM32F429
STM32F437
STM32F439
STM32F446
STM32F469
STM32F479
## Impact
For some of the aforementioned MCUs, core over-drive was not enabled, resulting in invalid clocking configuration.
Unfortunately as tested, at least on my board, the MCU happily accepted 180MHz without over-drive enabled and worked OK.
Thus it is very possible that many users of this chip are not aware of this miss-configuration. The chip though may be unstable in certain conditions.
## Testing
Tested on a custom board, using an STM32F427VI.
After the change, the respective code is indeed enabled. After boot, the core over-drive is enabled as it should, checked by the respective register value.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] fjpanag commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672003048
I added the option `STM32_HAVE_OVERDRIVE` for all the mentioned MCUs, cleaning things up.
Now regarding how this feature will be enabled. What do you think is best?
1. Add an option in Kconfig. The user explicitly chooses whether they want over-drive enabled or not.
2. Have the code to parse the RCC settings, determine whether the clock frequency will be greater than 168MHz, and automatically enable the over-drive if needed.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] davids5 commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672003094
@fjpanag - nice would be able to squash and force push, when you are done?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] davids5 commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672107061
>However, as I touched the files arch/arm/src/stm32/stm32_pwr.h & arch/arm/src/stm32/stm32_pwr.c CI complains about styling issues. I fixed as much as I could, but I am not sure what to do with this last error.
>Any hint?
@xiaoxiang781216 or @patacongo - Would you advise @fjpanag on what the style issues.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672125287
@fjpanag you can change "Public Functions" to "Public Function Prototypes"
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] fjpanag commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672047089
> > I added the option `STM32_HAVE_OVERDRIVE` for all the mentioned MCUs, cleaning things up.
> > Now regarding how this feature will be enabled. What do you think is best?
> >
> > 1. Add an option in Kconfig. The user explicitly chooses whether they want over-drive enabled or not.
> > 2. Have the code to parse the RCC settings, determine whether the clock frequency will be greater than 168MHz, and automatically enable the over-drive if needed.
>
> I think the 3rd option is to uses the `STM32_SYSCLK_FREQUENCY` > 168 Mhz
This however means that I will have to include `<arch/board/board.h>` in `stm32f40xxx_rcc.c`. Is this acceptable, architecturally?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] davids5 commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672006224
@fjpanag I set it to Draft. When you are done just un-Draft it
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] fjpanag commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672005496
> nice would be able to squash and force push, when you are done?
OK, I will do it.
Don't merge yet, let's just fix how/when over-drive will be enabled first.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] fjpanag commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672082852
I feel that this completes the changes related to over-drive.
I will work on another branch (possibly after vacation) on making regulator scale configurable too.
However, as I touched the files `arch/arm/src/stm32/stm32_pwr.h` & `arch/arm/src/stm32/stm32_pwr.c` CI complains about styling issues. I fixed as much as I could, but I am not sure what to do with this last error.
Any hint?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] fjpanag edited a comment on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
fjpanag edited a comment on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-671884881
Hi @davids5
Indeed STM32F437, STM32F439, STM32F479 are not included in Kconfig. I can remove those.
Certainly STM32F427 was missing though...
PWR_CR_VOS is not affected. It is a requirement though to be set to scale 1 or 2 only when over-drive is enabled. Currently it is set to scale 1 specifically, so this change shouldn't be a problem. It would be best to have PWR_CR_VOS configurable in Kconfig, but this is another PR...
Indeed the code previously was not checking the frequency before enabling over-drive. So I left it as is. But then also PWR_CR_VOS is not configurable...
I will check a bit later regarding STM32_HAS_OVERDRIVE.
I am will somehow improve this PR, and then maybe work separately on making power scale and over-drive configurable, as it may affect other MCUs than just the STM32F4.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] davids5 commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672004966
> I added the option `STM32_HAVE_OVERDRIVE` for all the mentioned MCUs, cleaning things up.
>
> Now regarding how this feature will be enabled. What do you think is best?
>
> 1. Add an option in Kconfig. The user explicitly chooses whether they want over-drive enabled or not.
> 2. Have the code to parse the RCC settings, determine whether the clock frequency will be greater than 168MHz, and automatically enable the over-drive if needed.
I think the 3rd option is to uses the `STM32_SYSCLK_FREQUENCY` > 168 Mhz
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] fjpanag commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-671884881
Hi @davids5
Indeed STM32F437, STM32F439, STM32F479 are not included in Kconfig. I can remove those.
Certainly STM32F427 was missing though...
PWR_CR_VOS is not affected. It is a requirement though to be set to scale 1 or 2 only. Currently it is set to scale 1 specifically, so this change shouldn't be a problem. It would be best to have PWR_CR_VOS configurable in Kconfig, but this is another PR...
Indeed the code previously was not checking the frequency before enabling over-drive. So I left it as is. But then also PWR_CR_VOS is not configurable...
I will check a bit later regarding STM32_HAS_OVERDRIVE.
I am will somehow improve this PR, and then maybe work separately on making power scale and over-drive configurable, as it may affect other MCUs than just the STM32F4.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] davids5 merged pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] patacongo commented on pull request #1561: Fix in STM32F4 over-drive configuration.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1561:
URL: https://github.com/apache/incubator-nuttx/pull/1561#issuecomment-672055511
> This however means that I will have to include `<arch/board/board.h>` in `stm32f40xxx_rcc.c`. Is this acceptable, architecturally?
Yes, board.h is a "public" header file and may be included from anywhere in the system. That is because arch/board is a symbolic link to the board/include directory.
----------------------------------------------------------------
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