proman21 / samd-dma

DMA wrapper library for SAM micro-controllers
https://proman21.github.io/samd-dma
MIT License
1 stars 3 forks source link

Alignment questions #4

Closed bradleyharden closed 3 years ago

bradleyharden commented 3 years ago

Hi,

Thanks for writing this library. It will save me a ton of time. It looks like maintaining this library isn't the highest on your priority list right now, which is totally fine. I just wanted to post something here for anyone else who might come across it.

Right now, the Storage types are composed of arrays of TransferDescriptors. The Storage type should be 16-byte aligned, but neither it nor TransferDescriptor has #[repr(align(16))]. I believe that means proper alignment is not guaranteed. Am I misunderstanding at all?

Also, the Storage types are not repr(C) either. Is that acceptable, because they are composed of exclusively repr(C) arrays? Or should they also have repr(C)?

ianrrees commented 3 years ago

I've had an email dialogue with @proman21 , he's mentioned the TransferDescriptor alignment and some other things are fixed in some changes he's got locally and will be pushing. Seems like we'll be bringing samd-dma in to https://github.com/atsamd-rs/atsamd !

proman21 commented 3 years ago

Thank you very much for your issue. The TransferDescriptor alignment solution has been sitting in my local repo for far too long, but I've finally pushed up those changes.

w.r.t the repr and alignment of the Storage types; these types simply provide a convenience for managing the descriptor storage memory, and thus don't have to be ABI compatible for them to work on an MCU. Rust arrays are contiguous and will respect type alignment, which is the part that needs to be ABI compatible.