New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STM32 : Clock source selection in json config file #4421

Merged
merged 1 commit into from Jun 29, 2017

Conversation

Projects
None yet
6 participants
@jeromecoutant
Contributor

jeromecoutant commented Jun 1, 2017

Description

We propose to set the clock source from json config file.

Default values are kept to the same values as before this pull request,
but if user wants to explicitly use the internal or external clock, he can modify the targets json file or overwrite the config value in the mbed app json file.

Goal is to :

  • avoid issues like #4390 => our test bench will be able now to verify each clock source
  • correct issues like #1408 => I create a new configuration for HSI and USB enabled

Status

IN DEVELOPMENT

I would like to get ARM and community feedback first.

@theotherjimmy

Just a suggestion: You could reduce copy paste code by creating a private target that all three of the targets you are modifying inherit from. For example:

diff --git i/targets/targets.json w/targets/targets.json
index 454ee6f3c..80b4c269b 100644
--- i/targets/targets.json
+++ w/targets/targets.json
@@ -686,17 +686,30 @@
         "release_versions": ["2"],
         "device_name": "STM32F042K6"
     },
+    "FAMILY_STM32": {
+        "inherits": ["Target"],
+        "public": false,
+        "supported_toolchains": ["ARM", "uARM", "IAR", "GCC_ARM"],
+        "default_toolchain": "ARM",
+        "extra_labels": ["STM"],
+        "release_versions": ["2", "5"],
+        "macros": ["TRANSACTION_QUEUE_SIZE_SPI=2"],
+        "device_has": ["ANALOGIN", "I2C", "I2CSLAVE", "I2C_ASYNCH", "INTERRUPTIN",  "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_FC", "SERIAL_ASYNCH", "SLEEP", "SPI", "SPISLAVE", "SPI_ASYNCH", "STDIO_MESSAGES"],
+        "config": {
+            "clock_source": {
+                "help": "Mask value : USE_PLL_HSE_EXTC | USE_PLL_HSE_XTAL (need HW patch) | USE_PLL_HSI",
+                "value": "USE_PLL_HSE_EXTC|USE_PLL_HSI",
+                "macro_name": "CLOCK_SOURCE"
+            }
+        }
+    },
     "NUCLEO_F070RB": {
+        "inherits": ["FAMILY_STM32"],
         "supported_form_factors": ["ARDUINO", "MORPHO"],
         "core": "Cortex-M0",
-        "default_toolchain": "ARM",
-        "extra_labels": ["STM", "STM32F0", "STM32F070RB"],
-        "supported_toolchains": ["ARM", "uARM", "IAR", "GCC_ARM"],
-        "inherits": ["Target"],
+        "extra_labels_add": ["STM32F0", "STM32F070RB"],
         "detect_code": ["0755"],
-        "macros": ["TRANSACTION_QUEUE_SIZE_SPI=2"],
-        "device_has": ["ANALOGIN", "I2C", "I2CSLAVE", "I2C_ASYNCH", "INTERRUPTIN", "LOWPOWERTIMER", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_FC", "SERIAL_ASYNCH", "SLEEP", "SPI", "SPISLAVE", "SPI_ASYNCH", "STDIO_MESSAGES"],
-        "release_versions": ["2", "5"],
+        "device_has_add": ["LOWPOWERTIMER"],
         "device_name": "STM32F070RB"
     },
     "NUCLEO_F072RB": {
@@ -728,14 +741,10 @@
     "NUCLEO_F103RB": {
         "supported_form_factors": ["ARDUINO", "MORPHO"],
         "core": "Cortex-M3",
-        "default_toolchain": "ARM",
-        "extra_labels": ["STM", "STM32F1", "STM32F103RB"],
-        "supported_toolchains": ["ARM", "uARM", "GCC_ARM", "IAR"],
-        "inherits": ["Target"],
+        "extra_labels_add": ["STM32F1", "STM32F103RB"],
+        "inherits": ["FAMILY_STM32"],
         "detect_code": ["0700"],
-        "macros": ["TRANSACTION_QUEUE_SIZE_SPI=2"],
-        "device_has": ["ANALOGIN", "CAN", "I2C", "I2CSLAVE", "I2C_ASYNCH", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_FC", "SERIAL_ASYNCH", "SLEEP", "SPI", "SPISLAVE", "SPI_ASYNCH", "STDIO_MESSAGES"],
-        "release_versions": ["2", "5"],
+        "device_has_add": ["CAN"],
         "device_name": "STM32F103RB"
     },
     "NUCLEO_F207ZG": {
@@ -1118,14 +1127,11 @@
     "NUCLEO_L476RG": {
         "supported_form_factors": ["ARDUINO", "MORPHO"],
         "core": "Cortex-M4F",
-        "default_toolchain": "ARM",
-        "extra_labels": ["STM", "STM32L4", "STM32L476RG", "STM32L476xG"],
-        "supported_toolchains": ["ARM", "uARM", "IAR", "GCC_ARM"],
-        "inherits": ["Target"],
+        "extra_labels_add": ["STM32L4", "STM32L476RG", "STM32L476xG"],
+        "inherits": ["FAMILY_STM32"],
         "detect_code": ["0765"],
-        "macros": ["TRANSACTION_QUEUE_SIZE_SPI=2","USBHOST_OTHER"],
-        "device_has": ["ANALOGIN", "ANALOGOUT", "CAN", "I2C", "I2CSLAVE", "I2C_ASYNCH", "INTERRUPTIN", "LOWPOWERTIMER", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "RTC", "SERIAL", "SERIAL_ASYNCH", "SERIAL_FC", "SLEEP", "SPI", "SPISLAVE", "SPI_ASYNCH", "STDIO_MESSAGES", "TRNG"],
-        "release_versions": ["2", "5"],
+        "macros_add": ["USBHOST_OTHER"],
+        "device_has_add": ["ANALOGOUT", "CAN", "LOWPOWERTIMER", "TRNG"],
         "device_name": "STM32L476RG"
     },
     "NUCLEO_L486RG": {
@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jun 2, 2017

@theotherjimmy
I like this idea, but I think this couldn't be applied to the clock_source config, as there are many different cases.
So let's optimize this targets.json file in another pull request?

@Willem23

This comment has been minimized.

Contributor

Willem23 commented Jun 2, 2017

The most important condition for a solution is that not-so-expert users should not have to worry about anything. Default clock or other settings should be initialised correctly for maximum performance on all mbed platforms, specific settings should be adjusted automatically when you include the USB device or host library in your project and/or any other lib (eg ethernet peripheral) that might require modifications (possibly generating a warning message when clockspeed or other default capabilities are affected).
An optional config file for clock (and perhaps other settings) would be welcome to overrule the default settings. However, it seems that the json file proposal above will only works for the CLI user and wont work for the online compiler.

Is is doable to have a mechanism that calls a weak external ''init'' function at the end of the system startup (ie before object instantiations and before main).
The default init function is empty, but could be overruled and provided as part of a library (eg USB device). The init function may be target dependent, maybe either hardcoded or interpret a config file that is part of the lib and do whatever is needed. There should be a method to sequentially call multiple ''ínit'' functions when there are multiple libs included.

The weak function approach is used on the original mbed lpc1768 platforms for the MAC address resolution: extern "C" void mbed_mac_address(char * mac) {} provides the unique MAC hardware address to the ethernet peripheral lib by interrogating the mbed host interface device.
You needed to provide your own version of mbed_mac_address() if you wanted to use the ethernet lib on non-mbed hardware that did not have the host interface device.

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jun 2, 2017

However, it seems that the json file proposal above will only works for the CLI user and wont work for the online compiler

No...

With OS2 online compiler, there is no change as default value in the targets json file is the same.
ex for most of NUCLEO boards: "value": "USE_PLL_HSE_EXTC|USE_PLL_HSI"
=> this will try to use HSE first, then HSI in case of HSE failure, which is already the current behavior

With OS5 online compiler, you can add a mbed_app json file to overwrite this config value
ex:
{
"target_overrides": {
"NUCLEO_F070RB": {
"clock_source": "USE_PLL_HSI"
}
}
}

@0xc0170 , please, correct me if I am wrong

@sg- sg- added the needs: review label Jun 3, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 12, 2017

Can you please resolve conflicts?

@0xc0170 0xc0170 added needs: work and removed needs: review labels Jun 12, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 19, 2017

@jeromecoutant Bump on the conflicts.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 20, 2017

The most important condition for a solution is that not-so-expert users should not have to worry about anything. Default clock or other settings should be initialised correctly for maximum performance on all mbed platforms, specific settings should be adjusted automatically when you include the USB device or host library in your project and/or any other lib (eg ethernet peripheral) that might require modifications (possibly generating a warning message when clockspeed or other default capabilities are affected).

+1, good summary !

This patch looks fine to me. Providing defaults as they are, and providing a configuration for setting other clock sources.

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jun 21, 2017

Thx Martin
I will complete the pull request with all STM32 targets

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jun 26, 2017

@jeromecoutant are you planning to continue this PR as is? if you are, could you rebase this PR to remove the merge conflicts?

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jun 26, 2017

Hi
Status is still "IN DEVELOPMENT"
Regards,

STM32F4: json clock source configuration
- default value is the same as before patch
- system_stm32f4xx.c file is copied to family level with all other ST cube files
- specific clock configuration is now in a new file: system_clock.c
- nvic_addr.h file is now in TARGET_STM level, and can be used everywhere

@jeromecoutant jeromecoutant force-pushed the jeromecoutant:PR_CONFIG_CLOCK branch to 2ae2d98 Jun 29, 2017

@jeromecoutant

This comment has been minimized.

Contributor

jeromecoutant commented Jun 29, 2017

Hi

Work done for all ST F4

  • default value is the same as before patch
  • system_stm32f4xx.c file is copied from target level to family level (easier to maintain at each cube update)
  • specific clock configuration is now in a new file: system_clock.c (target level)
  • nvic_addr.h file is now in TARGET_STM level, and can be used everywhere

Note that I try to set the maximum frequency for all:

  • some targets that worked at 96MHz are now working at 100
  • some targets that was at 168 are now at 180

Internal tests have been executed with HSE and HSI.

@theotherjimmy
You can start morph test
Thx

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Jun 29, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jun 29, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jun 29, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 687

All builds and test passed!

@theotherjimmy theotherjimmy merged commit ea5c2cf into ARMmbed:master Jun 29, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment