Rahix / avr-device

Register access crate for AVR microcontrollers
Apache License 2.0
170 stars 66 forks source link

atmega328p: Make PRR register writable #73

Closed gpgreen closed 3 years ago

gpgreen commented 3 years ago

changed atmega328p to use patch and allow writing of PRR register Fixed field names in PRR register to match datasheet

I am porting an existing C project to rust to learn how to do avr embedded programming. I needed access to the PRR register, which i did using unsafe pointer writes for now. I tried this fix to avr-device, and it seems to generate the rust code correctly, but when i try to use it in my project I get the following error:

warning: Linking globals named 'DEVICE_PERIPHERALS': symbol multiply defined!

error: failed to load bc of "avr_device-ee0b56ac06c1c352.avr_device.f0xkhm9n-cgu.7.rcgu.o":

Rahix commented 3 years ago
warning: Linking globals named 'DEVICE_PERIPHERALS': symbol multiply defined!

That means you're now including multiple versions of avr-device. You can use cargo tree to get a dependency graph and you'll probably see two versions of the crate in there. To fix this, you'll probably want to use a crate-patch instead of referencing your custom version of avr-device in [dependencies] somewhere. E.g.

[dependencies]
# ...
avr-device = "0.2.3"
# ...

[patch.crates-io]
avr-device = { path = "/path/to/your/modified/version" }

See The [patch] section for details.

gpgreen commented 3 years ago

I updated the change to have no common/cpu.yaml. I looked at some of the other registers and compared with the datasheet. Some have the numbers on the registers/fields and some don't. Very unsatisfying.

Rahix commented 3 years ago

Some have the numbers on the registers/fields and some don't. Very unsatisfying.

Yeah, it is a horrible mess and nothing is consistent anywhere... If you want to read some more discussions about how we should deal with this, here's a link: https://github.com/Rahix/avr-device/pull/55#discussion_r499269241

Honestly, I think trying to stay close to any kind of vendor scheme is close to impossible when the vendor doesn't even stay consistent with their own stuff. So let's just go with whatever is most ergonomic to use for us...

The generated docs don't have the same information as the code, ie what bit positions the field is covering.

Just FYI, this information is actually available - if you know where to look. The svd2rust API docs are unfortunately quite hard to decipher...

If you look at, for example, the write proxy for this field: https://docs.rs/avr-device/0.2.3/avr_device/atmega32u4/cpu/prr0/struct.W.html, the docs do have each bit position spelled out.

gpgreen commented 3 years ago

Ah ok. Thanks for the link to the discussion. Since i haven't tried doing a rust macro, the subtleties pass me by. :) I ran into another weird thing. What do you think about this register:

https://docs.rs/avr-device/0.2.3/avr_device/atmega328p/exint/eimsk/struct.INT_R.html

There are 2 interrupts, but the field is 2 bits wide, instead of the "logical" 2 fields for each interrupt, INT0 and INT1. Is this inconsistent svd? It still works but if you were dealing with both interrupts it would be more complicated to turn one of them on/off instead of that very nice rust field abstraction.

Rahix commented 3 years ago

There are 2 interrupts, but the field is 2 bits wide, instead of the "logical" 2 fields for each interrupt, INT0 and INT1. Is this inconsistent svd? It still works but if you were dealing with both interrupts it would be more complicated to turn one of them on/off instead of that very nice rust field abstraction.

Looks like the vendor's chip description is just wrong here... I'll gladly take a patch that splits it into two fields! :)

BTW, Microchip/Atmel don't provide the SVDs for these chips. They provide "ATDF" files which we first convert to SVD using atdf2svd.

gpgreen commented 3 years ago

I was looking at the atdf file originally, used the incorrect term. I will submit a PR for the this issue also. Thanks for all the help.