i2c,spi: add busno error detection

artiq/coredevice/exceptions.py

@@ -111,3 +111,11 @@ class WatchdogExpired(Exception):
 
 class ClockFailure(Exception):
     """Raised when RTIO PLL has lost lock."""
+
+class I2CError(Exception):
+    """Raised when a I2C transaction fails."""
+    pass
+
+class SPIError(Exception):
+    """Raised when a I2C transaction fails."""
+    pass

artiq/coredevice/i2c.py

@@ -1,10 +1,6 @@
 from artiq.language.core import syscall, kernel
 from artiq.language.types import TBool, TInt32, TNone
-
+from artiq.coredevice.exceptions import I2CError
-
-class I2CError(Exception):
-    """Raised when a I2C transaction fails."""
-    pass
 
 
 @syscall(flags={"nounwind", "nowrite"})

artiq/firmware/ksupport/nrt_bus.rs

@@ -5,20 +5,36 @@ pub mod i2c {
 
     pub extern fn start(busno: i32) {
         send(&I2cStartRequest { busno: busno as u8 });
+        recv!(&I2cBasicReply { succeeded } => if !succeeded {
+            raise!("I2CError", "I2C bus could not be accessed");
+        });
     }
 
     pub extern fn stop(busno: i32) {
         send(&I2cStopRequest { busno: busno as u8 });
+        recv!(&I2cBasicReply { succeeded } => if !succeeded {
+            raise!("I2CError", "I2C bus could not be accessed");
+        });
     }
 
     pub extern fn write(busno: i32, data: i32) -> bool {
         send(&I2cWriteRequest { busno: busno as u8, data: data as u8 });
-        recv!(&I2cWriteReply { ack } => ack)
+        recv!(&I2cWriteReply { succeeded, ack } => {
+            if !succeeded {
+                raise!("I2CError", "I2C bus could not be accessed");
+            }
+            ack
+        })
     }
 
     pub extern fn read(busno: i32, ack: bool) -> i32 {
         send(&I2cReadRequest { busno: busno as u8, ack: ack });
-        recv!(&I2cReadReply { data } => data) as i32
+        recv!(&I2cReadReply { succeeded, data } => {
+            if !succeeded {
+                raise!("I2CError", "I2C bus could not be accessed");
+            }
+            data
+        }) as i32
     }
 }
 
@@ -30,19 +46,33 @@ pub mod spi {
     pub extern fn set_config(busno: i32, flags: i32, write_div: i32, read_div: i32) {
         send(&SpiSetConfigRequest { busno: busno as u32, flags: flags as u8,
                                     write_div: write_div as u8, read_div: read_div as u8 });
+        recv!(&SpiBasicReply { succeeded } => if !succeeded {
+            raise!("SPIError", "SPI bus could not be accessed");
+        });
     }
 
     pub extern fn set_xfer(busno: i32, chip_select: i32, write_length: i32, read_length: i32) {
         send(&SpiSetXferRequest { busno: busno as u32, chip_select: chip_select as u16,
                                   write_length: write_length as u8, read_length: read_length as u8 });
+        recv!(&SpiBasicReply { succeeded } => if !succeeded {
+            raise!("SPIError", "SPI bus could not be accessed");
+        });
     }
 
     pub extern fn write(busno: i32, data: i32) {
         send(&SpiWriteRequest { busno: busno as u32, data: data as u32 });
+        recv!(&SpiBasicReply { succeeded } => if !succeeded {
+            raise!("SPIError", "SPI bus could not be accessed");
+        });
     }
 
     pub extern fn read(busno: i32) -> i32 {
         send(&SpiReadRequest { busno: busno as u32 });
-        recv!(&SpiReadReply { data } => data) as i32
+        recv!(&SpiReadReply { succeeded, data } => {
+            if !succeeded {
+                raise!("SPIError", "SPI bus could not be accessed");
+            }
+            data
+        }) as i32
     }
 }

artiq/firmware/libboard/i2c.rs

@@ -70,27 +70,38 @@ pub fn init() {
 }
 
 #[cfg(has_i2c)]
