Numerous minor fixes from github reviews

Apply various minor fixes.  None of these should affect the code
execution itself.

Signed-off-by: David Brown <[email protected]>

Author: d3zd3z

samples/blinky/CMakeLists.txt

@@ -1,4 +1,4 @@
-# SPDX-License-Identifier: Apache-2.0
+# SPDX-License-Identifier: Apache-2.0 OR MIT
 
 cmake_minimum_required(VERSION 3.20.0)
 find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})

samples/blinky/Cargo.toml

@@ -7,7 +7,7 @@ name = "rustapp"
 version = "0.1.0"
 edition = "2021"
 description = "Blink an LED forever using the GPIO API"
-license = "Apache-2.0 or MIT"
+license = "Apache-2.0 OR MIT"
 
 [lib]
 crate-type = ["staticlib"]

samples/blinky/src/lib.rs

@@ -21,9 +21,8 @@ extern "C" fn rust_main() {
     unsafe { zephyr::set_logger().unwrap(); }
 
     warn!("Starting blinky");
-    // println!("Blinky!");
+
     // Invoke "blink" as a user thread.
-    // blink();
     if false {
         unsafe {
             zephyr::raw::k_thread_user_mode_enter

samples/blinky/src/main.c

@@ -35,6 +35,7 @@ pub struct DeviceTree {
     /// The root of the tree.
     root: Rc<Node>,
     /// All of the labels.
+    /// Note that this is a BTree  so that the output will be deterministic.
     labels: BTreeMap<String, Rc<Node>>,
 }
 
@@ -70,7 +71,7 @@ pub struct Property {
 pub enum Value {
     Words(Vec<Word>),
     Bytes(Vec<u8>),
-    Phandle(Phandle), // TODO
+    Phandle(Phandle),
     String(String),
 }
 
@@ -123,34 +124,24 @@ impl Node {
     }
 
     fn parent_walk(self: &Rc<Self>) {
-        // *(self.parent.borrow_mut()) = Some(parent.clone());
-
         for child in &self.children {
             *(child.parent.borrow_mut()) = Some(self.clone());
             child.parent_walk()
         }
     }
 
     fn is_compatible(&self, name: &str) -> bool {
-        if let Some(prop) = self.properties.iter().find(|p| p.name == "compatible") {
-            prop.value.iter().any(|v| {
-                match v {
-                    Value::String(vn) if name == vn => true,
-                    _ => false,
-                }
-            })
-        } else {
-            // If there is no compatible field, we are clearly not compatible.
-            false
-        }
+        self.properties
+            .iter()
+            .filter(|p| p.name == "compatible")
+            .flat_map(|prop| prop.value.iter())
+            .any(|v| matches!(v, Value::String(vn) if name == vn))
     }
 
     /// A richer compatible test.  Walks a series of names, in reverse.  Any that are "Some(x)" must
     /// be compatible with "x" at that level.
     fn compatible_path(&self, path: &[Option<&str>]) -> bool {
-        let res = self.path_walk(path, 0);
-        // println!("compatible? {}: {} {:?}", res, self.path, path);
-        res
+        self.path_walk(path, 0)
     }
 
     /// Recursive path walk, to make borrowing simpler.
@@ -161,10 +152,8 @@ impl Node {
         }
 
         // Check the failure condition, where this node isn't compatible with this section of the path.
-        if let Some(name) = path[pos] {
-            if !self.is_compatible(name) {
-                return false;
-            }
+        if matches!(path[pos], Some(name) if !self.is_compatible(name)) {
+            return false;
         }
 
         // Walk down the tree.  We have to check for None here, as we can't recurse on the none
@@ -177,19 +166,17 @@ impl Node {
         }
     }
 
-    /// Is the named property present?
+    /// Returns `true` if there is a property with this name.
     fn has_prop(&self, name: &str) -> bool {
         self.properties.iter().any(|p| p.name == name)
     }
 
-    /// Get this property in its entirety.
+    /// Returns the slice of values of a property with this name as `Some` or `None` if the property
+    /// does not exist.
     fn get_property(&self, name: &str) -> Option<&[Value]> {
-        for p in &self.properties {
-            if p.name == name {
-                return Some(&p.value);
-            }
-        }
-        return None;
+        self.properties
+            .iter()
+            .find_map(|p| if p.name == name { Some(p.value.as_slice()) } else { None })
     }
 
     /// Attempt to retrieve the named property, as a single entry of Words.
@@ -244,12 +231,11 @@ impl Node {
 impl Value {
     fn phandle_walk(&self, labels: &BTreeMap<String, Rc<Node>>) {
         match self {
-            Value::Phandle(ph) => ph.phandle_resolve(labels),
-            Value::Words(words) => {
+            Self::Phandle(ph) => ph.phandle_resolve(labels),
+            Self::Words(words) => {
                 for w in words {
-                    match w {
-                        Word::Phandle(ph) => ph.phandle_resolve(labels),
-                        _ => (),
+                    if let Word::Phandle(ph) = w {
+                        ph.phandle_resolve(labels);
                     }
                 }
             }
@@ -260,8 +246,8 @@ impl Value {
 
 impl Phandle {
     /// Construct a phandle that is unresolved.
-    pub fn new(name: String) -> Phandle {
-        Phandle {
+    pub fn new(name: String) -> Self {
+        Self {
             name,
             node: RefCell::new(None),
         }
@@ -286,9 +272,9 @@ impl Phandle {
 }
 
 impl Word {
-    pub fn get_number(&self) -> Option<u32> {
+    pub fn as_number(&self) -> Option<u32> {
         match self {
-            Word::Number(n) => Some(*n),
+            Self::Number(n) => Some(*n),
             _ => None,
         }
     }
@@ -297,6 +283,9 @@ impl Word {
 // To avoid recursion, the debug printer for Phandle just prints the name.
 impl std::fmt::Debug for Phandle {
     fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(fmt, "Phandle({:?})", self.name)
+        fmt
+            .debug_struct("Phandle")
+            .field("name", &self.name)
+            .finish_non_exhaustive()
     }
 }

zephyr-build/src/devicetree.rs

@@ -191,7 +191,7 @@ impl RawInfo {
     fn generate(&self, node: &Node, device: &str) -> TokenStream {
         let device_id = str_to_path(device);
         match self {
-            RawInfo::Myself => {
+            Self::Myself => {
                 let ord = node.ord;
                 let rawdev = format_ident!("__device_dts_ord_{}", ord);
                 quote! {
@@ -209,7 +209,7 @@ impl RawInfo {
                     }
                 }
             }
-            RawInfo::Phandle(pname) => {
+            Self::Phandle(pname) => {
                 let words = node.get_words(pname).unwrap();
                 // We assume that elt 0 is the phandle, and that the rest are numbers.
                 let target = if let Word::Phandle(handle) = &words[0] {
@@ -221,7 +221,7 @@ impl RawInfo {
                 // TODO: We would try to correlate with parent node's notion of number of cells, and
                 // try to handle cases where there is more than one reference.  It is unclear when
                 // this will be needed.
-                let args: Vec<u32> = words[1..].iter().map(|n| n.get_number().unwrap()).collect();
+                let args: Vec<u32> = words[1..].iter().map(|n| n.as_number().unwrap()).collect();
 
                 let target_route = target.route_to_rust();
 
@@ -235,7 +235,7 @@ impl RawInfo {
                     }
                 }
             }
-            RawInfo::Parent { level, args } => {
+            Self::Parent { level, args } => {
                 let get_args = args.iter().map(|arg| arg.args(node));
 
                 assert!(*level > 0);

zephyr-build/src/devicetree/augment.rs

@@ -102,10 +102,8 @@ impl DeviceTree {
                                     pub const #tag: u32 = #n;
                                 };
                             }
-                            _ => return general_property(prop),
+                            _ => (),
                         }
-                    } else {
-                        return general_property(prop);
                     }
                 }
                 Value::Phandle(ref ph) => {
@@ -118,7 +116,7 @@ impl DeviceTree {
                         }
                     }
                 }
-                _ => return general_property(prop),
+                _ => (),
             }
         }
         general_property(prop)

zephyr-build/src/devicetree/output.rs

@@ -12,7 +12,7 @@
  */
 
 /*
- * This is getting build with KERNEL defined, which causes syscalls to not be implemented.  Work
+ * This is getting built with KERNEL defined, which causes syscalls to not be implemented.  Work
  * around this by just undefining this symbol.
  */
 #undef KERNEL

zephyr-sys/wrapper.h

@@ -5,6 +5,9 @@
 //!
 //! Most of these instances come from the device tree.
 
+// Allow for a Zephyr build that has no devices at all.
+#![allow(dead_code)]
+
 use crate::sync::atomic::{AtomicBool, Ordering};
 
 pub mod gpio;
@@ -20,7 +23,6 @@ pub mod flash;
 /// example, a [`GpioPin`] will reference a single pin, but the underlying device for the gpio
 /// driver will be shared among then.  Generally, the constructor for the individual device will
 /// call `get_instance_raw()` on the underlying device.
-#[allow(dead_code)]
 pub(crate) struct Unique(pub(crate) AtomicBool);
 
 impl Unique {
@@ -33,7 +35,6 @@ impl Unique {
 
     /// Indicates if this particular entity can be used.  This function, on a given `Unique` value
     /// will return true exactly once.
-    #[allow(dead_code)]
     pub(crate) fn once(&self) -> bool {
         // `fetch_add` is likely to be faster than compare_exchage.  This does have the limitation
         // that `once` is not called more than `usize::MAX` times.

zephyr/src/device.rs

@@ -1,5 +1,8 @@
 //! Device wrappers for flash controllers, and flash partitions.
 
+// Note that currently, the flash partition shares the controller, so the underlying operations
+// are not actually safe.  Need to rethink how to manage this.
+
 use crate::raw;
 use super::Unique;
 
@@ -54,6 +57,3 @@ impl FlashPartition {
         Some(FlashPartition { controller, offset, size })
     }
 }
-
-// Note that currently, the flash partition shares the controller, so the underlying operations
-// are not actually safe.  Need to rethink how to manage this.

zephyr/src/device/flash.rs

@@ -12,6 +12,8 @@ use super::Unique;
 
 /// Global instance to help make gpio in Rust slightly safer.
 ///
+/// # Safety
+///
 /// To help with safety, the rust types use a global instance of a gpio-token.  Methods will
 /// take a mutable reference to this, which will require either a single thread in the
 /// application code, or something like a mutex or critical section to manage.  The operation