Revert i2c wrapper refactor, untie io_expander from it

Signed-off-by: Egor Savkin <es@m-labs.hk>

src/libboard_artiq/src/i2c.rs

@@ -1,83 +0,0 @@
-use libboard_zynq;
-
-pub static mut I2C_BUS: Option<libboard_zynq::i2c::I2c> = None;
-
-pub fn start() -> Result<(), &'static str> {
-    unsafe {
-        if (&mut I2C_BUS).as_mut().unwrap().start().is_err() {
-            Err("I2C start failed")
-        } else {
-            Ok(())
-        }
-    }
-}
-
-pub fn restart() -> Result<(), &'static str> {
-    unsafe {
-        if (&mut I2C_BUS).as_mut().unwrap().restart().is_err() {
-            Err("I2C restart failed")
-        } else {
-            Ok(())
-        }
-    }
-}
-
-pub fn stop() -> Result<(), &'static str> {
-    unsafe {
-        if (&mut I2C_BUS).as_mut().unwrap().stop().is_err() {
-            Err("I2C stop failed")
-        } else {
-            Ok(())
-        }
-    }
-}
-
-pub fn write(data: i32) -> Result<bool, &'static str> {
-    unsafe {
-        match (&mut I2C_BUS).as_mut().unwrap().write(data as u8) {
-            Ok(r) =>Ok(r),
-            Err(_) => Err("I2C write failed"),
-        }
-    }
-}
-
-pub fn read(ack: bool) -> Result<i32, &'static str> {
-    unsafe {
-        match (&mut I2C_BUS).as_mut().unwrap().read(ack) {
-            Ok(r) => Ok(r as i32),
-            Err(_) => Err("I2C read failed"),
-        }
-    }
-}
-
-pub fn pca954x_select(address: i32, channel: Option<u8>) -> Result<(), &'static str> {
-    unsafe {
-        if (&mut I2C_BUS).as_mut().unwrap().pca954x_select(address as u8, channel).is_err() {
-            Err("switch select failed")
-        } else {
-            Ok(())
-        }
-    }
-}
-
-pub fn switch_select(address: i32, mask: i32) -> Result<(), &'static str> {
-    let ch = match mask { //decode from mainline, PCA9548-centric API
-        0x00 => None,
-        0x01 => Some(0),
-        0x02 => Some(1),
-        0x04 => Some(2),
-        0x08 => Some(3),
-        0x10 => Some(4),
-        0x20 => Some(5),
-        0x40 => Some(6),
-        0x80 => Some(7),
-        _ => return Err("switch select supports only one channel")
-    };
-    pca954x_select(address, ch)
-}
-
-pub fn init() {
-    let mut i2c = libboard_zynq::i2c::I2c::i2c0();
-    i2c.init().expect("I2C bus initialization failed");
-    unsafe { I2C_BUS = Some(i2c) };
-}

src/libboard_artiq/src/io_expander.rs

@@ -1,4 +1,4 @@
-use crate::i2c;
+use libboard_zynq::i2c;
 use log::info;
 
 // Only the bare minimum registers. Bits/IO connections equivalent between IC types.
@@ -10,9 +10,9 @@ struct Registers {
     gpiob: u8,  // Output Port 1
 }
 
