Skip to content

Commit

Permalink
better handle missing I2C muxes (#1425)
Browse files Browse the repository at this point in the history
After the fix to #1413, the system is unwilling to ignore mux related
errors -- which has generated a problem on some lab systems that are
missing muxes:  because we can never get the bus into a known mux
state these systems now become entirely unusable, as even devices that
aren't on the missing segments are not accessible.  This fixes that by
observing that a mux that is affirmatively missing -- that is, one
that doesn't reply to its in-band management at all -- can be assumed
to have segments that are similarly missing (and therefore as good as
disabled).  This (naturally) doesn't change the fact that accessing
any devices attached to the missing mux will generate an error, but it
allows the system to broadly drive on absent accesses to those
devices.
  • Loading branch information
bcantrill committed Jun 22, 2023
1 parent 23fdad6 commit 9932687
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 10 deletions.
4 changes: 2 additions & 2 deletions drv/i2c-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ pub enum ResponseCode {
SegmentDisconnected,
/// Mux disconnected during operation
MuxDisconnected,
/// Address used for mux in-band management is invalid
BadMuxAddress,
/// No device at address used for mux in-band management
MuxMissing,
/// Register used for mux in-band management is invalid
BadMuxRegister,
/// I2C bus was spontaneously reset during operation
Expand Down
39 changes: 32 additions & 7 deletions drv/stm32xx-i2c-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,37 @@ fn configure_mux(

Some(MuxState::Unknown) => {
//
// We are in an unknown mux state. Before we can do anything,
// we need to successfully talk to every mux, and disable every
// segment. If there is any failure through here, we'll just
// return the error, leaving our mux state as unknown.
// We are in an unknown mux state. Before we can do anything, we
// need to successfully talk to every mux (or successfully learn
// that the mux is gone entirely!), and disable every segment. If
// there is any failure through here that isn't the mux being
// affirmatively gone, we'll just return the error, leaving our
// mux state as unknown.
//
all_muxes(controller, port, muxes, |mux| {
mux.driver.enable_segment(mux, controller, None, ctrl)
match mux.driver.enable_segment(mux, controller, None, ctrl) {
Err(ResponseCode::MuxMissing) => {
//
// The mux is gone entirely. We really don't expect
// this on any production system, but it can be true on
// some special lab systems (you know who you are!).
// Regardless of its origin, we can limit the blast
// radius in this case: if the mux is affirmatively
// gone (that is, no device is acking its address), we
// can assume that the mux is absent rather than
// Byzantine -- and therefore assume that its segments
// are as good as disabled and allow other traffic on
// the bus. So on this error (and only this error), we
// note that we saw it, and drive on. (Note that
// attempting to speak to a device on a segment on the
// missing mux will properly return MuxMissing -- and
// set our bus's mux state to be unknown.)
//
ringbuf_entry!(Trace::MuxMissing(mux.address));
Ok(())
}
other => other,
}
})?;

//
Expand Down Expand Up @@ -208,7 +232,8 @@ enum Trace {
Reset((Controller, PortIndex)),
MuxUnknown((Controller, PortIndex)),
MuxUnknownRecover((Controller, PortIndex)),
ResetMux(Mux),
MuxMissing(u8),
ResetMux(u8),
SegmentFailed(ResponseCodeU8),
ConfigureFailed(ResponseCodeU8),
Wiggles(u8),
Expand All @@ -234,7 +259,7 @@ fn reset(

// And now reset all muxes on this bus, eating any errors.
let _ = all_muxes(controller, port, muxes, |mux| {
ringbuf_entry!(Trace::ResetMux(mux.id));
ringbuf_entry!(Trace::ResetMux(mux.address));
let _ = mux.driver.reset(mux, &sys);
Ok(())
});
Expand Down
2 changes: 1 addition & 1 deletion drv/stm32xx-i2c/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ impl I2cMux<'_> {
use drv_i2c_api::ResponseCode;

match code {
ResponseCode::NoDevice => ResponseCode::BadMuxAddress,
ResponseCode::NoDevice => ResponseCode::MuxMissing,
ResponseCode::NoRegister => ResponseCode::BadMuxRegister,
ResponseCode::BusLocked => ResponseCode::BusLockedMux,
ResponseCode::BusReset => ResponseCode::BusResetMux,
Expand Down

0 comments on commit 9932687

Please sign in to comment.