mciantyre / teensy4-rs

Rust support for the Teensy 4
Apache License 2.0
291 stars 32 forks source link

Make Peripherals constructors public to support using RTFM #52

Closed dflemstr closed 4 years ago

dflemstr commented 4 years ago

I'm using RTFM which handles peripherals already, and hence I need to be able to inject the peripherals from the outside.

dflemstr commented 4 years ago

I'm having limited success with this due to rtfm macros generating a custom entry point, so I might close this PR unless someone else finds it useful

mciantyre commented 4 years ago

I haven't used RTFM, but from my understanding, it relies on peripheral access crates (PAC), and not HALs or BSPs. From the docs (emphasis mine):

The app attribute has a mandatory device argument that takes a path as a value. This path must point to a peripheral access crate (PAC) generated using svd2rust v0.14.x or newer.

The imxrt1062-pac that's in this repository should be one of those valid PACs. Neither the imxrt1062-hal nor the teensy4-bsp would qualify.

When we started this project, we did a very quick POC showing that the imxrt1062-pac should work in RTFM. See this commit of my RTFM fork.

Edit: make the stale RTFM POC commit relevant to today's project.

dflemstr commented 4 years ago

Right, this change is about the fact that when you bind RTFM to a PAC crate, it will manage the lifecycle of your PAC Peripherals object, so it is not sound to take() the Peripherals from your entry point any more. However you CAN use HAL in RTFM, and I wanted to make that possible by tweaking this ctor.

dflemstr commented 4 years ago

So, in principle the code at the bottom of this comment should work. I ran it with the contents of this PR plus some changes to cortex-m-rtfm:

cortex-m-rtfm = { git = "https://github.com/dflemstr/cortex-m-rtfm.git", branch = "imxrt1062" }

However, there is an issue that you cannot actually bind to any interrupts. If I compile the code below, the Teensy loader fails with this:

Teensy Loader, Command Line, Version 2.1
Warning, HEX parse error line 2
error reading intel hex file "out/myapp.hex"

If I remove the #[task(binds = PIT, resources = [led])] line, it works fine, but of course nothing is bound to the actual interrupt so nothing ends up happening.

#![no_std]
#![no_main]

use teensy4_bsp as bsp;

#[panic_handler]
fn panic_handler(info: &core::panic::PanicInfo) -> ! {
    log::error!("panic: {}", info);
    loop {}
}

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

    #[init]
    fn init(cx: init::Context) -> init::LateResources {
        let mut periphs = bsp::Peripherals::new(bsp::hal::Peripherals::new(cx.device, cx.core));

        periphs.usb.init(bsp::usb::LoggingConfig {
            max_level: log::LevelFilter::Debug,
            filters: &[],
        });

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

        let cfg = periphs.ccm.perclk.configure(
            &mut periphs.ccm.handle,
            bsp::hal::ccm::perclk::PODF::DIVIDE_3,
            bsp::hal::ccm::perclk::CLKSEL::IPG(ipg_hz),
        );

        let (timer0, timer1, _, _) = periphs.pit.clock(cfg);
        let mut timer = bsp::hal::pit::chain(timer0, timer1);
        timer.set_interrupt_enable(true);

        let led = bsp::configure_led(&mut periphs.gpr, periphs.pins.p13);

        init::LateResources { led }
    }

    #[task(binds = PIT, resources = [led])]
    fn pit(cx: pit::Context) {
        use embedded_hal::digital::v2::ToggleableOutputPin;

        // TODO: check PIT peripheral state so that our desired timer actually ticked

        cx.resources.led.toggle().unwrap();
    }
};
mciantyre commented 4 years ago

Thanks for clarifying! I'll be OK with this change once we can show either the HAL / BSP working in the RTFM framework. We should add some docs that say 'only for use in RTFM'; maybe we could put the pub behind "rtfm" feature flags?

I gave this a try on my RTFM fork. I think I tracked down the Teensy loader issue to an additional link section, .uninit, that's not handled in the imxrt1062-rt linker script. Adding

.uninit (NOLOAD) :
{
    *(.uninit .uninit.*);
    . = ALIGN(16);
} > RAM

in SECTIONS of imxrt1062-rt/link.x, after the .dma section, seems to fix the Teensy loader issue.

I can't seem to get the PIT interrupt to trigger on its own though. I posted my test on the fork here.

Just noting that the BSP is going to override SYSTICK and USB_OTG1 interrupt handles. Not sure if that will play well with what RTFM expects.

dflemstr commented 4 years ago

I made some quick changes during my lunch break today, opted to add the linker section here (despite that there is an imxrt-rs fork now, I can't use it yet) and added a disclaimer comment for now. Feel free to merge this, or leave it open till the imxrt-rs move has settled and I can re-visit this issue. For now I'll use this branch as a basis for further testing later tonight.

mitchmindtree commented 4 years ago

@dflemstr you might be interested in #64 - another approach to getting teensy4_bsp working with rtic (formerly rtfm).

dflemstr commented 4 years ago

@mitchmindtree that looks a lot cleaner, thanks!