drivers/pulse_counter: Use C11 atomics & bugfix

- Previously the pulse counter used GCC's built-in functions for atomic memory
  access. This PR changes this to use C11 atomics instead:
    - Use of C11 atomics instead of atomic build-in functions makes the code
      more portable and readable.
- Previously pulse_counter_reset() did not use an atomic function, so that
  resets would be racy. (E.g. on the 8-bit AVR platform an 16-bit store is
  not atomic, unless special care (like using C11 atomics) is taken.)
This commit is contained in:
Marian Buschsieweke 2019-04-23 14:14:28 +02:00
parent 743d7170e2
commit 2d81738cb9
No known key found for this signature in database
GPG Key ID: 61F64C6599B1539F
2 changed files with 15 additions and 16 deletions

View File

@ -26,6 +26,11 @@
#define PULSE_COUNTER_H #define PULSE_COUNTER_H
#include <stdint.h> #include <stdint.h>
#ifdef __cplusplus
#include "c11_atomics_compat.hpp"
#else
#include <stdatomic.h>
#endif
#include "periph/gpio.h" #include "periph/gpio.h"
#ifdef __cplusplus #ifdef __cplusplus
@ -44,7 +49,7 @@ typedef struct {
* @brief Device descriptor for a pulse counter device * @brief Device descriptor for a pulse counter device
*/ */
typedef struct { typedef struct {
int16_t pulse_count; /**< pulse counter */ atomic_uint_least16_t pulse_count; /**< pulse counter */
} pulse_counter_t; } pulse_counter_t;
/** /**
@ -74,7 +79,7 @@ int16_t pulse_counter_read_with_reset(pulse_counter_t *dev);
* *
* @return Accumulated pulse counts * @return Accumulated pulse counts
*/ */
int16_t pulse_counter_read_without_reset(const pulse_counter_t *dev); int16_t pulse_counter_read_without_reset(pulse_counter_t *dev);
/** /**
* @brief Reset pulse counter value * @brief Reset pulse counter value

View File

@ -19,9 +19,8 @@
* @} * @}
*/ */
#include <string.h>
#include "pulse_counter_params.h" #include "pulse_counter_params.h"
#include "pulse_counter.h"
#define ENABLE_DEBUG (0) #define ENABLE_DEBUG (0)
#include "debug.h" #include "debug.h"
@ -31,8 +30,8 @@ static void pulse_counter_trigger(void *arg)
{ {
pulse_counter_t *dev = (pulse_counter_t *)arg; pulse_counter_t *dev = (pulse_counter_t *)arg;
/* Use atomic operations to avoid messing with IRQ flags */ /* Use C11 atomic operations to avoid messing with IRQ flags */
__atomic_fetch_add(&(dev->pulse_count), 1, __ATOMIC_SEQ_CST); atomic_fetch_add(&(dev->pulse_count), 1);
} }
/* Initialize pulse counter */ /* Initialize pulse counter */
@ -50,29 +49,24 @@ int pulse_counter_init(pulse_counter_t *dev, const pulse_counter_params_t *param
return -1; return -1;
} }
dev->pulse_count = 0; atomic_init(&dev->pulse_count, 0);
return 0; return 0;
} }
/* Return the accumulated pulse counts and reset the count to zero */ /* Return the accumulated pulse counts and reset the count to zero */
int16_t pulse_counter_read_with_reset(pulse_counter_t *dev) int16_t pulse_counter_read_with_reset(pulse_counter_t *dev)
{ {
int16_t pulse_count_output = 0; return atomic_exchange(&(dev->pulse_count), 0);
int16_t reset_value = 0;
/* Use atomic operations to avoid messing with IRQ flags */
__atomic_exchange(&(dev->pulse_count), &reset_value, &pulse_count_output, __ATOMIC_SEQ_CST);
return pulse_count_output;
} }
/* Return the accumulated pulse counts */ /* Return the accumulated pulse counts */
int16_t pulse_counter_read_without_reset(const pulse_counter_t *dev) int16_t pulse_counter_read_without_reset(pulse_counter_t *dev)
{ {
return dev->pulse_count; return atomic_load(&dev->pulse_count);
} }
/* Reset the pulse count value to zero */ /* Reset the pulse count value to zero */
void pulse_counter_reset(pulse_counter_t *dev) void pulse_counter_reset(pulse_counter_t *dev)
{ {
dev->pulse_count = 0; atomic_store(&(dev->pulse_count), 0);
} }