diff --git a/Cargo.lock b/Cargo.lock index 68764d724..9759d9366 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1958,6 +1958,7 @@ dependencies = [ "bitfield 0.13.2", "build-util", "cfg-if", + "counters", "drv-i2c-api", "drv-stm32xx-sys-api", "num-traits", diff --git a/app/demo-stm32h7-nucleo/app-h743.toml b/app/demo-stm32h7-nucleo/app-h743.toml index aeef5d5f8..95a6e6ece 100644 --- a/app/demo-stm32h7-nucleo/app-h743.toml +++ b/app/demo-stm32h7-nucleo/app-h743.toml @@ -34,6 +34,7 @@ task-slots = ["jefe"] [tasks.i2c_driver] name = "drv-stm32xx-i2c-server" +stacksize = 1048 features = ["h743"] priority = 2 max-sizes = {flash = 16384, ram = 4096} diff --git a/app/demo-stm32h7-nucleo/app-h753.toml b/app/demo-stm32h7-nucleo/app-h753.toml index 45c449862..ce49a814a 100644 --- a/app/demo-stm32h7-nucleo/app-h753.toml +++ b/app/demo-stm32h7-nucleo/app-h753.toml @@ -34,6 +34,7 @@ task-slots = ["jefe"] [tasks.i2c_driver] name = "drv-stm32xx-i2c-server" +stacksize = 1048 features = ["h753"] priority = 2 max-sizes = {flash = 16384, ram = 4096} diff --git a/app/gemini-bu/app.toml b/app/gemini-bu/app.toml index 869c51567..d380e374b 100644 --- a/app/gemini-bu/app.toml +++ b/app/gemini-bu/app.toml @@ -32,6 +32,7 @@ task-slots = ["jefe"] [tasks.i2c_driver] name = "drv-stm32xx-i2c-server" +stacksize = 1048 features = ["h753"] priority = 2 max-sizes = {flash = 16384, ram = 4096} diff --git a/app/gimlet/base.toml b/app/gimlet/base.toml index e554d0c43..b1cf64dd7 100644 --- a/app/gimlet/base.toml +++ b/app/gimlet/base.toml @@ -71,6 +71,7 @@ notifications = ["spi-irq"] [tasks.i2c_driver] name = "drv-stm32xx-i2c-server" +stacksize = 1048 features = ["h753"] priority = 3 max-sizes = {flash = 16384, ram = 4096} @@ -125,7 +126,7 @@ name = "task-power" features = ["gimlet"] priority = 6 max-sizes = {flash = 65536, ram = 16384 } -stacksize = 2504 +stacksize = 2800 start = true task-slots = ["i2c_driver", "sensor", "gimlet_seq"] notifications = ["timer"] @@ -135,7 +136,7 @@ name = "task-hiffy" features = ["h753", "stm32h7", "i2c", "gpio", "spi", "qspi", "hash", "sprot"] priority = 5 max-sizes = {flash = 32768, ram = 32768 } -stacksize = 1024 +stacksize = 1200 start = true task-slots = ["sys", "hf", "i2c_driver", "hash_driver", "update_server", "sprot"] diff --git a/app/gimletlet/base-gimletlet2.toml b/app/gimletlet/base-gimletlet2.toml index dcb1dc4e2..dffb02ea6 100644 --- a/app/gimletlet/base-gimletlet2.toml +++ b/app/gimletlet/base-gimletlet2.toml @@ -33,6 +33,7 @@ task-slots = ["jefe"] [tasks.i2c_driver] name = "drv-stm32xx-i2c-server" +stacksize = 1048 features = ["h753"] priority = 2 max-sizes = {flash = 16384, ram = 4096} diff --git a/app/psc/base.toml b/app/psc/base.toml index 4f1ed4565..c65c9afd8 100644 --- a/app/psc/base.toml +++ b/app/psc/base.toml @@ -43,6 +43,7 @@ task-slots = ["jefe"] [tasks.i2c_driver] name = "drv-stm32xx-i2c-server" +stacksize = 1048 features = ["h753"] priority = 2 max-sizes = {flash = 16384, ram = 4096} @@ -88,7 +89,7 @@ name = "task-hiffy" features = ["h753", "stm32h7", "i2c", "gpio", "sprot"] priority = 5 max-sizes = {flash = 32768, ram = 16384 } -stacksize = 1024 +stacksize = 1200 start = true task-slots = ["sys", "i2c_driver", "sprot"] @@ -207,7 +208,7 @@ notifications = ["timer"] name = "task-power" priority = 4 max-sizes = {flash = 32768, ram = 4096} -stacksize = 1504 +stacksize = 2504 start = true task-slots = ["i2c_driver", "sensor", "sys"] features = ["psc"] diff --git a/app/sidecar/base.toml b/app/sidecar/base.toml index d9174dc2c..1a5af3c76 100644 --- a/app/sidecar/base.toml +++ b/app/sidecar/base.toml @@ -150,6 +150,7 @@ interrupts = {"spi2.irq" = "spi-irq"} [tasks.i2c_driver] name = "drv-stm32xx-i2c-server" +stacksize = 1048 features = ["h753"] priority = 2 max-sizes = {flash = 16384, ram = 4096} @@ -173,7 +174,7 @@ name = "task-hiffy" features = ["h753", "stm32h7", "i2c", "gpio", "sprot"] priority = 5 max-sizes = {flash = 32768, ram = 32768 } -stacksize = 1024 +stacksize = 1200 start = true task-slots = ["sys", "i2c_driver", "sprot"] @@ -264,7 +265,7 @@ name = "task-power" features = ["sidecar"] priority = 6 max-sizes = {flash = 32768, ram = 8192 } -stacksize = 2048 +stacksize = 2800 start = true task-slots = ["i2c_driver", "sensor", "sequencer"] notifications = ["timer"] diff --git a/drv/stm32xx-i2c-server/src/main.rs b/drv/stm32xx-i2c-server/src/main.rs index 700f8ab47..def47b7cb 100644 --- a/drv/stm32xx-i2c-server/src/main.rs +++ b/drv/stm32xx-i2c-server/src/main.rs @@ -240,7 +240,7 @@ enum Trace { None, } -ringbuf!(Trace, 174, Trace::None); +ringbuf!(Trace, 160, Trace::None); fn reset( controller: &I2cController<'_>, @@ -340,8 +340,25 @@ fn main() -> ! { enable: |notification| { sys_irq_control(notification, true); }, - wfi: |notification| { - let _ = sys_recv_closed(&mut [], notification, TaskId::KERNEL); + wfi: |notification, timeout| { + const TIMER_NOTIFICATION: u32 = 1 << 31; + + let dead = sys_get_timer().now.checked_add(timeout.0).unwrap_lite(); + sys_set_timer(Some(dead), TIMER_NOTIFICATION); + + let m = sys_recv_closed( + &mut [], + notification | TIMER_NOTIFICATION, + TaskId::KERNEL, + ) + .unwrap_lite(); + + if m.operation == TIMER_NOTIFICATION { + I2cControlResult::TimedOut + } else { + sys_set_timer(None, TIMER_NOTIFICATION); + I2cControlResult::Interrupted + } }, }; diff --git a/drv/stm32xx-i2c/Cargo.toml b/drv/stm32xx-i2c/Cargo.toml index ff19c4e40..57f589dd5 100644 --- a/drv/stm32xx-i2c/Cargo.toml +++ b/drv/stm32xx-i2c/Cargo.toml @@ -14,6 +14,7 @@ zerocopy = { workspace = true } drv-i2c-api = { path = "../i2c-api" } drv-stm32xx-sys-api = { path = "../stm32xx-sys-api" } ringbuf = { path = "../../lib/ringbuf" } +counters = { path = "../../lib/counters" } userlib = { path = "../../sys/userlib" } [features] diff --git a/drv/stm32xx-i2c/src/lib.rs b/drv/stm32xx-i2c/src/lib.rs index 3a17d7561..caf769f57 100644 --- a/drv/stm32xx-i2c/src/lib.rs +++ b/drv/stm32xx-i2c/src/lib.rs @@ -73,6 +73,16 @@ pub struct I2cController<'a> { pub registers: &'a RegisterBlock, } +/// +/// A structure to denote an absolute number of ticks to wait. +/// +pub struct I2cTimeout(pub u64); + +pub enum I2cControlResult { + Interrupted, + 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 @@ -80,6 +90,11 @@ pub struct I2cController<'a> { /// 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), } @@ -148,33 +163,48 @@ pub enum ReadLength { Variable, } +#[allow(clippy::upper_case_acronyms)] #[derive(Copy, Clone, Eq, PartialEq)] +enum Register { + CR1, + CR2, + OAR1, + OAR2, + TIMINGR, + TIMEOUTR, + ISR, + PECR, +} + +#[derive(Copy, Clone, Eq, PartialEq, counters::Count)] enum Trace { - WaitISR(u32), - WriteISR(u32), - WriteWaitISR(u32), - ReadISR(u32), - ReadWaitISR(u32), - RxISR(u32), - KonamiISR(u32), - Konami(I2cKonamiCode), - ResetISR(u32), - ResetCR2(u32), - AddrISR(u32), + Wait(Register, u32), + Write(Register, u32), + WriteWait(Register, u32), + Read(Register, u32), + ReadWait(Register, u32), + KonamiOperation(I2cKonamiCode), + Konami(Register, u32), + Reset(Register, u32), + Addr(Register, u32), AddrMatch, AddrNack(u8), + RxReg(Register, u32), Rx(u8, u8), RxNack(u8, u8), Tx(u8, u8), TxBogus(u8), TxOverrun(u8), - TxISR(u32), + TxReg(Register, u32), WaitAddr, WaitRx, WaitTx, BusySleep, Stop, RepeatedStart(bool), + LostInterrupt, + Panic(Register, u32), + #[count(skip)] None, } @@ -427,7 +457,7 @@ impl I2cController<'_> { // ...wait until we see it disabled. loop { let cr1 = i2c.cr1.read(); - ringbuf_entry!(Trace::ResetISR(cr1.bits())); + ringbuf_entry!(Trace::Reset(Register::CR1, cr1.bits())); if cr1.pe().is_disabled() { break; } @@ -436,7 +466,7 @@ impl I2cController<'_> { // And then finally set it i2c.cr1.modify(|_, w| w.pe().set_bit()); - ringbuf_entry!(Trace::ResetCR2(i2c.cr2.read().bits())); + ringbuf_entry!(Trace::Reset(Register::CR2, i2c.cr2.read().bits())); } /// @@ -466,6 +496,62 @@ impl I2cController<'_> { Ok(()) } + /// + /// A routine to panic. This should not be called merely because something + /// has gone wrong with a device (which should rather be indicated by + /// returning an error and resetting the controller if/as needed), but with + /// the controller itself. + /// + fn panic(&self) -> ! { + let i2c = self.registers; + let tgr = &i2c.timingr; + let tor = &i2c.timeoutr; + + ringbuf_entry!(Trace::Panic(Register::CR1, i2c.cr1.read().bits())); + ringbuf_entry!(Trace::Panic(Register::CR2, i2c.cr2.read().bits())); + ringbuf_entry!(Trace::Panic(Register::OAR1, i2c.oar1.read().bits())); + ringbuf_entry!(Trace::Panic(Register::OAR2, i2c.oar2.read().bits())); + ringbuf_entry!(Trace::Panic(Register::TIMINGR, tgr.read().bits())); + ringbuf_entry!(Trace::Panic(Register::TIMEOUTR, tor.read().bits())); + ringbuf_entry!(Trace::Panic(Register::ISR, i2c.isr.read().bits())); + ringbuf_entry!(Trace::Panic(Register::PECR, i2c.pecr.read().bits())); + + panic!(); + } + + /// + /// A common routine to wait for interrupts with a timeout. + /// + fn wfi(&self, ctrl: &I2cControl) -> Result<(), drv_i2c_api::ResponseCode> { + // + // A 100 ms timeout is much, much longer than the I2C timeouts. + // + const TIMEOUT: I2cTimeout = I2cTimeout(100); + + match (ctrl.wfi)(self.notification, TIMEOUT) { + I2cControlResult::TimedOut => { + // + // This really shouldn't happen: it means that not only did + // we not get our expected interrupt, but that the configured + // timeout in the I2C block also didn't function as expected. + // That said, we got our OS timer interrupt (or we wouldn't be + // here at all), which gives us at least control. While we + // could conceivably return an error code in this condition, + // this condition is so unexpected that we want to instead + // make sure we can debug it: we are going to instead call + // our panic routine, which will record some additional data + // from the I2C controller and explicitly panic. This will + // result in a dump that will effectively preserve this state, + // and will (hopefuflly) allow it to be debugged long after it + // happens. + // + ringbuf_entry!(Trace::LostInterrupt); + self.panic(); + } + I2cControlResult::Interrupted => Ok(()), + } + } + fn wait_until_notbusy(&self) -> Result<(), drv_i2c_api::ResponseCode> { let i2c = self.registers; @@ -484,7 +570,7 @@ impl I2cController<'_> { for lap in 0..=BUSY_SLEEP_THRESHOLD + 1 { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::WaitISR(isr.bits())); + ringbuf_entry!(Trace::Wait(Register::ISR, isr.bits())); // // For reasons unclear and unknown, the timeout flag can become @@ -570,7 +656,7 @@ impl I2cController<'_> { while pos < wlen { loop { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::WriteISR(isr.bits())); + ringbuf_entry!(Trace::Write(Register::ISR, isr.bits())); self.check_errors(&isr)?; @@ -583,7 +669,7 @@ impl I2cController<'_> { break; } - (ctrl.wfi)(notification); + self.wfi(ctrl)?; (ctrl.enable)(notification); } @@ -600,7 +686,7 @@ impl I2cController<'_> { // we've been NACK'd (denoting an illegal register value) loop { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::WriteWaitISR(isr.bits())); + ringbuf_entry!(Trace::WriteWait(Register::ISR, isr.bits())); self.check_errors(&isr)?; @@ -613,7 +699,7 @@ impl I2cController<'_> { break; } - (ctrl.wfi)(notification); + self.wfi(ctrl)?; (ctrl.enable)(notification); } } @@ -661,11 +747,11 @@ impl I2cController<'_> { } loop { - (ctrl.wfi)(notification); + self.wfi(ctrl)?; (ctrl.enable)(notification); let isr = i2c.isr.read(); - ringbuf_entry!(Trace::ReadISR(isr.bits())); + ringbuf_entry!(Trace::Read(Register::ISR, isr.bits())); self.check_errors(&isr)?; @@ -708,7 +794,7 @@ impl I2cController<'_> { // All done; now block until our transfer is complete... loop { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::ReadWaitISR(isr.bits())); + ringbuf_entry!(Trace::ReadWait(Register::ISR, isr.bits())); if isr.tc().is_complete() { break; @@ -716,7 +802,7 @@ impl I2cController<'_> { self.check_errors(&isr)?; - (ctrl.wfi)(notification); + self.wfi(ctrl)?; (ctrl.enable)(notification); } } @@ -764,7 +850,7 @@ impl I2cController<'_> { I2cKonamiCode::Read => true, }; - ringbuf_entry!(Trace::Konami(*op)); + ringbuf_entry!(Trace::KonamiOperation(*op)); #[rustfmt::skip] i2c.cr2.modify(|_, w| { w @@ -782,7 +868,7 @@ impl I2cController<'_> { // at our Konami Code sequence). loop { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::KonamiISR(isr.bits())); + ringbuf_entry!(Trace::Konami(Register::ISR, isr.bits())); self.check_errors(&isr)?; @@ -795,7 +881,7 @@ impl I2cController<'_> { break; } - (ctrl.wfi)(notification); + self.wfi(ctrl)?; (ctrl.enable)(notification); } } @@ -846,7 +932,7 @@ impl I2cController<'_> { pub fn operate_as_target( &self, - ctrl: &I2cControl, + ctrl: &I2cTargetControl, mut initiate: impl FnMut(u8) -> bool, mut rxbyte: impl FnMut(u8, u8), mut txbyte: impl FnMut(u8) -> Option, @@ -866,7 +952,7 @@ impl I2cController<'_> { // Wait to be addressed. let (is_write, addr) = loop { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::AddrISR(isr.bits())); + ringbuf_entry!(Trace::Addr(Register::ISR, isr.bits())); // We expect STOPF to have been handled by the transaction loop // below, but given that there may be other irrelevant @@ -929,7 +1015,7 @@ impl I2cController<'_> { // handle below. 'rxloop: loop { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::RxISR(isr.bits())); + ringbuf_entry!(Trace::RxReg(Register::ISR, isr.bits())); // Note: the order of interrupt flag handling in this // routine is important. More details interleaved below. @@ -1025,7 +1111,7 @@ impl I2cController<'_> { 'txloop: loop { let isr = i2c.isr.read(); - ringbuf_entry!(Trace::TxISR(isr.bits())); + ringbuf_entry!(Trace::TxReg(Register::ISR, isr.bits())); // First, we want to see if we're still transmitting. diff --git a/task/spd/src/main.rs b/task/spd/src/main.rs index 6a075ecab..64ad7a2fe 100644 --- a/task/spd/src/main.rs +++ b/task/spd/src/main.rs @@ -25,7 +25,7 @@ use core::cell::Cell; use core::cell::RefCell; use drv_gimlet_seq_api::NUM_SPD_BANKS; use drv_gimlet_state::PowerState; -use drv_stm32xx_i2c::{I2cControl, I2cPins}; +use drv_stm32xx_i2c::{I2cPins, I2cTargetControl}; use drv_stm32xx_sys_api::{OutputType, Pull, Speed, Sys}; use ringbuf::{ringbuf, ringbuf_entry}; use task_jefe_api::Jefe; @@ -249,7 +249,7 @@ fn main() -> ! { rval }; - let ctrl = I2cControl { + let ctrl = I2cTargetControl { enable: |notification| { sys_irq_control(notification, true); },