Auto merge of #69 - japaric:revise-api, r=japaric

revise peripheral API

This PR changes the signature of many of the high level methods available on peripherals like
`NVIC::get_priority`. Additionally some instance methods have been turned into static methods. The
following guidelines have been used to apply the changes:

- If the method body contains a single, atomic read operation with *no* side effects (e.g. the read
  operation clears one of the bits of the register): the signature changed to make the method
  static, i.e. `&self` was removed from the signature.

- If the method involves writing to or a RMW operation on a register: the signature changed to take
  the singleton by `&mut self` reference.

- If the method involves only read operations where at least one of them modifies the value
  of a register: the signature changed to take the singleton by `&mut self` reference.

The rationale for this last guideline is that using `&self`, instead of `&mut self`, lets the user
(unintentionally) break abstractions in the presence of generators. Example below:

``` rust
let peripherals = Peripherals::take().unwrap();
let syst = &peripherals.SYST;

// tasks
let mut a = || {
    loop {
        // yielding "busy wait"
        while !a.has_wrapped() {
            yield;
        }

        // do stuff
    }
};

let mut b = || {
    // ..

    // *NOTE* the problem is in the line below: this `is_counter_enabled` method reads the CSR
    // register and that read operation clears the COUNTFLAG bit of the register (if set), which is
    // the bit the `has_wrapped` method checks for.
    if syst.is_counter_enabled() {
        // ..
    }

    // ..
};
```

One more guideline was considered but the required conditions didn't apply to any of the existing
methods:

- If the method involves only non side effectful, non necessarily atomic read operations: the
  signature of the method should remain as `&self`.

The rationale for this guideline is that a static method (no `self` argument) wouldn't be
appropriate because that can result in a torn read if the read operation can be preempted by some
context that modifies the register.

In any case, this last guideline doesn't seem to apply well to the peripherals structs exposed by
this crate because they *deref* to a `RegisterBlock` that allows mutation through a `&self`
reference. When these two properties (the guideline and `Deref<Target=RegisterBlock>`) are mixed
the user can potentially break abstractions using generators (as shown in the `syst` example).

cc @hannobraun
closes #67

1 files changed, 68 insertions, 1 deletions

diff --git a/src/peripheral/mod.rs b/src/peripheral/mod.rs
index bd658a4..ffbb56c 100644
--- a/src/peripheral/mod.rs
+++ b/src/peripheral/mod.rs
@@ -1,5 +1,66 @@
 //! Core peripherals
 //!
+//! # API
+//!
+//! To use (most of) the peripheral API first you must get an *instance* of the peripheral. All the
+//! core peripherals are modeled as singletons (there can only ever be, at most, one instance of
+//! them at any given point in time) and the only way to get an instance of them is through the
+//! [`Peripherals::take`](struct.Peripherals.html#method.take) method.
+//!
+//! ``` no_run
+//! extern crate cortex_m;
+//!
+//! use cortex_m::peripheral::Peripherals;
+//!
+//! fn main() {
+//!     let mut peripherals = Peripherals::take().unwrap();
+//!     peripherals.DWT.enable_cycle_counter();
+//! }
+//! ```
+//!
+//! This method can only be successfully called *once* -- this is why the method returns an
+//! `Option`. Subsequent calls to the method will result in a `None` value being returned.
+//!
+//! A part of the peripheral API doesn't require access to a peripheral instance. This part of the
+//! API is provided as static methods on the peripheral types. One example is the
+//! [`DWT::cyccnt`](struct.DWT.html#method.cyccnt) method.
+//!
+//! ``` no_run
+//! extern crate cortex_m;
+//!
+//! use cortex_m::peripheral::{DWT, Peripherals};
+//!
+//! fn main() {
+//!     {
+//!         let mut peripherals = Peripherals::take().unwrap();
+//!         peripherals.DWT.enable_cycle_counter();
+//!     } // all the peripheral singletons are destroyed here
+//!
+//!     // but this method can be called without a DWT instance
+//!     let cyccnt = DWT::get_cycle_count();
+//! }
+//! ```
+//!
+//! The singleton property can be *unsafely* bypassed using the `ptr` static method which is
+//! available on all the peripheral types. This method is a useful building block for implementing
+//! higher level and safe abstractions.
+//!
+//! ``` no_run
+//! extern crate cortex_m;
+//!
+//! use cortex_m::peripheral::{DWT, Peripherals};
+//!
+//! fn main() {
+//!     {
+//!         let mut peripherals = Peripherals::take().unwrap();
+//!         peripherals.DWT.enable_cycle_counter();
+//!     } // all the peripheral singletons are destroyed here
+//!
+//!     // actually safe because this is an atomic read with no side effects
+//!     let cyccnt = unsafe { (*DWT::ptr()).cyccnt.read() };
+//! }
+//! ```
+//!
 //! # References
 //!
 //! - ARMv7-M Architecture Reference Manual (Issue E.b) - Chapter B3
@@ -80,7 +141,7 @@ impl Peripherals {
         })
     }
 
-    /// Unchecked version of `Peripherals::steal`
+    /// Unchecked version of `Peripherals::take`
     pub unsafe fn steal() -> Self {
         debug_assert!(!CORE_PERIPHERALS);
 
@@ -136,6 +197,12 @@ pub struct CBP {
 
 #[cfg(armv7m)]
 impl CBP {
+    pub(crate) unsafe fn new() -> Self {
+        CBP {
+            _marker: PhantomData,
+        }
+    }
+
     /// Returns a pointer to the register block
     pub fn ptr() -> *const self::cbp::RegisterBlock {
         0xE000_EF50 as *const _