From 2f710d06f37b9ee964ad24dc1832d18ac321c664 Mon Sep 17 00:00:00 2001 From: JF Date: Fri, 23 Oct 2020 22:25:37 +0200 Subject: Workaround for bug https://github.com/JF002/Pinetime/issues/79 until a better fix is found. When the driver is stuck in an infinite loop for more than ~2.5ms, the TWI device is re-init and the transaction is retried. Read() and Write() return an error code. --- src/drivers/TwiMaster.cpp | 95 ++++++++++++++++++++++++++++++++++++++++++----- src/drivers/TwiMaster.h | 16 +++++--- 2 files changed, 96 insertions(+), 15 deletions(-) (limited to 'src/drivers') diff --git a/src/drivers/TwiMaster.cpp b/src/drivers/TwiMaster.cpp index a9eb5d0c..3ff8a952 100644 --- a/src/drivers/TwiMaster.cpp +++ b/src/drivers/TwiMaster.cpp @@ -60,24 +60,53 @@ void TwiMaster::Init() { } -void TwiMaster::Read(uint8_t deviceAddress, uint8_t registerAddress, uint8_t *data, size_t size) { +TwiMaster::ErrorCodes TwiMaster::Read(uint8_t deviceAddress, uint8_t registerAddress, uint8_t *data, size_t size) { xSemaphoreTake(mutex, portMAX_DELAY); - Write(deviceAddress, ®isterAddress, 1, false); - Read(deviceAddress, data, size, true); + auto ret = ReadWithRetry(deviceAddress, registerAddress, data, size); xSemaphoreGive(mutex); + + return ret; } -void TwiMaster::Write(uint8_t deviceAddress, uint8_t registerAddress, const uint8_t *data, size_t size) { +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); - Write(deviceAddress, internalBuffer, size+1, true); - xSemaphoreGive(mutex); + auto ret = Write(deviceAddress, internalBuffer, size+1, true); + if(ret != ErrorCodes::NoError) + ret = Write(deviceAddress, internalBuffer, size+1, true); + + return ret; } -void TwiMaster::Read(uint8_t deviceAddress, uint8_t *buffer, size_t size, bool stop) { +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; @@ -88,7 +117,15 @@ void TwiMaster::Read(uint8_t deviceAddress, uint8_t *buffer, size_t size, bool s while(!twiBaseAddress->EVENTS_RXSTARTED && !twiBaseAddress->EVENTS_ERROR); twiBaseAddress->EVENTS_RXSTARTED = 0x0UL; - while(!twiBaseAddress->EVENTS_LASTRX && !twiBaseAddress->EVENTS_ERROR); + 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) { @@ -105,9 +142,10 @@ void TwiMaster::Read(uint8_t deviceAddress, uint8_t *buffer, size_t size, bool s if (twiBaseAddress->EVENTS_ERROR) { twiBaseAddress->EVENTS_ERROR = 0x0UL; } + return ErrorCodes::NoError; } -void TwiMaster::Write(uint8_t deviceAddress, const uint8_t *data, size_t size, bool stop) { +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; @@ -118,7 +156,15 @@ void TwiMaster::Write(uint8_t deviceAddress, const uint8_t *data, size_t size, b while(!twiBaseAddress->EVENTS_TXSTARTED && !twiBaseAddress->EVENTS_ERROR); twiBaseAddress->EVENTS_TXSTARTED = 0x0UL; - while(!twiBaseAddress->EVENTS_LASTTX && !twiBaseAddress->EVENTS_ERROR); + 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) { @@ -137,6 +183,8 @@ void TwiMaster::Write(uint8_t deviceAddress, const uint8_t *data, size_t size, b uint32_t error = twiBaseAddress->ERRORSRC; twiBaseAddress->ERRORSRC = error; } + + return ErrorCodes::NoError; } void TwiMaster::Sleep() { @@ -152,3 +200,30 @@ 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 9b6b5070..52e39098 100644 --- a/src/drivers/TwiMaster.h +++ b/src/drivers/TwiMaster.h @@ -10,6 +10,7 @@ namespace Pinetime { public: enum class Modules { TWIM1 }; enum class Frequencies {Khz100, Khz250, Khz400}; + enum class ErrorCodes {NoError, TransactionFailed}; struct Parameters { uint32_t frequency; uint8_t pinSda; @@ -19,15 +20,19 @@ namespace Pinetime { TwiMaster(const Modules module, const Parameters& params); void Init(); - void Read(uint8_t deviceAddress, uint8_t registerAddress, uint8_t* buffer, size_t size); - void Write(uint8_t deviceAddress, uint8_t registerAddress, const uint8_t* data, size_t size); + ErrorCodes Read(uint8_t deviceAddress, uint8_t registerAddress, uint8_t* buffer, size_t size); + ErrorCodes Write(uint8_t deviceAddress, uint8_t registerAddress, const uint8_t* data, size_t size); void Sleep(); void Wakeup(); private: - void Read(uint8_t deviceAddress, uint8_t* buffer, size_t size, bool stop); - void Write(uint8_t deviceAddress, const uint8_t* data, size_t size, bool stop); + 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; const Modules module; @@ -35,7 +40,8 @@ namespace Pinetime { 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 -- cgit v1.2.3-70-g09d2 From 1bb2eb9dcd59fbb198be157e34b7ff25367adea0 Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 27 Oct 2020 19:38:45 +0100 Subject: Disable sleep mode on the SPI NOR Flash when the version is unknown. This is because the current bootloader (which does not exposes its version) cannot initialize the chip when it's in sleep mode. This feature will be re-enabled when the bootloader expses it's version. --- src/BootloaderVersion.cpp | 26 ++++++++++++++++++++++++++ src/BootloaderVersion.h | 12 ++++++++++++ src/CMakeLists.txt | 2 ++ src/drivers/SpiNorFlash.cpp | 6 +++--- src/systemtask/SystemTask.cpp | 7 ++++++- 5 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 src/BootloaderVersion.cpp create mode 100644 src/BootloaderVersion.h (limited to 'src/drivers') diff --git a/src/BootloaderVersion.cpp b/src/BootloaderVersion.cpp new file mode 100644 index 00000000..8555593f --- /dev/null +++ b/src/BootloaderVersion.cpp @@ -0,0 +1,26 @@ +#include +#include "BootloaderVersion.h" + +using namespace Pinetime; + +// NOTE : current bootloader does not export its version to the application firmware. + +uint32_t BootloaderVersion::Major() { + return 0; +} + +uint32_t BootloaderVersion::Minor() { + return 0; +} + +uint32_t BootloaderVersion::Patch() { + return 0; +} + +const char *BootloaderVersion::VersionString() { + return "0.0.0"; +} + +bool BootloaderVersion::IsValid() { + return false; +} diff --git a/src/BootloaderVersion.h b/src/BootloaderVersion.h new file mode 100644 index 00000000..c7fcbd98 --- /dev/null +++ b/src/BootloaderVersion.h @@ -0,0 +1,12 @@ +#pragma once + +namespace Pinetime { + class BootloaderVersion { + public: + static uint32_t Major(); + static uint32_t Minor(); + static uint32_t Patch(); + static const char* VersionString(); + static bool IsValid(); + }; +} \ No newline at end of file diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cd37810f..af0b110e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -322,6 +322,7 @@ list(APPEND IMAGE_FILES ) list(APPEND SOURCE_FILES + BootloaderVersion.cpp logging/NrfLogger.cpp displayapp/DisplayApp.cpp displayapp/screens/Screen.cpp @@ -397,6 +398,7 @@ list(APPEND GRAPHICS_SOURCE_FILES ) set(INCLUDE_FILES + BootloaderVersion.h logging/Logger.h logging/NrfLogger.h displayapp/DisplayApp.h diff --git a/src/drivers/SpiNorFlash.cpp b/src/drivers/SpiNorFlash.cpp index 351a9dfc..bd24834e 100644 --- a/src/drivers/SpiNorFlash.cpp +++ b/src/drivers/SpiNorFlash.cpp @@ -12,7 +12,7 @@ SpiNorFlash::SpiNorFlash(Spi& spi) : spi{spi} { void SpiNorFlash::Init() { device_id = ReadIdentificaion(); - NRF_LOG_INFO("[SPI FLASH] Manufacturer : %d, Memory type : %d, memory density : %d", device_id.manufacturer, device_id.type, device_id.density); + NRF_LOG_INFO("[SpiNorFlash] Manufacturer : %d, Memory type : %d, memory density : %d", device_id.manufacturer, device_id.type, device_id.density); } void SpiNorFlash::Uninit() { @@ -22,7 +22,7 @@ void SpiNorFlash::Uninit() { void SpiNorFlash::Sleep() { auto cmd = static_cast(Commands::DeepPowerDown); spi.Write(&cmd, sizeof(uint8_t)); - NRF_LOG_INFO("[FLASH] Sleep") + NRF_LOG_INFO("[SpiNorFlash] Sleep") } void SpiNorFlash::Wakeup() { @@ -38,7 +38,7 @@ void SpiNorFlash::Wakeup() { else { NRF_LOG_INFO("[SpiNorFlash] ID on Wakeup: %d", id); } - NRF_LOG_INFO("[FLASH] Wakeup") + NRF_LOG_INFO("[SpiNorFlash] Wakeup") } SpiNorFlash::Identification SpiNorFlash::ReadIdentificaion() { diff --git a/src/systemtask/SystemTask.cpp b/src/systemtask/SystemTask.cpp index 3efe21b8..dac4ce29 100644 --- a/src/systemtask/SystemTask.cpp +++ b/src/systemtask/SystemTask.cpp @@ -13,6 +13,7 @@ #include #include "main.h" #include "components/ble/NimbleController.h" +#include "../BootloaderVersion.h" using namespace Pinetime::System; @@ -161,7 +162,11 @@ void SystemTask::Work() { ReloadIdleTimer(); break; case Messages::OnDisplayTaskSleeping: - spiNorFlash.Sleep(); + if(BootloaderVersion::IsValid()) { + // First versions of the bootloader do not expose their version and cannot initialize the SPI NOR FLASH + // if it's in sleep mode. Avoid bricked device by disabling sleep mode on these versions. + spiNorFlash.Sleep(); + } lcd.Sleep(); touchPanel.Sleep(); -- cgit v1.2.3-70-g09d2 From 8a8c8aa86312840a84533318bff92fdb6c42b8de Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 27 Oct 2020 19:46:51 +0100 Subject: Handle error code when calling TwiMaster::Read(). --- src/drivers/Cst816s.cpp | 4 +++- src/drivers/Cst816s.h | 14 +++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) (limited to 'src/drivers') diff --git a/src/drivers/Cst816s.cpp b/src/drivers/Cst816s.cpp index f6816545..94db3b34 100644 --- a/src/drivers/Cst816s.cpp +++ b/src/drivers/Cst816s.cpp @@ -37,7 +37,9 @@ void Cst816S::Init() { Cst816S::TouchInfos Cst816S::GetTouchInfo() { Cst816S::TouchInfos info; - twiMaster.Read(twiAddress, 0, touchData, 63); + auto ret = twiMaster.Read(twiAddress, 0, touchData, 63); + if(ret != TwiMaster::ErrorCodes::NoError) return {}; + auto nbTouchPoints = touchData[2] & 0x0f; // uint8_t i = 0; diff --git a/src/drivers/Cst816s.h b/src/drivers/Cst816s.h index b115a688..4569e82f 100644 --- a/src/drivers/Cst816s.h +++ b/src/drivers/Cst816s.h @@ -18,13 +18,13 @@ namespace Pinetime { LongPress = 0x0C }; struct TouchInfos { - uint16_t x; - uint16_t y; - uint8_t action; - uint8_t finger; - uint8_t pressure; - uint8_t area; - Gestures gesture; + uint16_t x = 0; + uint16_t y = 0; + uint8_t action = 0; + uint8_t finger = 0; + uint8_t pressure = 0; + uint8_t area = 0; + Gestures gesture = Gestures::None; bool isTouch = false; }; -- cgit v1.2.3-70-g09d2