Support for different atmega controllers

[peacememories]

Hi, and thanks for creating this useful crate; especially the svd generator.
I've started working on modifying this library so it can support the atmega1280 mcu. It will obviously need a different svd, but at the same time it will use the same kind of interrupt logic etc.
The question here is, would it not be better to move the atmega core logic out of this crate into an atmega crate?

[Rahix]

Hi! Sorry for not responding for so long ...

The question here is, would it not be better to move the atmega core logic out of this crate into an atmega crate?

Yes! Back when I was working on this project, I had the plan to do exactly that. I have some files for attiny85 as well and I wanted to merge all of this into one big atmel crate similar to the stm-rs crates.

Sadly some other stuff got inbetween so I did not have time to work on this project anymore. I'd love to see it going forward though, so if you are still interested, we could devise a plan how we can make this happen.

I'll just brain-dump a few things here in case you care:

SVD

I'd still want to continue using svd2rust even though there are no svd files for AVR chips. I met the guy behind tinygo a while back and he showed me that there are actually machine-readable register definitions for avr chips as well. We thought that it might make sense to write a converter that both projects could then make use of.

Crate hierarchy

I think there are so little avr chips that it makes sense to stuff them all into one crate or at max into a few (attiny, atmega, ...).

I'd then suggest writing a crate for the hal macros (like the one for port) which are then used in the device-specific hal crates to instanciate the abstractions for each device. Here I am not yet sure if we should have a crate for each device or combine those in some way as well. I've heard that some people in the stm community are not too happy with the immense amount of crates they have ...

Interrupts

As you mentioned in #2, it makes sense to follow the design used in the stm crates.

[peacememories]

Hi, thanks for your reply!
I’d love to help making this happen. I’m still new to embedded Rust etc, so I’ll probably need a lot of guidance but I’m happy to help wherever I can.

My personal goal is to have rust running fairly well on an atmega1280 by October so I can do some course work at my uni that is typically done in C in Rust, but I’d like to not create a one-off solution but grow the ecosystem while I’m at it 😊

[Rahix]

To get a rough overview (I am not that deep in the AVR scene ...): Which chips do you think we will need to support? From my side, it is atmega328p, atmega32u8, and attiny85. You mentioned atmega1280. Are there any other chips that are widely used?

[peacememories]

The atmega2560 is the basis of the Arguino mega and generally relatively widely used. also the atmega8 for very small chips (the Cheapduino for example). Other than that I can't think of any really popular ones.

I'm honestly not that deep in the AVR scene myself, and lately I'm using more cortex-m based chips (mainly for ease of use), but the course uses the atmega, and of course most arduino products use one.

[Rahix]

I guess those six chips are a good starting point, we can always expand in the future ... But as long as the numbers stay small, I'd suggest keeping everything in one crate.

