From 3222d07d3563a5711d875cebf28b7ee6c077c0bb Mon Sep 17 00:00:00 2001 From: Bartosz Szczepanski Date: Mon, 13 Jun 2016 11:13:54 +0200 Subject: [PATCH 1/5] Partial revert of "[STM32Fxxx] Fix issue #816" This is a partial revert of 07b841b and currently we are only reverting the STM32F0xx family because new fix will be presented that's why we want to keep, still the original solution only F1xx family (it will be fixed in future). Change-Id: I147065299310c9fea499bf3ced8808a44699a1a1 --- .../hal/TARGET_STM/TARGET_STM32F0/us_ticker.c | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/hal/targets/hal/TARGET_STM/TARGET_STM32F0/us_ticker.c b/hal/targets/hal/TARGET_STM/TARGET_STM32F0/us_ticker.c index 4bb4bd3519..91ee9457de 100644 --- a/hal/targets/hal/TARGET_STM/TARGET_STM32F0/us_ticker.c +++ b/hal/targets/hal/TARGET_STM/TARGET_STM32F0/us_ticker.c @@ -41,7 +41,6 @@ static int us_ticker_inited = 0; volatile uint32_t SlaveCounter = 0; volatile uint32_t oc_int_part = 0; volatile uint16_t oc_rem_part = 0; -volatile uint16_t cnt_val = 0; void set_compare(uint16_t count) { TimMasterHandle.Instance = TIM_MST; @@ -59,15 +58,24 @@ void us_ticker_init(void) { } uint32_t us_ticker_read() { - uint32_t counter; + uint32_t counter, counter2; if (!us_ticker_inited) us_ticker_init(); - - //Current value of TIM_MST->CNT is stored in cnt_val and is - //updated in interrupt context + // A situation might appear when Master overflows right after Slave is read and before the + // new (overflowed) value of Master is read. Which would make the code below consider the + // previous (incorrect) value of Slave and the new value of Master, which would return a + // value in the past. Avoid this by computing consecutive values of the timer until they + // are properly ordered. counter = (uint32_t)(SlaveCounter << 16); - counter += cnt_val; - - return counter; + counter += TIM_MST->CNT; + while (1) { + counter2 = (uint32_t)(SlaveCounter << 16); + counter2 += TIM_MST->CNT; + if (counter2 > counter) { + break; + } + counter = counter2; + } + return counter2; } void us_ticker_set_interrupt(timestamp_t timestamp) { From ff1c172696f9398cf52cfbb64931eaccf1e28620 Mon Sep 17 00:00:00 2001 From: Bartosz Szczepanski Date: Mon, 13 Jun 2016 11:15:54 +0200 Subject: [PATCH 2/5] Revert "[NUCLEO_F070RB] 16-bit timer register update" This reverts commit 82d82d0b2a962cb3437d334d88e7f68e8652e5ff. --- .../TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c index f56a479e4c..34f30d2d91 100644 --- a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c +++ b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c @@ -43,7 +43,6 @@ void set_compare(uint16_t count); extern volatile uint32_t SlaveCounter; extern volatile uint32_t oc_int_part; extern volatile uint16_t oc_rem_part; -extern volatile uint16_t cnt_val; // Used to increment the slave counter void timer_update_irq_handler(void) @@ -60,7 +59,7 @@ void timer_update_irq_handler(void) // Used for mbed timeout (channel 1) and HAL tick (channel 2) void timer_oc_irq_handler(void) { - cnt_val = TIM_MST->CNT; + uint16_t cval = TIM_MST->CNT; TimMasterHandle.Instance = TIM_MST; // Channel 1 for mbed timeout @@ -72,7 +71,7 @@ void timer_oc_irq_handler(void) } else { if (oc_int_part > 0) { set_compare(0xFFFF); - oc_rem_part = cnt_val; // To finish the counter loop the next time + oc_rem_part = cval; // To finish the counter loop the next time oc_int_part--; } else { us_ticker_irq_handler(); From 91422cb7e1d3dfeec8baadea08791bcfcaf2493f Mon Sep 17 00:00:00 2001 From: Bartosz Szczepanski Date: Mon, 13 Jun 2016 11:16:05 +0200 Subject: [PATCH 3/5] Revert "[NUCLEO_F030R8] 16-bit timer register update" This reverts commit 01ff0b9ab79629524542a2bcdc9e0aa2465547bc. --- .../TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c index ec1839ebdc..7c7314d9f3 100644 --- a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c +++ b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c @@ -43,7 +43,6 @@ void set_compare(uint16_t count); extern volatile uint32_t SlaveCounter; extern volatile uint32_t oc_int_part; extern volatile uint16_t oc_rem_part; -extern volatile uint16_t cnt_val; // Used to increment the slave counter void timer_update_irq_handler(void) @@ -60,7 +59,7 @@ void timer_update_irq_handler(void) // Used for mbed timeout (channel 1) and HAL tick (channel 2) void timer_oc_irq_handler(void) { - cnt_val = TIM_MST->CNT; + uint16_t cval = TIM_MST->CNT; TimMasterHandle.Instance = TIM_MST; // Channel 1 for mbed timeout @@ -72,7 +71,7 @@ void timer_oc_irq_handler(void) } else { if (oc_int_part > 0) { set_compare(0xFFFF); - oc_rem_part = cnt_val; // To finish the counter loop the next time + oc_rem_part = cval; // To finish the counter loop the next time oc_int_part--; } else { us_ticker_irq_handler(); From 899cd446194cb9d8e9a136724365cb8e9aa4a64e Mon Sep 17 00:00:00 2001 From: Maciej Bojczuk Date: Mon, 13 Jun 2016 11:22:07 +0200 Subject: [PATCH 4/5] [NUCLEO_F070RB] Set NVIC Timer priorities This fix is a solution for issue #816 when having two separate IRQ handlers in Timers (UPDATE Irq and OutputCompare Irq). The update priority needs to be higher to avoid undefined behaviours. Change-Id: I5ef2c27926167ed22108901cd63586692a5f8f90 --- .../TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c index 34f30d2d91..7757d67aa6 100644 --- a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c +++ b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/hal_tick.c @@ -130,8 +130,10 @@ HAL_StatusTypeDef HAL_InitTick(uint32_t TickPriority) { // Output compare channel 2 interrupt for HAL tick NVIC_SetVector(TIM_MST_UP_IRQ, (uint32_t)timer_update_irq_handler); NVIC_EnableIRQ(TIM_MST_UP_IRQ); + NVIC_SetPriority(TIM_MST_UP_IRQ, 0); NVIC_SetVector(TIM_MST_OC_IRQ, (uint32_t)timer_oc_irq_handler); NVIC_EnableIRQ(TIM_MST_OC_IRQ); + NVIC_SetPriority(TIM_MST_OC_IRQ, 1); // Enable interrupts __HAL_TIM_ENABLE_IT(&TimMasterHandle, TIM_IT_UPDATE); // For 32-bit counter From bb65961089deaf78d086c27799885d4aa3034622 Mon Sep 17 00:00:00 2001 From: Maciej Bojczuk Date: Mon, 13 Jun 2016 11:25:32 +0200 Subject: [PATCH 5/5] [NUCLEO_F030R8] Set NVIC Timer priorities This fix is a solution for issue #816 when having two separate IRQ handlers in Timers (UPDATE Irq and OutputCompare Irq). The update priority needs to be higher to avoid undefined behaviours. Change-Id: Ic143ed0f3e4e42ad5f7b95337d8c005b7ec61274 --- .../TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c index 7c7314d9f3..9547c9bc45 100644 --- a/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c +++ b/hal/targets/cmsis/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/hal_tick.c @@ -128,8 +128,10 @@ HAL_StatusTypeDef HAL_InitTick(uint32_t TickPriority) { // Output compare channel 2 interrupt for HAL tick NVIC_SetVector(TIM_MST_UP_IRQ, (uint32_t)timer_update_irq_handler); NVIC_EnableIRQ(TIM_MST_UP_IRQ); + NVIC_SetPriority(TIM_MST_UP_IRQ, 0); NVIC_SetVector(TIM_MST_OC_IRQ, (uint32_t)timer_oc_irq_handler); NVIC_EnableIRQ(TIM_MST_OC_IRQ); + NVIC_SetPriority(TIM_MST_OC_IRQ, 1); // Enable interrupts __HAL_TIM_ENABLE_IT(&TimMasterHandle, TIM_IT_UPDATE); // For 32-bit counter