Remove PhantomData<*const ()> from peripherals

[therealprof]

Not sure which purpose they're supposed to serve but the do seem kind of
pointless.

Signed-off-by: Daniel Egger [email protected]

[rust-highfive]

r? @reitermarkus

(rust_highfive has picked a reviewer for you, use r? to override)

[hannobraun]

I think the purpose is to opt out of Send and Sync. PhantomData<*const ()> should prevent the compiler from deriving them automatically.

[therealprof]

I think the purpose is to opt out of Send and Sync. PhantomData<*const ()> should prevent the compiler from deriving them automatically.

We manually implement Send for those (which we conversely could also remove if this PR was acceptable). Since the peripherals are not actually structures in memory but MMIO and Sync doesn't have any effect on multi-core implementations in any way I can't see the downside of allowing them to beSync, too.

[adamgreig]

If these were Sync, it would be possible to store a peripheral without a Mutex in a static mut, allowing racing access between threads of execution. Since the peripherals have a modify method which is not thread-safe, I don't think we can make them Sync.

In any event I'd be very wary of removing the PhantomData so long as

Not sure which purpose they're supposed to serve

[therealprof]

If these were Sync, it would be possible to store a peripheral without a Mutex in a static mut, allowing racing access between threads of execution. Since the peripherals have a modify method which is not thread-safe, I don't think we can make them Sync.

• If you can conjure peripherals out of thin air, the point of not allowing to store them in a static mut is moot
• Manipulating static mut is per se unsafe
• We don't have a notion of threads in embedded, do we?

CC rust-embedded/wg#419

[adamgreig]

Manipulating static mut is per se unsafe

Actually, since the modify methods don't need &mut self, they could even be stored in a static, right?

We don't have a notion of threads in embedded, do we?

We do have threads of execution, namely ISRs and "thread mode" (on ARM), which can race each other and respect Send/Sync.

[therealprof]

We do have threads of execution, namely ISRs and "thread mode" (on ARM),

Sure do.

respect Send/Sync.

Can they though? Since they're purely implemented in hardware and you can conjure peripheral access abstractions out of thin air or completely bypass them I'm not sure how Sync really applies here or can be properly enforced by the compiler. If we wanted to make that completely safe and sound (for single-core anyway) we'd have to prevent the ability to conjure peripherals, i.e. make them true singletons.

I do agree with you though that it doesn't make sense to change this just for the sake of changing it.

[therealprof]

Let's drop this then.