Now, I just compiled the latest avr-rust and unfortunately it seems there is currently a bug that prevents me from getting anything working (avr-rust/rust-legacy-fork#128). Until that is resolved, we are stuck with theoretical work I'm afraid.

[peacememories]

I think I still have a working avr-rust from about a month ago, I’ll check when I get home

[peacememories]

I have a working rust-avr install on revision d830a3b63f9d17736b6350a464e0041666415b20 right now, which manages to build the repo

[Rahix]

Hmm ... That revision does not exist for me anymore when checking out avr-rust/rust. I guess that is because they rebased onto a newer upstream ... Interestingly, GitHub still shows that revision when specifically searching for it ...

Looking at the history, it is missing this patch anyway without which I won't be able to build for ATtiny85.

Anyway, I'll see if I can get some hackish solution working ... Until then, there is already quite a few other things that we can work on: Me and some friends started working on atdf2svd this weekend to get the peripheral access crate going. I'll push it to GitHub as soon as it is in a basic working state ...

[Rahix]

It's online: https://github.com/Rahix/atdf2svd 🎉

[peacememories]

Neat!
Where do I actually find atdfs^^ I tried finding one for an atmega1280 or some documentation and didn’t find anything.
Also, what are our next steps? Can I help with anything?

[octycs]

They're distributed here in the .atpacks: http://packs.download.atmel.com/

[Rahix]

Also, what are our next steps? Can I help with anything?

I'll try to get a basic version of the register-access crate running as soon as possible. This means basically writing a Makefile which calls atdf2svd, svd2rust, and a few other scripts ... One of those will be some kind of patch tool to fix missing/false information from the adtf files. I talked with @octycs and we decided to adopt the stm32-rs svd-patch tool.

Once this all works in a most minimal version, we can start integrating support for the different chips. I would suggest you help there if that is ok with you?

[peacememories]

Sounds great

[Rahix]

We have liftoff: https://github.com/Rahix/avr-device 🚀

[aykevl]

You can also get the .atdf files here, for example to put them in a submodule: https://github.com/avr-rust/avr-mcu

[peacememories]

So I guess I can try adding atmega1280 support? Do I have to consider anything when it comes to the svd patching, or is that universal?

[Rahix]

For each chip, there is an (optional) file in patch/<chip>.yaml which tells the patch-tool what to do. We tried factoring out common patches that multiple devices make use of into patch/common/*.yaml. You can include other patches using

_include:
  - "path/to/common.yaml

as you can see in the existing files. (Technically, this would even work recursively, but right now there is not much need for it ...)

A lot of patching will be the names of enumerated values, which is why @octycs added functionality for that to svdpatch.py (Thanks again!) You can take a look at the existing files (like this one) to get an idea of how that works ...

A little trick to get a starting point for things to patch is to run atdf2svd manually and look at the warnings:

# -v enables more messages
atdf2svd vendor/atmega1280.atdf /dev/null -v

Oh, and don't bother trying to patch everything before submitting, I'd rather start with a basic working version and then patch things as we notice them while working with it ...

[Rahix]

@aykevl: That might actually be a good idea at some point! Right now we only support <10 devices so I'm ok with storing the files in our tree as well. But thanks for the link, I wasn't aware Dylan had already pushed all of them into a crate ...

[Rahix]

@peacememories, finally got around to merging your PR. Thanks! I have some stuff ready, regarding the HAL design, though I still want to add a few more things before publishing it. Unfortunately, I'll be gone next week, so this might take a little longer ...

If you want to, you can take a look at the patching stuff that @octycs added. Maybe some of the patches in patch/common/ also apply to atmega1280?

[peacememories]

Thanks for merging!
I’ll have to look into what the common patches actually do, but I’ll try to find out ^^‘
Have a nice week :)

[Rahix]

I’ll have to look into what the common patches actually do, but I’ll try to find out ^^

Right, I should have done this before: I added some comments so you don't have to waste time ;)

[Rahix]

I did a thing: https://github.com/Rahix/avr-hal

Still very barebones, just digital IO works at this point. Next step for me is probably porting the PWM stuff from the old crate.

[peacememories]

should i just throw a barebones version of the atmega1280 in there as well?

[Rahix]

That would be great! And if you have a board based on atmega1280, add a crate for it as well! Right now, documentation is really lacking, but I hope that by looking at the code of the other crates you can figure out what you need to do.

You'll probably be able to implement the PORTx peripherals and if atmega1280 uses the same USART as atmega32u4, you can use that as well. I'm currently looking into the timers, specifically for PWM, and the "TWI" interface (which is just i2c).

[peacememories]

It seems I have destroyed my rust-avr install^^' when trying to compile core it complains about a host of stuff:

warning: Patch `rustc-std-workspace-core v1.0.0 (/Users/gabriel/.local/share/rust-avr/src/tools/rustc-std-workspace-core)` was not used in the crate graph.
Check that the patched package version and available features are compatible
with the dependency requirements. If the patch has a different version from
what is locked in the Cargo.lock file, run `cargo update` to use the new
version. This may also occur with an optional dependency that is not enabled.
   Compiling core v0.0.0 (/Users/gabriel/.local/share/rust-avr/src/libcore)
error[E0522]: definition of an unknown language item: `va_list`
   --> /Users/gabriel/.local/share/rust-avr/src/libcore/ffi.rs:118:1
    |
118 | #[lang = "va_list"]
    | ^^^^^^^^^^^^^^^^^^^ definition of unknown language item `va_list`

error[E0093]: unrecognized intrinsic function: `va_end`
   --> /Users/gabriel/.local/share/rust-avr/src/libcore/ffi.rs:211:5
    |
211 |     fn va_end(ap: &mut VaList);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ unrecognized intrinsic

error[E0093]: unrecognized intrinsic function: `va_copy`
   --> /Users/gabriel/.local/share/rust-avr/src/libcore/ffi.rs:217:5
    |
217 |     fn va_copy<'a>(src: &VaList<'a>) -> VaList<'a>;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unrecognized intrinsic

error[E0093]: unrecognized intrinsic function: `va_arg`
   --> /Users/gabriel/.local/share/rust-avr/src/libcore/ffi.rs:224:5
    |
224 |     fn va_arg<T: sealed_trait::VaArgSafe>(ap: &mut VaList) -> T;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unrecognized intrinsic

error: aborting due to 4 previous errors

Some errors occurred: E0093, E0522.
For more information about an error, try `rustc --explain E0093`.
error: Could not compile `core`.

I know that's nothing to do with the project itself, but I'm hoping you have some wisdom about how to fix this^^'

[peacememories]

I'm on rust-avr commit 09933ff8f5c2a282161165a88e37b5e16757aa91

[peacememories]

Okay, after a full rebuild this is getting even weirder:

warning: Patch `rustc-std-workspace-core v1.0.0 (/Users/gabriel/.local/share/rust-avr/src/tools/rustc-std-workspace-core)` was not used in the crate graph.
Check that the patched package version and available features are compatible
with the dependency requirements. If the patch has a different version from
what is locked in the Cargo.lock file, run `cargo update` to use the new
version. This may also occur with an optional dependency that is not enabled.
   Compiling core v0.0.0 (/Users/gabriel/.local/share/rust-avr/src/libcore)
Expected either Y or Z register
UNREACHABLE executed at /Users/gabriel/.local/share/rust-avr/src/llvm/lib/Target/AVR/MCTargetDesc/AVRMCCodeEmitter.cpp:146!

[peacememories]

Ah I see that's the same problem you encountered. How did you fix it?

[Rahix]

Right, yes ... I rolled back to a working commit, a3b0834328d73eae4f0a27afd7668b684e4a875f in this case. But you might want to wait a few more days, Dylan told me, the new revision will hit very soon. With it, we will have nightly 1.35 which should allow us to use ufmt and thus string formatting for the serial console.

[peacememories]

I'll try the rollback first. Got a few free days right now so might as well get some stuff done ;)

[peacememories]

Btw, I'm fine using this issue as general conversation channel, but it may make sense to move this somewhere else^^

[peacememories]

Needing pyyaml for avr-hal (and probably dependents) too right now is a bit awkward, especially if you haven't installed it globally (like me for example)^^
But I assume this will not be necessary anymore if we publish to cargo at some point?

[Rahix]

Needing pyyaml for avr-hal (and probably dependents) too right now is a bit awkward, especially if you haven't installed it globally (like me for example)^^
But I assume this will not be necessary anymore if we publish to cargo at some point?

Yes, that is a sideeffect of the way we currently build avr-device. I wanted to get it working in avr-hal without having to manually download avr-device, so I added a build-script that runs make. Of course, this will need pyYAML for applying the patches ...

Once the crates go up on crates.io, this issue will no longer exist, because avr-device will contain all the generated sources as well.

[peacememories]

Okay, I've gotten a bit further (leonardo-blink builds), but when trying to add an atmega1280-hal and bigavr6 crate i manage to crash my compiler^^
Maybe I'll create a PR just so the code in question is publicly visible.

[peacememories]

thread 'main' panicked at 'attempt to subtract with overflow', src/librustc_resolve/lib.rs:5054:45
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: rustc::util::common::panic_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::continue_panic_fmt
   7: rust_begin_unwind
   8: core::panicking::panic_fmt
   9: core::panicking::panic
  10: rustc_resolve::Resolver::report_conflict
  11: rustc_resolve::resolve_imports::ImportResolver::resolve_imports
  12: rustc_resolve::macros::<impl syntax::ext::base::Resolver for rustc_resolve::Resolver<'a, 'crateloader>>::resolve_imports
  13: syntax::ext::expand::MacroExpander::resolve_imports
  14: syntax::ext::expand::MacroExpander::expand_fragment
  15: syntax::ext::expand::MacroExpander::expand_crate
  16: rustc_driver::driver::phase_2_configure_and_expand_inner::{{closure}}
  17: rustc::util::common::time
  18: rustc_driver::driver::phase_2_configure_and_expand
  19: rustc_driver::driver::compile_input
  20: rustc_driver::run_compiler_with_pool
  21: <scoped_tls::ScopedKey<T>>::set
  22: rustc_driver::run_compiler
  23: syntax::with_globals
  24: __rust_maybe_catch_panic
  25: std::panicking::try
  26: rustc_driver::run
  27: rustc_driver::main
  28: std::rt::lang_start::{{closure}}
  29: std::panicking::try::do_call
  30: __rust_maybe_catch_panic
  31: std::panicking::try
  32: std::rt::lang_start_internal
  33: main

this is my stack trace ^

[Rahix]

Please push that somewhere, so I can take a look ... This should definitely not happen ...

[peacememories]

here you go: Rahix/avr-hal#1

[Rahix]

So ... I can't reproduce it ... I got some macro errors, but those were easy to fix with this patch: avr.patch

[peacememories]

Okay, apparently killing the shell and creating a new one fixed my problems. Something went very wrong there^^
Might have something to do with the fact that the shell stayed open while I recompiled rust-avr 3 times^^

[peacememories]

Btw, isn't it a bit weird to have to specify all peripheral ports when constructing Pins?

[peacememories]

The bigavr6 has all pins of the atmega1280 exposed, since it is a big development board.
Some of the pins are not exposed as buttons/leds though, so that might be nice to encode in the crate.
Also, a lot of them obviously have different functions as well, which I would also like to encode.
Can I, with the current setup, somehow make it easy to alias a pin between e.g. INT0 and PD0, while only allowing a user to grab it as either one? Is that even a good idea?

This will probably become interesting when adding drivers for the display and such. I'm not sure if it's good for the user to have to manually pipe all pins the display needs to the constructor. Okay, that's a lot of free association for one post^^ I'm just trying to wrap my head around the api choices

[peacememories]

I made some changes to the pr which make it mergeable, as long as you don't mind horribly incomplete hals^^

[Rahix]

Hmm, the points you make all have to do with basically the same underlying issue: Rust is not able to encode partial moves in function signatures. In more practical terms:

struct Parts {
    p1: P1,
    p2: P2,
    p3: P3,
    p4: P4,
    p5, P5,
}

struct Foo(P2, P3);

fn take_p2_and_p3(p: Parts) -> Foo {
  Foo(p.p2, p.p3)
}

fn main() {
  let parts = Parts { /* ... */ };

  let foo = take_p2_and_p3(parts);

  // Rust doesn't know that parts.{p1, p4, p5} have not been moved
  let p4 = parts.p4;
}

There is absolutely no way to encode the above properly. I tried writing some typesystem magic using a procedural macro once, which actually worked, but was weird to use and produced horrible error messages and even worse type signatures.

So unfortunately there isn't really a way around explicitly passing all PORTs/pins/whatever without dropping the strict tracking of ownership.

---

This means that, for now at least, you should just pass all pins individually. The aliasing not being possible boils down to the same reason, so just choose 'generic' names for pins which are used for multiple things ...

[Rahix]

The new compiler was released! avr-rust/rust-legacy-fork#144 🎉

[peacememories]

argh, i updated and now my compiler doesn't find avr-unknown-unknown anymore ^^'

[Rahix]

I added crude I2C support for ATmega32U4 ... If you want, you can try integrating that into your crates as well. There is an i2cdetect() method that you can use to quickly test the implementation a little ...

But don't look at the code too closely, it is still very messy and probably not quite correct in all cases ;)

[Rahix]

I also changed the cargo profiles to produce smaller binaries (up to 2x reduction for some cases!). I also made the debug-build usable, so you can test new code more quickly (with incremental & parallel codegen).

Be aware that the mkhex.sh script now by default looks for the debug binary. If you want to create an ihex from the release binary you have to use ./mkhex.sh --release <elfname>