From 301281cba82529e9ef0017bc76842eedd8ee8262 Mon Sep 17 00:00:00 2001 From: 9names <60134748+9names@users.noreply.github.com> Date: Thu, 2 Sep 2021 22:34:12 +1000 Subject: [PATCH] i2c lockup fix (#94) * Fix check for empty tx-fifo * Add read trait * Move from asserts to Err for read/write errors --- rp2040-hal/src/i2c.rs | 127 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 116 insertions(+), 11 deletions(-) diff --git a/rp2040-hal/src/i2c.rs b/rp2040-hal/src/i2c.rs index ef3c3c1..640f86e 100644 --- a/rp2040-hal/src/i2c.rs +++ b/rp2040-hal/src/i2c.rs @@ -14,7 +14,7 @@ use crate::{ typelevel::Sealed, }; use embedded_time::rate::Hertz; -use hal::blocking::i2c::{Write, WriteRead}; +use hal::blocking::i2c::{Read, Write, WriteRead}; use rp2040_pac::{I2C0, I2C1, RESETS}; /// I2C error @@ -23,6 +23,14 @@ use rp2040_pac::{I2C0, I2C1, RESETS}; pub enum Error { /// I2C abort with error Abort(u32), + /// User passed in a read buffer that was 0 or >255 length + InvalidReadBufferLength(usize), + /// User passed in a write buffer that was 0 or >255 length + InvalidWriteBufferLength(usize), + /// Target i2c address is out of range + AddressOutOfRange(u8), + /// Target i2c address is reserved + AddressReserved(u8), } /// SCL pin @@ -73,6 +81,8 @@ pub struct I2C { pins: Pins, } +const TX_FIFO_SIZE: u8 = 16; + fn i2c_reserved_addr(addr: u8) -> bool { (addr & 0x78) == 0 || (addr & 0x78) == 0x78 } @@ -176,15 +186,38 @@ macro_rules! hal { } } + impl I2C<$I2CX, PINS> { + /// Number of bytes currently in the TX FIFO + #[inline] + fn tx_fifo_used(&self) -> u8 { + self.i2c.ic_txflr.read().txflr().bits() + } + + /// Remaining capacity in the TX FIFO + #[inline] + fn tx_fifo_free(&self) -> u8 { + TX_FIFO_SIZE - self.tx_fifo_used() + } + + /// TX FIFO is at capacity + #[inline] + fn tx_fifo_full(&self) -> bool { + self.tx_fifo_free() == 0 + } + } + impl Write for I2C<$I2CX, PINS> { type Error = Error; fn write(&mut self, addr: u8, bytes: &[u8]) -> Result<(), Error> { // TODO support transfers of more than 255 bytes - assert!(bytes.len() < 256 && bytes.len() > 0); - - assert!(addr < 0x80); - assert!(!i2c_reserved_addr(addr)); + if (bytes.len() > 255 || bytes.len() == 0) { + return Err(Error::InvalidWriteBufferLength(bytes.len())); + } else if addr >= 0x80 { + return Err(Error::AddressOutOfRange(addr)); + } else if i2c_reserved_addr(addr) { + return Err(Error::AddressReserved(addr)); + } self.i2c.ic_enable.write(|w| w.enable().disabled()); self.i2c @@ -252,11 +285,15 @@ macro_rules! hal { fn write_read(&mut self, addr: u8, bytes: &[u8], buffer: &mut [u8]) -> Result<(), Error> { // TODO support transfers of more than 255 bytes - assert!(bytes.len() < 256 && bytes.len() > 0); - assert!(buffer.len() < 256 && buffer.len() > 0); - - assert!(addr < 0x80); - assert!(!i2c_reserved_addr(addr)); + if (bytes.len() > 255 || bytes.len() == 0) { + return Err(Error::InvalidWriteBufferLength(bytes.len())); + } else if (buffer.len() > 255 || buffer.len() == 0) { + return Err(Error::InvalidReadBufferLength(buffer.len())); + } else if addr >= 0x80 { + return Err(Error::AddressOutOfRange(addr)); + } else if i2c_reserved_addr(addr) { + return Err(Error::AddressReserved(addr)); + } self.i2c.ic_enable.write(|w| w.enable().disabled()); self.i2c @@ -309,7 +346,8 @@ macro_rules! hal { let first = i == 0; let last = i == bytes.len() - 1; - while 16 - self.i2c.ic_txflr.read().txflr().bits() > 0 {} + // wait until there is space in the FIFO to write the next byte + while self.tx_fifo_full() {} self.i2c.ic_data_cmd.write(|w| { if first { @@ -346,6 +384,73 @@ macro_rules! hal { } } } + + impl Read for I2C<$I2CX, PINS> { + type Error = Error; + + fn read(&mut self, addr: u8, buffer: &mut [u8]) -> Result<(), Error> { + // TODO support transfers of more than 255 bytes + if (buffer.len() > 255 || buffer.len() == 0) { + return Err(Error::InvalidReadBufferLength(buffer.len())); + } else if addr >= 0x80 { + return Err(Error::AddressOutOfRange(addr)); + } else if i2c_reserved_addr(addr) { + return Err(Error::AddressReserved(addr)); + } + + self.i2c.ic_enable.write(|w| w.enable().disabled()); + self.i2c + .ic_tar + .write(|w| unsafe { w.ic_tar().bits(addr as u16) }); + self.i2c.ic_enable.write(|w| w.enable().enabled()); + + let mut abort = false; + let mut abort_reason = 0; + + let lastindex = buffer.len() -1; + for (i, byte) in buffer.iter_mut().enumerate() { + let first = i == 0; + let last = i == lastindex; + + // wait until there is space in the FIFO to write the next byte + while self.tx_fifo_full() {} + + self.i2c.ic_data_cmd.write(|w| { + if first { + w.restart().enable(); + } else { + w.restart().disable(); + } + + if last { + w.stop().enable(); + } else { + w.stop().disable(); + } + + w.cmd().read() + }); + + while !abort && self.i2c.ic_rxflr.read().bits() == 0 { + abort_reason = self.i2c.ic_tx_abrt_source.read().bits(); + abort = self.i2c.ic_clr_tx_abrt.read().bits() > 0; + } + + if abort { + break; + } + + *byte = self.i2c.ic_data_cmd.read().dat().bits(); + } + + if abort { + Err(Error::Abort(abort_reason)) + } else { + Ok(()) + } + } + } + )+ } }