From 0b8e6c3fa20457bce931b1d289f187e46fc68307 Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 7 May 2020 19:53:51 +0200 Subject: Add SPI NOR Flash driver, WIP. --- src/drivers/SpiNorFlash.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/drivers/SpiNorFlash.h (limited to 'src/drivers/SpiNorFlash.h') diff --git a/src/drivers/SpiNorFlash.h b/src/drivers/SpiNorFlash.h new file mode 100644 index 00000000..839a1c2a --- /dev/null +++ b/src/drivers/SpiNorFlash.h @@ -0,0 +1,28 @@ +#pragma once +#include + +namespace Pinetime { + namespace Drivers { + class Spi; + class SpiNorFlash { + public: + explicit SpiNorFlash(Spi& spi); + SpiNorFlash(const SpiNorFlash&) = delete; + SpiNorFlash& operator=(const SpiNorFlash&) = delete; + SpiNorFlash(SpiNorFlash&&) = delete; + SpiNorFlash& operator=(SpiNorFlash&&) = delete; + + void Init(); + void Uninit(); + + + void Sleep(); + void Wakeup(); + private: + Spi& spi; + + }; + } +} + + -- cgit v1.2.3 From ee05577dd62c64d0e6a2e497b75710c7a1351557 Mon Sep 17 00:00:00 2001 From: JF Date: Mon, 11 May 2020 18:50:37 +0200 Subject: Fix race conditions on SPI and integrate the SPI NOR Flash driver into DFUService (WIP) --- src/Components/Ble/DfuService.cpp | 17 ++++- src/Components/Ble/DfuService.h | 7 ++- src/Components/Ble/NimbleController.cpp | 6 +- src/Components/Ble/NimbleController.h | 8 ++- src/DisplayApp/LittleVgl.cpp | 3 + src/SystemTask/SystemTask.cpp | 14 ++--- src/drivers/Spi.cpp | 11 +++- src/drivers/Spi.h | 3 +- src/drivers/SpiMaster.cpp | 85 +++++++++++++++++++------ src/drivers/SpiMaster.h | 11 +++- src/drivers/SpiNorFlash.cpp | 108 +++++++++++++++++++++++++++++--- src/drivers/SpiNorFlash.h | 32 ++++++++++ 12 files changed, 257 insertions(+), 48 deletions(-) (limited to 'src/drivers/SpiNorFlash.h') diff --git a/src/Components/Ble/DfuService.cpp b/src/Components/Ble/DfuService.cpp index 22983a83..0c8d357c 100644 --- a/src/Components/Ble/DfuService.cpp +++ b/src/Components/Ble/DfuService.cpp @@ -15,9 +15,10 @@ int DfuServiceCallback(uint16_t conn_handle, uint16_t attr_handle, return dfuService->OnServiceData(conn_handle, attr_handle, ctxt); } -DfuService::DfuService(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController) : +DfuService::DfuService(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController, Pinetime::Drivers::SpiNorFlash& spiNorFlash) : systemTask{systemTask}, bleController{bleController}, + spiNorFlash{spiNorFlash}, characteristicDefinition{ { .uuid = (ble_uuid_t *) &packetCharacteristicUuid, @@ -104,6 +105,15 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) { applicationSize = om->om_data[8] + (om->om_data[9] << 8) + (om->om_data[10] << 16) + (om->om_data[11] << 24); NRF_LOG_INFO("[DFU] -> Start data received : SD size : %d, BT size : %d, app size : %d", softdeviceSize, bootloaderSize, applicationSize); + for(int erased = 0; erased < applicationSize; erased += 0x1000) { + spiNorFlash.SectorErase(erased); + + auto p = spiNorFlash.ProgramFailed(); + auto e = spiNorFlash.EraseFailed(); + NRF_LOG_INFO("[DFU] Erasing sector %d - %d-%d", erased, p, e); + + } + uint8_t data[] {16, 1, 1}; SendNotification(connectionHandle, data, 3); state = States::Init; @@ -128,10 +138,15 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) { case States::Data: { nbPacketReceived++; + + spiNorFlash.Write(bytesReceived, om->om_data, om->om_len); + bytesReceived += om->om_len; bleController.FirmwareUpdateCurrentBytes(bytesReceived); NRF_LOG_INFO("[DFU] -> Bytes received : %d in %d packets", bytesReceived, nbPacketReceived); + + if((nbPacketReceived % nbPacketsToNotify) == 0) { uint8_t data[5]{static_cast(Opcodes::PacketReceiptNotification), (uint8_t)(bytesReceived&0x000000FFu),(uint8_t)(bytesReceived>>8u), (uint8_t)(bytesReceived>>16u),(uint8_t)(bytesReceived>>24u) }; diff --git a/src/Components/Ble/DfuService.h b/src/Components/Ble/DfuService.h index f59ba2c9..1406d6ad 100644 --- a/src/Components/Ble/DfuService.h +++ b/src/Components/Ble/DfuService.h @@ -8,17 +8,22 @@ namespace Pinetime { namespace System { class SystemTask; } + namespace Drivers { + class SpiNorFlash; + } namespace Controllers { class Ble; class DfuService { public: - DfuService(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController); + DfuService(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController, + Pinetime::Drivers::SpiNorFlash& spiNorFlash); void Init(); int OnServiceData(uint16_t connectionHandle, uint16_t attributeHandle, ble_gatt_access_ctxt *context); private: Pinetime::System::SystemTask& systemTask; Pinetime::Controllers::Ble& bleController; + Pinetime::Drivers::SpiNorFlash& spiNorFlash; static constexpr uint16_t dfuServiceId {0x1530}; static constexpr uint16_t packetCharacteristicId {0x1532}; diff --git a/src/Components/Ble/NimbleController.cpp b/src/Components/Ble/NimbleController.cpp index 1175022e..d2ee4801 100644 --- a/src/Components/Ble/NimbleController.cpp +++ b/src/Components/Ble/NimbleController.cpp @@ -24,12 +24,14 @@ using namespace Pinetime::Controllers; NimbleController::NimbleController(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController, DateTime& dateTimeController, - Pinetime::Controllers::NotificationManager& notificationManager) : + Pinetime::Controllers::NotificationManager& notificationManager, + Pinetime::Drivers::SpiNorFlash& spiNorFlash) : systemTask{systemTask}, bleController{bleController}, dateTimeController{dateTimeController}, notificationManager{notificationManager}, - dfuService{systemTask, bleController}, + spiNorFlash{spiNorFlash}, + dfuService{systemTask, bleController, spiNorFlash}, currentTimeClient{dateTimeController}, alertNotificationClient{systemTask, notificationManager} { diff --git a/src/Components/Ble/NimbleController.h b/src/Components/Ble/NimbleController.h index c4922bae..9f74be2b 100644 --- a/src/Components/Ble/NimbleController.h +++ b/src/Components/Ble/NimbleController.h @@ -8,12 +8,17 @@ #include namespace Pinetime { + namespace Drivers { + class SpiNorFlash; + } namespace Controllers { class DateTime; class NimbleController { public: - NimbleController(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController, DateTime& dateTimeController, Pinetime::Controllers::NotificationManager& notificationManager); + NimbleController(Pinetime::System::SystemTask& systemTask, Pinetime::Controllers::Ble& bleController, + DateTime& dateTimeController, Pinetime::Controllers::NotificationManager& notificationManager, + Pinetime::Drivers::SpiNorFlash& spiNorFlash); void Init(); void StartAdvertising(); int OnGAPEvent(ble_gap_event *event); @@ -34,6 +39,7 @@ namespace Pinetime { Pinetime::Controllers::Ble& bleController; DateTime& dateTimeController; Pinetime::Controllers::NotificationManager& notificationManager; + Pinetime::Drivers::SpiNorFlash& spiNorFlash; Pinetime::Controllers::DfuService dfuService; DeviceInformationService deviceInformationService; diff --git a/src/DisplayApp/LittleVgl.cpp b/src/DisplayApp/LittleVgl.cpp index b1b894f7..7b6cda94 100644 --- a/src/DisplayApp/LittleVgl.cpp +++ b/src/DisplayApp/LittleVgl.cpp @@ -74,6 +74,9 @@ void LittleVgl::SetFullRefresh(FullRefreshDirections direction) { void LittleVgl::FlushDisplay(const lv_area_t *area, lv_color_t *color_p) { ulTaskNotifyTake(pdTRUE, 500); + // NOtification is still needed (even if there is a mutex on SPI) because of the DataCommand pin + // which cannot be set/clear during a transfert. + // TODO refactore and remove duplicated code diff --git a/src/SystemTask/SystemTask.cpp b/src/SystemTask/SystemTask.cpp index c29a932a..022cade4 100644 --- a/src/SystemTask/SystemTask.cpp +++ b/src/SystemTask/SystemTask.cpp @@ -23,7 +23,7 @@ SystemTask::SystemTask(Drivers::SpiMaster &spi, Drivers::St7789 &lcd, spi{spi}, lcd{lcd}, spiNorFlash{spiNorFlash}, touchPanel{touchPanel}, lvgl{lvgl}, batteryController{batteryController}, bleController{bleController}, dateTimeController{dateTimeController}, watchdog{}, watchdogView{watchdog}, notificationManager{notificationManager}, - nimbleController(*this, bleController,dateTimeController, notificationManager) { + nimbleController(*this, bleController,dateTimeController, notificationManager, spiNorFlash) { systemTaksMsgQueue = xQueueCreate(10, 1); } @@ -39,19 +39,17 @@ void SystemTask::Process(void *instance) { } void SystemTask::Work() { - watchdog.Setup(7); - watchdog.Start(); +// watchdog.Setup(7); +// watchdog.Start(); NRF_LOG_INFO("Last reset reason : %s", Pinetime::Drivers::Watchdog::ResetReasonToString(watchdog.ResetReason())); APP_GPIOTE_INIT(2); -/* BLE */ + spi.Init(); + spiNorFlash.Init(); nimbleController.Init(); nimbleController.StartAdvertising(); -/* /BLE*/ - - spi.Init(); lcd.Init(); - spiNorFlash.Init(); + touchPanel.Init(); batteryController.Init(); diff --git a/src/drivers/Spi.cpp b/src/drivers/Spi.cpp index 76490aed..ec3a5e94 100644 --- a/src/drivers/Spi.cpp +++ b/src/drivers/Spi.cpp @@ -5,15 +5,16 @@ using namespace Pinetime::Drivers; Spi::Spi(SpiMaster& spiMaster, uint8_t pinCsn) : spiMaster{spiMaster}, pinCsn{pinCsn} { - + nrf_gpio_cfg_output(pinCsn); + nrf_gpio_pin_set(pinCsn); } bool Spi::Write(const uint8_t *data, size_t size) { return spiMaster.Write(pinCsn, data, size); } -bool Spi::Read(uint8_t *data, size_t size) { - return spiMaster.Read(pinCsn, data, size); +bool Spi::Read(uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize) { + return spiMaster.Read(pinCsn, cmd, cmdSize, data, dataSize); } void Spi::Sleep() { @@ -26,4 +27,8 @@ bool Spi::Init() { return true; } +bool Spi::WriteCmdAndBuffer(uint8_t *cmd, size_t cmdSize, uint8_t *data, size_t dataSize) { + return spiMaster.WriteCmdAndBuffer(pinCsn, cmd, cmdSize, data, dataSize); +} + diff --git a/src/drivers/Spi.h b/src/drivers/Spi.h index 7e155582..bee39af2 100644 --- a/src/drivers/Spi.h +++ b/src/drivers/Spi.h @@ -21,7 +21,8 @@ namespace Pinetime { bool Init(); bool Write(const uint8_t* data, size_t size); - bool Read(uint8_t* data, size_t size); + bool Read(uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); + bool WriteCmdAndBuffer(uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); void Sleep(); void Wakeup(); diff --git a/src/drivers/SpiMaster.cpp b/src/drivers/SpiMaster.cpp index 7e5bb935..4d44a435 100644 --- a/src/drivers/SpiMaster.cpp +++ b/src/drivers/SpiMaster.cpp @@ -9,7 +9,8 @@ using namespace Pinetime::Drivers; SpiMaster::SpiMaster(const SpiMaster::SpiModule spi, const SpiMaster::Parameters ¶ms) : spi{spi}, params{params} { - + mutex = xSemaphoreCreateBinary(); + ASSERT(mutex != NULL); } bool SpiMaster::Init() { @@ -67,6 +68,8 @@ bool SpiMaster::Init() { NRFX_IRQ_PRIORITY_SET(SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQn,2); NRFX_IRQ_ENABLE(SPIM0_SPIS0_TWIM0_TWIS0_SPI0_TWI0_IRQn); + + xSemaphoreGive(mutex); return true; } @@ -93,13 +96,17 @@ void SpiMaster::DisableWorkaroundForFtpan58(NRF_SPIM_Type *spim, uint32_t ppi_ch NRF_PPI->CH[ppi_channel].EEP = 0; NRF_PPI->CH[ppi_channel].TEP = 0; NRF_PPI->CHENSET = ppi_channel; + spiBaseAddress->EVENTS_END = 0; spim->INTENSET = (1<<6); spim->INTENSET = (1<<1); spim->INTENSET = (1<<19); } void SpiMaster::OnEndEvent() { - if(!busy) return; + if(currentBufferAddr == 0) { + asm("nop"); + return; + } auto s = currentBufferSize; if(s > 0) { @@ -112,7 +119,7 @@ void SpiMaster::OnEndEvent() { } else { uint8_t* buffer = nullptr; size_t size = 0; - busy = false; + if(taskToNotify != nullptr) { @@ -122,11 +129,14 @@ void SpiMaster::OnEndEvent() { } nrf_gpio_pin_set(this->pinCsn); + currentBufferAddr = 0; + BaseType_t xHigherPriorityTaskWoken = pdFALSE; + xSemaphoreGiveFromISR(mutex, &xHigherPriorityTaskWoken); + portYIELD_FROM_ISR(xHigherPriorityTaskWoken); } } void SpiMaster::OnStartedEvent() { - if(!busy) return; } void SpiMaster::PrepareTx(const volatile uint32_t bufferAddress, const volatile size_t size) { @@ -139,7 +149,7 @@ void SpiMaster::PrepareTx(const volatile uint32_t bufferAddress, const volatile spiBaseAddress->EVENTS_END = 0; } -void SpiMaster::PrepareRx(const volatile uint32_t bufferAddress, const volatile size_t size) { +void SpiMaster::PrepareRx(const volatile uint32_t cmdAddress, const volatile size_t cmdSize, const volatile uint32_t bufferAddress, const volatile size_t size) { spiBaseAddress->TXD.PTR = 0; spiBaseAddress->TXD.MAXCNT = 0; spiBaseAddress->TXD.LIST = 0; @@ -152,10 +162,10 @@ void SpiMaster::PrepareRx(const volatile uint32_t bufferAddress, const volatile bool SpiMaster::Write(uint8_t pinCsn, const uint8_t *data, size_t size) { if(data == nullptr) return false; + auto ok = xSemaphoreTake(mutex, portMAX_DELAY); + ASSERT(ok == true); taskToNotify = xTaskGetCurrentTaskHandle(); - while(busy) { - asm("nop"); - } + this->pinCsn = pinCsn; @@ -169,7 +179,6 @@ bool SpiMaster::Write(uint8_t pinCsn, const uint8_t *data, size_t size) { currentBufferAddr = (uint32_t)data; currentBufferSize = size; - busy = true; auto currentSize = std::min((size_t)255, (size_t)currentBufferSize); PrepareTx(currentBufferAddr, currentSize); @@ -179,34 +188,42 @@ bool SpiMaster::Write(uint8_t pinCsn, const uint8_t *data, size_t size) { if(size == 1) { while (spiBaseAddress->EVENTS_END == 0); - busy = false; + nrf_gpio_pin_set(this->pinCsn); + currentBufferAddr = 0; + xSemaphoreGive(mutex); } return true; } -bool SpiMaster::Read(uint8_t pinCsn, uint8_t *data, size_t size) { - while(busy) { - asm("nop"); - } +bool SpiMaster::Read(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize) { + xSemaphoreTake(mutex, portMAX_DELAY); + taskToNotify = nullptr; this->pinCsn = pinCsn; - SetupWorkaroundForFtpan58(spiBaseAddress, 0,0); + DisableWorkaroundForFtpan58(spiBaseAddress, 0,0); + spiBaseAddress->INTENCLR = (1<<6); + spiBaseAddress->INTENCLR = (1<<1); + spiBaseAddress->INTENCLR = (1<<19); nrf_gpio_pin_clear(this->pinCsn); + currentBufferAddr = 0; currentBufferSize = 0; - busy = true; - PrepareRx((uint32_t)data, size); + PrepareTx((uint32_t)cmd, cmdSize); + spiBaseAddress->TASKS_START = 1; + while (spiBaseAddress->EVENTS_END == 0); + + PrepareRx((uint32_t)cmd, cmdSize, (uint32_t)data, dataSize); spiBaseAddress->TASKS_START = 1; while (spiBaseAddress->EVENTS_END == 0); nrf_gpio_pin_set(this->pinCsn); - busy = false; + xSemaphoreGive(mutex); return true; } @@ -225,5 +242,37 @@ void SpiMaster::Wakeup() { Init(); } +bool SpiMaster::WriteCmdAndBuffer(uint8_t pinCsn, uint8_t *cmd, size_t cmdSize, uint8_t *data, size_t dataSize) { + xSemaphoreTake(mutex, portMAX_DELAY); + + taskToNotify = nullptr; + + this->pinCsn = pinCsn; + DisableWorkaroundForFtpan58(spiBaseAddress, 0,0); + spiBaseAddress->INTENCLR = (1<<6); + spiBaseAddress->INTENCLR = (1<<1); + spiBaseAddress->INTENCLR = (1<<19); + + nrf_gpio_pin_clear(this->pinCsn); + + + currentBufferAddr = 0; + currentBufferSize = 0; + + PrepareTx((uint32_t)cmd, cmdSize); + spiBaseAddress->TASKS_START = 1; + while (spiBaseAddress->EVENTS_END == 0); + + PrepareTx((uint32_t)data, dataSize); + spiBaseAddress->TASKS_START = 1; + + while (spiBaseAddress->EVENTS_END == 0); + nrf_gpio_pin_set(this->pinCsn); + + xSemaphoreGive(mutex); + + return true; +} + diff --git a/src/drivers/SpiMaster.h b/src/drivers/SpiMaster.h index 24b39b97..7b35dfc8 100644 --- a/src/drivers/SpiMaster.h +++ b/src/drivers/SpiMaster.h @@ -5,6 +5,7 @@ #include #include #include +#include #include "BufferProvider.h" namespace Pinetime { @@ -32,7 +33,9 @@ namespace Pinetime { bool Init(); bool Write(uint8_t pinCsn, const uint8_t* data, size_t size); - bool Read(uint8_t pinCsn, uint8_t* data, size_t size); + bool Read(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); + + bool WriteCmdAndBuffer(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); void OnStartedEvent(); void OnEndEvent(); @@ -44,7 +47,7 @@ namespace Pinetime { void SetupWorkaroundForFtpan58(NRF_SPIM_Type *spim, uint32_t ppi_channel, uint32_t gpiote_channel); void DisableWorkaroundForFtpan58(NRF_SPIM_Type *spim, uint32_t ppi_channel, uint32_t gpiote_channel); void PrepareTx(const volatile uint32_t bufferAddress, const volatile size_t size); - void PrepareRx(const volatile uint32_t bufferAddress, const volatile size_t size); + void PrepareRx(const volatile uint32_t cmdAddress, const volatile size_t cmdSize, const volatile uint32_t bufferAddress, const volatile size_t size); NRF_SPIM_Type * spiBaseAddress; uint8_t pinCsn; @@ -52,10 +55,12 @@ namespace Pinetime { SpiMaster::SpiModule spi; SpiMaster::Parameters params; - volatile bool busy = false; +// volatile bool busy = false; volatile uint32_t currentBufferAddr = 0; volatile size_t currentBufferSize = 0; volatile TaskHandle_t taskToNotify; + + SemaphoreHandle_t mutex; }; } } diff --git a/src/drivers/SpiNorFlash.cpp b/src/drivers/SpiNorFlash.cpp index d19548ee..8fbb53a1 100644 --- a/src/drivers/SpiNorFlash.cpp +++ b/src/drivers/SpiNorFlash.cpp @@ -11,16 +11,8 @@ SpiNorFlash::SpiNorFlash(Spi& spi) : spi{spi} { } void SpiNorFlash::Init() { - uint8_t cmd = 0x9F; - spi.Write(&cmd, 1); - - uint8_t data[3]; - data[0] = 0; - data[1] = 0; - data[2] = 0; - spi.Read(data, 3); - - NRF_LOG_INFO("Manufacturer : %d, Device : %d", data[0], (data[1] + (data[2]<<8))); + auto id = ReadIdentificaion(); + NRF_LOG_INFO("[SPI FLASH] Manufacturer : %d, Memory type : %d, memory density : %d", id.manufacturer, id.type, id.density); } void SpiNorFlash::Uninit() { @@ -34,3 +26,99 @@ void SpiNorFlash::Sleep() { void SpiNorFlash::Wakeup() { } + +SpiNorFlash::Identification SpiNorFlash::ReadIdentificaion() { + auto cmd = static_cast(Commands::ReadIdentification); + Identification identification; + spi.Read(&cmd, 1, reinterpret_cast(&identification), sizeof(Identification)); + return identification; +} + +uint8_t SpiNorFlash::ReadStatusRegister() { + auto cmd = static_cast(Commands::ReadStatusRegister); + uint8_t status; + spi.Read(&cmd, sizeof(cmd), &status, sizeof(uint8_t)); + return status; +} + +bool SpiNorFlash::WriteInProgress() { + return (ReadStatusRegister() & 0x01u) == 0x01u; +} + +bool SpiNorFlash::WriteEnabled() { + return (ReadStatusRegister() & 0x02u) == 0x02u; +} + +uint8_t SpiNorFlash::ReadConfigurationRegister() { + auto cmd = static_cast(Commands::ReadConfigurationRegister); + uint8_t status; + spi.Read(&cmd, sizeof(cmd), &status, sizeof(uint8_t)); + return status; +} + +void SpiNorFlash::Read(uint32_t address, uint8_t *buffer, size_t size) { + static constexpr uint8_t cmdSize = 4; + uint8_t cmd[cmdSize] = { static_cast(Commands::Read), (uint8_t)(address >> 16U), (uint8_t)(address >> 8U), + (uint8_t)address }; + spi.Read(reinterpret_cast(&cmd), cmdSize, buffer, size); +} + +void SpiNorFlash::WriteEnable() { + auto cmd = static_cast(Commands::WriteEnable); + spi.Read(&cmd, sizeof(cmd), nullptr, 0); +} + +void SpiNorFlash::SectorErase(uint32_t sectorAddress) { + static constexpr uint8_t cmdSize = 4; + uint8_t cmd[cmdSize] = { static_cast(Commands::SectorErase), (uint8_t)(sectorAddress >> 16U), (uint8_t)(sectorAddress >> 8U), + (uint8_t)sectorAddress }; + + WriteEnable(); + while(!WriteEnabled()) vTaskDelay(1); + + spi.Read(reinterpret_cast(&cmd), cmdSize, nullptr, 0); + + while(WriteInProgress()) vTaskDelay(1); +} + +uint8_t SpiNorFlash::ReadSecurityRegister() { + auto cmd = static_cast(Commands::ReadSecurityRegister); + uint8_t status; + spi.Read(&cmd, sizeof(cmd), &status, sizeof(uint8_t)); + return status; +} + +bool SpiNorFlash::ProgramFailed() { + return (ReadSecurityRegister() & 0x20u) == 0x20u; +} + +bool SpiNorFlash::EraseFailed() { + return (ReadSecurityRegister() & 0x40u) == 0x40u; +} + +void SpiNorFlash::Write(uint32_t address, uint8_t *buffer, size_t size) { + static constexpr uint8_t cmdSize = 4; + + size_t len = size; + uint32_t addr = address; + uint8_t* b = buffer; + while(len > 0) { + uint32_t pageLimit = (addr & ~(pageSize - 1u)) + pageSize; + uint32_t toWrite = pageLimit - addr > len ? len : pageLimit - addr; + + uint8_t cmd[cmdSize] = { static_cast(Commands::PageProgram), (uint8_t)(addr >> 16U), (uint8_t)(addr >> 8U), + (uint8_t)addr }; + + WriteEnable(); + while(!WriteEnabled()) vTaskDelay(1); + + spi.WriteCmdAndBuffer(cmd, cmdSize, b, toWrite); + + while(WriteInProgress()) vTaskDelay(1); + + addr += toWrite; + b += toWrite; + len -= toWrite; + } + +} diff --git a/src/drivers/SpiNorFlash.h b/src/drivers/SpiNorFlash.h index 839a1c2a..b5f19202 100644 --- a/src/drivers/SpiNorFlash.h +++ b/src/drivers/SpiNorFlash.h @@ -12,6 +12,26 @@ namespace Pinetime { SpiNorFlash(SpiNorFlash&&) = delete; SpiNorFlash& operator=(SpiNorFlash&&) = delete; + typedef struct __attribute__((packed)) { + uint8_t manufacturer = 0; + uint8_t type = 0; + uint8_t density = 0; + } Identification; + + Identification ReadIdentificaion(); + uint8_t ReadStatusRegister(); + bool WriteInProgress(); + bool WriteEnabled(); + uint8_t ReadConfigurationRegister(); + void Read(uint32_t address, uint8_t* buffer, size_t size); + void Write(uint32_t address, uint8_t *buffer, size_t size); + void WriteEnable(); + void SectorErase(uint32_t sectorAddress); + uint8_t ReadSecurityRegister(); + bool ProgramFailed(); + bool EraseFailed(); + + void Init(); void Uninit(); @@ -19,6 +39,18 @@ namespace Pinetime { void Sleep(); void Wakeup(); private: + enum class Commands : uint8_t { + PageProgram = 0x02, + Read = 0x03, + ReadStatusRegister = 0x05, + WriteEnable = 0x06, + ReadConfigurationRegister = 0x15, + SectorErase = 0x20, + ReadSecurityRegister = 0x2B, + ReadIdentification = 0x9F, + }; + static constexpr uint16_t pageSize = 256; + Spi& spi; }; -- cgit v1.2.3 From de822cc3a2f07033e881331ac8914b26023bb003 Mon Sep 17 00:00:00 2001 From: JF Date: Mon, 1 Jun 2020 18:32:46 +0200 Subject: Encapsulate DFU Image buffering and writing into spi flash in DfuImage. Add some const in SPI driver. --- src/CMakeLists.txt | 4 +- src/Components/Ble/DfuService.cpp | 181 +++++++++++++++++++++----------------- src/Components/Ble/DfuService.h | 37 ++++++-- src/drivers/Spi.cpp | 2 +- src/drivers/Spi.h | 2 +- src/drivers/SpiMaster.cpp | 2 +- src/drivers/SpiMaster.h | 2 +- src/drivers/SpiNorFlash.cpp | 4 +- src/drivers/SpiNorFlash.h | 2 +- 9 files changed, 137 insertions(+), 99 deletions(-) (limited to 'src/drivers/SpiNorFlash.h') diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c37a05a6..4799ea1c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -355,7 +355,6 @@ list(APPEND SOURCE_FILES Components/Ble/DfuService.cpp Components/Ble/CurrentTimeService.cpp Components/Ble/AlertNotificationService.cpp - Components/Ble/DfuImage.cpp drivers/Cst816s.cpp FreeRTOS/port.c FreeRTOS/port_cmsis_systick.c @@ -409,8 +408,7 @@ set(INCLUDE_FILES Components/Ble/CurrentTimeClient.h Components/Ble/AlertNotificationClient.h Components/Ble/DfuService.h - Components/Ble/DfuImage.h - drivers/Cst816s.h + drivers/Cst816s.h FreeRTOS/portmacro.h FreeRTOS/portmacro_cmsis.h libs/date/includes/date/tz.h diff --git a/src/Components/Ble/DfuService.cpp b/src/Components/Ble/DfuService.cpp index a170fdfa..e4dcdf38 100644 --- a/src/Components/Ble/DfuService.cpp +++ b/src/Components/Ble/DfuService.cpp @@ -30,7 +30,7 @@ DfuService::DfuService(Pinetime::System::SystemTask &systemTask, Pinetime::Contr Pinetime::Drivers::SpiNorFlash &spiNorFlash) : systemTask{systemTask}, bleController{bleController}, - spiNorFlash{spiNorFlash}, + dfuImage{spiNorFlash}, characteristicDefinition{ { .uuid = (ble_uuid_t *) &packetCharacteristicUuid, @@ -124,11 +124,7 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) { NRF_LOG_INFO("[DFU] -> Start data received : SD size : %d, BT size : %d, app size : %d", softdeviceSize, bootloaderSize, applicationSize); - for (int erased = 0; erased < maxImageSize; erased += 0x1000) { -#if 1 - spiNorFlash.SectorErase(writeOffset + erased); -#endif - } + dfuImage.Erase(); uint8_t data[]{16, 1, 1}; notificationManager.Send(connectionHandle, controlPointCharacteristicHandle, data, 3); @@ -157,13 +153,7 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) { case States::Data: { nbPacketReceived++; - auto offset = ((nbPacketReceived - 1) % nbPacketsToNotify) * 20; - std::memcpy(tempBuffer + offset, om->om_data, om->om_len); - - if (nbPacketReceived > 0 && (nbPacketReceived % nbPacketsToNotify) == 0) { - spiNorFlash.Write(writeOffset + ((nbPacketReceived - nbPacketsToNotify) * 20), tempBuffer, 200); - } - + dfuImage.Append(om->om_data, om->om_len); bytesReceived += om->om_len; bleController.FirmwareUpdateCurrentBytes(bytesReceived); @@ -174,15 +164,7 @@ int DfuService::WritePacketHandler(uint16_t connectionHandle, os_mbuf *om) { NRF_LOG_INFO("[DFU] -> Send packet notification: %d bytes received", bytesReceived); notificationManager.Send(connectionHandle, controlPointCharacteristicHandle, data, 5); } - if (bytesReceived == applicationSize) { - if ((nbPacketReceived % nbPacketsToNotify) != 0) { - auto remaningPacket = nbPacketReceived % nbPacketsToNotify; - spiNorFlash.Write(writeOffset + ((nbPacketReceived - remaningPacket) * 20), tempBuffer, remaningPacket * 20); - } - - if (applicationSize < maxImageSize) - WriteMagicNumber(); - + if (dfuImage.IsComplete()) { uint8_t data[3]{static_cast(Opcodes::Response), static_cast(Opcodes::ReceiveFirmwareImage), static_cast(ErrorCodes::NoError)}; @@ -257,6 +239,8 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) { NRF_LOG_INFO("[DFU] -> Receive firmware image requested, but we are not in Start Init"); return 0; } + // TODO the chunk size is dependant of the implementation of the host application... + dfuImage.Init(20, applicationSize, expectedCrc); NRF_LOG_INFO("[DFU] -> Starting receive firmware"); state = States::Data; return 0; @@ -268,8 +252,11 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) { NRF_LOG_INFO("[DFU] -> Validate firmware image requested -- %d", connectionHandle); - if(Validate()){ + if(dfuImage.Validate()){ state = States::Validated; + bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Validated); + NRF_LOG_INFO("Image OK"); + uint8_t data[3] { static_cast(Opcodes::Response), static_cast(Opcodes::ValidateFirmware), @@ -277,6 +264,9 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) { }; notificationManager.AsyncSend(connectionHandle, controlPointCharacteristicHandle, data, 3); } else { + bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Error); + NRF_LOG_INFO("Image Error : bad CRC"); + uint8_t data[3] { static_cast(Opcodes::Response), static_cast(Opcodes::ValidateFirmware), @@ -303,64 +293,6 @@ int DfuService::ControlPointHandler(uint16_t connectionHandle, os_mbuf *om) { } } - - -uint16_t DfuService::ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc) { - uint16_t crc = (p_crc == NULL) ? 0xFFFF : *p_crc; - - for (uint32_t i = 0; i < size; i++) { - crc = (uint8_t) (crc >> 8) | (crc << 8); - crc ^= p_data[i]; - crc ^= (uint8_t) (crc & 0xFF) >> 4; - crc ^= (crc << 8) << 4; - crc ^= ((crc & 0xFF) << 4) << 1; - } - - return crc; -} - -bool DfuService::Validate() { - uint32_t chunkSize = 200; - int currentOffset = 0; - uint16_t crc = 0; - - bool first = true; - while (currentOffset < applicationSize) { - uint32_t readSize = (applicationSize - currentOffset) > chunkSize ? chunkSize : (applicationSize - currentOffset); - - spiNorFlash.Read(writeOffset + currentOffset, tempBuffer, readSize); - if (first) { - crc = ComputeCrc(tempBuffer, readSize, NULL); - first = false; - } else - crc = ComputeCrc(tempBuffer, readSize, &crc); - currentOffset += readSize; - } - - NRF_LOG_INFO("Expected CRC : %u - Processed CRC : %u", expectedCrc, crc); - bool crcOk = (crc == expectedCrc); - if (crcOk) { - bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Validated); - NRF_LOG_INFO("Image OK"); - } else { - bleController.State(Pinetime::Controllers::Ble::FirmwareUpdateStates::Error); - NRF_LOG_INFO("Image Error : bad CRC"); - } - return crcOk; -} - -void DfuService::WriteMagicNumber() { - uint32_t magic[4] = { - 0xf395c277, - 0x7fefd260, - 0x0f505235, - 0x8079b62c, - }; - - uint32_t offset = writeOffset + (maxImageSize - (4 * sizeof(uint32_t))); - spiNorFlash.Write(offset, reinterpret_cast(magic), 4 * sizeof(uint32_t)); -} - void DfuService::OnTimeout() { Reset(); } @@ -415,3 +347,90 @@ void DfuService::NotificationManager::Reset() { size = 0; xTimerStop(timer, 0); } + +void DfuService::DfuImage::Init(size_t chunkSize, size_t totalSize, uint16_t expectedCrc) { + if(chunkSize != 20) return; + this->chunkSize = chunkSize; + this->totalSize = totalSize; + this->expectedCrc = expectedCrc; + this->ready = true; +} + +void DfuService::DfuImage::Append(uint8_t *data, size_t size) { + if(!ready) return; + ASSERT(size <= 20); + + std::memcpy(tempBuffer + bufferWriteIndex, data, size); + bufferWriteIndex += size; + + if(bufferWriteIndex == bufferSize) { + spiNorFlash.Write(writeOffset + totalWriteIndex, tempBuffer, bufferWriteIndex); + totalWriteIndex += bufferWriteIndex; + bufferWriteIndex = 0; + } + + if(bufferWriteIndex > 0 && totalWriteIndex + bufferWriteIndex == totalSize) { + spiNorFlash.Write(writeOffset + totalWriteIndex, tempBuffer, bufferWriteIndex); + totalWriteIndex += bufferWriteIndex; + if (totalSize < maxSize); + WriteMagicNumber(); + } +} + +void DfuService::DfuImage::WriteMagicNumber() { + static constexpr uint32_t magic[4] = { + 0xf395c277, + 0x7fefd260, + 0x0f505235, + 0x8079b62c, + }; + + uint32_t offset = writeOffset + (maxSize - (4 * sizeof(uint32_t))); + spiNorFlash.Write(offset, reinterpret_cast(magic), 4 * sizeof(uint32_t)); +} + +void DfuService::DfuImage::Erase() { + for (int erased = 0; erased < maxSize; erased += 0x1000) { + spiNorFlash.SectorErase(writeOffset + erased); + } +} + +bool DfuService::DfuImage::Validate() { + uint32_t chunkSize = 200; + int currentOffset = 0; + uint16_t crc = 0; + + bool first = true; + while (currentOffset < totalSize) { + uint32_t readSize = (totalSize - currentOffset) > chunkSize ? chunkSize : (totalSize - currentOffset); + + spiNorFlash.Read(writeOffset + currentOffset, tempBuffer, readSize); + if (first) { + crc = ComputeCrc(tempBuffer, readSize, NULL); + first = false; + } else + crc = ComputeCrc(tempBuffer, readSize, &crc); + currentOffset += readSize; + } + + return (crc == expectedCrc); +} + +uint16_t DfuService::DfuImage::ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc) { + uint16_t crc = (p_crc == NULL) ? 0xFFFF : *p_crc; + + for (uint32_t i = 0; i < size; i++) { + crc = (uint8_t) (crc >> 8) | (crc << 8); + crc ^= p_data[i]; + crc ^= (uint8_t) (crc & 0xFF) >> 4; + crc ^= (crc << 8) << 4; + crc ^= ((crc & 0xFF) << 4) << 1; + } + + return crc; +} + +bool DfuService::DfuImage::IsComplete() { + if(!ready) return false; + return totalWriteIndex == totalSize; +} diff --git a/src/Components/Ble/DfuService.h b/src/Components/Ble/DfuService.h index b0eccdf5..d7ba460c 100644 --- a/src/Components/Ble/DfuService.h +++ b/src/Components/Ble/DfuService.h @@ -20,7 +20,6 @@ namespace Pinetime { DfuService(Pinetime::System::SystemTask &systemTask, Pinetime::Controllers::Ble &bleController, Pinetime::Drivers::SpiNorFlash &spiNorFlash); void Init(); - bool Validate(); int OnServiceData(uint16_t connectionHandle, uint16_t attributeHandle, ble_gatt_access_ctxt *context); void OnTimeout(); void Reset(); @@ -40,11 +39,38 @@ namespace Pinetime { void OnNotificationTimer(); void Reset(); }; + class DfuImage { + public: + DfuImage(Pinetime::Drivers::SpiNorFlash& spiNorFlash) : spiNorFlash{spiNorFlash} {} + void Init(size_t chunkSize, size_t totalSize, uint16_t expectedCrc); + void Erase(); + void Append(uint8_t* data, size_t size); + bool Validate(); + bool IsComplete(); + + private: + Pinetime::Drivers::SpiNorFlash& spiNorFlash; + static constexpr size_t bufferSize = 200; + bool ready = false; + size_t chunkSize = 0; + size_t totalSize = 0; + size_t maxSize = 475136; + size_t bufferWriteIndex = 0; + size_t totalWriteIndex = 0; + static constexpr size_t writeOffset = 0x40000; + uint8_t tempBuffer[bufferSize]; + uint16_t expectedCrc = 0; + + void WriteMagicNumber(); + uint16_t ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc); + + }; private: Pinetime::System::SystemTask &systemTask; Pinetime::Controllers::Ble &bleController; - Pinetime::Drivers::SpiNorFlash &spiNorFlash; + DfuImage dfuImage; + NotificationManager notificationManager; static constexpr uint16_t dfuServiceId{0x1530}; static constexpr uint16_t packetCharacteristicId{0x1532}; @@ -119,22 +145,17 @@ namespace Pinetime { uint8_t nbPacketsToNotify = 0; uint32_t nbPacketReceived = 0; uint32_t bytesReceived = 0; - uint32_t writeOffset = 0x40000; uint32_t softdeviceSize = 0; uint32_t bootloaderSize = 0; uint32_t applicationSize = 0; - static constexpr uint32_t maxImageSize = 475136; uint16_t expectedCrc = 0; int SendDfuRevision(os_mbuf *om) const; int WritePacketHandler(uint16_t connectionHandle, os_mbuf *om); int ControlPointHandler(uint16_t connectionHandle, os_mbuf *om); - uint8_t tempBuffer[200]; - uint16_t ComputeCrc(uint8_t const *p_data, uint32_t size, uint16_t const *p_crc); - void WriteMagicNumber(); + TimerHandle_t timeoutTimer; - NotificationManager notificationManager; }; } } \ No newline at end of file diff --git a/src/drivers/Spi.cpp b/src/drivers/Spi.cpp index ec3a5e94..bf08178d 100644 --- a/src/drivers/Spi.cpp +++ b/src/drivers/Spi.cpp @@ -27,7 +27,7 @@ bool Spi::Init() { return true; } -bool Spi::WriteCmdAndBuffer(uint8_t *cmd, size_t cmdSize, uint8_t *data, size_t dataSize) { +bool Spi::WriteCmdAndBuffer(const uint8_t *cmd, size_t cmdSize, const uint8_t *data, size_t dataSize) { return spiMaster.WriteCmdAndBuffer(pinCsn, cmd, cmdSize, data, dataSize); } diff --git a/src/drivers/Spi.h b/src/drivers/Spi.h index bee39af2..82ba8a65 100644 --- a/src/drivers/Spi.h +++ b/src/drivers/Spi.h @@ -22,7 +22,7 @@ namespace Pinetime { bool Init(); bool Write(const uint8_t* data, size_t size); bool Read(uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); - bool WriteCmdAndBuffer(uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); + bool WriteCmdAndBuffer(const uint8_t* cmd, size_t cmdSize, const uint8_t *data, size_t dataSize); void Sleep(); void Wakeup(); diff --git a/src/drivers/SpiMaster.cpp b/src/drivers/SpiMaster.cpp index ff241184..8087d386 100644 --- a/src/drivers/SpiMaster.cpp +++ b/src/drivers/SpiMaster.cpp @@ -239,7 +239,7 @@ void SpiMaster::Wakeup() { Init(); } -bool SpiMaster::WriteCmdAndBuffer(uint8_t pinCsn, uint8_t *cmd, size_t cmdSize, uint8_t *data, size_t dataSize) { +bool SpiMaster::WriteCmdAndBuffer(uint8_t pinCsn, const uint8_t *cmd, size_t cmdSize, const uint8_t *data, size_t dataSize) { xSemaphoreTake(mutex, portMAX_DELAY); taskToNotify = nullptr; diff --git a/src/drivers/SpiMaster.h b/src/drivers/SpiMaster.h index 88b37a35..cd3193e4 100644 --- a/src/drivers/SpiMaster.h +++ b/src/drivers/SpiMaster.h @@ -37,7 +37,7 @@ namespace Pinetime { bool Write(uint8_t pinCsn, const uint8_t* data, size_t size); bool Read(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); - bool WriteCmdAndBuffer(uint8_t pinCsn, uint8_t* cmd, size_t cmdSize, uint8_t *data, size_t dataSize); + bool WriteCmdAndBuffer(uint8_t pinCsn, const uint8_t* cmd, size_t cmdSize, const uint8_t *data, size_t dataSize); void OnStartedEvent(); void OnEndEvent(); diff --git a/src/drivers/SpiNorFlash.cpp b/src/drivers/SpiNorFlash.cpp index 8fbb53a1..7e4da1ca 100644 --- a/src/drivers/SpiNorFlash.cpp +++ b/src/drivers/SpiNorFlash.cpp @@ -96,12 +96,12 @@ bool SpiNorFlash::EraseFailed() { return (ReadSecurityRegister() & 0x40u) == 0x40u; } -void SpiNorFlash::Write(uint32_t address, uint8_t *buffer, size_t size) { +void SpiNorFlash::Write(uint32_t address, const uint8_t *buffer, size_t size) { static constexpr uint8_t cmdSize = 4; size_t len = size; uint32_t addr = address; - uint8_t* b = buffer; + const uint8_t* b = buffer; while(len > 0) { uint32_t pageLimit = (addr & ~(pageSize - 1u)) + pageSize; uint32_t toWrite = pageLimit - addr > len ? len : pageLimit - addr; diff --git a/src/drivers/SpiNorFlash.h b/src/drivers/SpiNorFlash.h index b5f19202..98267c09 100644 --- a/src/drivers/SpiNorFlash.h +++ b/src/drivers/SpiNorFlash.h @@ -24,7 +24,7 @@ namespace Pinetime { bool WriteEnabled(); uint8_t ReadConfigurationRegister(); void Read(uint32_t address, uint8_t* buffer, size_t size); - void Write(uint32_t address, uint8_t *buffer, size_t size); + void Write(uint32_t address, const uint8_t *buffer, size_t size); void WriteEnable(); void SectorErase(uint32_t sectorAddress); uint8_t ReadSecurityRegister(); -- cgit v1.2.3