Skip to content
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

Soc arm cleanup systick #15212

Merged
merged 4 commits into from
Apr 26, 2019

Conversation

ioannisg
Copy link
Member

@ioannisg ioannisg commented Apr 5, 2019

The PR clean-up the inconsistencies with SysTick In ARM platforms:

  • Adds the CPU_HAS_SYSTICK selection in ARCH for ARMv7-M and ARMv8-M Mainline architectures
  • Renames it to CPU_CORTEX_M_HAS_SYSTICK, to look similar to all other CORTEX_M__HAS options.
  • Cleans up ARM ARMv7-M platforms definitions, so they don't need to define CPU_HAS_SYSTICK anymore
  • Removes the default system timer selection for ARM platforms
    • We have 3 timers at the moment (SysTick, Nordic RTC and SAM0 RTC). Remove default y option.
  • Explicitly set the system timer selection in all ARM boards/platforms. This was already done for 70% of the boards - now it is done for all.

In addition, the PR cleans up some left-out direct CPU_CORTEX_M Selections in ARM SOCs.

The main reason for the patch (besides the obvious enhancement of clean-up) is to decouple HW capability options (CPU_HAS_) from system configuration.

@ioannisg ioannisg added Enhancement Changes/Updates/Additions to existing features area: ARM ARM (32-bit) Architecture labels Apr 5, 2019
@ioannisg ioannisg added this to the v1.15.0 milestone Apr 5, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Apr 5, 2019

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@ioannisg ioannisg force-pushed the soc_arm_cleanup_systick branch from 88fd5cf to 022e070 Compare April 5, 2019 13:03
Copy link
Collaborator

@ulfalizer ulfalizer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable.

Maybe soc: arm: rename CPU_HAS_SYSTICK to CPU_CORTEX_M_HAS_SYSTICK could mention why it was renamed in the commit message.

@andrewboie andrewboie requested a review from andyross April 5, 2019 18:48
Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason not to do this, but is SysTick such a weird default? I mean, it's architecturally specified and always present. Even nRF and SAM can use it even though they have alternatives they'd like to be default.

@ioannisg ioannisg force-pushed the soc_arm_cleanup_systick branch from 022e070 to e082309 Compare April 8, 2019 15:07
@ioannisg
Copy link
Member Author

ioannisg commented Apr 8, 2019

Looks reasonable.

Maybe soc: arm: rename CPU_HAS_SYSTICK to CPU_CORTEX_M_HAS_SYSTICK could mention why it was renamed in the commit message.

Will do, thanks.
It is added only in the PR description.

Done now @ulfalizer

@ioannisg ioannisg force-pushed the soc_arm_cleanup_systick branch 3 times, most recently from 1f26fd4 to a7ae2e6 Compare April 14, 2019 18:37
@ioannisg ioannisg force-pushed the soc_arm_cleanup_systick branch 3 times, most recently from a949ffe to 0e25fdc Compare April 23, 2019 19:24
@ioannisg ioannisg requested a review from carlescufi April 23, 2019 19:25
CPU_CORTEX_M does not need to be selected by Kconfig
symbols that already select a CORTEX_M variant.

Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
ARM SysTick timer is implemented by default in ARMv7-M
and Mainline ARMv8-M processors, so we include the
corresponding Kconfig symbol in arch/arm/core/cortex-M/Kconfig
and remove the selections from the Cortex-M SOCs.

Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
This commit renames the symbol CPU_HAS_SYSTICK to
CPU_CORTEX_M_HAS_SYSTICK, to look similar to all
other CPU_CORTEX_M_HAS_ options, and moves the
K-config symbol definition from arm/core/Kconfig to
arm/core/cortex_m/Kconfig.

Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
We shall not enable by default a system timer in ARM
platforms, namely the SysTick, the Nordic, or the SAM0
RTC timer, simply by assessing the hardware capabilities
(e.g. by conditioning on CPU_CORTEX_M_HAS_SYSTICK).
Instead, now, all ARM platforms needs to explicitly set
their system timer module. Note that this has already
been the case for ca 80% of the ARM platforms.

This clean-up allows us to decouple HW capabilities from
system configuration (for example, Nordic platforms may
enable option CPU_CORTEX_M_HAS_SYSTICK, and still use
the platform-specific RTC timer for system timing).

Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
@ioannisg ioannisg force-pushed the soc_arm_cleanup_systick branch from 522c2c0 to 46a530c Compare April 24, 2019 16:31
@ioannisg
Copy link
Member Author

@carlescufi @galak @MaureenHelm, some help needed on this one - CI times-out continuously, due to the patch introducing changes in a relatively high number of platforms.

@galak galak merged commit 236c5ac into zephyrproject-rtos:master Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants