Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Peripherals::steal behind rtic feature. Add rtic-led.rs, rtic-blink.rs and rtic-uart-log.rs examples. #64

Merged
merged 6 commits into from
Jul 4, 2020

Conversation

mitchmindtree
Copy link
Contributor

@mitchmindtree mitchmindtree commented Jun 28, 2020

This PR is based on #63 and in turn the diff includes those changes.

Closes #62

Cargo.toml Outdated Show resolved Hide resolved
mciantyre added a commit that referenced this pull request Jun 29, 2020
As proposed by Mitch in #64. As of this commit, the Teensy CLI loader
does not reject the *.hex file. See the previous commit for details.
This adds two new changes for the `rtic` feature:

1. Re-export the `Interrupt` and `NVIC_PRIO_BITS` as `rtic` requires
   these for generated code related to interrupts.
2. Remove the `SysTick` definition to remove duplicating the `rtic`
   definition.
As of imxrt-rs/imxrt-hal#67 and imxrt-rs/imxrt-hal#68 the `ral` workaround
is no longer necessary.

As of imxrt-rs/imxrt-hal#69 `Peripherals::steal` is available on the
master branch.
@mitchmindtree
Copy link
Contributor Author

mitchmindtree commented Jul 1, 2020

I'm currently having a go at implementing a rtic_blink.rs example as a follow-up to the rtic_led.rs example. rtic_blink.rs aims to demonstrate how to trigger a periodic software task using the timer queue and how to share peripherals using late resources.

Here's my WIP based on following along with the rtic book example:

//! An adaptation of the `rtic_led.rs` example that demonstrates:
//!
//! 1. how to share late resources and
//! 2. how to use the systick interrupt to cause the LED to blink.
//!
//! NOTE: This example requires the `rtic` feature to be enabled.

#![no_std]
#![no_main]

use embedded_hal::digital::v2::{OutputPin, ToggleableOutputPin};
use panic_halt as _;
use rtic::cyccnt::U32Ext;
use teensy4_bsp as bsp;

// The CYCCNT counts in clock cycles - using the clock hz should give us a ~1 second period?
const PERIOD: u32 = bsp::hal::ccm::PLL1::ARM_HZ;

#[rtic::app(device = teensy4_bsp, monotonic = rtic::cyccnt::CYCCNT, peripherals = true)]
const APP: () = {
    struct Resources {
        led: bsp::LED,
    }

    #[init(schedule = [blink])]
    fn init(mut cx: init::Context) -> init::LateResources {
        // Initialise the monotonic timer.
        cx.core.DCB.enable_trace();
        cx.core.DWT.enable_cycle_counter();

        // Schedule the first blink.
        cx.schedule.blink(cx.start + PERIOD.cycles()).unwrap();

        let mut led = bsp::configure_led(&mut cx.device.gpr, cx.device.pins.p13);
        led.set_high().unwrap();

        init::LateResources { led }
    }

    #[task(resources = [led], schedule = [blink])]
    fn blink(cx: blink::Context) {
        cx.resources.led.toggle().unwrap();
        // Schedule the following blink.
        cx.schedule.blink(cx.scheduled + PERIOD.cycles()).unwrap();
    }

    // RTIC requires that unused interrupts are declared in an extern block when
    // using software tasks; these free interrupts will be used to dispatch the
    // software tasks.
    extern "C" {
        fn LPUART8();
    }
};

Currently the teensy seems to immediately halt (a few ms) after setting the LED high within init, though I'm unsure where exactly the halt occurs. The code does not appear to reach the blink function - if I comment out the statements in that function nothing appears to change. I wonder if my PERIOD is a reasonable value? Perhaps having such a fast CYCCNT and high clock rate causes teensy 4 to immediately overflow? If this might be the case maybe it's worth attempting to implement a custom Monotonic implementation for a microsecond/millisecond counter or something along these lines.

Anyway I'm finishing up for the night, but just thought I'd share progress in case you have any advice in the meantime.

@mciantyre
Copy link
Owner

mciantyre commented Jul 2, 2020

Currently the teensy seems to immediately halt (a few ms) after setting the LED high within init, though I'm unsure where exactly the halt occurs.

What does that look like? Does it look as if the LED turns on, it turns off, and the bootloader LED turns on? If so, I've seen that in other contexts. It seems to happen when we start from reprogram, and we quickly hit WFI. In fact, the simple led.rs example typically demonstrates the issue in my setup. Current workaround is to power-cycle the Teensy, and it'll come up. Or, put a spin-loop in init() like

for _ in 0..10_000_000 {
    core::sync::atomic::spin_loop_hint();
}

just before returning the late resources. I haven't figured this out yet, but it's definitely in the startup code...

Once I workaround that, I can show that the example has a steady, always-on LED. One thing that might be worth checking is how RTIC configures SYSTICK. SYSTICK can use one of two clock sources: the core, and an 'external' source. I've never tried using the core source. If we simply change External to Core, our systick.rs example doesn't blink. This could indicate that we either need additional configuration to support the 'core' SYSTICK source, or we need 'advanced SYSTICK configuration' in RTIC.

The BSP doesn't set the ARM clock by default, so don't forget to set the ARM clock if we're expecting PERIOD to be ARM_HZ. See below for an example of what that looks like.

let (_, ipg_hz) = periphs.ccm.pll1.set_arm_clock(
bsp::hal::ccm::PLL1::ARM_HZ,
&mut periphs.ccm.handle,
&mut periphs.dcdc,
);

A slightly more advanced adaptation of the `rtic_led.rs` example that
demonstrates:

1. how to share late resources and
2. how to use the systick interrupt to cause the LED to blink.
@mitchmindtree
Copy link
Contributor Author

What does that look like? Does it look as if the LED turns on, it turns off, and the bootloader LED turns on?

Exactly this.

After applying your 10_000_000 cycle delay (or simply power-cycling the device) I was also able to observe the LED remaining high as you mentioned! I wonder if the pjrc teensy libs have a similar delay along these lines that might give some hints... Will have to do some investigating.

After this I spent a while trying to work out why blink was never getting called. I started reading through the cortex-m-rtic API reference just in case something might click and I came across this under the idle decorator docs:

... If the idle task is not defined then the runtime sets the SLEEPONEXIT bit after executing init.

I had omitted the idle function to match the the examples presented within the rtic Timer Queue chapter. I tried re-adding the idle function with a simple spin loop to ensure that the teensy didn't set SLEEPONEXIT directly after init:

    #[idle]
    fn idle(_cx: idle::Context) -> ! {
        loop {
            core::sync::atomic::spin_loop_hint();
        }
    }

And sure enough the teensy started blinking!

The rate appears very close to my guesstimate of one second per ARM_HZ (I also added your set_arm_clock snippet btw) but I have not attempted to measure it just yet.

I have added a commit with the new working rtic_blink.rs example.

On a slightly related note, this PR looks very interesting w.r.t. to making timers a little more human-friendly in a device-agnostic manner - could be well worth keeping an eye on rather than trying to solve the problem a second time downstream.

See the comment in the `init()` method. This lets us remove the
busy-loop in idle, while still blinking at 1000ms.
@mciantyre
Copy link
Owner

Awesome work proving-out the scheduler! I measured the blinking interval with my logic analyzer: 1.00000454 seconds.

And good find regarding SLEEPONEXIT. I forgot one of the defaults on this processor: SYSTICK doesn't wake the processor from wait or sleep modes. There's a CCM method in the HAL, set_mode(), that lets us override the low-power state. After we set the low-power clock mode to 'RUN', we don't need an explicit idle function anymore; the default will do.

// Set the clock mode to 'RUN'
//
// i.MX RT (106x) processors will not wake on SYSTICK. When we enter
// WFI or WFE, we'll enter WAIT mode by default. This will disable
// SYSTICK. So, if you're waiting for SYSTICK to wake you up, it won't
// happen. By setting ourselves to 'RUN' low-power mode, SYSTICK will
// still wake us up.
//
// See the CCM_CLPCR register for more information. The HAL docs also note
// this aspect of the processor.
cx.device.ccm.set_mode(bsp::hal::ccm::ClockMode::Run);

Check out 3e260b8 for more information. If we'd like this approach to represent the rtic_blink example, feel free to cherry-pick that commit.

I wonder if the pjrc teensy libs have a similar delay along these lines

The standard Teensy startup code does have a delay. Not sure if it's to guard against what we're observing, or for another reason.

@mitchmindtree
Copy link
Contributor Author

Thanks for the explanation about CCM mode! I've added your commit to the PR 👍

mitchmindtree added a commit to mitchmindtree/teensy4-rs that referenced this pull request Jul 3, 2020
@mitchmindtree
Copy link
Contributor Author

OK, next on my list is an rtic_usb.rs example. I figure this will make for a nice demo of hardware tasks and also help a lot for debugging.

To begin, I'm aiming to get something simple working like this:

#![no_std]
#![no_main]

use embedded_hal::digital::v2::ToggleableOutputPin;
use panic_halt as _;
use teensy4_bsp as bsp;

#[rtic::app(device = teensy4_bsp, peripherals = true)]
const APP: () = {
    struct Resources {
        led: bsp::LED,
    }

    #[init]
    fn init(mut cx: init::Context) -> init::LateResources {
        init_delay();

        let led = bsp::configure_led(&mut cx.device.gpr, cx.device.pins.p13);
        init::LateResources { led }
    }

    #[task(binds = USB_OTG1, resources = [led])]
    fn usb(cx: usb::Context) {
        cx.resources.led.toggle().unwrap();
    }

    #[idle]
    fn idle(_cx: idle::Context) -> ! {
        loop {
            core::sync::atomic::spin_loop_hint();
        }
    }
};

fn init_delay() {
    for _ in 0..10_000_000 {
        core::sync::atomic::spin_loop_hint();
    }
}

I'm hoping to cause the LED to toggle by triggering the USB_OTG1 interrupt somehow - perhaps by connecting via screen /dev/ttyACM0 9600 and writing something? Once I have this working, I'll try to get some user-interaction going similar to the way examples/usb.rs does.

As expected this doesn't work just yet. Not only does /dev/ttyACM0 not show up, but teensy4 does not appear to show up under lsusb either. The usb.rs example does work as expected, so I tried copying over the usb.init.

I noticed that src/usb.rs already declares the USB_OTG1 interrupt that we do. I've added a #[cfg(not(feature = "rtic"))] to the USB_OTG1 interrupt function in the src/usb.rs module for now.

I then tried adding the following to init in case it might take care of the necessary USB initialisation:

        let usb_rx = cx.device.usb.init(bsp::usb::LoggingConfig {
            filters: &[("usb", None)],
            ..Default::default()
        });

While this seems to compile successfully (with the added usb-logging feature), running the code seems to cause the program to terminate during the call to usb.init. Adding some delay to match examples/usb.rs did not appear to make much of a difference, so I wonder if the method conflicts with rtic in some manner. I'll keep digging into this later on, but in the meantime my WIP can be found here.

@mciantyre
Copy link
Owner

Fully integrating the USB stack with RTIC might be tricky. There are some brief considerations documented here, in the teensy4-usb-sys crate. Here are some things to keep in mind as we consider RTIC integration:

  • The current USB stack expects a millisecond counter. Today, that's managed by SYSTICK, and we express that in the feature dependencies. Generally, the USB stack needs some blocking delay and time state. If RTIC is also assuming the SYSTICK exception handler for the RTIC scheduler, we may need to consider a different clock for the USB stack. We could also consider modifying the USB stack directly to remove dependencies on timers. (Back when I ported the USB library to this Rust project, I didn't do any more diligence than 'get it working for simple logging.' We might be able to simplify the driver for our usage.)
  • We drive the USB stack through repeated calls to isr(). If we disable USB_OTG1 in the BSP, we need to call isr() somewhere else (and maybe rename it to something like poll()). Maybe this could happen in RTIC's idle(); or, it could happen in RTIC's implementation of the hardware interrupt handler.

If the goal is a simple debugging interface, I'll also note that we have a serial logger. If you have a USB to serial cable (like this), you can have the same logging experience with the imxrt-uart-log crate (docs).

@mitchmindtree
Copy link
Contributor Author

I'll also note that we have a serial logger.

Oh great! I have one of these lying around, I might see if I can get debugging via UART working first then. If I can I imagine it might come in handy when hacking on the USB stack.

@mitchmindtree mitchmindtree force-pushed the rtic-example branch 2 times, most recently from fffd895 to 5b76bba Compare July 4, 2020 19:14
The example is an adaption of `rtic-blink.rs` that logs via the teensy
4.0 LPUART2 interface on each blink. The example also demonstrates the
LPUART2 interrupt and enqueues the received bytes so that they may be
logged on the next blink.
@mitchmindtree mitchmindtree changed the title Add Peripherals::steal behind rtic feature. Add examples/rtic_led.rs. Add Peripherals::steal behind rtic feature. Add rtic-led.rs, rtic-blink.rs and rtic-uart-log.rs examples. Jul 4, 2020
@mitchmindtree
Copy link
Contributor Author

OK, UART logging example added with a working interrupt!

@mciantyre
Copy link
Owner

Thanks again for putting all of this together! Is there anything else you'd like to add? All LGTM; I'm happy to merge it.

@mitchmindtree
Copy link
Contributor Author

Likewise, I'm happy for this to land as is!

I'd like to try and get an rtic-usb.rs example going at some point soon, though it's probably best to leave it for a future PR as it seems a little more involved.

Thanks for your timely responses and guidance on all this!

@mciantyre mciantyre merged commit 8cf06cf into mciantyre:master Jul 4, 2020
mitchmindtree added a commit to mitchmindtree/teensy4-rs that referenced this pull request Jul 5, 2020
bors bot added a commit that referenced this pull request Jan 2, 2021
91: De-couple SysTick and USB; support USB and RTIC r=mciantyre a=mciantyre

The PR separates the USB and SysTick drivers, so we can use USB CDC logging and I/O without running SysTick. Before this PR, the USB driver depended on SysTick for timing a polling loop. With this PR, the USB driver no longer depends on SysTick, and we can use these BSP features independently.

The PR addresses the limitations noted in #66 and #64. Now that we can use USB without SysTick, **we can use the USB module with RTIC**. The PR adds two RTIC examples that use USB for logging and direct CDC I/O.

The PR includes breaking changes throughout the USB interface. These changes should support BSP power-users who want to understand USB read / write events. The changes also make USB peripheral ownership explicit in user programs, and reduce the responsibility of the BSP. See the CHANGELOG for the specific breakages and migration tips.

This PR supersedes #66.

Co-authored-by: mitchmindtree <mitchell.nordine@gmail.com>
Co-authored-by: Ian McIntyre <ianpmcintyre@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Example using teensy4_bsp with rtic?
2 participants