Stupremee / rumio

Control your MMIO and CPU registers without pain using Rust.
Other
4 stars 0 forks source link

Looks great #1

Open kellda opened 3 years ago

kellda commented 3 years ago

I mainly wanted to share that this crate looks great. Things I really like are:

Here are also some ideas to make this even better:

Stupremee commented 3 years ago

First of all, thanks for the feedback! Tbh, I didn't expect anyone else to use this except me :sweat_smile: I will try to polish this crate and make it even better at some time.

  • Implement BitOrAssign for Value
  • Implement BitOr for Into instead of each specific field

Yea that sounds like a good idea.

Add a second type parameter to Value for the register it should be used with

That's on my TODO list since the crate existed, but I've never got around to implement it because it was much easier without it.

Be able to read a Value from a register

Oh wow, how did I not think of this. I will definitely add that at some point!

Maybe also have an "aliased" access

Can you show me an example how someone would use the Aliased register? I don't get it yet

I'm not sure that register should be Copy, maybe not even Clone

I thought about that a lot when creating this crate. My reasoning was that every "register" is actually just an address to the underlying memory. I'm fine with removing Copy, but I think at least Clone should stay. I need to think about this a bit more.

kellda commented 3 years ago

Implement BitOr for Into instead of each specific field

I since realized it is likely not possible (because it could create conflicting implementations).

Can you show me an example how someone would use the Aliased register?

I don't have a concrete example. It could be used for registers that have different meanings on read and on write, e.g. both status and config. It may also be used for registers that makes not much sense to be read and written back with some bits changed, like I2C or UART buffer registers. I saw it in tock-registers but it may not be that useful.

About Copy and Clone:

After thinking about it, I think either both or none should be implemented. Implementing only Clone just feel inconvenient.

I think as long as there is only one thread/processor and access from interrupts is properly synchronized, Copy is not a problem for safety or soundness. You may want to implement !Send and !Sync (or have a field of type PhantomData<*const ()>) to guarantee this.

Whether to implement Clone is in my opinion a more general design choice about guarantees vs ease of use. On one hand being able to pass register around without limits makes them far more easy to use. On the other hand, having unique access to a register allows peripherals abstractions to ensure the register will not be used outside the abstraction and may prevent conflicts (e.g. using the same pin with two different peripherals). In the end it's up too you to choose if you want more ease of use or more guarantees. (I think both are good for different use cases).

For me "an address to the underlying memory" is quite similar to &mut T, which is not Clone. That said &UnsafeCell<T> is Copy and mutable.