mirror of https://github.com/rusefi/rusefi-1.git
simplify getTimeNowNt() (#1876)
* remove old impl * add cpp wrapper to os_access * clean up os_access * remove touchTimeCounter * new impl * put comment about lock free impl * include cpp wrappers for BL Co-authored-by: Matthew Kennedy <makenne@microsoft.com>
This commit is contained in:
Matthew Kennedy
GitHub
parent
906675166e
commit
2083b34cf9
|
|
@ -139,6 +139,7 @@ include $(CHIBIOS)/os/hal/osal/rt/osal.mk
|
|||
# RTOS files (optional).
|
||||
include $(CHIBIOS)/os/rt/rt.mk
|
||||
include $(CHIBIOS)/os/common/ports/ARMCMx/compilers/GCC/mk/port_v7m.mk
|
||||
include $(CHIBIOS)/os/various/cpp_wrappers/chcpp.mk
|
||||
|
||||
include $(CONFIG)/boards/$(PROJECT_BOARD)/board.mk
|
||||
include $(PROJECT_DIR)/init/init.mk
|
||||
|
|
|
|||
|
|
@ -50,7 +50,6 @@
|
|||
#include "spark_logic.h"
|
||||
#include "aux_valves.h"
|
||||
#include "accelerometer.h"
|
||||
#include "counter64.h"
|
||||
#include "perf_trace.h"
|
||||
#include "boost_control.h"
|
||||
#include "launch_control.h"
|
||||
|
|
@ -220,8 +219,6 @@ static void doPeriodicSlowCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
|
|||
#if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT
|
||||
efiAssertVoid(CUSTOM_ERR_6661, getCurrentRemainingStack() > 64, "lowStckOnEv");
|
||||
#if EFI_PROD_CODE
|
||||
touchTimeCounter();
|
||||
|
||||
slowStartStopButtonCallback(PASS_ENGINE_PARAMETER_SIGNATURE);
|
||||
#endif /* EFI_PROD_CODE */
|
||||
|
||||
|
|
|
|||
|
|
@ -17,7 +17,6 @@ void commonInitEngineController(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_S
|
|||
void initStartStopButton(DECLARE_ENGINE_PARAMETER_SIGNATURE);
|
||||
|
||||
void initDataStructures(DECLARE_ENGINE_PARAMETER_SIGNATURE);
|
||||
void touchTimeCounter();
|
||||
|
||||
void slowStartStopButtonCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE);
|
||||
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@
|
|||
|
||||
#include "engine_controller.h"
|
||||
#include "perf_trace.h"
|
||||
#include "counter64.h"
|
||||
#include "os_access.h"
|
||||
#include "settings.h"
|
||||
|
||||
EXTERN_ENGINE;
|
||||
|
|
@ -84,8 +84,6 @@ void setMockState(brain_pin_e pin, bool state DECLARE_ENGINE_PARAMETER_SUFFIX) {
|
|||
#endif /* EFI_ENABLE_MOCK_ADC */
|
||||
|
||||
#if EFI_PROD_CODE
|
||||
static Overflow64Counter halTime;
|
||||
|
||||
/**
|
||||
* 64-bit result would not overflow, but that's complex stuff for our 32-bit MCU
|
||||
*/
|
||||
|
|
@ -95,34 +93,31 @@ efitimeus_t getTimeNowUs(void) {
|
|||
return getTimeNowNt() / (CORE_CLOCK / 1000000);
|
||||
}
|
||||
|
||||
//todo: macro to save method invocation
|
||||
efitick_t getTimeNowNt(void) {
|
||||
#if EFI_PROD_CODE
|
||||
/* Entering a reentrant critical zone.*/
|
||||
syssts_t sts = chSysGetStatusAndLockX();
|
||||
efitime_t localH = halTime.state.highBits;
|
||||
uint32_t localLow = halTime.state.lowBits;
|
||||
volatile uint32_t lastLowerNt = 0;
|
||||
volatile uint32_t upperTimeNt = 0;
|
||||
|
||||
uint32_t value = getTimeNowLowerNt();
|
||||
efitick_t getTimeNowNt() {
|
||||
chibios_rt::CriticalSectionLocker csl;
|
||||
|
||||
if (value < localLow) {
|
||||
// new value less than previous value means there was an overflow in that 32 bit counter
|
||||
localH += 0x100000000LL;
|
||||
uint32_t stamp = getTimeNowLowerNt();
|
||||
|
||||
// Lower 32 bits of the timer has wrapped - time to step upper bits
|
||||
if (stamp < lastLowerNt) {
|
||||
upperTimeNt++;
|
||||
}
|
||||
|
||||
efitime_t result = localH + value;
|
||||
lastLowerNt = stamp;
|
||||
|
||||
/* Leaving the critical zone.*/
|
||||
chSysRestoreStatusX(sts);
|
||||
return result;
|
||||
#else /* EFI_PROD_CODE */
|
||||
// todo: why is this implementation not used?
|
||||
/**
|
||||
* this method is lock-free and thread-safe, that's because the 'update' method
|
||||
* is atomic with a critical zone requirement.
|
||||
*
|
||||
* http://stackoverflow.com/questions/5162673/how-to-read-two-32bit-counters-as-a-64bit-integer-without-race-condition
|
||||
*/
|
||||
return ((int64_t)upperTimeNt << 32) | stamp;
|
||||
}
|
||||
|
||||
/* //Alternative lock free implementation (probably actually slower!)
|
||||
// this method is lock-free and thread-safe, that's because the 'update' method
|
||||
// is atomic with a critical zone requirement.
|
||||
//
|
||||
// http://stackoverflow.com/questions/5162673/how-to-read-two-32bit-counters-as-a-64bit-integer-without-race-condition
|
||||
|
||||
etitick_t getTimeNowNt() {
|
||||
efitime_t localH;
|
||||
efitime_t localH2;
|
||||
uint32_t localLow;
|
||||
|
|
@ -131,14 +126,11 @@ efitick_t getTimeNowNt(void) {
|
|||
localH = halTime.state.highBits;
|
||||
localLow = halTime.state.lowBits;
|
||||
localH2 = halTime.state.highBits;
|
||||
#if EFI_PROD_CODE
|
||||
if (counter++ == 10000)
|
||||
chDbgPanic("lock-free frozen");
|
||||
#endif /* EFI_PROD_CODE */
|
||||
} while (localH != localH2);
|
||||
/**
|
||||
* We need to take current counter after making a local 64 bit snapshot
|
||||
*/
|
||||
|
||||
// We need to take current counter after making a local 64 bit snapshot
|
||||
uint32_t value = getTimeNowLowerNt();
|
||||
|
||||
if (value < localLow) {
|
||||
|
|
@ -147,20 +139,8 @@ efitick_t getTimeNowNt(void) {
|
|||
}
|
||||
|
||||
return localH + value;
|
||||
#endif /* EFI_PROD_CODE */
|
||||
|
||||
}
|
||||
|
||||
void touchTimeCounter() {
|
||||
/**
|
||||
* We need to push current value into the 64 bit counter often enough so that we do not miss an overflow
|
||||
*/
|
||||
/* Entering a reentrant critical zone.*/
|
||||
syssts_t sts = chSysGetStatusAndLockX();
|
||||
updateAndSet(&halTime.state, getTimeNowLowerNt());
|
||||
/* Leaving the critical zone.*/
|
||||
chSysRestoreStatusX(sts);
|
||||
}
|
||||
*/
|
||||
|
||||
static void onStartStopButtonToggle(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
|
||||
engine->startStopStateToggleCounter++;
|
||||
|
|
|
|||
|
|
@ -9,19 +9,14 @@
|
|||
|
||||
#pragma once
|
||||
|
||||
#ifdef __cplusplus
|
||||
extern "C"
|
||||
{
|
||||
#endif /* __cplusplus */
|
||||
|
||||
#include <ch.h>
|
||||
#include <hal.h>
|
||||
#include "chprintf.h"
|
||||
|
||||
#ifdef __cplusplus
|
||||
}
|
||||
#endif /* __cplusplus */
|
||||
|
||||
#include "io_pins.h"
|
||||
|
||||
#ifdef __cplusplus
|
||||
// ChibiOS c++ wrappers
|
||||
#include "ch.hpp"
|
||||
#endif /* __cplusplus */
|
||||
|
||||
#define HAS_OS_ACCESS
|
||||
|
|
|
|||
|
|
@ -1,42 +0,0 @@
|
|||
/*
|
||||
* counter64.cpp
|
||||
*
|
||||
* Created on: Mar 31, 2019
|
||||
* @author Andrey Belomutskiy, (c) 2012-2020
|
||||
*/
|
||||
|
||||
#include "counter64.h"
|
||||
|
||||
/**
|
||||
* The main use-case of this class is to keep track of a 64-bit global number of CPU ticks from reset.
|
||||
*
|
||||
* stm32f4 hardware has a 32-bit Cycle Count Register (CYCCNT), which is incremented with every CPU cycle.
|
||||
* With 32 bits and 168MHz speed this counter overflows every 4B/168M = 23 seconds. The job of this class is to
|
||||
* keep track of the current CYCCNT value, detect these overflows, and provide a nice,
|
||||
* clean 64 bit global cycle counter.
|
||||
*
|
||||
* In order for this to function, it's your responsibility to invoke offer() method at least once a second.
|
||||
*/
|
||||
Overflow64Counter::Overflow64Counter() {
|
||||
state.highBits = 0;
|
||||
state.lowBits = 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* in order to have atomic writes this should be invoked within a critical section
|
||||
*/
|
||||
void updateAndSet(State64 *state, uint32_t value) {
|
||||
if (value < state->lowBits) {
|
||||
// new value less than previous value means there was an overflow in that 32 bit counter
|
||||
state->highBits += 0x100000000LL;
|
||||
}
|
||||
state->lowBits = value;
|
||||
}
|
||||
|
||||
#if EFI_UNIT_TEST
|
||||
efitime_t Overflow64Counter::update(uint32_t value) {
|
||||
updateAndSet(&state, value);
|
||||
return state.highBits + state.lowBits;
|
||||
}
|
||||
#endif
|
||||
|
||||
|
|
@ -1,31 +0,0 @@
|
|||
/*
|
||||
* @file counter64.h
|
||||
*
|
||||
* Created on: Mar 31, 2019
|
||||
* @author Andrey Belomutskiy, (c) 2012-2020
|
||||
*/
|
||||
|
||||
#pragma once
|
||||
|
||||
#include "global.h"
|
||||
|
||||
typedef struct {
|
||||
// todo: would probably be better to keep the high bits as 32 bit field to be sure
|
||||
volatile efitime_t highBits;
|
||||
volatile uint32_t lowBits;
|
||||
} State64;
|
||||
|
||||
void updateAndSet(State64 *state, uint32_t value);
|
||||
|
||||
class Overflow64Counter
|
||||
{
|
||||
public:
|
||||
Overflow64Counter();
|
||||
|
||||
efitime_t get();
|
||||
#if EFI_UNIT_TEST
|
||||
efitime_t update(uint32_t value);
|
||||
#endif
|
||||
|
||||
State64 state;
|
||||
};
|
||||
|
|
@ -8,7 +8,6 @@ UTILSRC = \
|
|||
UTILSRC_CPP = \
|
||||
$(UTIL_DIR)/containers/cyclic_buffer.cpp \
|
||||
$(UTIL_DIR)/containers/listener_array.cpp \
|
||||
$(UTIL_DIR)/containers/counter64.cpp \
|
||||
$(UTIL_DIR)/containers/local_version_holder.cpp \
|
||||
$(UTIL_DIR)/containers/table_helper.cpp \
|
||||
$(UTIL_DIR)/math/biquad.cpp \
|
||||
|
|
|
|||
|
|
@ -22,7 +22,6 @@
|
|||
#include "crc.h"
|
||||
#include "fl_stack.h"
|
||||
#include "io_pins.h"
|
||||
#include "counter64.h"
|
||||
#include "efi_gpio.h"
|
||||
#include "efilib.h"
|
||||
|
||||
|
|
@ -66,20 +65,6 @@ TEST(util, crc) {
|
|||
assertEqualsM("crc32 line inc", 0x4775a7b1, c);
|
||||
}
|
||||
|
||||
|
||||
TEST(util, Overflow64Counter) {
|
||||
print("*************************************** testOverflow64Counter\r\n");
|
||||
|
||||
Overflow64Counter o;
|
||||
ASSERT_EQ(0, o.update(0));
|
||||
ASSERT_EQ(10, o.update(10));
|
||||
|
||||
ASSERT_EQ(20, o.update(20));
|
||||
|
||||
// overflow
|
||||
ASSERT_EQ(4294967296, o.update(0));
|
||||
}
|
||||
|
||||
TEST(util, cyclicBufferContains) {
|
||||
cyclic_buffer<int> sb;
|
||||
sb.add(10);
|
||||
|
|
|
|||
Loading…
Reference in New Issue