[riot-notifications] [RIOT-OS/RIOT] add a systick interface for the ztimer (#16543)

Marian Buschsieweke notifications at github.com
Tue Jun 22 09:37:19 CEST 2021


@maribu commented on this pull request.

Thanks for the contribution.

First, a bunch of style fixes (mostly from the annotations by the CI). You can squash in the style fixes right in, if you want.

I can take an actual look at the code and test it later today.

> + * @brief   ztimer periph/systick backend API
+ *
+ * @author	Franz Freitag <franz.freitag at st.ovgu.de>

```suggestion
 * @brief       ztimer periph/systick backend API
 *
 * @author      Franz Freitag <franz.freitag at st.ovgu.de>
```

> + * @author	Justus Krebs <justus.krebs at st.ovgu.de>
+ * @author	Nick Weiler <nick.weiler at st.ovgu.de>

```suggestion
 * @author      Justus Krebs <justus.krebs at st.ovgu.de>
 * @author      Nick Weiler <nick.weiler at st.ovgu.de>
```

> +extern "C" {
+#endif
+
+/**
+ * @brief   ztimer periph/systick backend initialization function
+ *
+ * @param[in, out]  clock   ztimer_periph_systick_t object to initialize
+ */
+void ztimer_periph_systick_init(ztimer_clock_t *clock);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* ZTIMER_PERIPH_SYSTICK_H */
+/** @} */

```suggestion
/** @} */
```

> +extern "C" {
+#endif
+
+/**
+ * @brief   ztimer periph/systick backend initialization function
+ *
+ * @param[in, out]  clock   ztimer_periph_systick_t object to initialize
+ */
+void ztimer_periph_systick_init(ztimer_clock_t *clock);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* ZTIMER_PERIPH_SYSTICK_H */
+/** @} */

```suggestion
/** @} */

```

> +extern "C" {
+#endif
+
+/**
+ * @brief   ztimer periph/systick backend initialization function
+ *
+ * @param[in, out]  clock   ztimer_periph_systick_t object to initialize
+ */
+void ztimer_periph_systick_init(ztimer_clock_t *clock);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* ZTIMER_PERIPH_SYSTICK_H */
+/** @} */

missing end of line

> +#define ENABLE_MASK     (SysTick_CTRL_CLKSOURCE_Msk |\
+                         SysTick_CTRL_ENABLE_Msk    |\

```suggestion
#define ENABLE_MASK     (SysTick_CTRL_CLKSOURCE_Msk | \
                         SysTick_CTRL_ENABLE_Msk    | \
```

> +static ztimer_clock_t *clock_timer;
+
+void isr_systick(void)
+{
+    ztimer_handler(clock_timer);
+}
+
+static void _ztimer_periph_systick_set(ztimer_clock_t *clock, uint32_t val)
+{
+    (void)clock;
+    unsigned state = irq_disable();
+
+    /*Stop the systick to prevent race condition*/
+    SysTick->CTRL = 0;
+
+    /*val divided by 2^24 to garanty a suitable size*/

```suggestion
    /*val divided by 2^24 to guarantee a suitable size*/
```

> +
+    /*Stop the systick to prevent race condition*/
+    SysTick->CTRL = 0;
+
+    /*val divided by 2^24 to garanty a suitable size*/
+    alarm_ext = val >> 24;
+    alarm_val = val & SysTick_LOAD_RELOAD_Msk;
+
+    /*Systick is counting backwards*/
+    now += SysTick->LOAD - SysTick->VAL;
+
+    /*If the value is greater than the maximum value the value is
+    is set to the maximum*/
+    if (alarm_ext) {
+        SysTick->LOAD = SysTick_LOAD_RELOAD_Msk;
+    } else {

```suggestion
    }
    else {
```

> +
+    /* start the timer again */
+    SysTick->CTRL = ENABLE_MASK;
+
+    irq_restore(state);
+}
+
+static uint32_t _ztimer_periph_systick_now(ztimer_clock_t *clock)
+{
+    (void) clock;
+    return (now + SysTick->LOAD - SysTick->VAL) & 0xFFFFFFU;
+}
+
+static void _ztimer_periph_systick_cancel(ztimer_clock_t *clock)
+{
+    (void) clock;

```suggestion
    (void)clock;
```

> +    .cancel = _ztimer_periph_systick_cancel,
+};
+
+void ztimer_periph_systick_init(ztimer_clock_t *clock)
+{
+    clock->ops = &_ztimer_periph_systick_ops;
+    clock->max_value = 0xFFFFFFU;
+    clock_timer = clock;
+
+    SysTick->VAL  = 0;
+    SysTick->LOAD = SysTick_LOAD_RELOAD_Msk;
+    SysTick->CTRL = ENABLE_MASK;
+
+    NVIC_SetPriority(SysTick_IRQn, CPU_DEFAULT_IRQ_PRIO);
+    NVIC_EnableIRQ(SysTick_IRQn);
+}

missing end of line

> +    } else {
+        SysTick->LOAD = alarm_val;
+    }
+
+    /* force reload */
+    SysTick->VAL = 0;
+
+    /* start the timer again */
+    SysTick->CTRL = ENABLE_MASK;
+
+    irq_restore(state);
+}
+
+static uint32_t _ztimer_periph_systick_now(ztimer_clock_t *clock)
+{
+    (void) clock;

```suggestion
    (void)clock;
```

> @@ -0,0 +1,6 @@
+include ../Makefile.tests_common
+
+USEMODULE += ztimer_msec ztimer_usec ztimer_sec
+USEMODULE += ztimer_periph_systick
+
+include $(RIOTBASE)/Makefile.include

missing end of line

> +#include <stdint.h>
+#include <stdlib.h>
+
+#include "mutex.h"
+#include "ztimer.h"
+#include "ztimer/periph_systick.h"
+#include "periph_conf.h"
+#include "board.h"
+
+#define INTERVAL_SEC        2lu
+#define INTERVAL_LONG_SEC   30lu
+
+static void test_timer_set(unsigned period, unsigned iterations)
+{
+    while (iterations--) {
+        ztimer_sleep(ZTIMER_SEC, period); 

```suggestion
        ztimer_sleep(ZTIMER_SEC, period);
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/16543#pullrequestreview-689154420
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210622/7e0c2eaa/attachment.htm>


More information about the notifications mailing list