From 9808f6eb59185612f07b9420344c40ef7b45d330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Fri, 9 Feb 2018 19:21:56 +0100 Subject: [PATCH] Fixed soem ADC bugs and optimized things a bit --- TinyFrame/TF_Integration.c | 8 ++-- tasks/task_msg.c | 4 -- units/adc/_adc_api.c | 29 ------------ units/adc/_adc_core.c | 63 ++++++++++++++++++-------- units/adc/_adc_init.c | 93 ++++++++++++++++++++------------------ units/adc/_adc_internal.h | 8 +++- units/adc/unit_adc.c | 69 ++++++++++++++++++++++++++-- units/adc/unit_adc.h | 2 - 8 files changed, 171 insertions(+), 105 deletions(-) delete mode 100644 units/adc/_adc_api.c diff --git a/TinyFrame/TF_Integration.c b/TinyFrame/TF_Integration.c index a4eab53..6985d82 100644 --- a/TinyFrame/TF_Integration.c +++ b/TinyFrame/TF_Integration.c @@ -7,7 +7,6 @@ #include "platform.h" #include "task_main.h" -#include "utils/hexdump.h" #include "USB/usbd_cdc_if.h" #include "TinyFrame.h" @@ -20,13 +19,13 @@ void TF_WriteImpl(TinyFrame *tf, const uint8_t *buff, uint32_t len) #define CHUNK 64 // same as TF_SENDBUF_LEN, so we should always have only one run of the loop int32_t total = (int32_t) len; while (total > 0) { - int32_t mxStatus = osSemaphoreWait(semVcomTxReadyHandle, 250); + const int32_t mxStatus = osSemaphoreWait(semVcomTxReadyHandle, 100); if (mxStatus != osOK) { TF_Error("Tx stalled"); return; } - uint16_t chunksize = (uint16_t) MIN(total, CHUNK); + const uint16_t chunksize = (uint16_t) MIN(total, CHUNK); assert_param(USBD_OK == CDC_Transmit_FS((uint8_t *) buff, chunksize)); buff += chunksize; @@ -38,11 +37,10 @@ void TF_WriteImpl(TinyFrame *tf, const uint8_t *buff, uint32_t len) bool TF_ClaimTx(TinyFrame *tf) { (void) tf; - assert_param(osThreadGetId() != tskMainHandle); +// assert_param(osThreadGetId() != tskMainHandle); assert_param(!inIRQ()); assert_param(osOK == osMutexWait(mutTinyFrameTxHandle, 5000)); - return true; } diff --git a/tasks/task_msg.c b/tasks/task_msg.c index 353418e..5764788 100644 --- a/tasks/task_msg.c +++ b/tasks/task_msg.c @@ -11,7 +11,6 @@ volatile uint32_t msgQueHighWaterMark = 0; static bool que_safe_post(struct rx_sched_combined_que_item *slot) { uint32_t count = 0; - assert_param(slot != NULL); if (inIRQ()) { BaseType_t xHigherPriorityTaskWoken = pdFALSE; @@ -80,12 +79,9 @@ void TaskMsgJob(const void *argument) xQueueReceive(queMsgJobHandle, &slot, osWaitForever); if (slot.is_job) { - assert_param(slot.job.cb != NULL); slot.job.cb(&slot.job); } else { - assert_param(slot.msg.len > 0 && slot.msg.len <= MSG_QUE_SLOT_SIZE); // check the len is within bounds - #if CDC_LOOPBACK_TEST TF_WriteImpl(comm, slot.msg.data, slot.msg.len); #else diff --git a/units/adc/_adc_api.c b/units/adc/_adc_api.c deleted file mode 100644 index b91ce37..0000000 --- a/units/adc/_adc_api.c +++ /dev/null @@ -1,29 +0,0 @@ -// -// Created by MightyPork on 2018/02/03. -// - -#include "platform.h" -#include "unit_base.h" -#include "unit_adc.h" - -#define ADC_INTERNAL -#include "_adc_internal.h" - -error_t UU_ADC_AbortCapture(Unit *unit) -{ - CHECK_TYPE(unit, &UNIT_ADC); - struct priv *priv = unit->data; - - enum uadc_opmode old_opmode = priv->opmode; - - priv->auto_rearm = false; - UADC_SwitchMode(unit, ADC_OPMODE_IDLE); - - if (old_opmode == ADC_OPMODE_BLCAP || - old_opmode == ADC_OPMODE_STREAM || - old_opmode == ADC_OPMODE_TRIGD) { - UADC_ReportEndOfStream(unit); - } - - return E_SUCCESS; -} diff --git a/units/adc/_adc_core.c b/units/adc/_adc_core.c index 5990e3b..85a86ee 100644 --- a/units/adc/_adc_core.c +++ b/units/adc/_adc_core.c @@ -21,6 +21,8 @@ static void UADC_JobSendBlockChunk(Job *job) const bool close = (bool) (job->data3 & 0x80); const bool tc = (bool) (job->data3 & 0x01); +// assert_param(count <= priv->buf_itemcount); + TF_TYPE type = close ? EVT_CAPT_DONE : EVT_CAPT_MORE; TF_Msg msg = { @@ -28,7 +30,6 @@ static void UADC_JobSendBlockChunk(Job *job) .len = (TF_LEN) (1 /*seq*/ + count * sizeof(uint16_t)), .type = type, }; - TF_Respond_Multipart(comm, &msg); TF_Multipart_Payload(comm, &priv->stream_serial, 1); TF_Multipart_Payload(comm, (uint8_t *) (priv->dma_buffer + start), count * sizeof(uint16_t)); @@ -109,7 +110,7 @@ void UADC_ReportEndOfStream(Unit *unit) Job j = { .unit = unit, - .data1 = priv->stream_frame_id, + .data1 = priv->stream_frame_id, // copy the ID, it may be invalid by the time the cb gets executed .cb = UADC_JobSendEndOfStreamMsg }; scheduleJob(&j); @@ -128,14 +129,16 @@ static void handle_httc(Unit *unit, bool tc) if (ht) { end = (priv->buf_itemcount / 2); - LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum); } else { end = priv->buf_itemcount; - LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum); } if (start != end) { + if (end < start) { + trap("end < start! %d < %d, tc %d", (int)end, (int)start, (int)tc); + } + uint32_t sgcount = (end - start) / priv->nb_channels; if (m_trigd || m_fixcpt) { @@ -184,7 +187,7 @@ static void handle_httc(Unit *unit, bool tc) priv->stream_startpos = 0; } else { - priv->stream_startpos = end; + priv->stream_startpos = priv->buf_itemcount / 2; } } @@ -207,6 +210,9 @@ void UADC_DMA_Handler(void *arg) const bool ht = LL_DMA_IsActiveFlag_HT(isrsnapshot, priv->dma_chnum); const bool te = LL_DMA_IsActiveFlag_TE(isrsnapshot, priv->dma_chnum); + if (ht) LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum); + if (tc) LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum); + // check what mode we're in const bool m_trigd = priv->opmode == ADC_OPMODE_TRIGD; const bool m_stream = priv->opmode == ADC_OPMODE_STREAM; @@ -214,8 +220,8 @@ void UADC_DMA_Handler(void *arg) if (m_trigd || m_stream || m_fixcpt) { if (ht || tc) { + const uint32_t half = (uint32_t) (priv->buf_itemcount / 2); if (ht && tc) { - const uint32_t half = (uint32_t) (priv->buf_itemcount / 2); if (priv->stream_startpos > half) { handle_httc(unit, true); // TC handle_httc(unit, false); // HT @@ -224,19 +230,16 @@ void UADC_DMA_Handler(void *arg) handle_httc(unit, true); // TC } } else { + if (ht && priv->stream_startpos > half) { + // We missed the TC interrupt while e.g. setting up the stream / interrupt. catch up! + handle_httc(unit, true); // TC + } handle_httc(unit, tc); } } } else { // This shouldn't happen, the interrupt should be disabled in this opmode dbg("(!) not streaming, ADC DMA IT should be disabled"); - - if (ht) { - LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum); - } - else { - LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum); - } } if (te) { @@ -351,6 +354,23 @@ void UADC_HandleTrigger(Unit *unit, uint8_t edge_type, uint64_t timestamp) UADC_SwitchMode(unit, ADC_OPMODE_TRIGD); } +void UADC_AbortCapture(Unit *unit) +{ + struct priv *priv = unit->data; + + const enum uadc_opmode old_opmode = priv->opmode; + + priv->auto_rearm = false; + + if (old_opmode == ADC_OPMODE_BLCAP || + old_opmode == ADC_OPMODE_STREAM || + old_opmode == ADC_OPMODE_TRIGD) { + UADC_ReportEndOfStream(unit); + } + + UADC_SwitchMode(unit, ADC_OPMODE_IDLE); +} + void UADC_StartBlockCapture(Unit *unit, uint32_t len, TF_ID frame_id) { struct priv *priv = unit->data; @@ -370,8 +390,6 @@ void UADC_StartStream(Unit *unit, TF_ID frame_id) if (priv->opmode == ADC_OPMODE_UNINIT) return; priv->stream_frame_id = frame_id; - priv->stream_startpos = DMA_POS(priv); - priv->stream_serial = 0; UADC_SwitchMode(unit, ADC_OPMODE_STREAM); } @@ -394,9 +412,9 @@ void UADC_updateTick(Unit *unit) if (priv->opmode == ADC_OPMODE_EMERGENCY_SHUTDOWN) { adc_dbg("ADC recovering from emergency shutdown"); - UADC_SwitchMode(unit, ADC_OPMODE_IDLE); - LL_TIM_EnableCounter(priv->TIMx); UADC_ReportEndOfStream(unit); + LL_TIM_EnableCounter(priv->TIMx); + UADC_SwitchMode(unit, ADC_OPMODE_IDLE); unit->tick_interval = 0; return; } @@ -428,6 +446,8 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode) adc_dbg("ADC switch -> UNINIT"); // Stop the DMA, timer and disable ADC - this is called before tearing down the unit LL_TIM_DisableCounter(priv->TIMx); + LL_ADC_ClearFlag_EOS(priv->ADCx); + LL_ADC_DisableIT_EOS(priv->ADCx); // Switch off the ADC if (LL_ADC_IsEnabled(priv->ADCx)) { @@ -444,6 +464,8 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode) LL_DMA_DisableChannel(priv->DMAx, priv->dma_chnum); LL_DMA_DisableIT_HT(priv->DMAx, priv->dma_chnum); LL_DMA_DisableIT_TC(priv->DMAx, priv->dma_chnum); + LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum); + LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum); } else if (new_mode == ADC_OPMODE_IDLE || new_mode == ADC_OPMODE_REARM_PENDING) { // IDLE and ARMED are identical with the exception that the trigger condition is not checked @@ -501,7 +523,6 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode) else if (new_mode == ADC_OPMODE_TRIGD || new_mode == ADC_OPMODE_STREAM || new_mode == ADC_OPMODE_BLCAP) { - adc_dbg("ADC switch -> CAPTURE"); assert_param(old_mode == ADC_OPMODE_ARMED || old_mode == ADC_OPMODE_IDLE); @@ -514,6 +535,12 @@ void UADC_SwitchMode(Unit *unit, enum uadc_opmode new_mode) LL_DMA_ClearFlag_HT(priv->DMAx, priv->dma_chnum); LL_DMA_ClearFlag_TC(priv->DMAx, priv->dma_chnum); + // those must be as close as possible to the enabling + // if not trig'd, we don't care for lost samples before (this could cause a DMA irq miss / ht/tc mismatch with the startpos) + if (new_mode != ADC_OPMODE_TRIGD) { + priv->stream_startpos = DMA_POS(priv); + priv->stream_serial = 0; + } LL_DMA_EnableIT_HT(priv->DMAx, priv->dma_chnum); LL_DMA_EnableIT_TC(priv->DMAx, priv->dma_chnum); } diff --git a/units/adc/_adc_init.c b/units/adc/_adc_init.c index 99315fb..40930da 100644 --- a/units/adc/_adc_init.c +++ b/units/adc/_adc_init.c @@ -2,7 +2,6 @@ // Created by MightyPork on 2018/02/03. // -#include #include "platform.h" #include "unit_base.h" @@ -34,14 +33,12 @@ error_t UADC_SetSampleRate(Unit *unit, uint32_t hertz) uint16_t presc; uint32_t count; - if (!solve_timer(PLAT_APB1_HZ, hertz, true, &presc, &count, - &priv->real_frequency)) { + if (!solve_timer(PLAT_APB1_HZ, hertz, true, &presc, &count, &priv->real_frequency)) { dbg("Failed to resolve timer params."); return E_BAD_VALUE; } adc_dbg("Frequency error %d ppm, presc %d, count %d", - (int) lrintf(1000000.0f * - ((priv->real_frequency - hertz) / (float) hertz)), + (int) lrintf(1000000.0f * ((priv->real_frequency - hertz) / (float) hertz)), (int) presc, (int) count); LL_TIM_SetPrescaler(priv->TIMx, (uint32_t) (presc - 1)); @@ -52,6 +49,44 @@ error_t UADC_SetSampleRate(Unit *unit, uint32_t hertz) return E_SUCCESS; } +void UADC_SetupDMA(Unit *unit) +{ + struct priv *priv = unit->data; + + adc_dbg("Setting up DMA"); + { + uint32_t itemcount = priv->nb_channels * (priv->cfg.buffer_size / (priv->nb_channels)); + if (itemcount % 2 == 1) itemcount -= priv->nb_channels; // ensure the count is even + priv->buf_itemcount = itemcount; + + adc_dbg("DMA item count is %d (%d bytes), There are %d samples per group.", + (int)priv->buf_itemcount, + (int)(priv->buf_itemcount * sizeof(uint16_t)), + (int)priv->nb_channels); + + { + LL_DMA_InitTypeDef init; + LL_DMA_StructInit(&init); + init.Direction = LL_DMA_DIRECTION_PERIPH_TO_MEMORY; + + init.Mode = LL_DMA_MODE_CIRCULAR; + init.NbData = itemcount; + + init.PeriphOrM2MSrcAddress = (uint32_t) &priv->ADCx->DR; + init.PeriphOrM2MSrcDataSize = LL_DMA_PDATAALIGN_HALFWORD; + init.PeriphOrM2MSrcIncMode = LL_DMA_PERIPH_NOINCREMENT; + + init.MemoryOrM2MDstAddress = (uint32_t) priv->dma_buffer; + init.MemoryOrM2MDstDataSize = LL_DMA_MDATAALIGN_HALFWORD; + init.MemoryOrM2MDstIncMode = LL_DMA_MEMORY_INCREMENT; + + assert_param(SUCCESS == LL_DMA_Init(priv->DMAx, priv->dma_chnum, &init)); + } + +// LL_DMA_EnableChannel(priv->DMAx, priv->dma_chnum); + } +} + /** Finalize unit set-up */ error_t UADC_init(Unit *unit) { @@ -118,6 +153,12 @@ error_t UADC_init(Unit *unit) } } + // ---------------- Alloc the buffer ---------------------- + adc_dbg("Allocating buffer of size %d half-words", (int)priv->cfg.buffer_size); + priv->dma_buffer = calloc_ck(priv->cfg.buffer_size, sizeof(uint16_t)); + if (NULL == priv->dma_buffer) return E_OUT_OF_MEM; + assert_param(((uint32_t) priv->dma_buffer & 3) == 0); // must be aligned + // ------------------- ENABLE CLOCKS -------------------------- { // enable peripherals clock @@ -166,42 +207,7 @@ error_t UADC_init(Unit *unit) } // --------------------- CONFIGURE DMA ------------------------------- - adc_dbg("Setting up DMA"); - { - uint32_t itemcount = priv->nb_channels * (priv->cfg.buffer_size / (priv->nb_channels)); - if (itemcount % 2 == 1) itemcount -= priv->nb_channels; // ensure the count is even - priv->buf_itemcount = itemcount; - - adc_dbg("DMA item count is %d (%d bytes), There are %d samples per group.", - priv->buf_itemcount, - priv->buf_itemcount * sizeof(uint16_t), - priv->nb_channels); - - priv->dma_buffer = calloc_ck(priv->buf_itemcount, sizeof(uint16_t)); - if (NULL == priv->dma_buffer) return E_OUT_OF_MEM; - assert_param(((uint32_t) priv->dma_buffer & 3) == 0); // must be aligned - - { - LL_DMA_InitTypeDef init; - LL_DMA_StructInit(&init); - init.Direction = LL_DMA_DIRECTION_PERIPH_TO_MEMORY; - - init.Mode = LL_DMA_MODE_CIRCULAR; - init.NbData = itemcount; - - init.PeriphOrM2MSrcAddress = (uint32_t) &priv->ADCx->DR; - init.PeriphOrM2MSrcDataSize = LL_DMA_PDATAALIGN_HALFWORD; - init.PeriphOrM2MSrcIncMode = LL_DMA_PERIPH_NOINCREMENT; - - init.MemoryOrM2MDstAddress = (uint32_t) priv->dma_buffer; - init.MemoryOrM2MDstDataSize = LL_DMA_MDATAALIGN_HALFWORD; - init.MemoryOrM2MDstIncMode = LL_DMA_MEMORY_INCREMENT; - - assert_param(SUCCESS == LL_DMA_Init(priv->DMAx, priv->dma_chnum, &init)); - } - - LL_DMA_EnableChannel(priv->DMAx, priv->dma_chnum); - } + UADC_SetupDMA(unit); // prepare the avg factor float for the ISR if (priv->cfg.averaging_factor > 1000) priv->cfg.averaging_factor = 1000; // normalize @@ -238,10 +244,11 @@ void UADC_deInit(Unit *unit) irqd_detach(priv->ADCx, UADC_ADC_EOS_Handler); LL_DMA_DeInit(priv->DMAx, priv->dma_chnum); - - free_ck(priv->dma_buffer); } + // free buffer if not NULL + free_ck(priv->dma_buffer); + // Release all resources, deinit pins rsc_teardown(unit); diff --git a/units/adc/_adc_internal.h b/units/adc/_adc_internal.h index 88efcee..1207f22 100644 --- a/units/adc/_adc_internal.h +++ b/units/adc/_adc_internal.h @@ -11,7 +11,7 @@ #include "unit_base.h" -//#define adc_dbg(...) dbg(##__VA_ARGS__) +//#define adc_dbg dbg #define adc_dbg(...) do {} while(0) #define UADC_MAX_FREQ_FOR_AVERAGING 20000 @@ -115,6 +115,9 @@ error_t UADC_init(Unit *unit); /** Tear down the unit */ void UADC_deInit(Unit *unit); +/** Configure DMA (buffer count etc) */ +void UADC_SetupDMA(Unit *unit); + // ------------------------------------------------------------------------ /** DMA half/complete handler */ @@ -147,4 +150,7 @@ void UADC_StopStream(Unit *unit); /** Configure frequency */ error_t UADC_SetSampleRate(Unit *unit, uint32_t hertz); +/** Abort capture */ +void UADC_AbortCapture(Unit *unit); + #endif //GEX_F072_ADC_INTERNAL_H diff --git a/units/adc/unit_adc.c b/units/adc/unit_adc.c index e42b06d..4280c48 100644 --- a/units/adc/unit_adc.c +++ b/units/adc/unit_adc.c @@ -27,6 +27,8 @@ enum AdcCmd_ { CMD_STREAM_STOP = 27, CMD_SET_SMOOTHING_FACTOR = 28, CMD_SET_SAMPLE_RATE = 29, + CMD_ENABLE_CHANNELS = 30, + CMD_SET_SAMPLE_TIME = 31, }; /** Handle a request message */ @@ -35,8 +37,6 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P struct priv *priv = unit->data; PayloadBuilder pb = pb_start(unit_tmp512, UNIT_TMP_LEN, NULL); - // TODO toggling individual channels - would require DMA re-init and various changes in the usage of the struct - switch (command) { /** * Get enabled channels. @@ -81,6 +81,69 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P } return E_SUCCESS; + /** + * Set sample time + * pld: u8:0-7 + */ + case CMD_SET_SAMPLE_TIME: + { + uint8_t tim = pp_u8(pp); + if (tim > 7) return E_BAD_VALUE; + UADC_SwitchMode(unit, ADC_OPMODE_UNINIT); + { + LL_ADC_SetSamplingTimeCommonChannels(priv->ADCx, LL_ADC_SAMPLETIMES[tim]); + } + UADC_SwitchMode(unit, ADC_OPMODE_IDLE); + } + return E_SUCCESS; + + /** + * Enable channels. The channels must've been configured in the settings (except ch 16 and 17 which are available always) + * pld: u32: bitmap of channels + */ + case CMD_ENABLE_CHANNELS: + { + uint32_t new_channels = pp_u32(pp); + + // this tears down the peripherals sufficiently so we can re-configure them. Going back to IDLE re-inits this + UADC_SwitchMode(unit, ADC_OPMODE_UNINIT); + + uint32_t illegal_channels = new_channels & ~(priv->cfg.channels | (1<<16) | (1<<17)); // 16 and 17 may be enabled always + if (illegal_channels != 0) { + com_respond_str(MSG_ERROR, frame_id, "Some requested channels not available"); + UADC_SwitchMode(unit, ADC_OPMODE_IDLE); + return E_FAILURE; + } + + uint8_t nb_channels = 0; + // count the enabled channels + for(int i = 0; i < 32; i++) { + if (new_channels & (1<cfg.buffer_size < nb_channels * 2) { + com_respond_str(MSG_ERROR, frame_id, "Insufficient buf size"); + UADC_SwitchMode(unit, ADC_OPMODE_IDLE); + return E_BAD_CONFIG; + } + + priv->nb_channels = nb_channels; + priv->ADCx->CHSELR = new_channels; // apply it to the ADC + priv->channels_mask = new_channels; + + UADC_SetupDMA(unit); + UADC_SwitchMode(unit, ADC_OPMODE_IDLE); + } + return E_SUCCESS; + /** * Read raw values from the last measurement. * Response: interleaved (u8:channel, u16:value) for all channels @@ -246,7 +309,7 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P */ case CMD_ABORT:; adc_dbg("> Abort capture"); - TRY(UU_ADC_AbortCapture(unit)); + UADC_AbortCapture(unit); return E_SUCCESS; /** diff --git a/units/adc/unit_adc.h b/units/adc/unit_adc.h index 1ee3501..3675d67 100644 --- a/units/adc/unit_adc.h +++ b/units/adc/unit_adc.h @@ -11,6 +11,4 @@ extern const UnitDriver UNIT_ADC; -error_t UU_ADC_AbortCapture(Unit *unit); - #endif //U_TPL_H