From a8aabd38158def25f332cfbb61049040e5b5c732 Mon Sep 17 00:00:00 2001
From: Stephan Maka <stephan@spaceboyz.net>
Date: Fri, 16 Aug 2019 01:07:59 +0200
Subject: [PATCH] firwmare: turn i2c errors into &str

---
 artiq/firmware/libboard_artiq/i2c.rs        | 33 +++++++++++----------
 artiq/firmware/libboard_artiq/i2c_eeprom.rs | 30 ++++++++-----------
 2 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/artiq/firmware/libboard_artiq/i2c.rs b/artiq/firmware/libboard_artiq/i2c.rs
index ca40fbc5b..d13dc32ce 100644
--- a/artiq/firmware/libboard_artiq/i2c.rs
+++ b/artiq/firmware/libboard_artiq/i2c.rs
@@ -2,6 +2,8 @@
 mod imp {
     use board_misoc::{csr, clock};
 
+    const INVALID_BUS: &'static str = "Invalid I2C bus";
+
     fn half_period() { clock::spin_us(100) }
     fn sda_bit(busno: u8) -> u8 { 1 << (2 * busno + 1) }
     fn scl_bit(busno: u8) -> u8 { 1 << (2 * busno) }
@@ -76,9 +78,9 @@ mod imp {
         }
     }
 
-    pub fn start(busno: u8) -> Result<(), ()> {
+    pub fn start(busno: u8) -> Result<(), &'static str> {
         if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
-            return Err(())
+            return Err(INVALID_BUS)
         }
         // Set SCL high then SDA low
         scl_o(busno, true);
@@ -88,9 +90,9 @@ mod imp {
         Ok(())
     }
 
-    pub fn restart(busno: u8) -> Result<(), ()> {
+    pub fn restart(busno: u8) -> Result<(), &'static str> {
         if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
-            return Err(())
+            return Err(INVALID_BUS)
         }
         // Set SCL low then SDA high */
         scl_o(busno, false);
@@ -102,9 +104,9 @@ mod imp {
         Ok(())
     }
 
-    pub fn stop(busno: u8) -> Result<(), ()> {
+    pub fn stop(busno: u8) -> Result<(), &'static str> {
         if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
-            return Err(())
+            return Err(INVALID_BUS)
         }
         // First, make sure SCL is low, so that the target releases the SDA line
         scl_o(busno, false);
@@ -118,9 +120,9 @@ mod imp {
         Ok(())
     }
 
-    pub fn write(busno: u8, data: u8) -> Result<bool, ()> {
+    pub fn write(busno: u8, data: u8) -> Result<bool, &'static str> {
         if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
-            return Err(())
+            return Err(INVALID_BUS)
         }
         // MSB first
         for bit in (0..8).rev() {
@@ -144,9 +146,9 @@ mod imp {
         Ok(!sda_i(busno))
     }
 
-    pub fn read(busno: u8, ack: bool) -> Result<u8, ()> {
+    pub fn read(busno: u8, ack: bool) -> Result<u8, &'static str> {
         if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
-            return Err(())
+            return Err(INVALID_BUS)
         }
         // Set SCL low first, otherwise setting SDA as input may cause a transition
         // on SDA with SCL high which will be interpreted as START/STOP condition.
@@ -180,12 +182,13 @@ mod imp {
 
 #[cfg(not(has_i2c))]
 mod imp {
+    const NO_I2C: &'static str = "No I2C support on this platform";
     pub fn init() {}
-    pub fn start(_busno: u8) -> Result<(), ()> { Err(()) }
-    pub fn restart(_busno: u8) -> Result<(), ()> { Err(()) }
-    pub fn stop(_busno: u8) -> Result<(), ()> { Err(()) }
-    pub fn write(_busno: u8, _data: u8) -> Result<bool, ()> { Err(()) }
-    pub fn read(_busno: u8, _ack: bool) -> Result<u8, ()> { Err(()) }
+    pub fn start(_busno: u8) -> Result<(), &'static str> { Err(NO_I2C) }
+    pub fn restart(_busno: u8) -> Result<(), &'static str> { Err(NO_I2C) }
+    pub fn stop(_busno: u8) -> Result<(), &'static str> { Err(NO_I2C) }
+    pub fn write(_busno: u8, _data: u8) -> Result<bool, &'static str> { Err(NO_I2C) }
+    pub fn read(_busno: u8, _ack: bool) -> Result<u8, &'static str> { Err(NO_I2C) }
 }
 
 pub use self::imp::*;
diff --git a/artiq/firmware/libboard_artiq/i2c_eeprom.rs b/artiq/firmware/libboard_artiq/i2c_eeprom.rs
index fb89bd4a2..e5f6cb9b3 100644
--- a/artiq/firmware/libboard_artiq/i2c_eeprom.rs
+++ b/artiq/firmware/libboard_artiq/i2c_eeprom.rs
@@ -26,30 +26,26 @@ impl EEPROM {
     fn select(&self) -> Result<(), &'static str> {
         let mask: u16 = 1 << self.port;
         pca9548::select(self.busno, I2C_SWITCH0, mask as u8)?;
-        pca9548::select(self.busno, I2C_SWITCH1, (mask >> 8) as u8)
+        pca9548::select(self.busno, I2C_SWITCH1, (mask >> 8) as u8)?;
+
+        Ok(())
     }
 
     pub fn read<'a>(&self, addr: u8, buf: &'a mut [u8]) -> Result<(), &'static str> {
         self.select()?;
 
-        Ok(()).and_then(|()| {
-            i2c::start(self.busno)?;
-            i2c::write(self.busno, self.address)?;
-            i2c::write(self.busno, addr)?;
-            Ok(())
-        }).map_err(|()| "I2C address write error")?;
+        i2c::start(self.busno)?;
+        i2c::write(self.busno, self.address)?;
+        i2c::write(self.busno, addr)?;
 
-        Ok(()).and_then(|()| {
-            i2c::restart(self.busno)?;
-            i2c::write(self.busno, self.address | 1)?;
-            let buf_len = buf.len();
-            for (i, byte) in buf.iter_mut().enumerate() {
-                *byte = i2c::read(self.busno, i < buf_len - 1)?;
-            }
+        i2c::restart(self.busno)?;
+        i2c::write(self.busno, self.address | 1)?;
+        let buf_len = buf.len();
+        for (i, byte) in buf.iter_mut().enumerate() {
+            *byte = i2c::read(self.busno, i < buf_len - 1)?;
+        }
 
-            i2c::stop(self.busno)?;
-            Ok(())
-        }).map_err(|()| "I2C read error")?;
+        i2c::stop(self.busno)?;
 
         Ok(())
     }