initial support for avr

[djdisodo]

initial support for avr

tested

embassy_time driver for avr
example

[lulf]

Nice, thanks for the PR! 🚀

[lulf]

Just one thing, could you add this to ci.sh so that it gets built for PRs?

[Dirbaio]

The executor is polling in a hot loop. Is there an AVR equivalent of the "wait for interrupt" instruction? If so, it'd be good to use it. It helps both power consumption, and latency/jitter.

[djdisodo]

The executor is polling in a hot loop. Is there an AVR equivalent of the "wait for interrupt" instruction? If so, it'd be good to use it. It helps both power consumption, and latency/jitter.

seems like there are some sleep modes(MCUCR)
but i'm not sure how it should be implemented

how does it exit from wfi mode?
will ANY interrupt do it?

[djdisodo]

Just one thing, could you add this to ci.sh so that it gets built for PRs?

got some problem here on ci/build
error: unexpected argument '-Z' found

[janderholm]

The executor is polling in a hot loop. Is there an AVR equivalent of the "wait for interrupt" instruction? If so, it'd be good to use it. It helps both power consumption, and latency/jitter.

seems like there are some sleep modes(MCUCR) but i'm not sure how it should be implemented

how does it exit from wfi mode? will ANY interrupt do it?

As long as as you haven't changed the sleep mode select register, it should wake up on any interrupt:

https://microchipdeveloper.com/xwiki/bin/view/products/mcu-mpu/8-bit-avr/structure/sleep/

So an implementation similar to riscv should work:
https://github.com/embassy-rs/embassy/blob/main/embassy-executor/src/arch/riscv32.rs

But interrupts must be enabled before calling sleep so I don't think critical_section can be used:
https://onlinedocs.microchip.com/pr/GUID-A834D554-5741-41A3-B5E1-35ED7CD8250A-en-US-5/index.html?GUID-825A28EE-C4E4-4C03-864F-92AA9BA41231

I did a quick test based on the riscv implementation and this documentation in avrsim and it seem to work:

    pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! {
        init(self.inner.spawner());

        loop {
            unsafe {
                self.inner.poll();
                avr_device::interrupt::disable();
                if SIGNAL_WORK_THREAD_MODE.load(Ordering::SeqCst) {
                    SIGNAL_WORK_THREAD_MODE.store(false, Ordering::SeqCst);
                    avr_device::interrupt::enable();
                } else {
                    // sleep must be preceded by sei because the instruction after sei is guaranteed to run before any interrupts
                    // can happen. So keep them together.
                    avr_device::interrupt::enable();
                    avr_device::asm::sleep();
                }
            }
        }
    }

A caveat is that the SEN bit must have been set previously:

    let mut dp = arduino_hal::Peripherals::take().unwrap();
    dp.CPU.smcr.write(|w| w.se().set_bit());

This is defined separately in atmega and attiny so it's not generic for all AVR, which makes it a bit of a bummer. Also various AVR documentation recommend to set and unset the SEN bit before and after the sleep instruction. But from what I've been able to read it's just to avoid "accidentally" entering sleep by the programmer. So it seems like a waste of cycles to me. I'm not an experienced AVR developer so perhaps someone with a deeper knowledge could chime in.

Potentially bigger power savings could be made by setting a deeper sleep mode depending on what interrupts are used in the application.

[djdisodo]

@janderholm
in my implementation it wouldn't even sleep that long since it has to copy data from timer to sram quite often
is it still ok?
no drawbacks?

[janderholm]

It should be fine. Waking from idle should take 4 clock cycles before servicing the interrupt.
There are other power saving states where oscillators are stopped which is going to take quite some time to stabilize on startup. But idle basically keeps everything running except for the CPU and flash clock.

https://microchipdeveloper.com/xwiki/bin/view/products/mcu-mpu/8-bit-avr/structure/sleep/

[Dirbaio]

LGTM!

• Can you fix CI?
• Can you rebase, so no merge commits show up in the git history?

Thanks!

[djdisodo]

i decided to not touch interrupt or se bit, this makes user decide weather to use sleep or not

[djdisodo]

just going with @janderholm 's solution i think it might get too complex if i try to keep interrupt state
but still leaving sleep enable/disable up to user

[janderholm]

Yeah unfortunately sei has to be used just before sleep. For future reference there is another trick where you let __pender just clear the se bit. That way the sleep instruction is a no-op.

Then you can set the se bit just before poll and it will go to sleep after handling all waiting tasks if __pender is not executed again.

But it's not clear that is any better since you need to set both se and sm at the same time and the implementation is chip dependent I think.

[Dirbaio]

@djdisodo please make CI check the AVR build, don't remove it. If it needs nightly you can put it in ci-nightly.sh

[karioja]

@djdisodo I would really much like to try this out. However being a rust / embassy newbie I'm having hard time setting up the project. Could you please make the example (main.rs + Cargo.toml) available in some other form than as a screenshot?

[djdisodo]

@karioja sorry for being late, maybe i put the wrong hyperlink

here's what i intended for example
https://github.com/djdisodo/example-avr-embassy

i'll be updating the example soon, but here's what i've been working on now

it's not finished tho..

Cargo.zip

timer and serial driver might have some performance issue (it ended up being overly complicated)

[simmsb]

Hey, if anyone's interested I've also been playing around with embassy on avr devices, I have a project here which is some firmware for flashlights: https://github.com/simmsb/tyrfing

[djdisodo]

would be great if we had spi and i2c driver