From c1970512f3a71686ba7b3b8bfea50a9802879547 Mon Sep 17 00:00:00 2001 From: Aaron Hartwig Date: Fri, 12 Jan 2024 17:29:15 -0600 Subject: [PATCH] add enhanced failure and error reporting --- drv/sidecar-front-io/src/transceivers.rs | 149 ++++++++++-- drv/transceivers-api/src/lib.rs | 1 + drv/transceivers-server/src/udp.rs | 284 ++++++++++++++++++----- 3 files changed, 349 insertions(+), 85 deletions(-) diff --git a/drv/sidecar-front-io/src/transceivers.rs b/drv/sidecar-front-io/src/transceivers.rs index 48044d1542..28acca797a 100644 --- a/drv/sidecar-front-io/src/transceivers.rs +++ b/drv/sidecar-front-io/src/transceivers.rs @@ -6,6 +6,7 @@ use crate::{Addr, Reg}; use drv_fpga_api::{FpgaError, FpgaUserDesign, WriteOp}; use drv_transceivers_api::{ModuleStatus, TransceiversError, NUM_PORTS}; use transceiver_messages::ModuleId; +use userlib::FromPrimitive; use zerocopy::{byteorder, AsBytes, FromBytes, Unaligned, U16}; // The transceiver modules are split across two FPGAs on the QSFP Front IO @@ -24,7 +25,7 @@ pub enum FpgaController { } /// Physical port location -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq)] pub struct PortLocation { pub controller: FpgaController, pub port: PhysicalPort, @@ -37,7 +38,7 @@ impl From for PortLocation { } /// Physical port location within a particular FPGA, as a 0-15 index -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq)] pub struct PhysicalPort(pub u8); impl PhysicalPort { pub fn as_mask(&self) -> PhysicalPortMask { @@ -47,6 +48,20 @@ impl PhysicalPort { pub fn get(&self) -> u8 { self.0 } + + pub fn to_logical_port( + &self, + fpga: FpgaController, + ) -> Result { + let loc = PortLocation { + controller: fpga, + port: *self, + }; + match PORT_MAP.into_iter().position(|&l| l == loc) { + Some(p) => Ok(LogicalPort(p as u8)), + None => Err(TransceiversError::InvalidPhysicalToLogicalMap), + } + } } /// Physical port mask within a particular FPGA, as a 16-bit bitfield @@ -479,11 +494,26 @@ pub struct ModuleResult { success: LogicalPortMask, failure: LogicalPortMask, error: LogicalPortMask, + failure_types: LogicalPortFailureTypes, +} + +/// This is a slimmed down version of ModuleResult for use in the ringbuf +#[derive(Copy, Clone, PartialEq)] +pub struct ModuleResultSlim { + success: LogicalPortMask, + failure: LogicalPortMask, + error: LogicalPortMask, } impl From for ModuleResult { fn from(r: ModuleResultNoFailure) -> Self { - ModuleResult::new(r.success(), LogicalPortMask(0), r.error()).unwrap() + ModuleResult::new( + r.success(), + LogicalPortMask(0), + r.error(), + LogicalPortFailureTypes::default(), + ) + .unwrap() } } @@ -493,6 +523,7 @@ impl ModuleResult { success: LogicalPortMask, failure: LogicalPortMask, error: LogicalPortMask, + failure_types: LogicalPortFailureTypes, ) -> Result { if !(success & failure).is_empty() || !(success & error).is_empty() @@ -504,6 +535,7 @@ impl ModuleResult { success, failure, error, + failure_types, }) } @@ -519,6 +551,10 @@ impl ModuleResult { self.error } + pub fn failure_types(&self) -> LogicalPortFailureTypes { + self.failure_types + } + /// Combines two `ModuleResults` /// /// Intended to be used for a sequence of `ModuleResult` yielding function @@ -555,7 +591,29 @@ impl ModuleResult { // has subsequently occurred. let failure = (self.failure() | next.failure()) & !self.error(); - Self::new(success, failure, error).unwrap() + // merge the failure types observed, prefering newer failure types + // should both results have a failure at the same port. + let mut combined_failures = LogicalPortFailureTypes::default(); + for p in failure.to_indices() { + if next.failure().is_set(p) { + combined_failures.0[p.0 as usize] = + next.failure_types().0[p.0 as usize]; + } else if self.failure().is_set(p) { + combined_failures.0[p.0 as usize] = + self.failure_types().0[p.0 as usize]; + } + } + + Self::new(success, failure, error, combined_failures).unwrap() + } + + /// Helper to provide a nice way to get a ModuleResultSlim from this result + pub fn to_slim(&self) -> ModuleResultSlim { + ModuleResultSlim { + success: self.success(), + failure: self.failure(), + error: self.error(), + } } } @@ -592,6 +650,49 @@ impl ModuleResultNoFailure { } } +/// A type to provide more ergonomic access to the FPGA generated type +pub type FpgaI2CFailure = Reg::QSFP::PORT0_STATUS::Encoded; + +/// A type to consolidate per-module failure types. +/// +/// Currently the only types of operations that can be considered failures are +/// those that involve the FPGA doing I2C. Thus, that is the only supported type +/// right now. +#[derive(Copy, Clone, PartialEq)] +pub struct LogicalPortFailureTypes(pub [FpgaI2CFailure; NUM_PORTS as usize]); + +impl Default for LogicalPortFailureTypes { + fn default() -> Self { + Self([FpgaI2CFailure::NoError; NUM_PORTS as usize]) + } +} + +impl core::ops::Index for LogicalPortFailureTypes { + type Output = FpgaI2CFailure; + + fn index(&self, i: LogicalPort) -> &Self::Output { + &self.0[i.0 as usize] + } +} + +#[derive(Copy, Clone)] +pub struct PortI2CStatus { + pub done: bool, + pub error: FpgaI2CFailure, +} + +impl PortI2CStatus { + pub fn new(status: u8) -> Self { + Self { + done: (status & Reg::QSFP::PORT0_STATUS::BUSY) == 0, + error: FpgaI2CFailure::from_u8( + status & Reg::QSFP::PORT0_STATUS::ERROR, + ) + .unwrap(), + } + } +} + impl Transceivers { pub fn new(fpga_task: userlib::TaskId) -> Self { Self { @@ -1089,30 +1190,31 @@ impl Transceivers { let mut physical_failure = FpgaPortMasks::default(); let mut physical_error = FpgaPortMasks::default(); let phys_mask: FpgaPortMasks = mask.into(); + let mut failure_types = LogicalPortFailureTypes::default(); #[derive(AsBytes, Default, FromBytes)] #[repr(C)] - struct StatusAndErr { + struct BusyAndPortStatus { busy: u16, - err: [u8; 8], + status: [u8; 8], } for fpga_index in phys_mask.iter_fpgas() { let fpga = self.fpga(fpga_index); // This loop should break immediately, because I2C is fast - let status = loop { - // Two bytes of BUSY, followed by 8 bytes of error status - let status = match fpga.read(Addr::QSFP_I2C_BUSY0) { + let status_all = loop { + // Two bytes of BUSY, followed by 816bytes of port status + let status_all = match fpga.read(Addr::QSFP_I2C_BUSY0) { Ok(data) => data, // If there is an FPGA communication error, mark that as an // error on all of that FPGA's ports Err(_) => { *physical_error.get_mut(fpga_index) = PhysicalPortMask(0xffff); - StatusAndErr::default() + BusyAndPortStatus::default() } }; - if status.busy == 0 { - break status; + if status_all.busy == 0 { + break status_all; } userlib::hl::sleep_for(1); }; @@ -1123,25 +1225,30 @@ impl Transceivers { FpgaController::Right => phys_mask.right, }; for port in (0..16).map(PhysicalPort) { + // skip ports we didn't interact with if !phys_mask.is_set(port) { continue; } - // Each error byte packs together two ports - let err = status.err[port.0 as usize / 2] - >> ((port.0 as usize % 2) * 4); - - // For now, check for the presence of an error, but don't bother - // reporting the details. - let has_err = (err & 0b1000) != 0; - if has_err { + + // cast status byte into a type + let failure = FpgaI2CFailure::from_u8( + status_all.status[port.0 as usize] + & Reg::QSFP::PORT0_STATUS::ERROR, + ) + .unwrap(); + // if a failure occurred, mark it and record the failure type + if failure != FpgaI2CFailure::NoError { physical_failure.get_mut(fpga_index).set(port); + let logical_port = + port.to_logical_port(fpga_index).unwrap(); + failure_types.0[logical_port.0 as usize] = failure; } } } let error = mask & LogicalPortMask::from(physical_error); let failure = mask & LogicalPortMask::from(physical_failure); let success = mask & !(error | failure); - ModuleResult::new(success, failure, error).unwrap() + ModuleResult::new(success, failure, error, failure_types).unwrap() } } diff --git a/drv/transceivers-api/src/lib.rs b/drv/transceivers-api/src/lib.rs index 0ae111d0f5..274ada97bc 100644 --- a/drv/transceivers-api/src/lib.rs +++ b/drv/transceivers-api/src/lib.rs @@ -20,6 +20,7 @@ pub enum TransceiversError { InvalidPowerState, InvalidModuleResult, LedI2cError, + InvalidPhysicalToLogicalMap, #[idol(server_death)] ServerRestarted, diff --git a/drv/transceivers-server/src/udp.rs b/drv/transceivers-server/src/udp.rs index 72f2ee3fe3..8b3ce729a2 100644 --- a/drv/transceivers-server/src/udp.rs +++ b/drv/transceivers-server/src/udp.rs @@ -11,11 +11,9 @@ //! All of the API types in `transceiver_messages` operate on **physical** //! ports, i.e. an FPGA paired by a physical port index (or mask). use crate::ServerImpl; -use drv_sidecar_front_io::{ - transceivers::{ - LogicalPort, LogicalPortMask, ModuleResult, ModuleResultNoFailure, - }, - Reg, +use drv_sidecar_front_io::transceivers::{ + FpgaI2CFailure, LogicalPort, LogicalPortFailureTypes, LogicalPortMask, + ModuleResult, ModuleResultNoFailure, ModuleResultSlim, PortI2CStatus, }; use hubpack::SerializedSize; use ringbuf::*; @@ -53,12 +51,15 @@ enum Trace { MacAddrs, GotError(ProtocolError), ResponseSize(ResponseSize), - OperationResult(ModuleResult), + OperationResult(ModuleResultSlim), OperationNoFailResult(ModuleResultNoFailure), ClearPowerFault(ModuleId), LedState(ModuleId), SetLedState(ModuleId, LedState), ClearDisableLatch(ModuleId), + PageSelectI2CFailures(LogicalPort, FpgaI2CFailure), + ReadI2CFailures(LogicalPort, FpgaI2CFailure), + WriteI2CFailures(LogicalPort, FpgaI2CFailure), } ringbuf!(Trace, 16, Trace::None); @@ -366,7 +367,10 @@ impl ServerImpl { ringbuf_entry!(Trace::Read(modules, read)); // The host is not setting the the upper 32 bits at this time, // but should that happen we need to know how many HwErrors we - // will serialize due to invalid modules being specified. + // will serialize due to invalid modules being specified. We + // only precalculate error length requirements in Read as that + // is the only message whose response can approach + // MAX_PAYLOAD_SIZE. let num_invalid = ModuleId(modules.0 & 0xffffffff00000000) .selected_transceiver_count(); let mask = LogicalPortMask::from(modules); @@ -383,18 +387,33 @@ impl ServerImpl { ); } - let result = self.read(read, mask & !self.disabled, out); - ringbuf_entry!(Trace::OperationResult(result)); + let (data_len, result) = + if self.front_io_board_present.unwrap_or_default() { + let r = self.read(read, mask & !self.disabled, out); + let len = r.success().count() * read.len() as usize; + (len, r) + } else { + ( + 0, + ModuleResult::new( + LogicalPortMask(0), + LogicalPortMask(0), + mask, + LogicalPortFailureTypes::default(), + ) + .unwrap(), + ) + }; + + ringbuf_entry!(Trace::OperationResult(result.to_slim())); let success = ModuleId::from(result.success()); - let read_bytes = result.success().count() * read.len() as usize; let (err_len, failed_modules) = self .handle_errors_and_failures_and_disabled( modules, result, - HwError::I2cError, - &mut out[read_bytes..], + &mut out[data_len..], ); - let final_payload_len = read_bytes + err_len; + let final_payload_len = data_len + err_len; ( MessageBody::SpResponse(SpResponse::Read { @@ -416,16 +435,26 @@ impl ServerImpl { 0, ); } + let mask = LogicalPortMask::from(modules); - let result = self.write(write, mask & !self.disabled, data); - ringbuf_entry!(Trace::OperationResult(result)); + let result = if self.front_io_board_present.unwrap_or_default() + { + self.write(write, mask & !self.disabled, data) + } else { + ModuleResult::new( + LogicalPortMask(0), + LogicalPortMask(0), + mask, + LogicalPortFailureTypes::default(), + ) + .unwrap() + }; + + ringbuf_entry!(Trace::OperationResult(result.to_slim())); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = self .handle_errors_and_failures_and_disabled( - modules, - result, - HwError::I2cError, - out, + modules, result, out, ); ( @@ -440,7 +469,15 @@ impl ServerImpl { HostRequest::AssertReset(modules) => { ringbuf_entry!(Trace::AssertReset(modules)); let mask = LogicalPortMask::from(modules) & !self.disabled; - let result = self.transceivers.assert_reset(mask); + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.transceivers.assert_reset(mask) + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + ringbuf_entry!(Trace::OperationNoFailResult(result)); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = @@ -457,7 +494,15 @@ impl ServerImpl { HostRequest::DeassertReset(modules) => { ringbuf_entry!(Trace::DeassertReset(modules)); let mask = LogicalPortMask::from(modules) & !self.disabled; - let result = self.transceivers.deassert_reset(mask); + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.transceivers.deassert_reset(mask) + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + ringbuf_entry!(Trace::OperationNoFailResult(result)); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = @@ -474,7 +519,15 @@ impl ServerImpl { HostRequest::AssertLpMode(modules) => { ringbuf_entry!(Trace::AssertLpMode(modules)); let mask = LogicalPortMask::from(modules) & !self.disabled; - let result = self.transceivers.assert_lpmode(mask); + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.transceivers.assert_lpmode(mask) + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + ringbuf_entry!(Trace::OperationNoFailResult(result)); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = @@ -491,7 +544,15 @@ impl ServerImpl { HostRequest::DeassertLpMode(modules) => { ringbuf_entry!(Trace::DeassertLpMode(modules)); let mask = LogicalPortMask::from(modules) & !self.disabled; - let result = self.transceivers.deassert_lpmode(mask); + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.transceivers.deassert_lpmode(mask) + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + ringbuf_entry!(Trace::OperationNoFailResult(result)); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = @@ -508,7 +569,15 @@ impl ServerImpl { HostRequest::EnablePower(modules) => { ringbuf_entry!(Trace::EnablePower(modules)); let mask = LogicalPortMask::from(modules) & !self.disabled; - let result = self.transceivers.enable_power(mask); + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.transceivers.enable_power(mask) + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + ringbuf_entry!(Trace::OperationNoFailResult(result)); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = @@ -525,7 +594,15 @@ impl ServerImpl { HostRequest::DisablePower(modules) => { ringbuf_entry!(Trace::DisablePower(modules)); let mask = LogicalPortMask::from(modules) & !self.disabled; - let result = self.transceivers.disable_power(mask); + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.transceivers.disable_power(mask) + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + ringbuf_entry!(Trace::OperationNoFailResult(result)); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = @@ -568,7 +645,15 @@ impl ServerImpl { HostRequest::ClearPowerFault(modules) => { ringbuf_entry!(Trace::ClearPowerFault(modules)); let mask = LogicalPortMask::from(modules) & !self.disabled; - let result = self.transceivers.clear_power_fault(mask); + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.transceivers.clear_power_fault(mask) + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + ringbuf_entry!(Trace::OperationNoFailResult(result)); let success = ModuleId::from(result.success()); let (num_err_bytes, failed_modules) = @@ -585,32 +670,47 @@ impl ServerImpl { HostRequest::LedState(modules) => { ringbuf_entry!(Trace::LedState(modules)); let mask = LogicalPortMask::from(modules); - let (num_led_bytes, result) = - self.get_led_state_response(mask, out); + let (data_len, result) = if self + .front_io_board_present + .unwrap_or_default() + { + self.get_led_state_response(mask, out) + } else { + ( + 0, + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap(), + ) + }; + // This operation just queries internal SP state, so it is // always successful. However, invalid modules may been // specified by the host so we need to check that anyway. let success = ModuleId::from(result.success()); - let (num_err_bytes, failed_modules) = self.handle_errors( - modules, - result, - &mut out[num_led_bytes..], - ); + let (num_err_bytes, failed_modules) = + self.handle_errors(modules, result, &mut out[data_len..]); ( MessageBody::SpResponse(SpResponse::LedState { modules: success, failed_modules, }), - num_led_bytes + num_err_bytes, + data_len + num_err_bytes, ) } HostRequest::SetLedState { modules, state } => { ringbuf_entry!(Trace::SetLedState(modules, state)); let mask = LogicalPortMask::from(modules); - self.set_led_state(mask, state); - let result = + + let result = if self.front_io_board_present.unwrap_or_default() + { + self.set_led_state(mask, state); ModuleResultNoFailure::new(mask, LogicalPortMask(0)) - .unwrap(); + .unwrap() + } else { + ModuleResultNoFailure::new(LogicalPortMask(0), mask) + .unwrap() + }; + // This operation just sets internal SP state, so it is always // successful. However, invalid modules may been specified by // the host so we need to check that anyway. @@ -655,11 +755,14 @@ impl ServerImpl { /// operation before sending the response. For results where a /// `ModuleResultNoFailure` is returned, use `handle_errors` or /// `handle_errors_and_disabled` instead. + /// + /// Panics: This function can panic if a module has been marked to have had + /// a failure but the failure type enum is NoError. In practice this should + /// not happen. fn handle_errors_and_failures_and_disabled( &mut self, modules: ModuleId, result: ModuleResult, - failure_type: HwError, out: &mut [u8], ) -> (usize, ModuleId) { let mut error_idx: usize = 0; @@ -681,6 +784,21 @@ impl ServerImpl { .unwrap(); error_idx += err_size; } else if result.failure().is_set(module) { + let failure_type = match result.failure_types()[module] { + FpgaI2CFailure::NoModule => HwError::NotPresent, + FpgaI2CFailure::NoPower => HwError::NotPowered, + FpgaI2CFailure::PowerFault => HwError::PowerFault, + FpgaI2CFailure::NotInitialized => { + HwError::NotInitialized + } + FpgaI2CFailure::I2cAddressNack => { + HwError::I2cAddressNack + } + FpgaI2CFailure::I2cByteNack => HwError::I2cByteNack, + // We only mark failures when an error is set, so this + // branch should never match. + FpgaI2CFailure::NoError => panic!(), + }; // failure: whatever `HwError` specified by `failure_type` let err_size = hubpack::serialize( &mut out[error_idx..], @@ -689,12 +807,10 @@ impl ServerImpl { .unwrap(); error_idx += err_size; } else if result.error().is_set(module) { - // error: fpga communication issue - let err_size = hubpack::serialize( - &mut out[error_idx..], - &HwError::FpgaError, - ) - .unwrap(); + let err_type = self.resolve_error_type(); + let err_size = + hubpack::serialize(&mut out[error_idx..], &err_type) + .unwrap(); error_idx += err_size; } } else if requested_invalid_modules.is_set(module.0).unwrap() { @@ -740,14 +856,7 @@ impl ServerImpl { if module <= LogicalPortMask::MAX_PORT_INDEX && result.error().is_set(module) { - let err_type = match self.front_io_board_present { - // Front IO is present and ready, so the only other error - // path currently is if we handle an FpgaError. - Some(true) => HwError::FpgaError, - Some(false) => HwError::NoFrontIo, - None => HwError::FrontIoNotReady, - }; - + let err_type = self.resolve_error_type(); let err_size = hubpack::serialize(&mut out[error_idx..], &err_type) .unwrap(); @@ -771,6 +880,19 @@ impl ServerImpl { ) } + // shared logic between the various functions which handle errors + fn resolve_error_type(&self) -> HwError { + match self.front_io_board_present { + // Front IO is present and ready, so the only other error + // path currently is if we handle an FpgaError. + Some(true) => HwError::FpgaError, + // Front IO is not present. + Some(false) => HwError::NoFrontIo, + // Front IO is not yet ready. + None => HwError::FrontIoNotReady, + } + } + /// This function reads a `ModuleResultNoFailure` and populates error /// information at the end of the trailing data buffer, taking `self.data` /// into account. This means it should be called as the last operation @@ -954,9 +1076,13 @@ impl ServerImpl { const BANK_SELECT: u8 = 0x7E; const PAGE_SELECT: u8 = 0x7F; - let mut result = - ModuleResult::new(mask, LogicalPortMask(0), LogicalPortMask(0)) - .unwrap(); + let mut result = ModuleResult::new( + mask, + LogicalPortMask(0), + LogicalPortMask(0), + LogicalPortFailureTypes::default(), + ) + .unwrap(); // We can always write the lower page; upper pages require modifying // registers in the transceiver to select it. @@ -970,9 +1096,13 @@ impl ServerImpl { result = result.chain(self.wait_and_check_i2c(result.success())); } else { // If the request is to the lower page it is always successful - result = - ModuleResult::new(mask, LogicalPortMask(0), LogicalPortMask(0)) - .unwrap(); + result = ModuleResult::new( + mask, + LogicalPortMask(0), + LogicalPortMask(0), + LogicalPortFailureTypes::default(), + ) + .unwrap(); } if let Some(bank) = page.bank() { @@ -984,6 +1114,13 @@ impl ServerImpl { )); result = result.chain(self.wait_and_check_i2c(result.success())); } + + for port in result.failure().to_indices() { + ringbuf_entry!(Trace::PageSelectI2CFailures( + port, + result.failure_types()[port] + )) + } result } @@ -1017,6 +1154,7 @@ impl ServerImpl { let mut error = LogicalPortMask(0); let mut idx = 0; let buf_len = mem.len() as usize; + let mut failure_types = LogicalPortFailureTypes::default(); for port in result.success().to_indices() { // The status register is contiguous with the output buffer, so @@ -1040,15 +1178,16 @@ impl ServerImpl { break; }; - let status = buf[0]; + let status = PortI2CStatus::new(buf[0]); // Use QSFP::PORT0 for constants, since they're all identical - if status & Reg::QSFP::PORT0_STATUS::BUSY == 0 { + if status.done { // Check error mask - if status & Reg::QSFP::PORT0_STATUS::ERROR != 0 { + 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); @@ -1062,8 +1201,17 @@ impl ServerImpl { userlib::hl::sleep_for(1); } } - let final_result = ModuleResult::new(success, failure, error).unwrap(); - result.chain(final_result) + let mut final_result = + ModuleResult::new(success, failure, error, failure_types).unwrap(); + final_result = result.chain(final_result); + + for port in final_result.failure().to_indices() { + ringbuf_entry!(Trace::ReadI2CFailures( + port, + final_result.failure_types()[port] + )) + } + final_result } // The `LogicalPortMask` indicates which of the requested ports the @@ -1086,7 +1234,15 @@ impl ServerImpl { mem.len(), result.success(), )); - result.chain(self.wait_and_check_i2c(result.success())) + result = result.chain(self.wait_and_check_i2c(result.success())); + + for port in result.failure().to_indices() { + ringbuf_entry!(Trace::WriteI2CFailures( + port, + result.failure_types()[port] + )) + } + result } fn get_led_state_response(