From 5d63489080e49b9a3e81c4c085ff1dd29905fa74 Mon Sep 17 00:00:00 2001 From: Sebastien Bourdeauducq Date: Mon, 19 Jun 2017 14:22:59 +0800 Subject: [PATCH] i2c,spi: add busno error detection --- artiq/coredevice/exceptions.py | 8 +++ artiq/coredevice/i2c.py | 6 +- artiq/firmware/ksupport/nrt_bus.rs | 36 +++++++++++- artiq/firmware/libboard/i2c.rs | 44 +++++++++----- artiq/firmware/libboard/si5324.rs | 32 +++++----- artiq/firmware/libboard/spi.rs | 38 +++++++----- artiq/firmware/libproto/kernel_proto.rs | 8 ++- artiq/firmware/runtime/kern_hwreq.rs | 78 ++++++++++++++++--------- 8 files changed, 168 insertions(+), 82 deletions(-) diff --git a/artiq/coredevice/exceptions.py b/artiq/coredevice/exceptions.py index 4debf2318..9381bb729 100644 --- a/artiq/coredevice/exceptions.py +++ b/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 diff --git a/artiq/coredevice/i2c.py b/artiq/coredevice/i2c.py index 8fbe56ceb..e647cfd41 100644 --- a/artiq/coredevice/i2c.py +++ b/artiq/coredevice/i2c.py @@ -1,10 +1,6 @@ from artiq.language.core import syscall, kernel from artiq.language.types import TBool, TInt32, TNone - - -class I2CError(Exception): - """Raised when a I2C transaction fails.""" - pass +from artiq.coredevice.exceptions import I2CError @syscall(flags={"nounwind", "nowrite"}) diff --git a/artiq/firmware/ksupport/nrt_bus.rs b/artiq/firmware/ksupport/nrt_bus.rs index 1f05f189b..fedb41a64 100644 --- a/artiq/firmware/ksupport/nrt_bus.rs +++ b/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 } } diff --git a/artiq/firmware/libboard/i2c.rs b/artiq/firmware/libboard/i2c.rs index 4f9c6b7b3..ef4f36051 100644 --- a/artiq/firmware/libboard/i2c.rs +++ b/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 { + 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 { + 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 { Err(()) } #[cfg(not(has_i2c))] -pub fn read(_busno: u8, _ack: bool) { 0xff } +pub fn read(_busno: u8, _ack: bool) -> Result { Err(()) } diff --git a/artiq/firmware/libboard/si5324.rs b/artiq/firmware/libboard/si5324.rs index aee5efb0c..17119b1b9 100644 --- a/artiq/firmware/libboard/si5324.rs +++ b/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); - if !i2c::write(BUSNO, (0x74 << 1)) { + i2c::start(BUSNO).unwrap(); + 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 Result<()> { - i2c::start(BUSNO); - if !i2c::write(BUSNO, (ADDRESS << 1)) { + i2c::start(BUSNO).unwrap(); + 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 { - i2c::start(BUSNO); - if !i2c::write(BUSNO, (ADDRESS << 1)) { + i2c::start(BUSNO).unwrap(); + 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); - if !i2c::write(BUSNO, (ADDRESS << 1) | 1) { + i2c::restart(BUSNO).unwrap(); + if !i2c::write(BUSNO, (ADDRESS << 1) | 1).unwrap() { return Err("Si5324 failed to ack read address") } - let val = i2c::read(BUSNO, false); - i2c::stop(BUSNO); + let val = i2c::read(BUSNO, false).unwrap(); + i2c::stop(BUSNO).unwrap(); Ok(val) } diff --git a/artiq/firmware/libboard/spi.rs b/artiq/firmware/libboard/spi.rs index afacb652a..3c0611311 100644 --- a/artiq/firmware/libboard/spi.rs +++ b/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 { - unsafe { - csr::converter_spi::data_read_read() +pub fn read(busno: u8) -> Result { + if busno != 0 { + 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 { Err(()) } diff --git a/artiq/firmware/libproto/kernel_proto.rs b/artiq/firmware/libproto/kernel_proto.rs index 22fd38a18..809bbdc47 100644 --- a/artiq/firmware/libproto/kernel_proto.rs +++ b/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) diff --git a/artiq/firmware/runtime/kern_hwreq.rs b/artiq/firmware/runtime/kern_hwreq.rs index ca21b5ed7..9312a0d12 100644 --- a/artiq/firmware/runtime/kern_hwreq.rs +++ b/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_xfer(_busno: u32, _chip_select: u16, _write_length: u8, _read_length: u8) {} - pub fn write(_busno: u32, _data: u32) {} - pub fn read(_busno: u32) -> u32 { 0 } + pub fn set_config(_busno: u32, _flags: u8, _write_div: u8, _read_div: u8) -> Result<(), ()> { + Err(()) + } + + 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 { + 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 { 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 } &kern::I2cStartRequest { busno } => { - board::i2c::start(busno); - kern_acknowledge() + let succeeded = board::i2c::start(busno).is_ok(); + kern_send(io, &kern::I2cBasicReply { succeeded: succeeded }) } &kern::I2cStopRequest { busno } => { - board::i2c::stop(busno); - kern_acknowledge() + let succeeded = board::i2c::stop(busno).is_ok(); + kern_send(io, &kern::I2cBasicReply { succeeded: succeeded }) } &kern::I2cWriteRequest { busno, data } => { - let ack = board::i2c::write(busno, data); - kern_send(io, &kern::I2cWriteReply { ack: ack }) + match board::i2c::write(busno, data) { + 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); - kern_send(io, &kern::I2cReadReply { data: data }) + match board::i2c::read(busno, ack) { + 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); - kern_acknowledge() + let succeeded = spi::set_config(busno, flags, write_div, read_div).is_ok(); + 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); - kern_acknowledge() + let succeeded = spi::set_xfer(busno, chip_select, write_length, read_length).is_ok(); + kern_send(io, &kern::SpiBasicReply { succeeded: succeeded }) } &kern::SpiWriteRequest { busno, data } => { - spi::write(busno, data); - kern_acknowledge() + let succeeded = spi::write(busno, data).is_ok(); + kern_send(io, &kern::SpiBasicReply { succeeded: succeeded }) } &kern::SpiReadRequest { busno } => { - let data = spi::read(busno); - kern_send(io, &kern::SpiReadReply { data: data }) + match spi::read(busno) { + Ok(data) => kern_send(io, &kern::SpiReadReply { succeeded: true, data: data }), + Err(_) => kern_send(io, &kern::SpiReadReply { succeeded: false, data: 0 }) + } }, _ => return Ok(false)