-pub fn start(busno: u8) {
+pub fn start(busno: u8) -> Result<(), ()> {
+    if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
+        return Err(())
+    }
     // Set SCL high then SDA low
     io::scl_o(busno, true);
     io::half_period();
     io::sda_oe(busno, true);
     io::half_period();
+    Ok(())
 }
 
 #[cfg(has_i2c)]
-pub fn restart(busno: u8) {
+pub fn restart(busno: u8) -> Result<(), ()> {
+    if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
+        return Err(())
+    }
     // Set SCL low then SDA high */
     io::scl_o(busno, false);
     io::half_period();
     io::sda_oe(busno, false);
     io::half_period();
     // Do a regular start
-    start(busno);
+    start(busno)?;
+    Ok(())
 }
 
 #[cfg(has_i2c)]
-pub fn stop(busno: u8) {
+pub fn stop(busno: u8) -> Result<(), ()> {
+    if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
+        return Err(())
+    }
     // First, make sure SCL is low, so that the target releases the SDA line
     io::scl_o(busno, false);
     io::half_period();
@@ -100,10 +111,14 @@ pub fn stop(busno: u8) {
     io::half_period();
     io::sda_oe(busno, false);
     io::half_period();
+    Ok(())
 }
 
 #[cfg(has_i2c)]
-pub fn write(busno: u8, data: u8) -> bool {
+pub fn write(busno: u8, data: u8) -> Result<bool, ()> {
+    if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
+        return Err(())
+    }
     // MSB first
     for bit in (0..8).rev() {
         // Set SCL low and set our bit on SDA
@@ -123,11 +138,14 @@ pub fn write(busno: u8, data: u8) -> bool {
     io::scl_o(busno, true);
     io::half_period();
     // returns true if acked (I2C target pulled SDA low)
-    !io::sda_i(busno)
+    Ok(!io::sda_i(busno))
 }
 
 #[cfg(has_i2c)]
-pub fn read(busno: u8, ack: bool) -> u8 {
+pub fn read(busno: u8, ack: bool) -> Result<u8, ()> {
+    if busno as u32 >= csr::CONFIG_I2C_BUS_COUNT {
+        return Err(())
+    }
     // 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.
     io::scl_o(busno, false);
@@ -154,18 +172,18 @@ pub fn read(busno: u8, ack: bool) -> u8 {
     io::scl_o(busno, true);
     io::half_period();
 
-    data
+    Ok(data)
 }
 
 #[cfg(not(has_i2c))]
 pub fn init() {}
 #[cfg(not(has_i2c))]
-pub fn start(_busno: u8) {}
+pub fn start(_busno: u8) -> Result<(), ()> { Err(()) }
 #[cfg(not(has_i2c))]
-pub fn restart(_busno: u8) {}
+pub fn restart(_busno: u8) -> Result<(), ()> { Err(()) }
 #[cfg(not(has_i2c))]
-pub fn stop(_busno: u8) {}
+pub fn stop(_busno: u8) -> Result<(), ()> { Err(()) }
 #[cfg(not(has_i2c))]
-pub fn write(_busno: u8, _data: u8) -> bool { false }
+pub fn write(_busno: u8, _data: u8) -> Result<bool, ()> { Err(()) }
 #[cfg(not(has_i2c))]
-pub fn read(_busno: u8, _ack: bool) { 0xff }
+pub fn read(_busno: u8, _ack: bool) -> Result<u8, ()> { Err(()) }

artiq/firmware/libboard/si5324.rs

@@ -10,14 +10,14 @@ const ADDRESS: u8 = 0x68;
 
 #[cfg(soc_platform = "kc705")]
 fn pca9548_select(channel: u8) -> Result<()> {
-    i2c::start(BUSNO);
+    i2c::start(BUSNO).unwrap();
-    if !i2c::write(BUSNO, (0x74 << 1)) {
+    if !i2c::write(BUSNO, (0x74 << 1)).unwrap() {
         return Err("PCA9548 failed to ack write address")
     }
-    if !i2c::write(BUSNO, 1 << channel) {
+    if !i2c::write(BUSNO, 1 << channel).unwrap() {
         return Err("PCA9548 failed to ack control word")
     }
-    i2c::stop(BUSNO);
+    i2c::stop(BUSNO).unwrap();
     Ok(())
 }
 
@@ -93,34 +93,34 @@ fn map_frequency_settings(settings: &FrequencySettings) -> Result<FrequencySetti
 }
 
 fn write(reg: u8, val: u8) -> Result<()> {
-    i2c::start(BUSNO);
+    i2c::start(BUSNO).unwrap();
-    if !i2c::write(BUSNO, (ADDRESS << 1)) {
+    if !i2c::write(BUSNO, (ADDRESS << 1)).unwrap() {
         return Err("Si5324 failed to ack write address")
     }
-    if !i2c::write(BUSNO, reg) {
+    if !i2c::write(BUSNO, reg).unwrap() {
         return Err("Si5324 failed to ack register")
     }
-    if !i2c::write(BUSNO, val) {
+    if !i2c::write(BUSNO, val).unwrap() {
         return Err("Si5324 failed to ack value")
     }
-    i2c::stop(BUSNO);
+    i2c::stop(BUSNO).unwrap();
     Ok(())
 }
 
 fn read(reg: u8) -> Result<u8> {
-    i2c::start(BUSNO);
+    i2c::start(BUSNO).unwrap();
-    if !i2c::write(BUSNO, (ADDRESS << 1)) {
+    if !i2c::write(BUSNO, (ADDRESS << 1)).unwrap() {
         return Err("Si5324 failed to ack write address")
     }
-    if !i2c::write(BUSNO, reg) {
+    if !i2c::write(BUSNO, reg).unwrap() {
         return Err("Si5324 failed to ack register")
     }
-    i2c::restart(BUSNO);
+    i2c::restart(BUSNO).unwrap();
-    if !i2c::write(BUSNO, (ADDRESS << 1) | 1) {
+    if !i2c::write(BUSNO, (ADDRESS << 1) | 1).unwrap() {
         return Err("Si5324 failed to ack read address")
     }
-    let val = i2c::read(BUSNO, false);
+    let val = i2c::read(BUSNO, false).unwrap();
-    i2c::stop(BUSNO);
+    i2c::stop(BUSNO).unwrap();
     Ok(val)
 }
 

artiq/firmware/libboard/spi.rs

@@ -1,11 +1,11 @@
 #[cfg(has_converter_spi)]
 use csr;
 
-// Later this module should support other buses than the converter SPI bus,
-// and add a busno parameter to differentiate them.
-
 #[cfg(has_converter_spi)]
-pub fn set_config(flags: u8, write_div: u8, read_div: u8) {
+pub fn set_config(busno: u8, flags: u8, write_div: u8, read_div: u8) -> Result<(), ()> {
+    if busno != 0 {
+        return Err(())
+    }
     unsafe {
         csr::converter_spi::offline_write(1);
         csr::converter_spi::cs_polarity_write(flags >> 3 & 1);
@@ -17,38 +17,50 @@ pub fn set_config(flags: u8, write_div: u8, read_div: u8) {
         csr::converter_spi::clk_div_read_write(read_div);
         csr::converter_spi::offline_write(0);
     }
+    Ok(())
 }
 
 #[cfg(has_converter_spi)]
-pub fn set_xfer(chip_select: u16, write_length: u8, read_length: u8) {
+pub fn set_xfer(busno: u8, chip_select: u16, write_length: u8, read_length: u8) -> Result<(), ()> {
+    if busno != 0 {
+        return Err(())
+    }
     unsafe {
         csr::converter_spi::cs_write(chip_select as _);
         csr::converter_spi::xfer_len_write_write(write_length);
         csr::converter_spi::xfer_len_read_write(read_length);
     }
+    Ok(())
 }
 
 #[cfg(has_converter_spi)]
-pub fn write(data: u32) {
+pub fn write(busno: u8, data: u32) -> Result<(), ()> {
+    if busno != 0 {
+        return Err(())
+    }
     unsafe {
         csr::converter_spi::data_write_write(data);
         while csr::converter_spi::pending_read() != 0 {}
         while csr::converter_spi::active_read() != 0 {}
     }
+    Ok(())
 }
 
 #[cfg(has_converter_spi)]
-pub fn read() -> u32 {
+pub fn read(busno: u8) -> Result<u32, ()> {
-    unsafe {
+    if busno != 0 {
-        csr::converter_spi::data_read_read()
+        return Err(())
     }
+    Ok(unsafe {
+        csr::converter_spi::data_read_read()
+    })
 }
 
 #[cfg(not(has_converter_spi))]
-pub fn set_config(_flags: u8, _write_div: u8, _read_div: u8) {}
+pub fn set_config(_busno: u8, _flags: u8, _write_div: u8, _read_div: u8) -> Result<(), ()> { Err(()) }
 #[cfg(not(has_converter_spi))]
-pub fn set_xfer(_chip_select: u16, _write_length: u8, _read_length: u8) {}
+pub fn set_xfer(_busno: u8,_chip_select: u16, _write_length: u8, _read_length: u8) -> Result<(), ()> { Err(()) }
 #[cfg(not(has_converter_spi))]
-pub fn write(_data: u32) {}
+pub fn write(_busno: u8,_data: u32) -> Result<(), ()> { Err(()) }
 #[cfg(not(has_converter_spi))]
-pub fn read() -> u32 { 0 }
+pub fn read(_busno: u8,) -> Result<u32, ()> { Err(()) }

artiq/firmware/libproto/kernel_proto.rs

@@ -83,15 +83,17 @@ pub enum Message<'a> {
     I2cStartRequest { busno: u8 },
     I2cStopRequest { busno: u8 },
     I2cWriteRequest { busno: u8, data: u8 },
-    I2cWriteReply { ack: bool },
+    I2cWriteReply { succeeded: bool, ack: bool },
     I2cReadRequest { busno: u8, ack: bool },
-    I2cReadReply { data: u8 },
+    I2cReadReply { succeeded: bool, data: u8 },
+    I2cBasicReply { succeeded: bool },
 
     SpiSetConfigRequest { busno: u32, flags: u8, write_div: u8, read_div: u8 },
     SpiSetXferRequest { busno: u32, chip_select: u16, write_length: u8, read_length: u8 },
     SpiWriteRequest { busno: u32, data: u32 },
     SpiReadRequest { busno: u32 },
-    SpiReadReply { data: u32 },
+    SpiReadReply { succeeded: bool, data: u32 },
+    SpiBasicReply { succeeded: bool },
 
     Log(fmt::Arguments<'a>),
     LogSlice(&'a str)

artiq/firmware/runtime/kern_hwreq.rs

@@ -5,49 +5,63 @@ use kernel_proto as kern;
 use std::io;
 use sched::Io;
 
-// TODO
 mod drtio_spi {
-    pub fn set_config(_busno: u32, _flags: u8, _write_div: u8, _read_div: u8) {}
+    pub fn set_config(_busno: u32, _flags: u8, _write_div: u8, _read_div: u8) -> Result<(), ()> {
-    pub fn set_xfer(_busno: u32, _chip_select: u16, _write_length: u8, _read_length: u8) {}
+        Err(())
-    pub fn write(_busno: u32, _data: u32) {}
+    }
-    pub fn read(_busno: u32) -> u32 { 0 }
+
+    pub fn set_xfer(_busno: u32, _chip_select: u16, _write_length: u8, _read_length: u8) -> Result<(), ()> {
+        Err(())
+    }
+
+    pub fn write(_busno: u32, _data: u32) -> Result<(), ()> {
+        Err(())
+    }
+
+    pub fn read(_busno: u32) -> Result<u32, ()> {
+        Err(())
+    }
 }
 
 mod spi {
     use board;
     use super::drtio_spi;
 
-    pub fn set_config(busno: u32, flags: u8, write_div: u8, read_div: u8) {
+    pub fn set_config(busno: u32, flags: u8, write_div: u8, read_div: u8) -> Result<(), ()> {
         let drtio = busno >> 16;
+        let dev_busno = busno as u8;
         if drtio == 0 {
-            board::spi::set_config(flags, write_div, read_div)
+            board::spi::set_config(dev_busno, flags, write_div, read_div)
         } else {
             drtio_spi::set_config(busno, flags, write_div, read_div)
         }
     }
 
-    pub fn set_xfer(busno: u32, chip_select: u16, write_length: u8, read_length: u8) {
+    pub fn set_xfer(busno: u32, chip_select: u16, write_length: u8, read_length: u8) -> Result<(), ()> {
         let drtio = busno >> 16;
+        let dev_busno = busno as u8;
         if drtio == 0 {
-            board::spi::set_xfer(chip_select, write_length, read_length)
+            board::spi::set_xfer(dev_busno, chip_select, write_length, read_length)
         } else {
             drtio_spi::set_xfer(busno, chip_select, write_length, read_length)
         }
     }
 
-    pub fn write(busno: u32, data: u32) {
+    pub fn write(busno: u32, data: u32) -> Result<(), ()> {
         let drtio = busno >> 16;
+        let dev_busno = busno as u8;
         if drtio == 0 {
-            board::spi::write(data)
+            board::spi::write(dev_busno, data)
         } else {
             drtio_spi::write(busno, data)
         }
     }
 
-    pub fn read(busno: u32) -> u32 {
+    pub fn read(busno: u32) -> Result<u32, ()> {
         let drtio = busno >> 16;
+        let dev_busno = busno as u8;
         if drtio == 0 {
-            board::spi::read()
+            board::spi::read(dev_busno)
         } else {
             drtio_spi::read(busno)
         }
@@ -85,37 +99,43 @@ pub fn process_kern_hwreq(io: &Io, request: &kern::Message) -> io::Result<bool>
         }
 
         &kern::I2cStartRequest { busno } => {
-            board::i2c::start(busno);
+            let succeeded = board::i2c::start(busno).is_ok();
-            kern_acknowledge()
+            kern_send(io, &kern::I2cBasicReply { succeeded: succeeded })
         }
         &kern::I2cStopRequest { busno } => {
-            board::i2c::stop(busno);
+            let succeeded = board::i2c::stop(busno).is_ok();
-            kern_acknowledge()
+            kern_send(io, &kern::I2cBasicReply { succeeded: succeeded })
         }
         &kern::I2cWriteRequest { busno, data } => {
-            let ack = board::i2c::write(busno, data);
+            match board::i2c::write(busno, data) {
-            kern_send(io, &kern::I2cWriteReply { ack: ack })
+                Ok(ack) => kern_send(io, &kern::I2cWriteReply { succeeded: true, ack: ack }),
+                Err(_) => kern_send(io, &kern::I2cWriteReply { succeeded: false, ack: false })
+            }
         }
         &kern::I2cReadRequest { busno, ack } => {
-            let data = board::i2c::read(busno, ack);
+            match board::i2c::read(busno, ack) {
-            kern_send(io, &kern::I2cReadReply { data: data })
+                Ok(data) => kern_send(io, &kern::I2cReadReply { succeeded: true, data: data }),
+                Err(_) => kern_send(io, &kern::I2cReadReply { succeeded: false, data: 0xff })
+            }
         },
 
         &kern::SpiSetConfigRequest { busno, flags, write_div, read_div } => {
-            spi::set_config(busno, flags, write_div, read_div);
+            let succeeded = spi::set_config(busno, flags, write_div, read_div).is_ok();
-            kern_acknowledge()
+            kern_send(io, &kern::SpiBasicReply { succeeded: succeeded })
         },
         &kern::SpiSetXferRequest { busno, chip_select, write_length, read_length } => {
-            spi::set_xfer(busno, chip_select, write_length, read_length);
+            let succeeded = spi::set_xfer(busno, chip_select, write_length, read_length).is_ok();
-            kern_acknowledge()
+            kern_send(io, &kern::SpiBasicReply { succeeded: succeeded })
         }
         &kern::SpiWriteRequest { busno, data } => {
-            spi::write(busno, data);
+            let succeeded = spi::write(busno, data).is_ok();
-            kern_acknowledge()
+            kern_send(io, &kern::SpiBasicReply { succeeded: succeeded })
         }
         &kern::SpiReadRequest { busno } => {
-            let data = spi::read(busno);
+            match spi::read(busno) {
-            kern_send(io, &kern::SpiReadReply { data: data })
+                Ok(data) => kern_send(io, &kern::SpiReadReply { succeeded: true, data: data }),
+                Err(_) => kern_send(io, &kern::SpiReadReply { succeeded: false, data: 0 })
+            }
         },
 
         _ => return Ok(false)