From bec3f5b634cf03132791b7202a546863404ffb4e Mon Sep 17 00:00:00 2001 From: Peter Harper Date: Thu, 2 Jan 2025 19:59:36 +0000 Subject: [PATCH 1/2] Add a delay after rtc_set_datetime If you call rtc_get_datetime immediately after rtc_set_datetime you get junk back. According to the datasheet "Writing to the RTC will take 2 clk_rtc clock periods to arrive". So add a delay after calling rtc_set_datetime in aon_timer_set_time_calendar. Fixes #2148 --- src/rp2_common/pico_aon_timer/aon_timer.c | 6 +++ test/pico_time_test/CMakeLists.txt | 2 +- test/pico_time_test/pico_time_test.c | 50 +++++++++++++++++++---- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/rp2_common/pico_aon_timer/aon_timer.c b/src/rp2_common/pico_aon_timer/aon_timer.c index a4d594f63..e727677da 100644 --- a/src/rp2_common/pico_aon_timer/aon_timer.c +++ b/src/rp2_common/pico_aon_timer/aon_timer.c @@ -13,6 +13,8 @@ static aon_timer_alarm_handler_t aon_timer_alarm_handler; #if HAS_RP2040_RTC #include "hardware/rtc.h" #include "pico/util/datetime.h" +#include "pico/time.h" +#include "hardware/clocks.h" #elif HAS_POWMAN_TIMER #include "hardware/powman.h" @@ -56,6 +58,10 @@ bool aon_timer_set_time_calendar(const struct tm *tm) { datetime_t dt; tm_to_datetime(tm, &dt); rtc_set_datetime(&dt); + + // Writing to the RTC will take 2 clk_rtc clock periods to arrive + uint rtc_freq = clock_get_hz(clk_rtc); + busy_wait_us((1000000 / rtc_freq) * 2); return true; #elif HAS_POWMAN_TIMER struct timespec ts; diff --git a/test/pico_time_test/CMakeLists.txt b/test/pico_time_test/CMakeLists.txt index d36a3da07..8c670c68c 100644 --- a/test/pico_time_test/CMakeLists.txt +++ b/test/pico_time_test/CMakeLists.txt @@ -3,6 +3,6 @@ if (NOT PICO_TIME_NO_ALARM_SUPPORT) target_compile_definitions(pico_time_test PRIVATE PICO_TIME_DEFAULT_ALARM_POOL_MAX_TIMERS=250 ) - target_link_libraries(pico_time_test PRIVATE pico_test) + target_link_libraries(pico_time_test PRIVATE pico_test pico_aon_timer) pico_add_extra_outputs(pico_time_test) endif() \ No newline at end of file diff --git a/test/pico_time_test/pico_time_test.c b/test/pico_time_test/pico_time_test.c index 8092abbef..f0eed12aa 100644 --- a/test/pico_time_test/pico_time_test.c +++ b/test/pico_time_test/pico_time_test.c @@ -11,6 +11,7 @@ #include "hardware/clocks.h" #include "pico/stdlib.h" #include "pico/test.h" +#include "pico/aon_timer.h" // Include sys/types.h before inttypes.h to work around issue with // certain versions of GCC and newlib which causes omission of PRIi64 #include @@ -72,10 +73,11 @@ static bool repeating_timer_callback(struct repeating_timer *t) { #define RESOLUTION_ALLOWANCE PICO_HARDWARE_TIMER_RESOLUTION_US #endif -int issue_195_test(void); -int issue_1812_test(void); -int issue_1953_test(void); -int issue_2118_test(void); +static int issue_195_test(void); +static int issue_1812_test(void); +static int issue_1953_test(void); +static int issue_2118_test(void); +static int issue_2148_test(void); int main() { setup_default_uart(); @@ -250,6 +252,8 @@ int main() { issue_2118_test(); + issue_2148_test(); + PICOTEST_END_TEST(); } @@ -260,7 +264,7 @@ int64_t issue_195_callback(alarm_id_t id, void *user_data) { return -ISSUE_195_TIMER_DELAY; } -int issue_195_test(void) { +static int issue_195_test(void) { PICOTEST_START_SECTION("Issue #195 race condition - without fix may hang on gcc 10.2.1 release builds"); absolute_time_t t1 = get_absolute_time(); int id = add_alarm_in_us(ISSUE_195_TIMER_DELAY, issue_195_callback, NULL, true); @@ -279,7 +283,7 @@ int issue_195_test(void) { } // Setting an alarm should not swallow a sev -int issue_1812_test(void) { +static int issue_1812_test(void) { PICOTEST_START_SECTION("Issue #1812 defect - Setting an alarm should not ignore a sev"); __sev(); // Make sure the call below does not ignore this @@ -303,7 +307,7 @@ static void alarm_pool_stuck_issue_1953(uint alarm) { hard_assert(false); } -int issue_1953_test(void) { +static int issue_1953_test(void) { PICOTEST_START_SECTION("Issue #1953 defect - Alarm can be set in the past"); int alarm = hardware_alarm_claim_unused(true); hardware_alarm_set_callback(alarm, alarm_pool_stuck_issue_1953); @@ -336,7 +340,7 @@ static bool timer_callback_issue_2118(repeating_timer_t *rt) { return true; } -int issue_2118_test(void) { +static int issue_2118_test(void) { PICOTEST_START_SECTION("Issue #2118 defect - failure to set an alarm"); // this problem only happens when running the clock fast as it requires the time between @@ -364,3 +368,33 @@ int issue_2118_test(void) { PICOTEST_END_SECTION(); return 0; } + +static int issue_2148_test(void) { +#if HAS_RP2040_RTC + PICOTEST_START_SECTION("Issue #2148 defect - get time after rtc start"); + struct tm tm = { 0 }; + struct tm tm_check = { 0 }; + + tm.tm_sec = 55; + tm.tm_min = 36; + tm.tm_hour = 20; + tm.tm_mday = 21; + tm.tm_mon = 10; + tm.tm_year = 124; + tm.tm_wday = 4; + tm.tm_yday = 325; + tm.tm_isdst = 0; + hard_assert(aon_timer_start_calendar(&tm)); + hard_assert(aon_timer_get_time_calendar(&tm_check)); + + PICOTEST_CHECK(tm.tm_sec == tm_check.tm_sec || tm.tm_sec == tm_check.tm_sec - 1, "failed to get seconds"); + PICOTEST_CHECK(tm.tm_min == tm_check.tm_min, "failed to get minutes"); + PICOTEST_CHECK(tm.tm_hour == tm_check.tm_hour, "failed to get hour"); + PICOTEST_CHECK(tm.tm_mday == tm_check.tm_mday, "failed to get day"); + PICOTEST_CHECK(tm.tm_mon == tm_check.tm_mon, "failed to get month"); + PICOTEST_CHECK(tm.tm_year == tm_check.tm_year, "failed to get year"); + + aon_timer_stop(); + PICOTEST_END_SECTION(); +#endif +} From 936ed202b19d0ca2843da586603cabefc2fc4179 Mon Sep 17 00:00:00 2001 From: Peter Harper Date: Mon, 6 Jan 2025 19:31:11 +0000 Subject: [PATCH 2/2] Fix bazel build issues --- src/rp2_common/pico_aon_timer/BUILD.bazel | 1 + test/pico_time_test/BUILD.bazel | 1 + test/pico_time_test/pico_time_test.c | 1 + 3 files changed, 3 insertions(+) diff --git a/src/rp2_common/pico_aon_timer/BUILD.bazel b/src/rp2_common/pico_aon_timer/BUILD.bazel index 6ceaf2e52..5bed7c852 100644 --- a/src/rp2_common/pico_aon_timer/BUILD.bazel +++ b/src/rp2_common/pico_aon_timer/BUILD.bazel @@ -10,6 +10,7 @@ cc_library( target_compatible_with = compatible_with_rp2(), deps = [ "//src/common/pico_util", + "//src/common/pico_time", "//src/rp2_common:hardware_regs", "//src/rp2_common:pico_platform", "//src/rp2_common/hardware_irq", diff --git a/test/pico_time_test/BUILD.bazel b/test/pico_time_test/BUILD.bazel index b7a437629..038738601 100644 --- a/test/pico_time_test/BUILD.bazel +++ b/test/pico_time_test/BUILD.bazel @@ -12,6 +12,7 @@ cc_binary( target_compatible_with = compatible_with_rp2(), deps = [ "//src/rp2_common/pico_stdlib", + "//src/rp2_common/pico_aon_timer", "//test/pico_test", ], ) diff --git a/test/pico_time_test/pico_time_test.c b/test/pico_time_test/pico_time_test.c index f0eed12aa..a9f5d7f08 100644 --- a/test/pico_time_test/pico_time_test.c +++ b/test/pico_time_test/pico_time_test.c @@ -397,4 +397,5 @@ static int issue_2148_test(void) { aon_timer_stop(); PICOTEST_END_SECTION(); #endif + return 0; }