Implementation of RTC driver by ahmedfalah01 · Pull Request #28 · jlab-sensing/tock

ahmedfalah01 · 2026-02-26T19:07:48Z

Pull Request Overview

This pull request adds RTC driver Implementation

Testing Strategy

This pull request was tested by creating the rtc_dummy.rs test file

Documentation Updated

Updated the relevant files in /docs, or no updates are required.

Formatting

Ran make prepush.

tyler-potyondy

This is great work @ahmedfalah01! A few minor changes, but overall this is mostly good to go.

(I'm not an expert on the STM32 RTC implementation, so may be worthwhile for @jmadden173 / @stevegtaylor to have a quick review on chips/stm32wle5xx/src/rtc.rs as well)

tyler-potyondy · 2026-03-02T17:33:41Z

boards/seeed_studio_lora_e5_mini/src/test/rtc_dummy.rs

@@ -0,0 +1,496 @@
+// Licensed under the Apache License, Version 2.0 or the MIT License.
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+// Copyright Tock Contributors 2025.


Suggested change

// Copyright Tock Contributors 2025.

// Copyright Tock Contributors 2026.

tyler-potyondy · 2026-03-02T17:54:37Z

boards/seeed_studio_lora_e5_mini/src/main.rs

+    //--------------------------------------------------------------------
+    // RTC
+    //--------------------------------------------------------------------
+    debug!("=== RTC Test Starting ===");


Suggested change

debug!("=== RTC Test Starting ===");

debug!() has a code size overhead and typically we want to avoid debugs!() on the common path. The exception to this is one off errors, but even this we usually want to be sparing about the usage.

(these are fine while debugging, we just want to remove these for what we're merging)

tyler-potyondy · 2026-03-02T17:56:43Z

boards/seeed_studio_lora_e5_mini/src/main.rs

+    // Wire up RTC to peripherals for interrupt handling
+    peripherals.set_rtc(rtc);
+
+    // Enable RTC interrupts in NVIC
+    cortexm4::nvic::Nvic::new(stm32wle5jc::nvic::RTC_WKUP).enable();
+    cortexm4::nvic::Nvic::new(stm32wle5jc::nvic::RTC_Alarm).enable();
+
+    debug!("RTC: Initializing...");
+    match rtc.rtc_init() {
+        Ok(()) => debug!("RTC: Initialization successful"),
+        Err(e) => debug!("RTC: Initialization failed: {:?}", e),
+    }
+
+    // Set up test client for basic datetime operations
+    let rtc_test_client = static_init!(
+        test::rtc_dummy::RtcTestClient<'static>,
+        test::rtc_dummy::RtcTestClient::new()
+    );
+    rtc_test_client.set_rtc(rtc);
+    rtc.set_client(rtc_test_client);
+
+    // Set up extended test client for alarm and wakeup timer features
+    let rtc_ext_client = static_init!(
+        test::rtc_dummy::RtcExtendedTestClient<'static>,
+        test::rtc_dummy::RtcExtendedTestClient::new()
+    );
+    rtc_ext_client.set_rtc(rtc);
+    rtc_ext_client.set_test_client(rtc_test_client); // Link clients for state coordination
+
+    // Link test client to extended client for triggering alarm test
+    rtc_test_client.set_ext_client(rtc_ext_client);
+
+    // Wire up all client callbacks
+    rtc.set_alarm_client(rtc_ext_client);
+    rtc.set_wakeup_client(rtc_ext_client);
+
+    // Run the complete RTC test sequence:
+    // 1. GET current time
+    // 2. SET new time (2025-01-15 10:00:00)
+    // 3. GET time again (verify change)
+    // 4. Alarm: fires at second 15 (triggered automatically after GET→SET→GET)
+    // 5. Wakeup timer: 5 times, every 2 seconds (triggered by alarm)
+    debug!("RTC: Starting comprehensive test sequence...");
+    test::rtc_dummy::run_complete_rtc_test(rtc, rtc_test_client, rtc_ext_client);
+
+    debug!("=== RTC Comprehensive Test Initiated ===");


We should move this to a component (I can do this since they are a bit tricky conceptually / use some weird macros).

I think we will want a standard rtc component and also an rtc test component (that we can comment out / uncomment for the test)

tyler-potyondy · 2026-03-02T17:59:20Z

chips/stm32wle5jc/src/interrupt_service.rs

+    /// Set the RTC peripheral reference for interrupt handling
+    pub fn set_rtc(&self, rtc: &'a crate::rtc::Rtc<'a>) {
+        self.stm32wle.set_rtc(rtc);
+    }


I don't think we want this set_rtc method here. I suspect this is needed since we are using a dyn to pass in the rtc when we probably should just be passing this using a generic rtc type.

tyler-potyondy · 2026-03-02T17:59:37Z

chips/stm32wle5xx/src/chip.rs

    pub subghz_radio_interrupt: crate::subghz_radio::SubGhzRadioInterrupt<'a>,
    pub pwr: crate::pwr::Pwr,
    pub uid64: crate::device_signature::Uid64,
+    rtc: kernel::utilities::cells::OptionalCell<&'a crate::rtc::Rtc<'a>>,


Suggested change

rtc: kernel::utilities::cells::OptionalCell<&'a crate::rtc::Rtc<'a>>,

pub rtc: kernel::utilities::cells::OptionalCell<&'a crate::rtc::Rtc<'a>>,

tyler-potyondy · 2026-03-02T18:00:35Z

chips/stm32wle5xx/src/chip.rs

            subghz_radio_interrupt: crate::subghz_radio::SubGhzRadioInterrupt::new(),
            pwr: crate::pwr::Pwr::new(),
            uid64: crate::device_signature::Uid64::new(),
+            rtc: kernel::utilities::cells::OptionalCell::empty(),


I don't think we want this to be an OptionalCell. Instead we should have this just be the stm32 Rtc object (or generic?).

tyler-potyondy · 2026-03-02T18:04:34Z

chips/stm32wle5xx/src/rcc.rs

+/// PWR CR1 register for backup domain access control
+const PWR_CR1: StaticRef<ReadWrite<u32, PWR_CR1_REG::Register>> =
+    unsafe { StaticRef::new(0x5800_0400 as *const _) };
+


We don't want to do this unsafe op here and create a new reference to the Pwr peripheral. What is the Pwr peripheral needed for in the rtc driver?

If we need it, we need to pass a reference to it when creating the Rcc peripheral (via the Rcc new() method).

tyler-potyondy · 2026-03-02T18:08:38Z

chips/stm32wle5xx/src/rcc.rs

+        // Enable backup domain write access (required for BDCR register)
+        // the DBP bit in PWR_CR1 must be set to enable write access 
+		// to RTC and backup registers
+        PWR_CR1.modify(PWR_CR1_REG::DBP::SET);
+
+        // Enable RTC APB clock for CPU access to RTC registers
+        self.enable_rtcapb_clock();


Based on this, I'm guessing you need to do something with PWR_CR1 before you can enable the RTC clock. I don't think the Rcc peripheral needs to have a reference to the Pwr peripheral (as mentioned in an earlier comment). Instead, you can have the Rtc take a reference to the Pwr peripheral (see https://github.com/jlab-sensing/tock/blob/ents-dev/chips/stm32wle5xx/src/subghz_radio.rs for an example of this). You then can expose a method from the Rtc peripheral that calls the functionality you have here (avoiding the unsafe earlier and having two references to the peripheral, which in and of itself likely violates Rust soundness)

tyler-potyondy · 2026-03-02T18:16:13Z

chips/stm32wle5xx/src/rtc.rs

+        // Wait for INITF flag to be set, indicating initialization mode is active
+        for _ in 0..Self::INIT_MODE_TIMEOUT {
+            if self.registers.icsr.is_set(ICSR::INITF) {
+                return Ok(());
+            }
+        }


This worries me slightly since the time this takes to run is dependent on the speed our processor needs to run. This is likely okay as is (and I don't have a better idea right now for how else to do this). Given this returns a Result, we can gracefully handle failure (I'd be much more concerned if this was a panic path).

tyler-potyondy · 2026-03-02T18:29:37Z

Also, to get CI to pass try running make prepush at the root of the directory (it looks like the clippy/formatting checks are failing).

Implementation of RTC driver

dd87b36

ahmedfalah01 requested a review from jmadden173 February 26, 2026 19:08

jmadden173 mentioned this pull request Feb 26, 2026

Implementation of RTC driver #27

Closed

2 tasks

tyler-potyondy requested changes Mar 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of RTC driver#28

Implementation of RTC driver#28
ahmedfalah01 wants to merge 1 commit intoents-devfrom
rtc_driver_impl-ents_dev

ahmedfalah01 commented Feb 26, 2026

Uh oh!

tyler-potyondy left a comment

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy Mar 2, 2026

Uh oh!

tyler-potyondy commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// Copyright Tock Contributors 2025.
	// Copyright Tock Contributors 2026.

	rtc: kernel::utilities::cells::OptionalCell<&'a crate::rtc::Rtc<'a>>,
	pub rtc: kernel::utilities::cells::OptionalCell<&'a crate::rtc::Rtc<'a>>,

Conversation

ahmedfalah01 commented Feb 26, 2026

Pull Request Overview

Testing Strategy

Documentation Updated

Formatting

Uh oh!

tyler-potyondy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tyler-potyondy commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants