This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 564c9d7dcdc arm: systick: fix off-by-one in SysTick RELOAD programming
564c9d7dcdc is described below
commit 564c9d7dcdccf99167939923853264729e53322e
Author: Gao Jiawei <[email protected]>
AuthorDate: Wed Jan 22 11:54:51 2025 +0800
arm: systick: fix off-by-one in SysTick RELOAD programming
ARM specifies that SysTick RELOAD must be programmed with N-1 to get a
period of N CPU cycles.
Adjust the timeout/reload conversions accordingly: subtract 1 when writing
RELOAD and add 1 back when converting RELOAD to a timeout/interval.
Also clamp RELOAD to the valid 24-bit range (1..0x00ffffff) for
Armv7-M/Armv8-M to avoid invalid values.
Reference:
https://developer.arm.com/documentation/dui0646/c/Cortex-M7-Peripherals/System-timer--SysTick/SysTick-Reload-Value-Register
Signed-off-by: Gao Jiawei <[email protected]>
---
arch/arm/src/armv7-m/arm_systick.c | 38 +++++++++++++++++++++++++++++---------
arch/arm/src/armv7-m/nvic.h | 13 ++++++++++---
arch/arm/src/armv8-m/arm_systick.c | 34 +++++++++++++++++++++++++++-------
arch/arm/src/armv8-m/nvic.h | 13 ++++++++++---
include/sys/param.h | 5 +++++
5 files changed, 81 insertions(+), 22 deletions(-)
diff --git a/arch/arm/src/armv7-m/arm_systick.c
b/arch/arm/src/armv7-m/arm_systick.c
index eccdae96af6..a25df880f5f 100644
--- a/arch/arm/src/armv7-m/arm_systick.c
+++ b/arch/arm/src/armv7-m/arm_systick.c
@@ -30,6 +30,7 @@
#include <nuttx/irq.h>
#include <stdio.h>
+#include <sys/param.h>
#include "nvic.h"
#include "systick.h"
@@ -37,6 +38,22 @@
#ifdef CONFIG_ARMV7M_SYSTICK
+/****************************************************************************
+ * Pre-processor definitions
+ ****************************************************************************/
+
+/* According to what arm specified, we should set the
+ * RELOAD value to N-1 for a desired systick interval of
+ * N processor clock cycles.
+ * Therefore, when reading the RELOAD value, it is important
+ * to add one clock cycle back.
+ */
+#define RELOAD2TIMEOUT(reload) ((reload) + 1)
+#define TIMEOUT2RELOAD(timeout) ((timeout) - 1)
+
+#define CLAMP_RELOAD(reload) \
+CLAMP(reload, NVIC_MIN_SYSTICK_CNT, NVIC_MAX_SYSTICK_CNT)
+
/****************************************************************************
* Private Types
****************************************************************************/
@@ -141,10 +158,10 @@ static int systick_getstatus(struct timer_lowerhalf_s
*lower_,
status->flags = lower->callback ? TCFLAGS_HANDLER : 0;
status->flags |= systick_is_running() ? TCFLAGS_ACTIVE : 0;
- status->timeout = usec_from_count(getreg32(NVIC_SYSTICK_RELOAD),
- lower->freq);
- status->timeleft = usec_from_count(getreg32(NVIC_SYSTICK_CURRENT),
- lower->freq);
+ status->timeout = usec_from_count(
+ RELOAD2TIMEOUT(getreg32(NVIC_SYSTICK_RELOAD)), lower->freq);
+ status->timeleft = usec_from_count(
+ getreg32(NVIC_SYSTICK_CURRENT), lower->freq);
if (systick_irq_pending(lower))
{
@@ -185,8 +202,9 @@ static int systick_settimeout(struct timer_lowerhalf_s
*lower_,
{
uint32_t reload;
- reload = usec_to_count(timeout, lower->freq);
- putreg32(reload, NVIC_SYSTICK_RELOAD);
+ reload = TIMEOUT2RELOAD(usec_to_count(timeout, lower->freq));
+
+ putreg32(CLAMP_RELOAD(reload), NVIC_SYSTICK_RELOAD);
if (systick_is_running())
{
if (reload != getreg32(NVIC_SYSTICK_CURRENT))
@@ -237,7 +255,8 @@ static int systick_interrupt(int irq, void *context, void
*arg)
if (lower->callback && systick_is_running())
{
uint32_t reload = getreg32(NVIC_SYSTICK_RELOAD);
- uint32_t interval = usec_from_count(reload, lower->freq);
+ uint32_t interval = usec_from_count(
+ RELOAD2TIMEOUT(reload), lower->freq);
uint32_t next_interval = interval;
lower->next_interval = &next_interval;
@@ -245,8 +264,9 @@ static int systick_interrupt(int irq, void *context, void
*arg)
{
if (next_interval && next_interval != interval)
{
- reload = usec_to_count(next_interval, lower->freq);
- putreg32(reload, NVIC_SYSTICK_RELOAD);
+ reload = TIMEOUT2RELOAD(
+ usec_to_count(next_interval, lower->freq));
+ putreg32(CLAMP_RELOAD(reload), NVIC_SYSTICK_RELOAD);
putreg32(0, NVIC_SYSTICK_CURRENT);
}
}
diff --git a/arch/arm/src/armv7-m/nvic.h b/arch/arm/src/armv7-m/nvic.h
index 49d0af16338..e6e1d8171ad 100644
--- a/arch/arm/src/armv7-m/nvic.h
+++ b/arch/arm/src/armv7-m/nvic.h
@@ -470,18 +470,25 @@
/* SysTick reload value register (SYSTICK_RELOAD) */
+/* The armv7-m systick RELOAD regitser has 24 bits
+ * ranging from 0x1 ~ 0x00ffffff
+ * noting that, setting reload to 0 is valid but doesn't have any effects
+ */
+
+#define NVIC_MIN_SYSTICK_CNT (0x00000001)
+#define NVIC_MAX_SYSTICK_CNT (0x00ffffff)
#define NVIC_SYSTICK_RELOAD_SHIFT 0 /* Bits 23-0: Timer reload
value */
-#define NVIC_SYSTICK_RELOAD_MASK (0x00ffffff <<
NVIC_SYSTICK_RELOAD_SHIFT)
+#define NVIC_SYSTICK_RELOAD_MASK (NVIC_MAX_SYSTICK_CNT <<
NVIC_SYSTICK_RELOAD_SHIFT)
/* SysTick current value register (SYSTICK_CURRENT) */
#define NVIC_SYSTICK_CURRENT_SHIFT 0 /* Bits 23-0: Timer current
value */
-#define NVIC_SYSTICK_CURRENT_MASK (0x00ffffff <<
NVIC_SYSTICK_RELOAD_SHIFT)
+#define NVIC_SYSTICK_CURRENT_MASK (NVIC_MAX_SYSTICK_CNT <<
NVIC_SYSTICK_RELOAD_SHIFT)
/* SysTick calibration value register (SYSTICK_CALIB) */
#define NVIC_SYSTICK_CALIB_TENMS_SHIFT 0 /* Bits 23-0: Calibration
value */
-#define NVIC_SYSTICK_CALIB_TENMS_MASK (0x00ffffff <<
NVIC_SYSTICK_CALIB_TENMS_SHIFT)
+#define NVIC_SYSTICK_CALIB_TENMS_MASK (NVIC_MAX_SYSTICK_CNT <<
NVIC_SYSTICK_CALIB_TENMS_SHIFT)
#define NVIC_SYSTICK_CALIB_SKEW (1 << 30) /* Bit 30: Calibration value
inexact */
#define NVIC_SYSTICK_CALIB_NOREF (1 << 31) /* Bit 31: No external
reference clock */
diff --git a/arch/arm/src/armv8-m/arm_systick.c
b/arch/arm/src/armv8-m/arm_systick.c
index 7d7e54c9bbc..9a0d5f52112 100644
--- a/arch/arm/src/armv8-m/arm_systick.c
+++ b/arch/arm/src/armv8-m/arm_systick.c
@@ -29,6 +29,7 @@
#include <nuttx/arch.h>
#include <nuttx/irq.h>
+#include <sys/param.h>
#include <stdio.h>
#include "nvic.h"
@@ -37,6 +38,22 @@
#ifdef CONFIG_ARMV8M_SYSTICK
+/****************************************************************************
+ * Pre-processor definitions
+ ****************************************************************************/
+
+/* According to what arm specified, we should set the
+ * RELOAD value to N-1 for a desired systick interval of
+ * N processor clock cycles.
+ * Therefore, when reading the RELOAD value, it is important
+ * to add one clock cycle back.
+ */
+#define RELOAD2TIMEOUT(reload) ((reload) + 1)
+#define TIMEOUT2RELOAD(timeout) ((timeout) - 1)
+
+#define CLAMP_RELOAD(reload) \
+CLAMP(reload, NVIC_MIN_SYSTICK_CNT, NVIC_MAX_SYSTICK_CNT)
+
/****************************************************************************
* Private Types
****************************************************************************/
@@ -141,8 +158,8 @@ static int systick_getstatus(struct timer_lowerhalf_s
*lower_,
status->flags = lower->callback ? TCFLAGS_HANDLER : 0;
status->flags |= systick_is_running() ? TCFLAGS_ACTIVE : 0;
- status->timeout = usec_from_count(getreg32(NVIC_SYSTICK_RELOAD),
- lower->freq);
+ status->timeout = usec_from_count(
+ RELOAD2TIMEOUT(getreg32(NVIC_SYSTICK_RELOAD)), lower->freq);
status->timeleft = usec_from_count(getreg32(NVIC_SYSTICK_CURRENT),
lower->freq);
@@ -185,8 +202,9 @@ static int systick_settimeout(struct timer_lowerhalf_s
*lower_,
{
uint32_t reload;
- reload = usec_to_count(timeout, lower->freq);
- putreg32(reload, NVIC_SYSTICK_RELOAD);
+ reload = TIMEOUT2RELOAD(usec_to_count(timeout, lower->freq));
+
+ putreg32(CLAMP_RELOAD(reload), NVIC_SYSTICK_RELOAD);
if (systick_is_running())
{
if (reload != getreg32(NVIC_SYSTICK_CURRENT))
@@ -237,7 +255,8 @@ static int systick_interrupt(int irq, void *context, void
*arg)
if (lower->callback && systick_is_running())
{
uint32_t reload = getreg32(NVIC_SYSTICK_RELOAD);
- uint32_t interval = usec_from_count(reload, lower->freq);
+ uint32_t interval = usec_from_count(
+ RELOAD2TIMEOUT(reload), lower->freq);
uint32_t next_interval = interval;
lower->next_interval = &next_interval;
@@ -245,8 +264,9 @@ static int systick_interrupt(int irq, void *context, void
*arg)
{
if (next_interval && next_interval != interval)
{
- reload = usec_to_count(next_interval, lower->freq);
- putreg32(reload, NVIC_SYSTICK_RELOAD);
+ reload = TIMEOUT2RELOAD(
+ usec_to_count(next_interval, lower->freq));
+ putreg32(CLAMP_RELOAD(reload), NVIC_SYSTICK_RELOAD);
putreg32(0, NVIC_SYSTICK_CURRENT);
}
}
diff --git a/arch/arm/src/armv8-m/nvic.h b/arch/arm/src/armv8-m/nvic.h
index 646ebbf33e9..0df6a9e60db 100644
--- a/arch/arm/src/armv8-m/nvic.h
+++ b/arch/arm/src/armv8-m/nvic.h
@@ -544,18 +544,25 @@
/* SysTick reload value register (SYSTICK_RELOAD) */
+/* The armv8-m systick RELOAD regitser has 24 bits
+ * ranging from 0x1 ~ 0x00ffffff
+ * noting that, setting reload to 0 is valid but doesn't have any effects
+ */
+
+#define NVIC_MIN_SYSTICK_CNT (0x00000001)
+#define NVIC_MAX_SYSTICK_CNT (0x00ffffff)
#define NVIC_SYSTICK_RELOAD_SHIFT 0 /* Bits 23-0: Timer reload
value */
-#define NVIC_SYSTICK_RELOAD_MASK (0x00ffffff <<
NVIC_SYSTICK_RELOAD_SHIFT)
+#define NVIC_SYSTICK_RELOAD_MASK (NVIC_MAX_SYSTICK_CNT <<
NVIC_SYSTICK_RELOAD_SHIFT)
/* SysTick current value register (SYSTICK_CURRENT) */
#define NVIC_SYSTICK_CURRENT_SHIFT 0 /* Bits 23-0: Timer current
value */
-#define NVIC_SYSTICK_CURRENT_MASK (0x00ffffff <<
NVIC_SYSTICK_RELOAD_SHIFT)
+#define NVIC_SYSTICK_CURRENT_MASK (NVIC_MAX_SYSTICK_CNT <<
NVIC_SYSTICK_RELOAD_SHIFT)
/* SysTick calibration value register (SYSTICK_CALIB) */
#define NVIC_SYSTICK_CALIB_TENMS_SHIFT 0 /* Bits 23-0: Calibration
value */
-#define NVIC_SYSTICK_CALIB_TENMS_MASK (0x00ffffff <<
NVIC_SYSTICK_CALIB_TENMS_SHIFT)
+#define NVIC_SYSTICK_CALIB_TENMS_MASK (NVIC_MAX_SYSTICK_CNT <<
NVIC_SYSTICK_CALIB_TENMS_SHIFT)
#define NVIC_SYSTICK_CALIB_SKEW (1 << 30) /* Bit 30: Calibration value
inexact */
#define NVIC_SYSTICK_CALIB_NOREF (1 << 31) /* Bit 31: No external
reference clock */
diff --git a/include/sys/param.h b/include/sys/param.h
index ff7ebb38ae3..ebce5444ccc 100644
--- a/include/sys/param.h
+++ b/include/sys/param.h
@@ -45,6 +45,11 @@
# define MAX(a,b) (((a) > (b)) ? (a) : (b))
#endif /* MAX */
+#ifndef CLAMP
+/* inclusively clamping a value into a given range */
+# define CLAMP(x, min, max) ((x) <= (min) ? (min) : MIN(x, max))
+#endif /* CLAMP */
+
/* Macros for number of items.
* (aka. ARRAY_SIZE, ArraySize, Size of an Array)
*/
