From f530acf1c5ade458e5967ab3c44c2d0cad25671d Mon Sep 17 00:00:00 2001 From: Martin Kojtal <0xc0170@gmail.com> Date: Mon, 26 Jun 2017 12:41:01 +0100 Subject: [PATCH 1/2] lpc1768 and beetle: fix us ticker set interrupt for timestamp in the past If the event is in the past, set pending interrupt instead of directly using timestamp. --- targets/TARGET_ARM_SSG/TARGET_BEETLE/us_ticker.c | 2 +- targets/TARGET_NXP/TARGET_LPC176X/us_ticker.c | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/targets/TARGET_ARM_SSG/TARGET_BEETLE/us_ticker.c b/targets/TARGET_ARM_SSG/TARGET_BEETLE/us_ticker.c index 809961f98db..b4a6842380b 100644 --- a/targets/TARGET_ARM_SSG/TARGET_BEETLE/us_ticker.c +++ b/targets/TARGET_ARM_SSG/TARGET_BEETLE/us_ticker.c @@ -99,7 +99,7 @@ void us_ticker_set_interrupt(timestamp_t timestamp) { /* Check if the event was in the past */ if (delta <= 0) { /* This event was in the past */ - Timer_SetInterrupt(TIMER0, 0); + NVIC_SetPendingIRQ(Timer_GetIRQn(TIMER0)); return; } diff --git a/targets/TARGET_NXP/TARGET_LPC176X/us_ticker.c b/targets/TARGET_NXP/TARGET_LPC176X/us_ticker.c index b46d75e6bcc..cc107d3dbd3 100644 --- a/targets/TARGET_NXP/TARGET_LPC176X/us_ticker.c +++ b/targets/TARGET_NXP/TARGET_LPC176X/us_ticker.c @@ -49,10 +49,17 @@ uint32_t us_ticker_read() { } void us_ticker_set_interrupt(timestamp_t timestamp) { - // set match value - US_TICKER_TIMER->MR0 = (uint32_t)timestamp; - // enable match interrupt - US_TICKER_TIMER->MCR |= 1; + int current_time = us_ticker_read(); + int delta = (int)(timestamp - current_time); + if (delta <= 0) { + NVIC_SetPendingIRQ(US_TICKER_TIMER_IRQn); + } else { + // set match value + US_TICKER_TIMER->MR0 = (uint32_t)timestamp; + // enable match interrupt + US_TICKER_TIMER->MCR |= 1; + } + } void us_ticker_disable_interrupt(void) { From 45e6164bb2da67a7cbb7fe1ee1fb4694adf91b26 Mon Sep 17 00:00:00 2001 From: Russ Butler Date: Sat, 24 Jun 2017 18:00:43 -0500 Subject: [PATCH 2/2] Add port tests for ticker and lp ticker Add a test to make sure timer events scheduled for the past fire immediately, and add a test to ensure many events occurring at once do not cause a stack overflow. --- TESTS/mbed_hal/ticker_port/main.cpp | 151 ++++++++++++++++++++++++++++ hal/mbed_lp_ticker_api.c | 4 +- hal/mbed_us_ticker_api.c | 4 +- 3 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 TESTS/mbed_hal/ticker_port/main.cpp diff --git a/TESTS/mbed_hal/ticker_port/main.cpp b/TESTS/mbed_hal/ticker_port/main.cpp new file mode 100644 index 00000000000..c8b45fe8be3 --- /dev/null +++ b/TESTS/mbed_hal/ticker_port/main.cpp @@ -0,0 +1,151 @@ +/* mbed Microcontroller Library + * Copyright (c) 2017 ARM Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "utest/utest.h" +#include "unity/unity.h" +#include "greentea-client/test_env.h" + +#include "mbed.h" +#include "us_ticker_api.h" +#include "lp_ticker_api.h" + +using namespace utest::v1; + +static const uint32_t QUICK_EVENT_COUNT = 100000; + +static const ticker_interface_t* intf; +static volatile uint32_t irq_count; +static void (*irq_cb)(void); + +static void ticker_irq_handler(void) +{ + intf->clear_interrupt(); + irq_count++; + if (irq_cb) { + irq_cb(); + } +} + +void us_ticker_irq_handler(void) +{ + ticker_irq_handler(); +} + +#if DEVICE_LOWPOWERTIMER +void lp_ticker_irq_handler(void) +{ + ticker_irq_handler(); +} +#endif + +static void run_next_event(void) +{ + if (irq_count < QUICK_EVENT_COUNT) { + intf->set_interrupt(intf->read() - 1); + } +} + +/** + * Test that events schedule in the past still trigger an interrupt + * + * Events can be scheduled in the past if there is a delay between the + * call to timer->read() and timer->set_interrupt(). This delay can be + * caused by a task switch or interrupt, in addition to other things. + * This test ensures that the hardware gracefully handles this case. + */ +void test_events_in_past(void) +{ + static const uint32_t us_in_past[] = { + 0, + 1, + 10, + 1000000, + 0x70000000 + }; + irq_count = 0; + irq_cb = 0; + intf->init(); + + for (uint32_t i = 0; i < sizeof(us_in_past) / sizeof(us_in_past[0]); i++) { + printf("Testing interrupt %lu us in the past\r\n", us_in_past[i]); + intf->set_interrupt(intf->read() - us_in_past[i]); + wait(0.01); + TEST_ASSERT_EQUAL(i + 1, irq_count); + } + +} + +/** + * Test that the port can handle events being added non-stop + * + * In the past some ports processed new pending events recursively. + * With this type of implementation, if too many events are pending at + * the same time then the recursion will be deep enough to cause a stack + * overflow. This test simulates that to ensure there will not be a stack + * overflow. + */ +void test_events_quickly(void) +{ + irq_count = 0; + irq_cb = run_next_event; + intf->init(); + + printf("Running %lu events now\r\n", QUICK_EVENT_COUNT); + intf->set_interrupt(intf->read() - 1); + while (irq_count < QUICK_EVENT_COUNT) { + wait(0.01); + } + TEST_ASSERT_EQUAL(QUICK_EVENT_COUNT, irq_count); +} + +utest::v1::status_t us_ticker_setup(const Case *const source, const size_t index_of_case) +{ + intf = get_us_ticker_data()->interface; + irq_count = 0; + irq_cb = 0; + return greentea_case_setup_handler(source, index_of_case); +} + +#if DEVICE_LOWPOWERTIMER +utest::v1::status_t lp_ticker_setup(const Case *const source, const size_t index_of_case) +{ + intf = get_lp_ticker_data()->interface; + irq_count = 0; + irq_cb = 0; + return greentea_case_setup_handler(source, index_of_case); +} +#endif + +Case cases[] = { + Case("Test us ticker events in the past", us_ticker_setup, test_events_in_past), + Case("Test us ticker events occurring quickly", us_ticker_setup, test_events_quickly), +#if DEVICE_LOWPOWERTIMER + Case("Test lp ticker events in the past", lp_ticker_setup, test_events_in_past), + Case("Test lp ticker events occurring quickly", lp_ticker_setup, test_events_quickly), +#endif +}; + + +utest::v1::status_t greentea_test_setup(const size_t number_of_cases) { + GREENTEA_SETUP(1200, "default_auto"); + return greentea_test_setup_handler(number_of_cases); +} + +Specification specification(greentea_test_setup, cases, greentea_test_teardown_handler); + +int main() { + return Harness::run(specification); +} diff --git a/hal/mbed_lp_ticker_api.c b/hal/mbed_lp_ticker_api.c index 6a90b153f61..4e6f181d3f1 100644 --- a/hal/mbed_lp_ticker_api.c +++ b/hal/mbed_lp_ticker_api.c @@ -14,6 +14,7 @@ * limitations under the License. */ #include "hal/lp_ticker_api.h" +#include "mbed_toolchain.h" #if DEVICE_LOWPOWERTIMER @@ -37,7 +38,8 @@ const ticker_data_t* get_lp_ticker_data(void) return &lp_data; } -void lp_ticker_irq_handler(void) +// MBED_WEAK for testing only +MBED_WEAK void lp_ticker_irq_handler(void) { ticker_irq_handler(&lp_data); } diff --git a/hal/mbed_us_ticker_api.c b/hal/mbed_us_ticker_api.c index 2f2cf2cb252..1e630272543 100644 --- a/hal/mbed_us_ticker_api.c +++ b/hal/mbed_us_ticker_api.c @@ -14,6 +14,7 @@ * limitations under the License. */ #include "hal/us_ticker_api.h" +#include "mbed_toolchain.h" static ticker_event_queue_t events = { 0 }; @@ -35,7 +36,8 @@ const ticker_data_t* get_us_ticker_data(void) return &us_data; } -void us_ticker_irq_handler(void) +// MBED_WEAK for testing only +MBED_WEAK void us_ticker_irq_handler(void) { ticker_irq_handler(&us_data); }