STM32: add setting for LSE drive load level #13797
Conversation
ciarmcom
added
the
release-type: patch
|
@JojoS62, thank you for your changes. |
|
@ARMmbed/team-st-mcd |
Next steps after:
|
|
I will check for defaults in target.json and add some lines in the documentation later. |
|
See Travis info, seems there is some compilation issue with NUCLEO_F103RB |
|
I see, that happens for targets where this setting is not applicable but LSE is present. |
transconductance
use unique #if defined()
set all defaults to initial low as after a reset most STM32 eval boards use low power crystals and work with this setting
|
Let's start CI |
|
what about the CI? |
|
CI started |
Jenkins CI Test : ❌ FAILEDBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
|
not sure what the errors are meaning, is a rebase necessary? |
|
CI restarted It was internal CI issue, it should be fixed now |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
mbedmain
removed
release-type: patch
| // set defaults for LSE drive load level | ||
| #if defined(LSE_CONFIG_AVAILABLE) | ||
|
|
||
| # if defined(MBED_CONF_TARGET_LSE_DRIVE_LOAD_LEVEL) |
There was a problem hiding this comment.
I'm not entirely sure if something changed since [1] was documented, but I think the build system will interpret the "macro_name" from target.json without adding the "MBED_CONF_" prefix, which will lead to the setting never being applied. I noticed this while I was backporting this to mbedos 5 so I might be wrong.
[1] https://os.mbed.com/docs/mbed-os/v6.11/program-setup/advanced-configuration.html
Edited: added the comment here, as adding it to all the target.json lines would have been a pain.
There was a problem hiding this comment.
Hi
I am sorry, I didn't understand well the question.
I don't think there is something different between mbedos 5 and 6 about json configuration.
And no need to add this line for each target as it can be defined at the family level.
There was a problem hiding this comment.
Sorry, I didn't made my point in an understandable way:
The implementation requires MBED_CONF_TARGET_LSE_DRIVE_LOAD_LEVEL to be defined to set LSEDRV from the config. However, in target.json the actual parameter is set to "macro_name": "TARGET_LSE_DRIVE_LOAD_LEVEL" which will result in the target not being generated as MBED_CONF_TARGET_LSE_DRIVE_LOAD_LEVEL, but without the prefix MBED_CONF_. (See [1] above comment)
This results in the value from target.json (or anywhere else, where the setting is overridden from) never being used in the first place, as far as I understand.
There was a problem hiding this comment.
I'm sorry, was resolved here #13939
Never mind. My fault.
Summary of changes
This PR is related to #13781
It adds a setting for the LSE drive load level.
This settings ist target hardware dependend and
depends on the crystal that is used for LSE clock.
For low power requirements, crystals with low load capacitors can be used and
driver setting is RCC_LSEDRIVE_LOW.
For higher stablity, crystals with higher load capacitys can be used and
driver setting is RCC_LSEDRIVE_HIGH.
When the setting is not matching the hardware, the LSE oscillator may not start or have large jitter.
Symptoms are Mbed Exceptions with LSE error or RTC wakeup error, or sleep_for() call never returns.
This PR fixes also a missing
#include "platform/mbed_error.h"for the error() function that was used conditionally already.Impact of changes
For known targets, the setting can be added to targets.json. For custom_targets, a default is used, but the user should provide a value that matches the hardware (crystal parameter depending).
Migration actions required
The defaults may be changed and the settings for existing targets in target.json need to be defined.
Documentation
A hint for this setting is necessary in the documentation, specially for custom_targets.
Pull request type
Test results
Reviewers