From 9ac4be8b759bb2cedeb999ce5e87d983261beded Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Fran=C3=A7ois=20Milants?= Date: Thu, 8 Apr 2021 20:07:24 +0200 Subject: [PATCH] TwiMaster is now based on the NRFX TWI driver, as it handles more edge cases and workarounds for errors on the bus. Reset the TWI bus after the soft-reset of the motion sensor to workaround issues on the TWI bus. --- src/CMakeLists.txt | 2 +- src/drivers/Bma421.cpp | 15 ++- src/drivers/Bma421.h | 4 + src/drivers/TwiMaster.cpp | 227 ++++++---------------------------- src/drivers/TwiMaster.h | 14 +-- src/sdk_config.h | 4 +- src/systemtask/SystemTask.cpp | 8 +- 7 files changed, 67 insertions(+), 207 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 89b1bc5a..3e199442 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -83,7 +83,7 @@ set(SDK_SOURCE_FILES "${NRF5_SDK_PATH}/external/fprintf/nrf_fprintf_format.c" # TWI - "${NRF5_SDK_PATH}/modules/nrfx/drivers/src/nrfx_twi.c" + "${NRF5_SDK_PATH}/modules/nrfx/drivers/src/nrfx_twim.c" # GPIOTE "${NRF5_SDK_PATH}/components/libraries/gpiote/app_gpiote.c" diff --git a/src/drivers/Bma421.cpp b/src/drivers/Bma421.cpp index 10d3e5e8..ea705d8e 100644 --- a/src/drivers/Bma421.cpp +++ b/src/drivers/Bma421.cpp @@ -35,12 +35,9 @@ Bma421::Bma421(TwiMaster& twiMaster, uint8_t twiAddress) : twiMaster{twiMaster}, } void Bma421::Init() { - auto ret = bma4_soft_reset(&bma); - if(ret != BMA4_OK) return; + if(not isResetOk) return; // Call SoftReset (and reset TWI device) first! - nrf_delay_ms(1); - - ret = bma423_init(&bma); + auto ret = bma423_init(&bma); if(ret != BMA4_OK) return; ret = bma423_write_config_file(&bma); @@ -109,3 +106,11 @@ bool Bma421::IsOk() const { void Bma421::ResetStepCounter() { bma423_reset_step_counter(&bma); } + +void Bma421::SoftReset() { + auto ret = bma4_soft_reset(&bma); + if(ret == BMA4_OK) { + isResetOk = true; + nrf_delay_ms(1); + } +} diff --git a/src/drivers/Bma421.h b/src/drivers/Bma421.h index d36d1db5..da021cbf 100644 --- a/src/drivers/Bma421.h +++ b/src/drivers/Bma421.h @@ -18,6 +18,9 @@ namespace Pinetime { Bma421(Bma421&&) = delete; Bma421& operator=(Bma421&&) = delete; + /// The chip freezes the TWI bus after the softreset operation. Softreset is separated from the + /// Init() method to allow the caller to uninit and then reinit the TWI device after the softreset. + void SoftReset(); void Init(); Values Process(); void ResetStepCounter(); @@ -34,6 +37,7 @@ namespace Pinetime { uint8_t deviceAddress = 0x18; struct bma4_dev bma; bool isOk = false; + bool isResetOk = false; }; } } \ No newline at end of file diff --git a/src/drivers/TwiMaster.cpp b/src/drivers/TwiMaster.cpp index 6a063ecf..646823d0 100644 --- a/src/drivers/TwiMaster.cpp +++ b/src/drivers/TwiMaster.cpp @@ -2,195 +2,77 @@ #include #include #include - +#include +#include using namespace Pinetime::Drivers; // TODO use shortcut to automatically send STOP when receive LastTX, for example // TODO use DMA/IRQ -TwiMaster::TwiMaster(const Modules module, const Parameters& params) : module{module}, params{params} { - mutex = xSemaphoreCreateBinary(); +TwiMaster::TwiMaster(const Modules module, const Parameters& params) : module{module}, params{params}, mutex{xSemaphoreCreateBinary()} { + ASSERT(mutex != nullptr); + switch(module) { + case Modules::TWIM1: + default: + twim = NRFX_TWIM_INSTANCE(1); + break; + } } void TwiMaster::Init() { - NRF_GPIO->PIN_CNF[params.pinScl] = ((uint32_t)GPIO_PIN_CNF_DIR_Input << GPIO_PIN_CNF_DIR_Pos) - | ((uint32_t)GPIO_PIN_CNF_INPUT_Connect << GPIO_PIN_CNF_INPUT_Pos) - | ((uint32_t)GPIO_PIN_CNF_PULL_Pullup << GPIO_PIN_CNF_PULL_Pos) - | ((uint32_t)GPIO_PIN_CNF_DRIVE_S0D1 << GPIO_PIN_CNF_DRIVE_Pos) - | ((uint32_t)GPIO_PIN_CNF_SENSE_Disabled << GPIO_PIN_CNF_SENSE_Pos); - - NRF_GPIO->PIN_CNF[params.pinSda] = ((uint32_t)GPIO_PIN_CNF_DIR_Input << GPIO_PIN_CNF_DIR_Pos) - | ((uint32_t)GPIO_PIN_CNF_INPUT_Connect << GPIO_PIN_CNF_INPUT_Pos) - | ((uint32_t)GPIO_PIN_CNF_PULL_Pullup << GPIO_PIN_CNF_PULL_Pos) - | ((uint32_t)GPIO_PIN_CNF_DRIVE_S0D1 << GPIO_PIN_CNF_DRIVE_Pos) - | ((uint32_t)GPIO_PIN_CNF_SENSE_Disabled << GPIO_PIN_CNF_SENSE_Pos); - - switch(module) { - case Modules::TWIM1: twiBaseAddress = NRF_TWIM1; break; - default: - return; - } - - switch(static_cast(params.frequency)) { - case Frequencies::Khz100 : twiBaseAddress->FREQUENCY = TWIM_FREQUENCY_FREQUENCY_K100; break; - case Frequencies::Khz250 : twiBaseAddress->FREQUENCY = TWIM_FREQUENCY_FREQUENCY_K250; break; - case Frequencies::Khz400 : twiBaseAddress->FREQUENCY = TWIM_FREQUENCY_FREQUENCY_K400; break; - } - - twiBaseAddress->PSEL.SCL = params.pinScl; - twiBaseAddress->PSEL.SDA = params.pinSda; - twiBaseAddress->EVENTS_LASTRX = 0; - twiBaseAddress->EVENTS_STOPPED = 0; - twiBaseAddress->EVENTS_LASTTX = 0; - twiBaseAddress->EVENTS_ERROR = 0; - twiBaseAddress->EVENTS_RXSTARTED = 0; - twiBaseAddress->EVENTS_SUSPENDED = 0; - twiBaseAddress->EVENTS_TXSTARTED = 0; - - twiBaseAddress->ENABLE = (TWIM_ENABLE_ENABLE_Enabled << TWIM_ENABLE_ENABLE_Pos); - - - /* // IRQ - NVIC_ClearPendingIRQ(_IRQn); - NVIC_SetPriority(_IRQn, 2); - NVIC_EnableIRQ(_IRQn); - */ + nrfx_twim_config_t config; + config.frequency = static_cast(params.frequency); + config.hold_bus_uninit = false; + config.interrupt_priority = 0; + config.scl = params.pinScl; + config.sda = params.pinSda; + nrfx_twim_init(&twim, + &config, + nullptr, + nullptr); + nrfx_twim_enable(&twim); xSemaphoreGive(mutex); - } TwiMaster::ErrorCodes TwiMaster::Read(uint8_t deviceAddress, uint8_t registerAddress, uint8_t *data, size_t size) { xSemaphoreTake(mutex, portMAX_DELAY); - auto ret = ReadWithRetry(deviceAddress, registerAddress, data, size); + TwiMaster::ErrorCodes ret; + + auto err = nrfx_twim_tx(&twim, deviceAddress, ®isterAddress, 1, false); + if(err != 0) { + return TwiMaster::ErrorCodes::TransactionFailed; + } + + err = nrfx_twim_rx(&twim, deviceAddress, data, size); + if(err != 0) { + return TwiMaster::ErrorCodes::TransactionFailed; + } xSemaphoreGive(mutex); - return ret; + return TwiMaster::ErrorCodes::NoError; } TwiMaster::ErrorCodes TwiMaster::Write(uint8_t deviceAddress, uint8_t registerAddress, const uint8_t *data, size_t size) { ASSERT(size <= maxDataSize); xSemaphoreTake(mutex, portMAX_DELAY); - - auto ret = WriteWithRetry(deviceAddress, registerAddress, data, size); - xSemaphoreGive(mutex); - return ret; -} - -/* Execute a read transaction (composed of a write and a read operation). If one of these opeartion fails, - * it's retried once. If it fails again, an error is returned */ -TwiMaster::ErrorCodes TwiMaster::ReadWithRetry(uint8_t deviceAddress, uint8_t registerAddress, uint8_t *data, size_t size) { TwiMaster::ErrorCodes ret; - ret = Write(deviceAddress, ®isterAddress, 1, false); - if(ret != ErrorCodes::NoError) - ret = Write(deviceAddress, ®isterAddress, 1, false); - if(ret != ErrorCodes::NoError) return ret; - - ret = Read(deviceAddress, data, size, true); - if(ret != ErrorCodes::NoError) - ret = Read(deviceAddress, data, size, true); - - return ret; -} - -/* Execute a write transaction. If it fails, it is retried once. If it fails again, an error is returned. */ -TwiMaster::ErrorCodes TwiMaster::WriteWithRetry(uint8_t deviceAddress, uint8_t registerAddress, const uint8_t *data, size_t size) { internalBuffer[0] = registerAddress; std::memcpy(internalBuffer+1, data, size); - auto ret = Write(deviceAddress, internalBuffer, size+1, true); - if(ret != ErrorCodes::NoError) - ret = Write(deviceAddress, internalBuffer, size+1, true); - - return ret; -} - - -TwiMaster::ErrorCodes TwiMaster::Read(uint8_t deviceAddress, uint8_t *buffer, size_t size, bool stop) { - twiBaseAddress->ADDRESS = deviceAddress; - twiBaseAddress->TASKS_RESUME = 0x1UL; - twiBaseAddress->RXD.PTR = (uint32_t)buffer; - twiBaseAddress->RXD.MAXCNT = size; - - twiBaseAddress->TASKS_STARTRX = 1; - - while(!twiBaseAddress->EVENTS_RXSTARTED && !twiBaseAddress->EVENTS_ERROR); - twiBaseAddress->EVENTS_RXSTARTED = 0x0UL; - - txStartedCycleCount = DWT->CYCCNT; - uint32_t currentCycleCount; - while(!twiBaseAddress->EVENTS_LASTRX && !twiBaseAddress->EVENTS_ERROR) { - currentCycleCount = DWT->CYCCNT; - if ((currentCycleCount-txStartedCycleCount) > HwFreezedDelay) { - FixHwFreezed(); - return ErrorCodes::TransactionFailed; - } - } - twiBaseAddress->EVENTS_LASTRX = 0x0UL; - - if (stop || twiBaseAddress->EVENTS_ERROR) { - twiBaseAddress->TASKS_STOP = 0x1UL; - while(!twiBaseAddress->EVENTS_STOPPED); - twiBaseAddress->EVENTS_STOPPED = 0x0UL; - } - else { - twiBaseAddress->TASKS_SUSPEND = 0x1UL; - while(!twiBaseAddress->EVENTS_SUSPENDED); - twiBaseAddress->EVENTS_SUSPENDED = 0x0UL; + auto err = nrfx_twim_tx(&twim, deviceAddress, internalBuffer , size+1, false); + if(err != 0){ + return TwiMaster::ErrorCodes::TransactionFailed; } - if (twiBaseAddress->EVENTS_ERROR) { - twiBaseAddress->EVENTS_ERROR = 0x0UL; - } - return ErrorCodes::NoError; -} - -TwiMaster::ErrorCodes TwiMaster::Write(uint8_t deviceAddress, const uint8_t *data, size_t size, bool stop) { - twiBaseAddress->ADDRESS = deviceAddress; - twiBaseAddress->TASKS_RESUME = 0x1UL; - twiBaseAddress->TXD.PTR = (uint32_t)data; - twiBaseAddress->TXD.MAXCNT = size; - - twiBaseAddress->TASKS_STARTTX = 1; - - while(!twiBaseAddress->EVENTS_TXSTARTED && !twiBaseAddress->EVENTS_ERROR); - twiBaseAddress->EVENTS_TXSTARTED = 0x0UL; - - txStartedCycleCount = DWT->CYCCNT; - uint32_t currentCycleCount; - while(!twiBaseAddress->EVENTS_LASTTX && !twiBaseAddress->EVENTS_ERROR) { - currentCycleCount = DWT->CYCCNT; - if ((currentCycleCount-txStartedCycleCount) > HwFreezedDelay) { - FixHwFreezed(); - return ErrorCodes::TransactionFailed; - } - } - twiBaseAddress->EVENTS_LASTTX = 0x0UL; - - if (stop || twiBaseAddress->EVENTS_ERROR) { - twiBaseAddress->TASKS_STOP = 0x1UL; - while(!twiBaseAddress->EVENTS_STOPPED); - twiBaseAddress->EVENTS_STOPPED = 0x0UL; - } - else { - twiBaseAddress->TASKS_SUSPEND = 0x1UL; - while(!twiBaseAddress->EVENTS_SUSPENDED); - twiBaseAddress->EVENTS_SUSPENDED = 0x0UL; - } - - if (twiBaseAddress->EVENTS_ERROR) { - twiBaseAddress->EVENTS_ERROR = 0x0UL; - uint32_t error = twiBaseAddress->ERRORSRC; - twiBaseAddress->ERRORSRC = error; - } - - return ErrorCodes::NoError; + xSemaphoreGive(mutex); + return TwiMaster::ErrorCodes::NoError; } void TwiMaster::Sleep() { - while(twiBaseAddress->ENABLE != 0) { - twiBaseAddress->ENABLE = (TWIM_ENABLE_ENABLE_Disabled << TWIM_ENABLE_ENABLE_Pos); - } + nrfx_twim_disable(&twim); + nrfx_twim_uninit(&twim); + nrf_gpio_cfg_default(6); nrf_gpio_cfg_default(7); NRF_LOG_INFO("[TWIMASTER] Sleep"); @@ -200,30 +82,3 @@ void TwiMaster::Wakeup() { Init(); NRF_LOG_INFO("[TWIMASTER] Wakeup"); } - -/* Sometimes, the TWIM device just freeze and never set the event EVENTS_LASTTX. - * This method disable and re-enable the peripheral so that it works again. - * This is just a workaround, and it would be better if we could find a way to prevent - * this issue from happening. - * */ -void TwiMaster::FixHwFreezed() { - NRF_LOG_INFO("I2C device frozen, reinitializing it!"); - // Disable I²C - uint32_t twi_state = NRF_TWI1->ENABLE; - twiBaseAddress->ENABLE = TWIM_ENABLE_ENABLE_Disabled << TWI_ENABLE_ENABLE_Pos; - - NRF_GPIO->PIN_CNF[params.pinScl] = ((uint32_t)GPIO_PIN_CNF_DIR_Input << GPIO_PIN_CNF_DIR_Pos) - | ((uint32_t)GPIO_PIN_CNF_INPUT_Connect << GPIO_PIN_CNF_INPUT_Pos) - | ((uint32_t)GPIO_PIN_CNF_PULL_Pullup << GPIO_PIN_CNF_PULL_Pos) - | ((uint32_t)GPIO_PIN_CNF_DRIVE_S0S1 << GPIO_PIN_CNF_DRIVE_Pos) - | ((uint32_t)GPIO_PIN_CNF_SENSE_Disabled << GPIO_PIN_CNF_SENSE_Pos); - - NRF_GPIO->PIN_CNF[params.pinSda] = ((uint32_t)GPIO_PIN_CNF_DIR_Input << GPIO_PIN_CNF_DIR_Pos) - | ((uint32_t)GPIO_PIN_CNF_INPUT_Connect << GPIO_PIN_CNF_INPUT_Pos) - | ((uint32_t)GPIO_PIN_CNF_PULL_Pullup << GPIO_PIN_CNF_PULL_Pos) - | ((uint32_t)GPIO_PIN_CNF_DRIVE_S0S1 << GPIO_PIN_CNF_DRIVE_Pos) - | ((uint32_t)GPIO_PIN_CNF_SENSE_Disabled << GPIO_PIN_CNF_SENSE_Pos); - - // Re-enable I²C - twiBaseAddress->ENABLE = twi_state; -} diff --git a/src/drivers/TwiMaster.h b/src/drivers/TwiMaster.h index 399e3d0f..6e3ff721 100644 --- a/src/drivers/TwiMaster.h +++ b/src/drivers/TwiMaster.h @@ -3,13 +3,13 @@ #include #include // NRF_TWIM_Type #include +#include namespace Pinetime { namespace Drivers { class TwiMaster { public: enum class Modules { TWIM1 }; - enum class Frequencies {Khz100, Khz250, Khz400}; enum class ErrorCodes {NoError, TransactionFailed}; struct Parameters { uint32_t frequency; @@ -27,21 +27,13 @@ namespace Pinetime { void Wakeup(); private: - ErrorCodes ReadWithRetry(uint8_t deviceAddress, uint8_t registerAddress, uint8_t* buffer, size_t size); - ErrorCodes WriteWithRetry(uint8_t deviceAddress, uint8_t registerAddress, const uint8_t* data, size_t size); - - ErrorCodes Read(uint8_t deviceAddress, uint8_t* buffer, size_t size, bool stop); - ErrorCodes Write(uint8_t deviceAddress, const uint8_t* data, size_t size, bool stop); - void FixHwFreezed(); - NRF_TWIM_Type* twiBaseAddress; - SemaphoreHandle_t mutex; + nrfx_twim_t twim; const Modules module; const Parameters params; + SemaphoreHandle_t mutex; static constexpr uint8_t maxDataSize{8}; static constexpr uint8_t registerSize{1}; uint8_t internalBuffer[maxDataSize + registerSize]; - uint32_t txStartedCycleCount = 0; - static constexpr uint32_t HwFreezedDelay{161000}; }; } } \ No newline at end of file diff --git a/src/sdk_config.h b/src/sdk_config.h index aa1bbb3a..bc97ef14 100644 --- a/src/sdk_config.h +++ b/src/sdk_config.h @@ -4992,7 +4992,7 @@ // NRFX_TWIM_ENABLED - nrfx_twim - TWIM peripheral driver //========================================================== #ifndef NRFX_TWIM_ENABLED -#define NRFX_TWIM_ENABLED 0 +#define NRFX_TWIM_ENABLED 1 #endif // NRFX_TWIM0_ENABLED - Enable TWIM0 instance @@ -5005,7 +5005,7 @@ #ifndef NRFX_TWIM1_ENABLED -#define NRFX_TWIM1_ENABLED 0 +#define NRFX_TWIM1_ENABLED 1 #endif // NRFX_TWIM_DEFAULT_CONFIG_FREQUENCY - Frequency diff --git a/src/systemtask/SystemTask.cpp b/src/systemtask/SystemTask.cpp index 4605ba95..6163c105 100644 --- a/src/systemtask/SystemTask.cpp +++ b/src/systemtask/SystemTask.cpp @@ -84,11 +84,15 @@ void SystemTask::Work() { touchPanel.Init(); batteryController.Init(); motorController.Init(); + motionSensor.SoftReset(); + + // Reset the TWI device because the motion sensor chip most probably crashed it... + twiMaster.Sleep(); + twiMaster.Init(); + motionSensor.Init(); - settingsController.Init(); - displayApp = std::make_unique(lcd, lvgl, touchPanel, batteryController, bleController, dateTimeController, watchdogView, *this, notificationManager, heartRateController, settingsController, motionController);