Skip to content

stm32xx-i2c: remove I2cControl function table #2121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

stm32xx-i2c: remove I2cControl function table #2121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5aed2a0
stm32xx-i2c: remove I2cControl function table
cbiffle Jun 30, 2025
febfc35
stm32xx-i2c: update docstring
mkeeter Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 8 additions & 50 deletions drv/stm32xx-i2c-server/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ fn configure_mux(
port: PortIndex,
mux: Option<(Mux, Segment)>,
muxes: &[I2cMux<'_>],
ctrl: &I2cControl,
) -> Result<(), ResponseCode> {
let bus = (controller.controller, port);

Expand Down Expand Up @@ -127,7 +126,7 @@ fn configure_mux(
// (errant) address conflict can be pretty brutal.
//
find_mux(controller, port, muxes, current_id, |mux| {
mux.driver.enable_segment(mux, controller, None, ctrl)
mux.driver.enable_segment(mux, controller, None)
})
.map_err(|err| {
//
Expand Down Expand Up @@ -157,7 +156,7 @@ fn configure_mux(
// mux state as unknown.
//
all_muxes(controller, port, muxes, |mux| {
match mux.driver.enable_segment(mux, controller, None, ctrl) {
match mux.driver.enable_segment(mux, controller, None) {
Err(ResponseCode::MuxMissing) => {
//
// The mux is gone entirely. We really don't expect
Expand Down Expand Up @@ -201,7 +200,7 @@ fn configure_mux(
if let Some((id, segment)) = mux {
find_mux(controller, port, muxes, id, |mux| {
mux.driver
.enable_segment(mux, controller, Some(segment), ctrl)
.enable_segment(mux, controller, Some(segment))
.map_err(|err| {
//
// We have failed to enable our new mux+segment.
Expand Down Expand Up @@ -378,40 +377,7 @@ fn main() -> ! {
// Field messages.
let mut buffer = [0; 4];

let ctrl = I2cControl {
enable: |notification| {
sys_irq_control(notification, true);
},
wfi: |notification, timeout| {
const TIMER_NOTIFICATION: u32 = 1 << 31;

// If the driver passes in a timeout that is large enough that it
// would overflow the kernel's 64-bit timestamp space... well, we'll
// do the best we can without compiling in an unlikely panic.
let dead = sys_get_timer().now.saturating_add(timeout.0);

sys_set_timer(Some(dead), TIMER_NOTIFICATION);

let notification =
sys_recv_notification(notification | TIMER_NOTIFICATION);

if notification == TIMER_NOTIFICATION {
I2cControlResult::TimedOut
} else {
sys_set_timer(None, TIMER_NOTIFICATION);
I2cControlResult::Interrupted
}
},
};

configure_muxes(
&muxes,
&controllers,
&pins,
&mut portmap,
&mut muxmap,
&ctrl,
);
configure_muxes(&muxes, &controllers, &pins, &mut portmap, &mut muxmap);

loop {
hl::recv_without_notification(&mut buffer, |op, msg| match op {
Expand All @@ -438,14 +404,8 @@ fn main() -> ! {

configure_port(&mut portmap, controller, port, &pins);

match configure_mux(
&mut muxmap,
controller,
port,
mux,
&muxes,
&ctrl,
) {
match configure_mux(&mut muxmap, controller, port, mux, &muxes)
{
Ok(_) => {}
Err(code) => {
ringbuf_entry!(Trace::MuxError(code.into()));
Expand Down Expand Up @@ -511,7 +471,6 @@ fn main() -> ! {

rbuf.write_at(pos, byte)
},
&ctrl,
);
match controller_result {
Err(code) => {
Expand Down Expand Up @@ -775,7 +734,6 @@ fn configure_muxes(
pins: &[I2cPins],
map: &mut PortMap,
muxmap: &mut MuxMap,
ctrl: &I2cControl,
) {
let sys = SYS.get_task_id();
let sys = Sys::from(sys);
Expand All @@ -788,7 +746,7 @@ fn configure_muxes(
let mut reset_attempted = false;

loop {
match mux.driver.configure(mux, controller, &sys, ctrl) {
match mux.driver.configure(mux, controller, &sys) {
Ok(_) => {
//
// We are going to attempt to disable all segments. If we
Expand All @@ -815,7 +773,7 @@ fn configure_muxes(
// deal with the reset).
//
if let Err(code) =
mux.driver.enable_segment(mux, controller, None, ctrl)
mux.driver.enable_segment(mux, controller, None)
{
ringbuf_entry!(Trace::SegmentFailed(code.into()));

Expand Down
78 changes: 49 additions & 29 deletions drv/stm32xx-i2c/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,6 @@ pub enum I2cControlResult {
TimedOut,
}

///
/// A structure that defines interrupt control flow functions that will be
/// used to pass control flow into the kernel to either enable or wait for
/// interrupts. Note that this is deliberately a struct and not a trait,
/// allowing the [`I2cMuxDriver`] trait to itself be a trait object.
///
pub struct I2cControl {
pub enable: fn(u32),
pub wfi: fn(u32, I2cTimeout) -> I2cControlResult,
}

pub struct I2cTargetControl {
pub enable: fn(u32),
pub wfi: fn(u32),
Expand All @@ -109,7 +98,6 @@ pub trait I2cMuxDriver {
mux: &I2cMux<'_>,
controller: &I2cController<'_>,
sys: &sys_api::Sys,
ctrl: &I2cControl,
) -> Result<(), drv_i2c_api::ResponseCode>;

/// Reset the mux
Expand All @@ -126,7 +114,6 @@ pub trait I2cMuxDriver {
mux: &I2cMux<'_>,
controller: &I2cController<'_>,
segment: Option<drv_i2c_api::Segment>,
ctrl: &I2cControl,
) -> Result<(), drv_i2c_api::ResponseCode>;
}

Expand Down Expand Up @@ -503,15 +490,18 @@ impl I2cController<'_> {
}

///
/// A common routine to wait for interrupts with a timeout.
/// A common routine to wait for interrupts, panicking the driver if the
/// interrupt doesn't arrive in an arbitrarily chosen period of time.
///
fn wfi(&self, ctrl: &I2cControl) -> Result<(), drv_i2c_api::ResponseCode> {
fn wfi(&self) -> Result<(), drv_i2c_api::ResponseCode> {
//
// A 100 ms timeout is much, much longer than the I2C timeouts.
// A 100 ms timeout is 4x longer than the I2C timeouts, but much shorter
// than potential context switches when dumps are being recorded by a
// high priority task.
//
const TIMEOUT: I2cTimeout = I2cTimeout(100);

match (ctrl.wfi)(self.notification, TIMEOUT) {
match wfi_raw(self.notification, TIMEOUT) {
I2cControlResult::TimedOut => {
//
// This really shouldn't happen: it means that not only did
Expand Down Expand Up @@ -607,7 +597,6 @@ impl I2cController<'_> {
getbyte: impl Fn(usize) -> Option<u8>,
mut rlen: ReadLength,
mut putbyte: impl FnMut(usize, u8) -> Option<()>,
ctrl: &I2cControl,
) -> Result<(), drv_i2c_api::ResponseCode> {
// Assert our preconditions as described above
assert!(wlen > 0 || rlen != ReadLength::Fixed(0));
Expand Down Expand Up @@ -652,8 +641,8 @@ impl I2cController<'_> {
break;
}

self.wfi(ctrl)?;
(ctrl.enable)(notification);
self.wfi()?;
sys_irq_control(notification, true);
}

// Get a single byte.
Expand Down Expand Up @@ -682,8 +671,8 @@ impl I2cController<'_> {
break;
}

self.wfi(ctrl)?;
(ctrl.enable)(notification);
self.wfi()?;
sys_irq_control(notification, true);
}
}

Expand Down Expand Up @@ -730,8 +719,8 @@ impl I2cController<'_> {
}

loop {
self.wfi(ctrl)?;
(ctrl.enable)(notification);
self.wfi()?;
sys_irq_control(notification, true);

let isr = i2c.isr.read();
ringbuf_entry!(Trace::Read(Register::ISR, isr.bits()));
Expand Down Expand Up @@ -785,8 +774,8 @@ impl I2cController<'_> {

self.check_errors(&isr)?;

self.wfi(ctrl)?;
(ctrl.enable)(notification);
self.wfi()?;
sys_irq_control(notification, true);
}
}

Expand Down Expand Up @@ -820,7 +809,6 @@ impl I2cController<'_> {
&self,
addr: u8,
ops: &[I2cKonamiCode],
ctrl: &I2cControl,
) -> Result<(), drv_i2c_api::ResponseCode> {
let i2c = self.registers;
let notification = self.notification;
Expand Down Expand Up @@ -864,8 +852,8 @@ impl I2cController<'_> {
break;
}

self.wfi(ctrl)?;
(ctrl.enable)(notification);
self.wfi()?;
sys_irq_control(notification, true);
}
}

Expand Down Expand Up @@ -1191,3 +1179,35 @@ impl I2cController<'_> {
}
}
}

/// Waits for `event_mask` or `timeout`, whichever comes first. This is
/// factored out of `wfi` above for clarity.
///
/// This function, like `userlib::hl`, assumes that notification bit 31 is safe
/// to use for the timer. If `event_mask` also has bit 31 set, weird things
/// will happen, don't do that.
fn wfi_raw(event_mask: u32, timeout: I2cTimeout) -> I2cControlResult {
const TIMER_NOTIFICATION: u32 = 1 << 31;

// If the driver passes in a timeout that is large enough that it would
// overflow the kernel's 64-bit timestamp space... well, we'll do the
// best we can without compiling in an unlikely panic.
let dead = sys_get_timer().now.saturating_add(timeout.0);

sys_set_timer(Some(dead), TIMER_NOTIFICATION);

let received = sys_recv_notification(event_mask | TIMER_NOTIFICATION);

// If the event arrived _and_ our timer went off, prioritize the event and
// ignore the timeout.
if received & event_mask != 0 {
// Attempt to clear our timer. Note that this does not protect against
// the race where the kernel decided to wake us up, but the timer went
// off before we got to this point.
sys_set_timer(None, TIMER_NOTIFICATION);
I2cControlResult::Interrupted
} else {
// The event_mask bit was not set, so:
I2cControlResult::TimedOut
}
}
10 changes: 2 additions & 8 deletions drv/stm32xx-i2c/src/ltc4306.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ fn read_reg_u8(
mux: &I2cMux<'_>,
controller: &I2cController<'_>,
reg: u8,
ctrl: &I2cControl,
) -> Result<u8, ResponseCode> {
let mut rval = 0u8;
let wlen = 1;
Expand All @@ -102,7 +101,6 @@ fn read_reg_u8(
rval = byte;
Some(())
},
ctrl,
);
match controller_result {
Err(code) => Err(mux.error_code(code)),
Expand All @@ -115,15 +113,13 @@ fn write_reg_u8(
controller: &I2cController<'_>,
reg: u8,
val: u8,
ctrl: &I2cControl,
) -> Result<(), ResponseCode> {
match controller.write_read(
mux.address,
2,
|pos| Some(if pos == 0 { reg } else { val }),
ReadLength::Fixed(0),
|_, _| Some(()),
ctrl,
) {
Err(code) => Err(mux.error_code(code)),
_ => Ok(()),
Expand All @@ -136,7 +132,6 @@ impl I2cMuxDriver for Ltc4306 {
mux: &I2cMux<'_>,
_controller: &I2cController<'_>,
gpio: &sys_api::Sys,
_ctrl: &I2cControl,
) -> Result<(), drv_i2c_api::ResponseCode> {
mux.configure(gpio)
}
Expand All @@ -146,7 +141,6 @@ impl I2cMuxDriver for Ltc4306 {
mux: &I2cMux<'_>,
controller: &I2cController<'_>,
segment: Option<Segment>,
ctrl: &I2cControl,
) -> Result<(), ResponseCode> {
let mut reg3 = Register3(0);

Expand All @@ -170,8 +164,8 @@ impl I2cMuxDriver for Ltc4306 {
}
}

write_reg_u8(mux, controller, 3, reg3.0, ctrl)?;
let reg0 = Register0(read_reg_u8(mux, controller, 0, ctrl)?);
write_reg_u8(mux, controller, 3, reg3.0)?;
let reg0 = Register0(read_reg_u8(mux, controller, 0)?);

if !reg0.not_failed() {
Err(ResponseCode::SegmentDisconnected)
Expand Down
Loading
Loading