From bc82dbd15774f4e5e8bd5dc07782a6bb12f0dd11 Mon Sep 17 00:00:00 2001 From: bunnie Date: Thu, 6 May 2021 05:05:17 +0800 Subject: [PATCH] transition i2c api on codec, finally found the damn race condition --- services/codec/src/backend/tlv320aic3100.rs | 125 +++----------------- services/llio/src/api.rs | 3 +- services/llio/src/i2c/hardware.rs | 14 ++- services/llio/src/i2c/mod.rs | 2 +- services/llio/src/main.rs | 2 +- services/rtc/src/main.rs | 2 +- services/shellchat/src/cmds/audio.rs | 4 +- 7 files changed, 34 insertions(+), 118 deletions(-) diff --git a/services/codec/src/backend/tlv320aic3100.rs b/services/codec/src/backend/tlv320aic3100.rs index d964bc5c8..495714879 100644 --- a/services/codec/src/backend/tlv320aic3100.rs +++ b/services/codec/src/backend/tlv320aic3100.rs @@ -3,20 +3,13 @@ use utralib::generated::*; use xous::MemoryRange; use susres::{RegManager, RegOrField, SuspendResume}; -use core::sync::atomic::{AtomicU8, AtomicBool, Ordering}; -use llio::{I2cStatus, I2cTransaction}; +use llio::I2cStatus; use crate::api::*; use num_traits::*; pub const TLV320AIC3100_I2C_ADR: u8 = 0b0011_000; const I2C_TIMEOUT: u32 = 50; -static I2C_PENDING: AtomicBool = AtomicBool::new(false); -static I2C_READ: [AtomicU8; 8] = [AtomicU8::new(0), AtomicU8::new(0), AtomicU8::new(0), AtomicU8::new(0), - AtomicU8::new(0), AtomicU8::new(0), AtomicU8::new(0), AtomicU8::new(0), ]; -static I2C_READLEN: AtomicU8 = AtomicU8::new(0); - - pub struct Codec { csr: utralib::CSR, fifo: MemoryRange, @@ -33,28 +26,6 @@ pub struct Codec { conn: xous::CID, } -fn i2c_callback(trans: I2cTransaction) { - if trans.status == I2cStatus::ResponseReadOk { - I2C_PENDING.store(false, Ordering::Relaxed); - - if let Some(rxbuf) = trans.rxbuf { - let mut len = trans.rxlen as usize; - if len > I2C_READ.len() { - log::error!("Received more bytes via I2C than we can return inside our handler"); - len = I2C_READ.len(); - } - for i in 0..len { - I2C_READ[i].store(rxbuf[i], Ordering::Relaxed); - } - I2C_READLEN.store(len as u8, Ordering::Relaxed); - } else { - log::error!("i2c_callback: no rx data to unpack!") - } - } else if trans.status == I2cStatus::ResponseWriteOk { - I2C_PENDING.store(false, Ordering::Relaxed); - } -} - fn audio_handler(_irq_no: usize, arg: *mut usize) { let codec = unsafe { &mut *(arg as *mut Codec) }; let volatile_audio = codec.fifo.as_mut_ptr() as *mut u32; @@ -112,8 +83,7 @@ impl Codec { ) .expect("couldn't map Audio CSR range"); - let mut llio = llio::Llio::new(xns).expect("can't connect to LLIO"); - llio.hook_i2c_callback(i2c_callback).expect("can't hook I2C callback"); + let llio = llio::Llio::new(xns).expect("can't connect to LLIO"); let mut codec = Codec { csr: CSR::new(csr.as_mut_ptr() as *mut u32), @@ -211,91 +181,30 @@ impl Codec { } fn w(&mut self, adr: u8, data: &[u8]) -> bool { - let mut transaction = I2cTransaction::new(); - let mut txbuf: [u8; llio::I2C_MAX_LEN] = [0; llio::I2C_MAX_LEN]; - txbuf[0] = adr; - for i in 0..data.len() { - txbuf[i+1] = data[i]; - } - transaction.bus_addr = TLV320AIC3100_I2C_ADR; - transaction.txbuf = Some(txbuf); - transaction.txlen = (data.len() + 1) as u32; - transaction.status = I2cStatus::RequestIncoming; - transaction.timeout_ms = I2C_TIMEOUT; - log::trace!("writing to 0x{:x}, {:x?}", adr, data); - let mut sent = false; - while self.llio.poll_i2c_busy().unwrap() { - xous::yield_slice(); - } - let mut retries = 0; - while !sent && retries < 4 { - match self.llio.send_i2c_request(transaction) { - Ok(status) => { - match status { - I2cStatus::ResponseInProgress => { - sent = true; - I2C_PENDING.store(true, Ordering::Relaxed); - }, - I2cStatus::ResponseBusy => sent = false, - _ => {log::error!("try_send_i2c unhandled response"); return false;}, - } + match self.llio.i2c_write_sync(TLV320AIC3100_I2C_ADR, adr, data) { + Ok(status) => { + log::trace!("write returned with status {:?}", status); + match status { + I2cStatus::ResponseWriteOk => true, + I2cStatus::ResponseBusy => false, + _ => {log::error!("try_send_i2c unhandled response: {:?}", status); false}, } - _ => {log::error!("try_send_i2c unhandled error"); return false;} } - retries += 1; + _ => {log::error!("try_send_i2c unhandled error"); false} } - // block until completed - while I2C_PENDING.load(Ordering::Relaxed) { - xous::yield_slice(); - } - log::trace!("write done"); - true } fn r(&mut self, adr: u8, data: &mut[u8]) -> bool { - let mut transaction = I2cTransaction::new(); - let mut txbuf: [u8; llio::I2C_MAX_LEN] = [0; llio::I2C_MAX_LEN]; - txbuf[0] = adr; - transaction.bus_addr = TLV320AIC3100_I2C_ADR; - transaction.txbuf = Some(txbuf); - transaction.txlen = 1; - transaction.status = I2cStatus::RequestIncoming; - transaction.timeout_ms = I2C_TIMEOUT; - - let rxbuf: [u8; llio::I2C_MAX_LEN] = [0; llio::I2C_MAX_LEN]; - transaction.rxbuf = Some(rxbuf); - transaction.rxlen = data.len() as u32; - - let mut sent = false; - while self.llio.poll_i2c_busy().unwrap() { - xous::yield_slice(); - } - let mut retries = 0; - while !sent && retries < 4 { - match self.llio.send_i2c_request(transaction) { - Ok(status) => { - match status { - I2cStatus::ResponseInProgress => { - sent = true; - I2C_PENDING.store(true, Ordering::Relaxed); - }, - I2cStatus::ResponseBusy => sent = false, - _ => {log::error!("try_send_i2c unhandled response"); return false;}, - } + match self.llio.i2c_read_sync(TLV320AIC3100_I2C_ADR, adr, data) { + Ok(status) => { + match status { + I2cStatus::ResponseReadOk => true, + I2cStatus::ResponseBusy => false, + _ => {log::error!("try_send_i2c unhandled response: {:?}", status); false}, } - _ => {log::error!("try_send_i2c unhandled error"); return false;} } - retries += 1; + _ => {log::error!("try_send_i2c unhandled error"); false} } - // block until completed - while I2C_PENDING.load(Ordering::Relaxed) { - xous::yield_slice(); - } - - for i in 0..I2C_READLEN.load(Ordering::Relaxed) as usize { - data[i] = I2C_READ[i].load(Ordering::Relaxed); - } - true } pub fn get_headset_code(&mut self) -> u8 { diff --git a/services/llio/src/api.rs b/services/llio/src/api.rs index f5d080243..7eb0c40a5 100644 --- a/services/llio/src/api.rs +++ b/services/llio/src/api.rs @@ -40,8 +40,7 @@ impl Into for UartType { } /////////////////////// I2C -// TODO: change this to pub(crate) once we've gotten rid of legacy API -pub const I2C_MAX_LEN: usize = 33; +pub (crate) const I2C_MAX_LEN: usize = 33; // a small book-keeping struct used to report back to I2C requestors as to the status of a transaction #[derive(Debug, Copy, Clone, rkyv::Archive, rkyv::Serialize, rkyv::Deserialize, Eq, PartialEq)] pub enum I2cStatus { diff --git a/services/llio/src/i2c/hardware.rs b/services/llio/src/i2c/hardware.rs index 984a79f94..fc4c1b1fc 100644 --- a/services/llio/src/i2c/hardware.rs +++ b/services/llio/src/i2c/hardware.rs @@ -124,13 +124,16 @@ impl I2cStateMachine { } pub fn initiate(&mut self, transaction: I2cTransaction ) -> I2cStatus { - if !self.workqueue.is_empty() { + // state idle means the transaction is done + // listener None means any callback notifications are also done + // workqueue empty means, we have a clear path to do our thing + if self.workqueue.is_empty() && self.state == I2cState::Idle && self.listener == None { + self.checked_initiate(transaction) + } else { match self.workqueue.enqueue(transaction) { Ok(_) => return I2cStatus::ResponseInProgress, _ => return I2cStatus::ResponseBusy, } - } else { - self.checked_initiate(transaction) } } @@ -159,10 +162,12 @@ impl I2cStateMachine { self.error = false; self.timestamp = now; self.transaction = transaction.clone(); + assert!(self.listener == None, "initiating when previous transaction is still in progress!"); match transaction.listener { None => self.listener = None, Some((s0, s1, s2, s3)) => self.listener = Some(xous::SID::from_u32(s0, s1, s2, s3)), } + log::trace!("initiate with listener {:?}", self.listener); if self.transaction.status == I2cStatus::RequestIncoming { self.transaction.status = I2cStatus::ResponseInProgress; @@ -206,10 +211,13 @@ impl I2cStateMachine { fn i2c_followup(&mut self, trans: I2cTransaction) -> Result<(), xous::Error> { if let Some(listener) = self.listener.take() { + log::trace!("followup to listener {:?}: {:?}", listener, trans); let cid = xous::connect(listener).unwrap(); let buf = xous_ipc::Buffer::into_buf(trans).or(Err(xous::Error::InternalError))?; buf.lend(cid, I2cCallback::Result.to_u32().unwrap()).map(|_|())?; unsafe{xous::disconnect(cid).unwrap()}; + } else { + log::trace!("completed with transaction, but no listener! {:?}", trans); }; if let Some(work) = self.workqueue.dequeue() { if self.checked_initiate(work) != I2cStatus::ResponseInProgress { diff --git a/services/llio/src/i2c/mod.rs b/services/llio/src/i2c/mod.rs index 2756ea3fb..fcce8adb9 100644 --- a/services/llio/src/i2c/mod.rs +++ b/services/llio/src/i2c/mod.rs @@ -6,4 +6,4 @@ pub use crate::backend::hosted::*; #[cfg(target_os = "none")] mod hardware; #[cfg(target_os = "none")] -pub use crate::i2c::hardware::*; +pub(crate) use crate::i2c::hardware::*; diff --git a/services/llio/src/main.rs b/services/llio/src/main.rs index c4174bda2..1d03ac779 100644 --- a/services/llio/src/main.rs +++ b/services/llio/src/main.rs @@ -537,7 +537,7 @@ fn xmain() -> ! { let gpio_base = crate::implementation::log_init(); log_server::init_wait().unwrap(); - log::set_max_level(log::LevelFilter::Trace); + log::set_max_level(log::LevelFilter::Info); info!("my PID is {}", xous::process::id()); let xns = xous_names::XousNames::new().unwrap(); diff --git a/services/rtc/src/main.rs b/services/rtc/src/main.rs index 4da1125e5..7c57577ee 100644 --- a/services/rtc/src/main.rs +++ b/services/rtc/src/main.rs @@ -520,7 +520,7 @@ fn xmain() -> ! { use crate::implementation::Rtc; log_server::init_wait().unwrap(); - log::set_max_level(log::LevelFilter::Trace); + log::set_max_level(log::LevelFilter::Info); log::info!("my PID is {}", xous::process::id()); let xns = xous_names::XousNames::new().unwrap(); diff --git a/services/shellchat/src/cmds/audio.rs b/services/shellchat/src/cmds/audio.rs index 78fc795bb..a48443445 100644 --- a/services/shellchat/src/cmds/audio.rs +++ b/services/shellchat/src/cmds/audio.rs @@ -31,7 +31,7 @@ impl Audio { unsafe{ raw_header[i] = (*samples)[i] }; log::trace!("0x{:x}", raw_header[i]); } - /* + log::trace!("setting up audio stream"); codec.setup_8k_stream().expect("couldn't set the CODEC to expected defaults"); log::trace!("getting a callback ID"); @@ -39,7 +39,7 @@ impl Audio { log::trace!("hooking frame callback"); codec.hook_frame_callback(0xDEAD_BEEF, callback_conn).unwrap(); // any non-handled IDs get routed to our callback port log::trace!("returning from setup"); - */ + Audio { codec, sample,