diff --git a/drv/sidecar-front-io/src/transceivers.rs b/drv/sidecar-front-io/src/transceivers.rs index bf84c3ced..d339f0af8 100644 --- a/drv/sidecar-front-io/src/transceivers.rs +++ b/drv/sidecar-front-io/src/transceivers.rs @@ -677,13 +677,22 @@ impl core::ops::Index for LogicalPortFailureTypes { #[derive(Copy, Clone)] pub struct PortI2CStatus { + pub rdata_fifo_empty: bool, + pub wdata_fifo_empty: bool, pub done: bool, pub error: FpgaI2CFailure, } impl PortI2CStatus { pub fn new(status: u8) -> Self { + // Use QSFP::PORT0 for constants, since they're all identical Self { + rdata_fifo_empty: (status + & Reg::QSFP::PORT0_STATUS::RDATA_FIFO_EMPTY) + != 0, + wdata_fifo_empty: (status + & Reg::QSFP::PORT0_STATUS::WDATA_FIFO_EMPTY) + != 0, done: (status & Reg::QSFP::PORT0_STATUS::BUSY) == 0, error: FpgaI2CFailure::try_from(status).unwrap_lite(), } @@ -1028,13 +1037,13 @@ impl Transceivers { .read(Addr::QSFP_PORT0_STATUS as u16 + port_loc.port.0 as u16) } - /// Get `buf.len()` bytes of data, where the first byte is port status and - /// trailing bytes are the I2C read buffer for a `port`. The buffer stores - /// data from the last I2C read transaction done and thus only the number of - /// bytes read will be valid in the buffer. + /// Returns a PortI2CStatus and will fill `buf.len()` bytes of data with the + /// I2C read buffer for a `port`. The buffer stores data from the last I2C + /// read transaction done and thus only the number of bytes read will be + /// valid in the buffer. /// /// Poll the status register for a port until an I2C transaction is done. - /// Upon completion, pass the status byte back alongside the read data. The + /// Upon completion, return the status struct back and the read data. The /// read data buffer stores I2C read data from the last transaction so the /// FIFO will only contain that many bytes. The FPGA automatically clears /// the read data FIFO before starting a new read, so there's no need for @@ -1043,14 +1052,23 @@ impl Transceivers { &self, port: P, buf: &mut [u8], - ) -> Result<(), FpgaError> { + ) -> Result { let port_loc = port.into(); - buf[0] = self.get_i2c_status(port_loc)?; - self.fpga(port_loc.controller).read_bytes( - ReadOp::ReadNoAddrIncr, - Addr::QSFP_PORT0_I2C_DATA as u16 + port_loc.port.0 as u16, - &mut buf[1..], - ) + loop { + let status_byte = self.get_i2c_status(port_loc)?; + let status = PortI2CStatus::new(status_byte); + + if status.done { + self.fpga(port_loc.controller).read_bytes( + ReadOp::ReadNoAddrIncr, + Addr::QSFP_PORT0_I2C_DATA as u16 + port_loc.port.0 as u16, + buf, + )?; + return Ok(status); + } + + userlib::hl::sleep_for(1); + } } /// Write `buf.len()` bytes of data into the I2C write buffer of the `mask` diff --git a/drv/transceivers-server/src/main.rs b/drv/transceivers-server/src/main.rs index 29f419a83..2618526a3 100644 --- a/drv/transceivers-server/src/main.rs +++ b/drv/transceivers-server/src/main.rs @@ -16,7 +16,9 @@ use drv_fpga_api::FpgaError; use drv_i2c_devices::pca9956b::Error; use drv_sidecar_front_io::{ leds::{FullErrorSummary, Leds}, - transceivers::{LogicalPort, LogicalPortMask, Transceivers}, + transceivers::{ + FpgaI2CFailure, LogicalPort, LogicalPortMask, Transceivers, + }, Reg, }; use drv_sidecar_seq_api::{SeqError, Sequencer}; @@ -253,28 +255,24 @@ impl ServerImpl { #[derive(Copy, Clone, FromBytes, AsBytes)] #[repr(C)] - struct StatusAndTemperature { - status: u8, + struct Temperature { temperature: zerocopy::I16, } - loop { - let mut out = StatusAndTemperature::new_zeroed(); - self.transceivers - .get_i2c_status_and_read_buffer(port, out.as_bytes_mut())?; - if out.status & Reg::QSFP::PORT0_STATUS::BUSY == 0 { - if out.status & Reg::QSFP::PORT0_STATUS::ERROR != 0 { - return Err(FpgaError::ImplError(out.status)); - } else { - // "Internally measured free side device temperatures are - // represented as a 16-bit signed twos complement value in - // increments of 1/256 degrees Celsius" - // - // - SFF-8636 rev 2.10a, Section 6.2.4 - return Ok(Celsius(out.temperature.get() as f32 / 256.0)); - } - } - userlib::hl::sleep_for(1); + let mut out = Temperature::new_zeroed(); + let status = self + .transceivers + .get_i2c_status_and_read_buffer(port, out.as_bytes_mut())?; + + if status.error == FpgaI2CFailure::NoError { + // "Internally measured free side device temperatures are + // represented as a 16-bit signed twos complement value in + // increments of 1/256 degrees Celsius" + // + // - SFF-8636 rev 2.10a, Section 6.2.4 + Ok(Celsius(out.temperature.get() as f32 / 256.0)) + } else { + Err(FpgaError::ImplError(status.error as u8)) } } @@ -287,20 +285,15 @@ impl ServerImpl { return Err(FpgaError::CommsError); } - let mut out = [0u8; 2]; // [status, SFF8024Identifier] + let mut out = [0u8; 1]; // [SFF8024Identifier] // Wait for the I2C transaction to complete - loop { - self.transceivers - .get_i2c_status_and_read_buffer(port, &mut out)?; - if out[0] & Reg::QSFP::PORT0_STATUS::BUSY == 0 { - break; - } - userlib::hl::sleep_for(1); - } + let status = self + .transceivers + .get_i2c_status_and_read_buffer(port, &mut out)?; - if out[0] & Reg::QSFP::PORT0_STATUS::ERROR == 0 { - match out[1] { + if status.error == FpgaI2CFailure::NoError { + match out[0] { 0x1E => Ok(ManagementInterface::Cmis), 0x0D | 0x11 => Ok(ManagementInterface::Sff8636), i => Ok(ManagementInterface::Unknown(i)), @@ -308,7 +301,7 @@ impl ServerImpl { } else { // TODO: how should we handle this? // Right now, we'll retry on the next pass through the loop. - Err(FpgaError::ImplError(out[0])) + Err(FpgaError::ImplError(status.error as u8)) } } diff --git a/drv/transceivers-server/src/udp.rs b/drv/transceivers-server/src/udp.rs index 38efe7711..cd0ec26cf 100644 --- a/drv/transceivers-server/src/udp.rs +++ b/drv/transceivers-server/src/udp.rs @@ -19,7 +19,7 @@ use userlib::UnwrapLite; use crate::{FrontIOStatus, ServerImpl}; use drv_sidecar_front_io::transceivers::{ FpgaI2CFailure, LogicalPort, LogicalPortFailureTypes, LogicalPortMask, - ModuleResult, ModuleResultNoFailure, ModuleResultSlim, PortI2CStatus, + ModuleResult, ModuleResultNoFailure, ModuleResultSlim, }; use task_net_api::*; use transceiver_messages::{ @@ -1156,48 +1156,34 @@ impl ServerImpl { let mut failure_types = LogicalPortFailureTypes::default(); for port in result.success().to_indices() { - // The status register is contiguous with the output buffer, so - // we'll read them all in a single pass. This should normally - // terminate with a single read, since I2C is faster than Hubris - // IPC. - let mut buf = [0u8; 129]; + let mut buf = [0u8; 128]; let port_loc = port.get_physical_location(); - loop { - // If we have not encountered any errors, keep pulling full - // status + buffer payloads. - if self - .transceivers - .get_i2c_status_and_read_buffer( - port_loc, - &mut buf[0..(buf_len + 1)], - ) - .is_err() - { - error.set(port); - break; - }; - - let status = PortI2CStatus::new(buf[0]); - // Use QSFP::PORT0 for constants, since they're all identical - if status.done { - // Check error mask - if status.error != FpgaI2CFailure::NoError { - // Record which port the error ocurred at so we can - // give the host a more meaningful error. - failure.set(port); - failure_types.0[port.0 as usize] = status.error - } else { - // Add data to payload - success.set(port); - let end_idx = idx + buf_len; - out[idx..end_idx].copy_from_slice(&buf[1..][..buf_len]); - idx = end_idx; - } + // If we have not encountered any errors, keep pulling full + // status + buffer payloads. + let status = match self + .transceivers + .get_i2c_status_and_read_buffer(port_loc, &mut buf[0..buf_len]) + { + Ok(status) => status, + Err(_) => { + error.set(port); break; } + }; - userlib::hl::sleep_for(1); + // Check error mask + if status.error != FpgaI2CFailure::NoError { + // Record which port the error ocurred at so we can + // give the host a more meaningful error. + failure.set(port); + failure_types.0[port.0 as usize] = status.error + } else { + // Add data to payload + success.set(port); + let end_idx = idx + buf_len; + out[idx..end_idx].copy_from_slice(&buf[..buf_len]); + idx = end_idx; } } let mut final_result =