-pub struct IoExpander {
+pub struct IoExpander<'a> {
-    port: u8,
+    i2c: &'a mut i2c::I2c,
-    address: i32,
+    address: u8,
     iodir: [u8; 2],
     out_current: [u8; 2],
     out_target: [u8; 2],
@@ -20,13 +20,13 @@ pub struct IoExpander {
 }
 
 
-impl IoExpander {
+impl<'a> IoExpander<'a> {
-    pub fn new(index: u8) -> Result<Self, &'static str> {
+    pub fn new(i2c: &'a mut i2c::I2c, index: u8) -> Result<Self, &'static str> {
 
         // Both expanders on SHARED I2C bus
         let mut io_expander = match index {
             0 => IoExpander {
-                port: 11,
+                i2c,
                 address: 0x40,
                 iodir: [0xff; 2],
                 out_current: [0; 2],
@@ -39,7 +39,7 @@ impl IoExpander {
                 },
             },
             1 => IoExpander {
-                port: 11,
+                i2c,
                 address: 0x42,
                 iodir: [0xff; 2],
                 out_current: [0; 2],
@@ -72,32 +72,31 @@ impl IoExpander {
         Ok(io_expander)
     }
 
-    fn select(&self) -> Result<(), &'static str> {
+    fn select(&mut self) -> Result<(), &'static str> {
-        let mask: u16 = 1 << self.port;
+        self.i2c.pca954x_select(0x70, None)?;
-        i2c::switch_select(0x70, mask as u8 as i32)?;
+        self.i2c.pca954x_select(0x71, Some(3))?;
-        i2c::switch_select(0x71, (mask >> 8) as u8 as i32)?;
         Ok(())
     }
 
-    fn write(&self, addr: u8, value: u8) -> Result<(), &'static str> {
+    fn write(&mut self, addr: u8, value: u8) -> Result<(), &'static str> {
-        i2c::start()?;
+        self.i2c.start()?;
-        i2c::write(self.address as i32)?;
+        self.i2c.write(self.address)?;
-        i2c::write(addr as i32)?;
+        self.i2c.write(addr)?;
-        i2c::write(value as i32)?;
+        self.i2c.write(value)?;
-        i2c::stop()?;
+        self.i2c.stop()?;
         Ok(())
     }
 
-    fn check_ack(&self) -> Result<bool, &'static str> {
+    fn check_ack(&mut self) -> Result<bool, &'static str> {
         // Check for ack from io expander
         self.select()?;
-        i2c::start()?;
+        self.i2c.start()?;
-        let ack = i2c::write(self.address)?;
+        let ack = self.i2c.write(self.address)?;
-        i2c::stop()?;
+        self.i2c.stop()?;
         Ok(ack)
     }
 
-    fn update_iodir(&self) -> Result<(), &'static str> {
+    fn update_iodir(&mut self) -> Result<(), &'static str> {
         self.write(self.registers.iodira, self.iodir[0])?;
         self.write(self.registers.iodirb, self.iodir[1])?;
         Ok(())

src/libboard_artiq/src/lib.rs

@@ -27,7 +27,6 @@ pub mod drtioaux_async;
 #[cfg(has_drtio)]
 #[path = "../../../build/mem.rs"]
 pub mod mem;
-pub mod i2c;
 #[cfg(feature = "target_kasli_soc")]
 pub mod io_expander;
 

src/runtime/src/i2c.rs

@@ -0,0 +1,90 @@
+use libboard_zynq;
+use crate::artiq_raise;
+
+pub static mut I2C_BUS: Option<libboard_zynq::i2c::I2c> = None;
+
+pub extern fn start(busno: i32) {
+    if busno > 0 {
+        artiq_raise!("I2CError", "I2C bus could not be accessed");
+    }
+    unsafe {
+        if (&mut I2C_BUS).as_mut().unwrap().start().is_err() {
+            artiq_raise!("I2CError", "I2C start failed");
+        }
+    }
+}
+
+pub extern fn restart(busno: i32) {
+    if busno > 0 {
+        artiq_raise!("I2CError", "I2C bus could not be accessed");
+    }
+    unsafe {
+        if (&mut I2C_BUS).as_mut().unwrap().restart().is_err() {
+            artiq_raise!("I2CError", "I2C restart failed");
+        }
+    }
+}
+
+pub extern fn stop(busno: i32) {
+    if busno > 0 {
+        artiq_raise!("I2CError", "I2C bus could not be accessed");
+    }
+    unsafe {
+        if (&mut I2C_BUS).as_mut().unwrap().stop().is_err() {
+            artiq_raise!("I2CError", "I2C stop failed");
+        }
+    }
+}
+
+pub extern fn write(busno: i32, data: i32) -> bool {
+    if busno > 0 {
+        artiq_raise!("I2CError", "I2C bus could not be accessed");
+    }
+    unsafe {
+        match (&mut I2C_BUS).as_mut().unwrap().write(data as u8) {
+            Ok(r) => r,
+            Err(_) => artiq_raise!("I2CError", "I2C write failed"),
+        }
+    }
+}
+
+pub extern fn read(busno: i32, ack: bool) -> i32 {
+    if busno > 0 {
+        artiq_raise!("I2CError", "I2C bus could not be accessed");
+    }
+    unsafe {
+        match (&mut I2C_BUS).as_mut().unwrap().read(ack) {
+            Ok(r) => r as i32,
+            Err(_) => artiq_raise!("I2CError", "I2C read failed"),
+        }
+    }
+}
+
+pub extern fn switch_select(busno: i32, address: i32, mask: i32) {
+    if busno > 0 {
+        artiq_raise!("I2CError", "I2C bus could not be accessed");
+    }
+    let ch = match mask { //decode from mainline, PCA9548-centric API
+        0x00 => None,
+        0x01 => Some(0),
+        0x02 => Some(1),
+        0x04 => Some(2),
+        0x08 => Some(3),
+        0x10 => Some(4),
+        0x20 => Some(5),
+        0x40 => Some(6),
+        0x80 => Some(7),
+        _ => artiq_raise!("I2CError", "switch select supports only one channel")
+    };
+    unsafe {
+        if (&mut I2C_BUS).as_mut().unwrap().pca954x_select(address as u8, ch).is_err() {
+            artiq_raise!("I2CError", "switch select failed");
+        }
+    }
+}
+
+pub fn init() {
+    let mut i2c = libboard_zynq::i2c::I2c::i2c0();
+    i2c.init().expect("I2C bus initialization failed");
+    unsafe { I2C_BUS = Some(i2c) };
+}

src/runtime/src/main.rs

@@ -21,7 +21,7 @@ use nb;
 use void::Void;
 use libconfig::Config;
 use libcortex_a9::l2c::enable_l2_cache;
-use libboard_artiq::{logger, identifier_read, pl, i2c, io_expander};
+use libboard_artiq::{logger, identifier_read, pl, io_expander};
 
 const ASYNC_ERROR_COLLISION: u8 = 1 << 0;
 const ASYNC_ERROR_BUSY: u8 = 1 << 1;
@@ -45,6 +45,7 @@ mod panic;
 mod mgmt;
 mod analyzer;
 mod irq;
+mod i2c;
 
 static mut SEEN_ASYNC_ERRORS: u8 = 0;
 
@@ -114,21 +115,17 @@ pub fn main_core0() {
 
     #[cfg(feature = "target_kasli_soc")]
     {
-        let (mut io_expander0, mut io_expander1) = (io_expander::IoExpander::new(0).unwrap(), io_expander::IoExpander::new(1).unwrap());
+        let i2c = unsafe { (&mut i2c::I2C_BUS).as_mut().unwrap() };
-        io_expander0.init().expect("I2C I/O expander #0 initialization failed");
+        for expander_i in 0..2 {
-        io_expander1.init().expect("I2C I/O expander #1 initialization failed");
+            let mut io_expander = io_expander::IoExpander::new(i2c, expander_i).unwrap();
-
+            io_expander.init().expect("I2C I/O expander #0 initialization failed");
-        // Actively drive TX_DISABLE to false on SFP0..3
+            // Actively drive TX_DISABLE to false on SFP0..3
-        io_expander0.set_oe(0, 1 << 1).unwrap();
+            io_expander.set_oe(0, 1 << 1).unwrap();
-        io_expander0.set_oe(1, 1 << 1).unwrap();
+            io_expander.set_oe(1, 1 << 1).unwrap();
-        io_expander1.set_oe(0, 1 << 1).unwrap();
+            io_expander.set(0, 1, false);
-        io_expander1.set_oe(1, 1 << 1).unwrap();
+            io_expander.set(1, 1, false);
-        io_expander0.set(0, 1, false);
+            io_expander.service().unwrap();
-        io_expander0.set(1, 1, false);
+        }
-        io_expander1.set(0, 1, false);
-        io_expander1.set(1, 1, false);
-        io_expander0.service().unwrap();
-        io_expander1.service().unwrap();
     }
 
     let cfg = match Config::new() {

src/satman/src/main.rs

@@ -18,12 +18,11 @@ extern crate unwind;
 
 extern crate alloc;
 
-use libboard_zynq::{timer::GlobalTimer, time::Milliseconds, print, println, mpcore, gic, stdio};
+use libboard_zynq::{i2c::I2c, timer::GlobalTimer, time::Milliseconds, print, println, mpcore, gic, stdio};
 use libsupport_zynq::ram;
 #[cfg(has_si5324)]
 use libboard_artiq::si5324;
-use libboard_artiq::{pl::csr, drtio_routing, drtioaux, logger,
+use libboard_artiq::{pl::csr, drtio_routing, drtioaux, logger, identifier_read, io_expander};
-                     identifier_read, i2c, io_expander};
 use libcortex_a9::{spin_lock_yield, interrupt_handler, regs::{MPIDR, SP}, notify_spin_lock, asm, l2c::enable_l2_cache};
 use libregister::{RegisterW, RegisterR};
 #[cfg(feature = "target_kasli_soc")]
@@ -85,7 +84,7 @@ macro_rules! forward {
 
 fn process_aux_packet(_repeaters: &mut [repeater::Repeater],
         _routing_table: &mut drtio_routing::RoutingTable, _rank: &mut u8,
-        packet: drtioaux::Packet, timer: &mut GlobalTimer) -> Result<(), drtioaux::Error> {
+        packet: drtioaux::Packet, timer: &mut GlobalTimer, i2c: &mut I2c) -> Result<(), drtioaux::Error> {
     // In the code below, *_chan_sel_write takes an u8 if there are fewer than 256 channels,
     // and u16 otherwise; hence the `as _` conversion.
     match packet {
@@ -254,22 +253,22 @@ fn process_aux_packet(_repeaters: &mut [repeater::Repeater],
 
         drtioaux::Packet::I2cStartRequest { destination: _destination, busno: _busno } => {
             forward!(_routing_table, _destination, *_rank, _repeaters, &packet, timer);
-            let succeeded = i2c::start().is_ok();
+            let succeeded = i2c.start().is_ok();
             drtioaux::send(0, &drtioaux::Packet::I2cBasicReply { succeeded: succeeded })
         }
         drtioaux::Packet::I2cRestartRequest { destination: _destination, busno: _busno } => {
             forward!(_routing_table, _destination, *_rank, _repeaters, &packet, timer);
-            let succeeded = i2c::restart().is_ok();
+            let succeeded = i2c.restart().is_ok();
             drtioaux::send(0, &drtioaux::Packet::I2cBasicReply { succeeded: succeeded })
         }
         drtioaux::Packet::I2cStopRequest { destination: _destination, busno: _busno } => {
             forward!(_routing_table, _destination, *_rank, _repeaters, &packet, timer);
-            let succeeded = i2c::stop().is_ok();
+            let succeeded = i2c.stop().is_ok();
             drtioaux::send(0, &drtioaux::Packet::I2cBasicReply { succeeded: succeeded })
         }
         drtioaux::Packet::I2cWriteRequest { destination: _destination, busno: _busno, data } => {
             forward!(_routing_table, _destination, *_rank, _repeaters, &packet, timer);
-            match i2c::write(data as i32) {
+            match i2c.write(data) {
                 Ok(ack) => drtioaux::send(0,
                     &drtioaux::Packet::I2cWriteReply { succeeded: true, ack: ack }),
                 Err(_) => drtioaux::send(0,
@@ -278,9 +277,9 @@ fn process_aux_packet(_repeaters: &mut [repeater::Repeater],
         }
         drtioaux::Packet::I2cReadRequest { destination: _destination, busno: _busno, ack } => {
             forward!(_routing_table, _destination, *_rank, _repeaters, &packet, timer);
-            match i2c::read(ack) {
+            match i2c.read(ack) {
                 Ok(data) => drtioaux::send(0,
-                    &drtioaux::Packet::I2cReadReply { succeeded: true, data: data as u8 }),
+                    &drtioaux::Packet::I2cReadReply { succeeded: true, data: data }),
                 Err(_) => drtioaux::send(0,
                     &drtioaux::Packet::I2cReadReply { succeeded: false, data: 0xff })
             }
@@ -299,7 +298,7 @@ fn process_aux_packet(_repeaters: &mut [repeater::Repeater],
                 0x80 => Some(7),
                 _ => { return drtioaux::send(0, &drtioaux::Packet::I2cBasicReply { succeeded: false }); }
             };
-            let succeeded = i2c::pca954x_select(address as i32, ch).is_ok();
+            let succeeded = i2c.pca954x_select(address, ch).is_ok();
             drtioaux::send(0, &drtioaux::Packet::I2cBasicReply { succeeded: succeeded })
         }
 
@@ -340,11 +339,11 @@ fn process_aux_packet(_repeaters: &mut [repeater::Repeater],
 
 fn process_aux_packets(repeaters: &mut [repeater::Repeater],
         routing_table: &mut drtio_routing::RoutingTable, rank: &mut u8, 
-        timer: &mut GlobalTimer) {
+        timer: &mut GlobalTimer, i2c: &mut I2c) {
     let result =
         drtioaux::recv(0).and_then(|packet| {
             if let Some(packet) = packet {
-                process_aux_packet(repeaters, routing_table, rank, packet, timer)
+                process_aux_packet(repeaters, routing_table, rank, packet, timer, i2c)
             } else {
                 Ok(())
             }
@@ -448,32 +447,24 @@ pub extern fn main_core0() -> i32 {
 
     ram::init_alloc_core0();
 
-    i2c::init();
+    let mut i2c = I2c::i2c0();
-
+    i2c.init().expect("I2C initialization failed");
     #[cfg(feature = "target_kasli_soc")]
     {
-        let (mut io_expander0, mut io_expander1) = (io_expander::IoExpander::new(0).unwrap(), io_expander::IoExpander::new(1).unwrap());
+        for expander_i in 0..2 {
-        io_expander0.init().expect("I2C I/O expander #0 initialization failed");
+            let mut io_expander = io_expander::IoExpander::new(&mut i2c, expander_i).unwrap();
-        io_expander1.init().expect("I2C I/O expander #1 initialization failed");
+            io_expander.init().expect("I2C I/O expander #0 initialization failed");
-
+            // Actively drive TX_DISABLE to false on SFP0..3
-        // Actively drive TX_DISABLE to false on SFP0..3
+            io_expander.set_oe(0, 1 << 1).unwrap();
-        io_expander0.set_oe(0, 1 << 1).unwrap();
+            io_expander.set_oe(1, 1 << 1).unwrap();
-        io_expander0.set_oe(1, 1 << 1).unwrap();
+            io_expander.set(0, 1, false);
-        io_expander1.set_oe(0, 1 << 1).unwrap();
+            io_expander.set(1, 1, false);
-        io_expander1.set_oe(1, 1 << 1).unwrap();
+            io_expander.service().unwrap();
-        io_expander0.set(0, 1, false);
+        }
-        io_expander0.set(1, 1, false);
-        io_expander1.set(0, 1, false);
-        io_expander1.set(1, 1, false);
-        io_expander0.service().unwrap();
-        io_expander1.service().unwrap();
     }
 
     #[cfg(has_si5324)]
-    {
+    si5324::setup(&mut i2c, &SI5324_SETTINGS, si5324::Input::Ckin1, &mut timer).expect("cannot initialize Si5324");
-        let mut i2c = unsafe { (&mut i2c::I2C_BUS).as_mut().unwrap() };
-        si5324::setup(&mut i2c, &SI5324_SETTINGS, si5324::Input::Ckin1, &mut timer).expect("cannot initialize Si5324");
-    }
 
     timer.delay_us(100_000);
     info!("Switching SYS clocks...");
@@ -517,7 +508,6 @@ pub extern fn main_core0() -> i32 {
         info!("uplink is up, switching to recovered clock");
         #[cfg(has_siphaser)]
         {
-            let mut i2c = unsafe { (&mut i2c::I2C_BUS).as_mut().unwrap() };
             si5324::siphaser::select_recovered_clock(&mut i2c, true, &mut timer).expect("failed to switch clocks");
             si5324::siphaser::calibrate_skew(&mut timer).expect("failed to calibrate skew");
         }
@@ -528,7 +518,7 @@ pub extern fn main_core0() -> i32 {
 
         while drtiosat_link_rx_up() {
             drtiosat_process_errors();
-            process_aux_packets(&mut repeaters, &mut routing_table, &mut rank, &mut timer);
+            process_aux_packets(&mut repeaters, &mut routing_table, &mut rank, &mut timer, &mut i2c);
             #[allow(unused_mut)]
             for mut rep in repeaters.iter_mut() {
                 rep.service(&routing_table, rank, &mut timer);
@@ -552,10 +542,7 @@ pub extern fn main_core0() -> i32 {
         drtiosat_tsc_loaded();
         info!("uplink is down, switching to local oscillator clock");
         #[cfg(has_siphaser)]
-        {
+        si5324::siphaser::select_recovered_clock(&mut i2c, false, &mut timer).expect("failed to switch clocks");
-            let mut i2c = unsafe { (&mut i2c::I2C_BUS).as_mut().unwrap() };
-            si5324::siphaser::select_recovered_clock(&mut i2c, false, &mut timer).expect("failed to switch clocks");
-        }
     }
 }