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

make nRF SoC timer source configurable #15491

Closed
wants to merge 1 commit into from
Closed

make nRF SoC timer source configurable #15491

wants to merge 1 commit into from

Conversation

elsonwei
Copy link

Only nRF51 does not implement the systick timer.
User can use systick to drive the timer in nRF52/nRF91 series if
they need higher resolution timer.

Only nRF51 does not implement the systick timer.
User can use systick to drive the timer in nRF52/nRF91 series if
they need higher resolution timer.

Signed-off-by: Elson Wei <elson.wei@gmail.com>
@elsonwei elsonwei requested a review from ioannisg as a code owner April 16, 2019 15:34
@ioannisg
Copy link
Member

Thanks for your contribution!
I wonder - will this be covered by #15212 ?

@ioannisg ioannisg added the platform: nRF Nordic nRFx label Apr 17, 2019
@pizi-nordic
Copy link
Collaborator

pizi-nordic commented Apr 17, 2019

@elsonwei: Have you confirmed that Systick is not stopped when __WFE()/__WFI() is executed?
Have you measured current consumption of the SoC when Systick is used as a system timer?

@elsonwei
Copy link
Author

I use SysTick as the timer source on my nrf52_sparkfun board. It works fine.
I don't measure the power consumption since I'm not developing a portable device.

@elsonwei
Copy link
Author

#15212 makes the SysTick be selectable.
But NRF_RTC_TIMER, CORTEX_M_SYSTICK and SAM0_RTC_TIMER should be mutually exclusive.
In addition, SYS_CLOCK_HW_CYCLES_PER_SEC should be changed according to the timer source.

@@ -11,7 +11,7 @@ config SOC_SERIES_NRF52X
select CPU_CORTEX_M4
select CPU_HAS_ARM_MPU
select SOC_FAMILY_NRF
select NRF_RTC_TIMER
select NRF_HAS_SYSTICK
Copy link
Member

Choose a reason for hiding this comment

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

Not needed any more - CPU_HAS_SYSTICK is selected in ARCH KConfig

config SYSTICK_AS_TIMER
bool "Cortex-M SYSTICK timer"
depends on NRF_HAS_SYSTICK
select CPU_HAS_SYSTICK
Copy link
Member

Choose a reason for hiding this comment

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

Not needed any more - CPU_HAS_SYSTICK is selected in ARCH KConfig

@@ -18,4 +18,24 @@ config SOC_FAMILY
source "soc/arm/nordic_nrf/Kconfig.peripherals"
source "soc/arm/nordic_nrf/*/Kconfig.soc"

config NRF_HAS_SYSTICK
Copy link
Member

Choose a reason for hiding this comment

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

Not needed any more - CPU_HAS_SYSTICK is selected in ARCH KConfig

@@ -14,7 +14,8 @@ config SOC_SERIES

config SYS_CLOCK_HW_CYCLES_PER_SEC
int
default 32768
default 32768 if RTC_AS_TIMER
Copy link
Member

Choose a reason for hiding this comment

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

This could work, but why not using, instead, the existing options:
NRF_RTC_TIMER and CORTEX_M_SYSTICK?

@@ -13,7 +13,7 @@ config SOC_SERIES_NRF91X
select CPU_HAS_FPU
select ARMV8_M_DSP
select SOC_FAMILY_NRF
select NRF_RTC_TIMER
select NRF_HAS_SYSTICK
Copy link
Member

Choose a reason for hiding this comment

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

Not needed any more - CPU_HAS_SYSTICK is selected in ARCH KConfig

@@ -14,7 +14,8 @@ config SOC_SERIES

config SYS_CLOCK_HW_CYCLES_PER_SEC
int
default 32768
default 32768 if RTC_AS_TIMER
Copy link
Member

Choose a reason for hiding this comment

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

This could work, but why not using, instead, the existing options:
NRF_RTC_TIMER and CORTEX_M_SYSTICK?

@ioannisg
Copy link
Member

So, first, I agree that you should have the option to use any available HW timer you prefer, as a source for your system timer. Let's agree that the up-streamed nRF Boards need to have a default timer. And that shall be the NRF_RTC_TIMER.

In #15212 we have done some cleanup and removed the "default y" in SYSTICK and NRF_RTC timer Kconfig options. IMO, if you want to use the SysTick, you should simply do:

CONFIG_CORTEX_M_SYSTICK=y
CONFIG_NRF_RTC_TIMER=n

in your prj.conf.

And the same for the cycles-per-second: CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC=64 MHz in your prj.conf (although, for that one, your proposal to embed this in the existing definition is OK).

We might still need to remove the default setting of the NRF_RTC_TIMER from SOC definition and move it to nrf Board definition, if the above setting does not work.

@ioannisg
Copy link
Member

ioannisg commented Apr 29, 2019

But NRF_RTC_TIMER, CORTEX_M_SYSTICK and SAM0_RTC_TIMER should be mutually exclusive.

That's really interesting; could you solve this problem for all ARM boards?

Copy link
Member

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

Added a few comments, and proposed a simpler solution, please, re-visit :)

@github-actions github-actions bot added has-conflicts Issue/PR has conflicts with another issue/PR and removed has-conflicts Issue/PR has conflicts with another issue/PR labels Jun 29, 2020
@nashif nashif added the Stale label Sep 4, 2020
@github-actions github-actions bot closed this Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-conflicts Issue/PR has conflicts with another issue/PR platform: nRF Nordic nRFx Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants