From 3a9676800f870eedec4a0bfcc297e7f2b962a928 Mon Sep 17 00:00:00 2001 From: James Munns Date: Tue, 11 Aug 2026 19:37:22 +0200 Subject: [PATCH 01/12] Start adding ereport mechanisms for thermals --- Cargo.lock | 3 ++ task/thermal/Cargo.toml | 5 ++ task/thermal/src/control.rs | 104 +++++++++++++++++++++++++++++++----- task/thermal/src/main.rs | 5 +- 4 files changed, 103 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e7b05a34a7..ef5d71dbd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6861,14 +6861,17 @@ dependencies = [ "drv-onewire-devices", "drv-sidecar-seq-api", "drv-transceivers-api", + "ereports", "hubpack", "idol", "idol-runtime", + "microcbor", "mutable-statics", "num-traits", "ringbuf", "serde", "static-cell", + "task-packrat-api", "task-sensor-api", "task-thermal-api", "userlib", diff --git a/task/thermal/Cargo.toml b/task/thermal/Cargo.toml index cbceefef37..31d5617fa1 100644 --- a/task/thermal/Cargo.toml +++ b/task/thermal/Cargo.toml @@ -29,6 +29,11 @@ static-cell.path = "../../lib/static-cell" task-sensor-api.path = "../sensor-api" task-thermal-api.path = "../thermal-api" +# ereports deps +task-packrat-api.path= "../packrat-api" +ereports = { path = "../../lib/ereports", features = ["ereporter-macro"] } +microcbor = { path = "../../lib/microcbor" } + [build-dependencies] anyhow = { workspace = true } idol = { workspace = true } diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index 5d5ae31082..0c71b1fee8 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -72,7 +72,9 @@ use crate::{ThermalError, Trace, bsp::PowerBitmask}; use drv_i2c_devices::max31790::I2cWatchdog; +use microcbor::Encode; use ringbuf::ringbuf_entry_root as ringbuf_entry; +use task_packrat_api::Packrat; use task_sensor_api::{NoData, Sensor as SensorApi, SensorId}; use task_thermal_api::{ FanProperties, SensorReadError, ThermalAutoState, ThermalProperties, @@ -576,6 +578,9 @@ pub(crate) struct ThermalControl<'a, B: BspInterface> { /// Task to which we should post sensor data updates sensor_api: SensorApi, + /// Task to which we should post ereports + ereporter: Ereporter, + /// Target temperature margin. This must be >= 0; as it increases, parts /// are kept cooler than their target temperature value. target_margin: Celsius, @@ -909,7 +914,11 @@ impl<'a, B: BspInterface> ThermalControl<'a, B> { /// # Panics /// This function can only be called once, because it claims mutable static /// buffers. - pub fn new(bsp: &'a mut B, sensor_api: SensorApi) -> Self { + pub fn new( + bsp: &'a mut B, + sensor_api: SensorApi, + packrat_api: Packrat, + ) -> Self { use static_cell::ClaimOnceCell; let [err_blackbox, prev_err_blackbox] = { @@ -933,6 +942,7 @@ impl<'a, B: BspInterface> ThermalControl<'a, B> { prev_err_blackbox, fan_watchdog_configured: false, overheat_timer: None, + ereporter: Ereporter::claim_static_resources(packrat_api), } } @@ -1028,7 +1038,7 @@ impl<'a, B: BspInterface> ThermalControl<'a, B> { // Read fan data and log it to the sensors task let now = sys_get_timer().now; for fan in self.bsp.poll_fan_rpms() { - report_fan_state(fan, &self.sensor_api, now); + report_fan_state(fan, &self.sensor_api, now, &mut self.ereporter); } // Read miscellaneous temperature data and log it to the sensors task @@ -1536,7 +1546,12 @@ impl<'a, B: BspInterface> ThermalControl<'a, B> { /// - Sensor API data /// - Ringbuf logging on state changes /// - ereport logging on state changes -fn report_fan_state(fan: &mut Fan, sensor_api: &SensorApi, now_ms: u64) { +fn report_fan_state( + fan: &mut Fan, + sensor_api: &SensorApi, + now_ms: u64, + ereporter: &mut Ereporter, +) { // Make state matches a little less verbose use FanPresentState as Fps; use FanState as Fs; @@ -1544,11 +1559,16 @@ fn report_fan_state(fan: &mut Fan, sensor_api: &SensorApi, now_ms: u64) { // Step one: report presence, if necessary let id = fan.rpm_sensor_id; if !fan.presence_acked { - let trace = match fan.cur_state { - Fs::NotPresent => Trace::FanRemoved(id), - Fs::Present(_) => Trace::FanAdded(id), + match fan.cur_state { + Fs::NotPresent => { + ringbuf_entry!(Trace::FanRemoved(id)); + _ = ereporter.deliver_ereport(&FanRemoved {}); + } + Fs::Present(_) => { + ringbuf_entry!(Trace::FanAdded(id)); + _ = ereporter.deliver_ereport(&FanInserted {}) + } }; - ringbuf_entry!(trace); fan.presence_acked = true; } @@ -1576,13 +1596,71 @@ fn report_fan_state(fan: &mut Fan, sensor_api: &SensorApi, now_ms: u64) { // Step three: handle state reporting, if unreported if !fan.state_acked { - let trace = match pres { - Fps::Unresponsive(e) => Trace::FanReadFailed(id, e), - Fps::Nominal(_) => Trace::FanNominal(id), - Fps::TooFast(rpm) => Trace::FanOverspeed(id, rpm), - Fps::TooSlow(rpm) => Trace::FanUnderspeed(id, rpm), + match pres { + Fps::Unresponsive(e) => { + _ = ereporter.deliver_ereport(&FanRpmReadFailed {}); + ringbuf_entry!(Trace::FanReadFailed(id, e)); + } + Fps::Nominal(_) => { + _ = ereporter.deliver_ereport(&FanNominal {}); + ringbuf_entry!(Trace::FanNominal(id)); + } + Fps::TooFast(rpm) => { + _ = ereporter.deliver_ereport(&FanOverspeed {}); + ringbuf_entry!(Trace::FanOverspeed(id, rpm)); + } + Fps::TooSlow(rpm) => { + _ = ereporter.deliver_ereport(&FanUnderspeed {}); + ringbuf_entry!(Trace::FanUnderspeed(id, rpm)); + } }; - ringbuf_entry!(trace); fan.state_acked = true; } } + +ereports::declare_ereporter! { + struct Ereporter { + FanRemoved(FanRemoved), + FanInserted(FanInserted), + FanNominal(FanNominal), + FanOverspeed(FanOverspeed), + FanUnderspeed(FanUnderspeed), + FanRpmReadFailed(FanRpmReadFailed), + FanPwmWriteFailed(FanPwmWriteFailed), + } +} + +/// An ereport represent a host reported panic +#[derive(Encode)] +#[ereport(class = "hw.remove.fan", version = 0)] +struct FanRemoved {} + +/// An ereport represent a host reported boot failure +#[derive(Encode)] +#[ereport(class = "hw.insert.fan", version = 0)] +struct FanInserted {} + +/// An ereport represent a host reported boot failure +#[derive(Encode)] +#[ereport(class = "hw.fan.good", version = 0)] +struct FanNominal {} + +/// An ereport represent a host reported boot failure +#[derive(Encode)] +#[ereport(class = "hw.fan.overspeed", version = 0)] +struct FanOverspeed {} + +/// An ereport represent a host reported boot failure +#[derive(Encode)] +#[ereport(class = "hw.fan.underspeed", version = 0)] +struct FanUnderspeed {} + +/// An ereport represent a host reported boot failure +#[derive(Encode)] +#[ereport(class = "hw.fan.rpmfail", version = 0)] +struct FanRpmReadFailed {} + +/// An ereport represent a host reported boot failure +#[derive(Encode)] +#[ereport(class = "hw.fan.pwmfail", version = 0)] +struct FanPwmWriteFailed {} diff --git a/task/thermal/src/main.rs b/task/thermal/src/main.rs index 9383e0185c..6fe7b90ec7 100644 --- a/task/thermal/src/main.rs +++ b/task/thermal/src/main.rs @@ -51,6 +51,7 @@ use drv_i2c_api::ResponseCode; use drv_i2c_devices::max31790::I2cWatchdog; use idol_runtime::{NotificationHandler, RequestError}; use ringbuf::*; +use task_packrat_api::Packrat; use task_sensor_api::{Sensor as SensorApi, SensorId}; use task_thermal_api::{ SensorReadError, ThermalAutoState, ThermalError, ThermalMode, @@ -62,6 +63,7 @@ use userlib::{ }; task_slot!(I2C, i2c_driver); +task_slot!(PACKRAT, packrat); task_slot!(SENSOR, sensor); #[derive(Copy, Clone, PartialEq, counters::Count)] @@ -394,11 +396,12 @@ impl<'a, B: control::BspInterface> NotificationHandler for ServerImpl<'a, B> { fn main() -> ! { let i2c_task = I2C.get_task_id(); let sensor_api = SensorApi::from(SENSOR.get_task_id()); + let packrat = Packrat::from(PACKRAT.get_task_id()); ringbuf_entry!(Trace::Start); let mut bsp = Bsp::new(i2c_task); - let control = ThermalControl::new(&mut bsp, sensor_api); + let control = ThermalControl::new(&mut bsp, sensor_api, packrat); // This will put our timer in the past, and should immediately kick us. let deadline = sys_get_timer().now; From c4b2b31d8162230d6cac469c5fcb596278f397d3 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 13 Aug 2026 12:22:44 +0200 Subject: [PATCH 02/12] Fill in thermal ereports --- app/cosmo/base.toml | 2 +- app/gimlet/base.toml | 2 +- app/grapefruit/rev-a-ruby.toml | 2 +- app/minibar/base.toml | 2 +- app/sidecar/base.toml | 2 +- task/thermal/src/bsp/common/emc2305.rs | 19 +++--- task/thermal/src/bsp/common/max31790.rs | 23 +++---- task/thermal/src/bsp/cosmo_ab.rs | 7 +-- task/thermal/src/bsp/gimlet_bcdef.rs | 7 +-- task/thermal/src/bsp/grapefruit.rs | 7 +-- task/thermal/src/bsp/sidecar_bcd.rs | 8 +-- task/thermal/src/control.rs | 79 +++++++++++++++++-------- 12 files changed, 88 insertions(+), 72 deletions(-) diff --git a/app/cosmo/base.toml b/app/cosmo/base.toml index a74389e29d..de5acab565 100644 --- a/app/cosmo/base.toml +++ b/app/cosmo/base.toml @@ -154,7 +154,7 @@ priority = 8 max-sizes = {flash = 32768, ram = 8192 } stacksize = 3000 start = true -task-slots = ["i2c_driver", "sensor", "cosmo_seq", "jefe"] +task-slots = ["i2c_driver", "sensor", "cosmo_seq", "jefe", "packrat"] notifications = ["timer"] [tasks.power] diff --git a/app/gimlet/base.toml b/app/gimlet/base.toml index bdd75ca3bd..6cbf38bd6d 100644 --- a/app/gimlet/base.toml +++ b/app/gimlet/base.toml @@ -138,7 +138,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 8192 } stacksize = 2000 start = true -task-slots = ["i2c_driver", "sensor", "gimlet_seq", "jefe"] +task-slots = ["i2c_driver", "sensor", "gimlet_seq", "jefe", "thermal"] notifications = ["timer"] [tasks.power] diff --git a/app/grapefruit/rev-a-ruby.toml b/app/grapefruit/rev-a-ruby.toml index 39af4f5ee5..da307e9854 100644 --- a/app/grapefruit/rev-a-ruby.toml +++ b/app/grapefruit/rev-a-ruby.toml @@ -16,7 +16,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 8192 } stacksize = 6000 start = true -task-slots = ["i2c_driver", "sensor", "jefe"] +task-slots = ["i2c_driver", "sensor", "jefe", "thermal"] notifications = ["timer"] [config] diff --git a/app/minibar/base.toml b/app/minibar/base.toml index 31e2871993..a64c4bcccd 100644 --- a/app/minibar/base.toml +++ b/app/minibar/base.toml @@ -112,7 +112,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 16384 } stacksize = 8096 start = true -task-slots = ["i2c_driver", "sensor"] +task-slots = ["i2c_driver", "sensor", "packrat"] notifications = ["timer"] [tasks.power] diff --git a/app/sidecar/base.toml b/app/sidecar/base.toml index 3ae33705a4..84ab2d5c38 100644 --- a/app/sidecar/base.toml +++ b/app/sidecar/base.toml @@ -299,7 +299,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 16384 } stacksize = 4000 start = true -task-slots = ["i2c_driver", "sensor", "sequencer"] +task-slots = ["i2c_driver", "sensor", "sequencer", "thermal"] notifications = ["timer"] [tasks.power] diff --git a/task/thermal/src/bsp/common/emc2305.rs b/task/thermal/src/bsp/common/emc2305.rs index 3bf6f95745..aed3294ddb 100644 --- a/task/thermal/src/bsp/common/emc2305.rs +++ b/task/thermal/src/bsp/common/emc2305.rs @@ -6,6 +6,7 @@ use drv_i2c_api::{I2cDevice, ResponseCode}; use drv_i2c_devices::emc2305::Emc2305; +use drv_i2c_devices::emc2305::Fan as EmcFan; use ringbuf::ringbuf_entry_root; use task_sensor_api::SensorId; use task_thermal_api::{SensorReadError, ThermalError}; @@ -96,20 +97,18 @@ impl From for SensorReadError { #[allow(dead_code)] pub(crate) const fn make_consecutive_nonremovable_fans( sensors: &'static [SensorId; N], -) -> [crate::control::Fan; N] { - const ONE: crate::control::Fan = - crate::control::Fan::new( - SensorId::new(0), - drv_i2c_devices::emc2305::Fan::new_const(0), - ); +) -> [crate::control::Fan; N] { + const ONE: crate::control::Fan = crate::control::Fan::new( + SensorId::new(0), + SANYO_DENKI_FAN_PROPERTIES, + EmcFan::new_const(0), + ); let mut out = [ONE; N]; let mut idx = 0; while idx < N { - out[idx] = crate::control::Fan::new( - sensors[idx], - drv_i2c_devices::emc2305::Fan::new_const(idx as u8), - ); + out[idx].rpm_sensor_id = sensors[idx]; + out[idx].bsp_data = EmcFan::new_const(idx as u8); out[idx].cur_state = FanState::Present(FanPresentState::Unresponsive( SensorReadError::NoData, )); diff --git a/task/thermal/src/bsp/common/max31790.rs b/task/thermal/src/bsp/common/max31790.rs index a8bf5b69be..e526fe7ab7 100644 --- a/task/thermal/src/bsp/common/max31790.rs +++ b/task/thermal/src/bsp/common/max31790.rs @@ -5,10 +5,13 @@ //! Common types and helpers for Max31790 Fan Controller use drv_i2c_api::{I2cDevice, ResponseCode}; +use drv_i2c_devices::max31790::Fan as MaxFan; use drv_i2c_devices::max31790::Max31790; use ringbuf::ringbuf_entry_root; use task_sensor_api::SensorId; -use task_thermal_api::{SensorReadError, ThermalError}; +use task_thermal_api::{ + SANYO_DENKI_FAN_PROPERTIES, SensorReadError, ThermalError, +}; use crate::{ Trace, @@ -103,20 +106,18 @@ impl From for SensorReadError { #[allow(dead_code)] pub(crate) const fn make_consecutive_nonremovable_fans( sensors: &'static [SensorId; N], -) -> [crate::control::Fan; N] { - const ONE: crate::control::Fan = - crate::control::Fan::new( - SensorId::new(0), - drv_i2c_devices::max31790::Fan::new_const(0), - ); +) -> [crate::control::Fan; N] { + const ONE: crate::control::Fan = crate::control::Fan::new( + SensorId::new(0), + SANYO_DENKI_FAN_PROPERTIES, + MaxFan::new_const(0), + ); let mut out = [ONE; N]; let mut idx = 0; while idx < N { - out[idx] = crate::control::Fan::new( - sensors[idx], - drv_i2c_devices::max31790::Fan::new_const(idx as u8), - ); + out[idx].rpm_sensor_id = sensors[idx]; + out[idx].bsp_data = MaxFan::new_const(idx as u8); out[idx].cur_state = FanState::Present(FanPresentState::Unresponsive( SensorReadError::NoData, )); diff --git a/task/thermal/src/bsp/cosmo_ab.rs b/task/thermal/src/bsp/cosmo_ab.rs index b1868c4ae5..ab9f7eb52a 100644 --- a/task/thermal/src/bsp/cosmo_ab.rs +++ b/task/thermal/src/bsp/cosmo_ab.rs @@ -15,10 +15,7 @@ pub use drv_cpu_seq_api::SeqError; use drv_cpu_seq_api::{PowerState, Sequencer, StateChangeReason}; use drv_i2c_devices::max31790::I2cWatchdog; use task_sensor_api::{Sensor, SensorId}; -use task_thermal_api::{ - SANYO_DENKI_FAN_PROPERTIES, SensorReadError, ThermalError, - ThermalProperties, -}; +use task_thermal_api::{SensorReadError, ThermalError, ThermalProperties}; use userlib::{ TaskId, task_slot, units::{Celsius, PWMDuty}, @@ -126,7 +123,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl.try_initialize() { for fan in self.fans.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(&SANYO_DENKI_FAN_PROPERTIES, || { + fan.poll_rpm_with(|| { fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) }); } diff --git a/task/thermal/src/bsp/gimlet_bcdef.rs b/task/thermal/src/bsp/gimlet_bcdef.rs index 05f2914955..0c69b2cae9 100644 --- a/task/thermal/src/bsp/gimlet_bcdef.rs +++ b/task/thermal/src/bsp/gimlet_bcdef.rs @@ -15,10 +15,7 @@ pub use drv_cpu_seq_api::SeqError; use drv_cpu_seq_api::{PowerState, Sequencer, StateChangeReason}; use drv_i2c_devices::max31790::I2cWatchdog; use task_sensor_api::{Sensor, SensorId}; -use task_thermal_api::{ - SANYO_DENKI_FAN_PROPERTIES, SensorReadError, ThermalError, - ThermalProperties, -}; +use task_thermal_api::{SensorReadError, ThermalError, ThermalProperties}; use userlib::{ TaskId, task_slot, units::{Celsius, PWMDuty}, @@ -171,7 +168,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl.try_initialize() { for fan in self.fans.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(&SANYO_DENKI_FAN_PROPERTIES, || { + fan.poll_rpm_with(|| { fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) }); } diff --git a/task/thermal/src/bsp/grapefruit.rs b/task/thermal/src/bsp/grapefruit.rs index f072e7cc2d..decd40d1ee 100644 --- a/task/thermal/src/bsp/grapefruit.rs +++ b/task/thermal/src/bsp/grapefruit.rs @@ -8,10 +8,7 @@ use crate::control::{ActiveInputState, MiscSensorPollingOutcome}; use crate::control::{ChannelType, PidConfig}; use drv_i2c_devices::max31790::I2cWatchdog; use task_sensor_api::SensorId; -use task_thermal_api::{ - SANYO_DENKI_FAN_PROPERTIES, SensorReadError, ThermalError, - ThermalProperties, -}; +use task_thermal_api::{SensorReadError, ThermalError, ThermalProperties}; use userlib::TaskId; use userlib::units::{Celsius, PWMDuty}; @@ -91,7 +88,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl.try_initialize() { for fan in self.fans.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(&SANYO_DENKI_FAN_PROPERTIES, || { + fan.poll_rpm_with(|| { fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) }); } diff --git a/task/thermal/src/bsp/sidecar_bcd.rs b/task/thermal/src/bsp/sidecar_bcd.rs index 59ac381ea6..d5d327a28f 100644 --- a/task/thermal/src/bsp/sidecar_bcd.rs +++ b/task/thermal/src/bsp/sidecar_bcd.rs @@ -16,9 +16,7 @@ use drv_sidecar_seq_api::{Sequencer, TofinoSeqState, TofinoSequencerPolicy}; use ringbuf::ringbuf_entry_root; use task_sensor_api::SensorId; use task_thermal_api::ThermalError; -use task_thermal_api::{ - SANYO_DENKI_FAN_PROPERTIES, SensorReadError, ThermalProperties, -}; +use task_thermal_api::{SensorReadError, ThermalProperties}; use userlib::{TaskId, task_slot, units::Celsius}; include!(concat!(env!("OUT_DIR"), "/i2c_config.rs")); @@ -155,7 +153,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl_east.try_initialize() { for fan in east.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(&SANYO_DENKI_FAN_PROPERTIES, || { + fan.poll_rpm_with(|| { fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) }); } @@ -163,7 +161,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl_west.try_initialize() { for fan in west.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(&SANYO_DENKI_FAN_PROPERTIES, || { + fan.poll_rpm_with(|| { fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) }); } diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index 0c71b1fee8..bb4157d11c 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -240,18 +240,25 @@ pub struct Fan { pub cur_state: FanState, /// A BSP-specific ID used to identify the fan pub bsp_data: D, + /// Parameter model for this fan + pub model: FanProperties, } #[allow(dead_code)] // Not all bsps have fans! impl Fan { /// Create a new fan - pub const fn new(rpm_sensor_id: SensorId, bsp_data: D) -> Self { + pub const fn new( + rpm_sensor_id: SensorId, + model: FanProperties, + bsp_data: D, + ) -> Self { Self { rpm_sensor_id, presence_acked: false, state_acked: false, cur_state: FanState::NotPresent, bsp_data, + model, } } @@ -346,7 +353,6 @@ impl Fan { /// retrieve the RPM. Used to share logic across different fan controllers pub(crate) fn poll_rpm_with>( &mut self, - model: &FanProperties, poll_rpm: impl FnOnce() -> Result, ) { // If this fan is not present, then do not attempt to poll it. Presence @@ -361,9 +367,9 @@ impl Fan { Ok(rpm) => { // The poll went well! Use the model to determine if this // reading is nominal or not, and report that as the state. - let state = if rpm < model.underspeed_rpm { + let state = if rpm < self.model.underspeed_rpm { FanPresentState::TooSlow(rpm) - } else if rpm > model.overspeed_rpm { + } else if rpm > self.model.overspeed_rpm { FanPresentState::TooFast(rpm) } else { FanPresentState::Nominal(rpm) @@ -1562,11 +1568,11 @@ fn report_fan_state( match fan.cur_state { Fs::NotPresent => { ringbuf_entry!(Trace::FanRemoved(id)); - _ = ereporter.deliver_ereport(&FanRemoved {}); + _ = ereporter.deliver_ereport(&FanRemoved { id: id.into() }); } Fs::Present(_) => { ringbuf_entry!(Trace::FanAdded(id)); - _ = ereporter.deliver_ereport(&FanInserted {}) + _ = ereporter.deliver_ereport(&FanInserted { id: id.into() }) } }; fan.presence_acked = true; @@ -1596,21 +1602,29 @@ fn report_fan_state( // Step three: handle state reporting, if unreported if !fan.state_acked { + let fan_info = || FanInfo { + id: id.into(), + lo_rpm_lim: fan.model.underspeed_rpm.0, + hi_rpm_lim: fan.model.overspeed_rpm.0, + }; match pres { Fps::Unresponsive(e) => { - _ = ereporter.deliver_ereport(&FanRpmReadFailed {}); + _ = ereporter + .deliver_ereport(&FanRpmReadFailed { id: id.into() }); ringbuf_entry!(Trace::FanReadFailed(id, e)); } Fps::Nominal(_) => { - _ = ereporter.deliver_ereport(&FanNominal {}); + _ = ereporter.deliver_ereport(&FanNominal { info: fan_info() }); ringbuf_entry!(Trace::FanNominal(id)); } Fps::TooFast(rpm) => { - _ = ereporter.deliver_ereport(&FanOverspeed {}); + _ = ereporter + .deliver_ereport(&FanOverspeed { info: fan_info() }); ringbuf_entry!(Trace::FanOverspeed(id, rpm)); } Fps::TooSlow(rpm) => { - _ = ereporter.deliver_ereport(&FanUnderspeed {}); + _ = ereporter + .deliver_ereport(&FanUnderspeed { info: fan_info() }); ringbuf_entry!(Trace::FanUnderspeed(id, rpm)); } }; @@ -1626,41 +1640,54 @@ ereports::declare_ereporter! { FanOverspeed(FanOverspeed), FanUnderspeed(FanUnderspeed), FanRpmReadFailed(FanRpmReadFailed), - FanPwmWriteFailed(FanPwmWriteFailed), } } +#[derive(Encode)] +struct FanInfo { + id: u32, + lo_rpm_lim: u16, + hi_rpm_lim: u16, +} + /// An ereport represent a host reported panic #[derive(Encode)] #[ereport(class = "hw.remove.fan", version = 0)] -struct FanRemoved {} +struct FanRemoved { + id: u32, +} /// An ereport represent a host reported boot failure #[derive(Encode)] #[ereport(class = "hw.insert.fan", version = 0)] -struct FanInserted {} - -/// An ereport represent a host reported boot failure -#[derive(Encode)] -#[ereport(class = "hw.fan.good", version = 0)] -struct FanNominal {} +struct FanInserted { + id: u32, +} /// An ereport represent a host reported boot failure #[derive(Encode)] -#[ereport(class = "hw.fan.overspeed", version = 0)] -struct FanOverspeed {} +#[ereport(class = "hw.fan.ok", version = 0)] +struct FanNominal { + info: FanInfo, +} /// An ereport represent a host reported boot failure #[derive(Encode)] -#[ereport(class = "hw.fan.underspeed", version = 0)] -struct FanUnderspeed {} +#[ereport(class = "hw.fan.rpm.hi", version = 0)] +struct FanOverspeed { + info: FanInfo, +} /// An ereport represent a host reported boot failure #[derive(Encode)] -#[ereport(class = "hw.fan.rpmfail", version = 0)] -struct FanRpmReadFailed {} +#[ereport(class = "hw.fan.rpm.lo", version = 0)] +struct FanUnderspeed { + info: FanInfo, +} /// An ereport represent a host reported boot failure #[derive(Encode)] -#[ereport(class = "hw.fan.pwmfail", version = 0)] -struct FanPwmWriteFailed {} +#[ereport(class = "hw.fan.rpm.err", version = 0)] +struct FanRpmReadFailed { + id: u32, +} From 28b4ead3124061813a902e9958ccfa2de9edde0a Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 13 Aug 2026 12:37:02 +0200 Subject: [PATCH 03/12] Fix a few things --- app/gimlet/base.toml | 2 +- app/grapefruit/rev-a-ruby.toml | 2 +- app/sidecar/base.toml | 2 +- task/thermal/src/bsp/sidecar_bcd.rs | 58 +++++++++++------------------ task/thermal/src/control.rs | 26 ++++++++----- 5 files changed, 40 insertions(+), 50 deletions(-) diff --git a/app/gimlet/base.toml b/app/gimlet/base.toml index 6cbf38bd6d..836ec187eb 100644 --- a/app/gimlet/base.toml +++ b/app/gimlet/base.toml @@ -138,7 +138,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 8192 } stacksize = 2000 start = true -task-slots = ["i2c_driver", "sensor", "gimlet_seq", "jefe", "thermal"] +task-slots = ["i2c_driver", "sensor", "gimlet_seq", "jefe", "packrat"] notifications = ["timer"] [tasks.power] diff --git a/app/grapefruit/rev-a-ruby.toml b/app/grapefruit/rev-a-ruby.toml index da307e9854..a6d24b68e5 100644 --- a/app/grapefruit/rev-a-ruby.toml +++ b/app/grapefruit/rev-a-ruby.toml @@ -16,7 +16,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 8192 } stacksize = 6000 start = true -task-slots = ["i2c_driver", "sensor", "jefe", "thermal"] +task-slots = ["i2c_driver", "sensor", "jefe", "packrat"] notifications = ["timer"] [config] diff --git a/app/sidecar/base.toml b/app/sidecar/base.toml index 84ab2d5c38..533618768f 100644 --- a/app/sidecar/base.toml +++ b/app/sidecar/base.toml @@ -299,7 +299,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 16384 } stacksize = 4000 start = true -task-slots = ["i2c_driver", "sensor", "sequencer", "thermal"] +task-slots = ["i2c_driver", "sensor", "sequencer", "packrat"] notifications = ["timer"] [tasks.power] diff --git a/task/thermal/src/bsp/sidecar_bcd.rs b/task/thermal/src/bsp/sidecar_bcd.rs index d5d327a28f..35a7b39345 100644 --- a/task/thermal/src/bsp/sidecar_bcd.rs +++ b/task/thermal/src/bsp/sidecar_bcd.rs @@ -15,7 +15,7 @@ pub use drv_sidecar_seq_api::SeqError; use drv_sidecar_seq_api::{Sequencer, TofinoSeqState, TofinoSequencerPolicy}; use ringbuf::ringbuf_entry_root; use task_sensor_api::SensorId; -use task_thermal_api::ThermalError; +use task_thermal_api::{SANYO_DENKI_FAN_PROPERTIES, ThermalError}; use task_thermal_api::{SensorReadError, ThermalProperties}; use userlib::{TaskId, task_slot, units::Celsius}; @@ -520,39 +520,23 @@ const MISC_SENSORS: [TemperatureSensor; NUM_TEMPERATURE_SENSORS] = [ // 6 West WSW 0 (1) // 7 West WNW 1 (2) type Fan = crate::control::Fan; -const FANS: [Fan; NUM_FANS] = [ - // EAST FANS - Fan::new( - sensors::MAX31790_SPEED_SENSORS[0], - drv_i2c_devices::max31790::Fan::new_const(2), - ), - Fan::new( - sensors::MAX31790_SPEED_SENSORS[1], - drv_i2c_devices::max31790::Fan::new_const(3), - ), - Fan::new( - sensors::MAX31790_SPEED_SENSORS[2], - drv_i2c_devices::max31790::Fan::new_const(0), - ), - Fan::new( - sensors::MAX31790_SPEED_SENSORS[3], - drv_i2c_devices::max31790::Fan::new_const(1), - ), - // WEST FANS - Fan::new( - sensors::MAX31790_SPEED_SENSORS[4], - drv_i2c_devices::max31790::Fan::new_const(2), - ), - Fan::new( - sensors::MAX31790_SPEED_SENSORS[5], - drv_i2c_devices::max31790::Fan::new_const(3), - ), - Fan::new( - sensors::MAX31790_SPEED_SENSORS[6], - drv_i2c_devices::max31790::Fan::new_const(0), - ), - Fan::new( - sensors::MAX31790_SPEED_SENSORS[7], - drv_i2c_devices::max31790::Fan::new_const(1), - ), -]; +use drv_i2c_devices::max31790::Fan as MaxFan; +const FAN_ORDER: [u8; NUM_FANS] = [2, 3, 0, 1, 2, 3, 0, 1]; +const fn make_fans() -> [Fan; NUM_FANS] { + const ONE_FAN: Fan = Fan::new( + SensorId::new(0), + SANYO_DENKI_FAN_PROPERTIES, + MaxFan::new_const(0), + ); + let mut fans = [ONE_FAN; NUM_FANS]; + let mut idx = 0; + while idx < NUM_FANS { + fans[idx].rpm_sensor_id = sensors::MAX31790_SPEED_SENSORS[idx]; + fans[idx].bsp_data = MaxFan::new_const(FAN_ORDER[idx]); + idx += 1; + } + + fans +} + +const FANS: [Fan; NUM_FANS] = make_fans(); diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index bb4157d11c..5b5c658e11 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -1618,13 +1618,17 @@ fn report_fan_state( ringbuf_entry!(Trace::FanNominal(id)); } Fps::TooFast(rpm) => { - _ = ereporter - .deliver_ereport(&FanOverspeed { info: fan_info() }); + _ = ereporter.deliver_ereport(&FanOverspeed { + info: fan_info(), + rpm: rpm.0, + }); ringbuf_entry!(Trace::FanOverspeed(id, rpm)); } Fps::TooSlow(rpm) => { - _ = ereporter - .deliver_ereport(&FanUnderspeed { info: fan_info() }); + _ = ereporter.deliver_ereport(&FanUnderspeed { + info: fan_info(), + rpm: rpm.0, + }); ringbuf_entry!(Trace::FanUnderspeed(id, rpm)); } }; @@ -1650,42 +1654,44 @@ struct FanInfo { hi_rpm_lim: u16, } -/// An ereport represent a host reported panic +/// An ereport representing a fan being removed #[derive(Encode)] #[ereport(class = "hw.remove.fan", version = 0)] struct FanRemoved { id: u32, } -/// An ereport represent a host reported boot failure +/// An ereport representing a fan being inserted #[derive(Encode)] #[ereport(class = "hw.insert.fan", version = 0)] struct FanInserted { id: u32, } -/// An ereport represent a host reported boot failure +/// An ereport representing a fan entering a nominal state #[derive(Encode)] #[ereport(class = "hw.fan.ok", version = 0)] struct FanNominal { info: FanInfo, } -/// An ereport represent a host reported boot failure +/// An ereport representing a fan becoming overspeed #[derive(Encode)] #[ereport(class = "hw.fan.rpm.hi", version = 0)] struct FanOverspeed { info: FanInfo, + rpm: u16, } -/// An ereport represent a host reported boot failure +/// An ereport representing a fan becoming underspeed #[derive(Encode)] #[ereport(class = "hw.fan.rpm.lo", version = 0)] struct FanUnderspeed { info: FanInfo, + rpm: u16, } -/// An ereport represent a host reported boot failure +/// An ereport representing a failure to remove a fan #[derive(Encode)] #[ereport(class = "hw.fan.rpm.err", version = 0)] struct FanRpmReadFailed { From 0500cc19e121e546e32cb74787de9518cbbcd288 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 13 Aug 2026 12:43:58 +0200 Subject: [PATCH 04/12] Remove unused task slot --- app/cosmo/base.toml | 2 +- app/gimlet/base.toml | 2 +- app/grapefruit/rev-a-ruby.toml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/cosmo/base.toml b/app/cosmo/base.toml index de5acab565..e265db9612 100644 --- a/app/cosmo/base.toml +++ b/app/cosmo/base.toml @@ -154,7 +154,7 @@ priority = 8 max-sizes = {flash = 32768, ram = 8192 } stacksize = 3000 start = true -task-slots = ["i2c_driver", "sensor", "cosmo_seq", "jefe", "packrat"] +task-slots = ["i2c_driver", "sensor", "cosmo_seq", "packrat"] notifications = ["timer"] [tasks.power] diff --git a/app/gimlet/base.toml b/app/gimlet/base.toml index 836ec187eb..fe44528d13 100644 --- a/app/gimlet/base.toml +++ b/app/gimlet/base.toml @@ -138,7 +138,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 8192 } stacksize = 2000 start = true -task-slots = ["i2c_driver", "sensor", "gimlet_seq", "jefe", "packrat"] +task-slots = ["i2c_driver", "sensor", "gimlet_seq", "packrat"] notifications = ["timer"] [tasks.power] diff --git a/app/grapefruit/rev-a-ruby.toml b/app/grapefruit/rev-a-ruby.toml index a6d24b68e5..07fedbd06e 100644 --- a/app/grapefruit/rev-a-ruby.toml +++ b/app/grapefruit/rev-a-ruby.toml @@ -16,7 +16,7 @@ priority = 5 max-sizes = {flash = 32768, ram = 8192 } stacksize = 6000 start = true -task-slots = ["i2c_driver", "sensor", "jefe", "packrat"] +task-slots = ["i2c_driver", "sensor", "packrat"] notifications = ["timer"] [config] From 1dd736d3dc5998e3d5b7abce1e9a433850463db4 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 13 Aug 2026 12:51:26 +0200 Subject: [PATCH 05/12] Add missing import --- task/thermal/src/bsp/common/emc2305.rs | 4 +++- task/thermal/src/bsp/sidecar_bcd.rs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/task/thermal/src/bsp/common/emc2305.rs b/task/thermal/src/bsp/common/emc2305.rs index aed3294ddb..754a4b6391 100644 --- a/task/thermal/src/bsp/common/emc2305.rs +++ b/task/thermal/src/bsp/common/emc2305.rs @@ -9,7 +9,9 @@ use drv_i2c_devices::emc2305::Emc2305; use drv_i2c_devices::emc2305::Fan as EmcFan; use ringbuf::ringbuf_entry_root; use task_sensor_api::SensorId; -use task_thermal_api::{SensorReadError, ThermalError}; +use task_thermal_api::{ + SANYO_DENKI_FAN_PROPERTIES, SensorReadError, ThermalError, +}; use crate::{ Trace, diff --git a/task/thermal/src/bsp/sidecar_bcd.rs b/task/thermal/src/bsp/sidecar_bcd.rs index 35a7b39345..12d4000839 100644 --- a/task/thermal/src/bsp/sidecar_bcd.rs +++ b/task/thermal/src/bsp/sidecar_bcd.rs @@ -9,6 +9,7 @@ use crate::control::{ DynamicTemperatureState, MiscSensorPollingOutcome, PidConfig, TimestampedTemperatureReading, }; +use drv_i2c_devices::max31790::Fan as MaxFan; use drv_i2c_devices::max31790::Max31790; use drv_i2c_devices::tmp451::*; pub use drv_sidecar_seq_api::SeqError; @@ -519,8 +520,7 @@ const MISC_SENSORS: [TemperatureSensor; NUM_TEMPERATURE_SENSORS] = [ // 5 West NW 3 (4) // 6 West WSW 0 (1) // 7 West WNW 1 (2) -type Fan = crate::control::Fan; -use drv_i2c_devices::max31790::Fan as MaxFan; +type Fan = crate::control::Fan; const FAN_ORDER: [u8; NUM_FANS] = [2, 3, 0, 1, 2, 3, 0, 1]; const fn make_fans() -> [Fan; NUM_FANS] { const ONE_FAN: Fan = Fan::new( From 6c1795b637355c85694b0f5f056f7e4f2a9a2228 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 13 Aug 2026 13:03:21 +0200 Subject: [PATCH 06/12] Adjust ruby sizes --- app/grapefruit/rev-a-ruby.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/grapefruit/rev-a-ruby.toml b/app/grapefruit/rev-a-ruby.toml index 07fedbd06e..87a1440dd4 100644 --- a/app/grapefruit/rev-a-ruby.toml +++ b/app/grapefruit/rev-a-ruby.toml @@ -13,8 +13,8 @@ interrupts = {"usart6.irq" = "usart-irq"} name = "task-thermal" features = ["grapefruit"] priority = 5 -max-sizes = {flash = 32768, ram = 8192 } -stacksize = 6000 +max-sizes = {flash = 32768, ram = 4096 } +stacksize = 1536 start = true task-slots = ["i2c_driver", "sensor", "packrat"] notifications = ["timer"] From 88443160d06e86b50f8a5537c4deb502d0e2b5c7 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 13 Aug 2026 14:43:34 +0200 Subject: [PATCH 07/12] Don't get reply-faulted by packrat because ereports aren't enabled --- app/grapefruit/rev-a-ruby.toml | 20 ++++++++++++++++++++ task/thermal/src/control.rs | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/grapefruit/rev-a-ruby.toml b/app/grapefruit/rev-a-ruby.toml index 87a1440dd4..f48332e502 100644 --- a/app/grapefruit/rev-a-ruby.toml +++ b/app/grapefruit/rev-a-ruby.toml @@ -9,6 +9,26 @@ features = ["usart6", "hardware_flow_control"] uses = ["usart6"] interrupts = {"usart6.irq" = "usart-irq"} +# needed for thermal ereports +[tasks.packrat] +features = ["ereport"] +stacksize = 1280 + +# We don't actually tell jefe to notify us on faults, +# but this is still required +notifications = ["task-faulted"] +task-slots = ["jefe"] + +# needed for thermal ereports +[tasks.rng_driver] +features = ["h753", "packrat"] +name = "drv-stm32h7-rng" +priority = 6 +uses = ["rng"] +start = true +stacksize = 512 +task-slots = ["sys", "packrat"] + [tasks.thermal] name = "task-thermal" features = ["grapefruit"] diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index 5b5c658e11..152a90c3c0 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -1647,7 +1647,7 @@ ereports::declare_ereporter! { } } -#[derive(Encode)] +#[derive(microcbor::EncodeFields)] struct FanInfo { id: u32, lo_rpm_lim: u16, @@ -1672,6 +1672,7 @@ struct FanInserted { #[derive(Encode)] #[ereport(class = "hw.fan.ok", version = 0)] struct FanNominal { + #[cbor(flatten)] info: FanInfo, } @@ -1679,6 +1680,7 @@ struct FanNominal { #[derive(Encode)] #[ereport(class = "hw.fan.rpm.hi", version = 0)] struct FanOverspeed { + #[cbor(flatten)] info: FanInfo, rpm: u16, } @@ -1687,6 +1689,7 @@ struct FanOverspeed { #[derive(Encode)] #[ereport(class = "hw.fan.rpm.lo", version = 0)] struct FanUnderspeed { + #[cbor(flatten)] info: FanInfo, rpm: u16, } From 5c162513c54f9d9e1439ae5920e28c0c3b82f8f9 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 20 Aug 2026 17:25:57 +0200 Subject: [PATCH 08/12] Switch from sensor_id to name --- Cargo.lock | 1 + task/sensor-api/src/lib.rs | 2 +- task/thermal/Cargo.toml | 1 + task/thermal/src/control.rs | 22 +++++++++++++--------- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8996f74310..4e0a52aa70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6921,6 +6921,7 @@ dependencies = [ "drv-sidecar-seq-api", "drv-transceivers-api", "ereports", + "fixedstr", "hubpack", "idol", "idol-runtime", diff --git a/task/sensor-api/src/lib.rs b/task/sensor-api/src/lib.rs index fb35430cd9..4b6a697e9d 100644 --- a/task/sensor-api/src/lib.rs +++ b/task/sensor-api/src/lib.rs @@ -95,7 +95,7 @@ impl SensorId { /// Returns the name of this sensor. #[cfg(feature = "sensor-name-lookup")] - pub fn name( + pub const fn name( &self, ) -> fixedstr::FixedStr<'static, { config::MAX_SENSOR_NAME_LEN }> { config::SENSOR_ID_TO_NAME[self.0 as usize] diff --git a/task/thermal/Cargo.toml b/task/thermal/Cargo.toml index b674034838..768c1e0d40 100644 --- a/task/thermal/Cargo.toml +++ b/task/thermal/Cargo.toml @@ -33,6 +33,7 @@ task-thermal-api.path = "../thermal-api" task-packrat-api.path= "../packrat-api" ereports = { path = "../../lib/ereports", features = ["ereporter-macro"] } microcbor = { path = "../../lib/microcbor" } +fixedstr = { path = "../../lib/fixedstr", features = ["microcbor"] } [build-dependencies] anyhow = { workspace = true } diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index 152a90c3c0..b5db07ab03 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -75,7 +75,9 @@ use drv_i2c_devices::max31790::I2cWatchdog; use microcbor::Encode; use ringbuf::ringbuf_entry_root as ringbuf_entry; use task_packrat_api::Packrat; -use task_sensor_api::{NoData, Sensor as SensorApi, SensorId}; +use task_sensor_api::{ + NoData, Sensor as SensorApi, SensorId, config::MAX_SENSOR_NAME_LEN, +}; use task_thermal_api::{ FanProperties, SensorReadError, ThermalAutoState, ThermalProperties, }; @@ -242,6 +244,7 @@ pub struct Fan { pub bsp_data: D, /// Parameter model for this fan pub model: FanProperties, + pub name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, } #[allow(dead_code)] // Not all bsps have fans! @@ -259,6 +262,7 @@ impl Fan { cur_state: FanState::NotPresent, bsp_data, model, + name: rpm_sensor_id.name(), } } @@ -1568,11 +1572,11 @@ fn report_fan_state( match fan.cur_state { Fs::NotPresent => { ringbuf_entry!(Trace::FanRemoved(id)); - _ = ereporter.deliver_ereport(&FanRemoved { id: id.into() }); + _ = ereporter.deliver_ereport(&FanRemoved { name: fan.name }); } Fs::Present(_) => { ringbuf_entry!(Trace::FanAdded(id)); - _ = ereporter.deliver_ereport(&FanInserted { id: id.into() }) + _ = ereporter.deliver_ereport(&FanInserted { name: fan.name }) } }; fan.presence_acked = true; @@ -1603,14 +1607,14 @@ fn report_fan_state( // Step three: handle state reporting, if unreported if !fan.state_acked { let fan_info = || FanInfo { - id: id.into(), + name: fan.name, lo_rpm_lim: fan.model.underspeed_rpm.0, hi_rpm_lim: fan.model.overspeed_rpm.0, }; match pres { Fps::Unresponsive(e) => { _ = ereporter - .deliver_ereport(&FanRpmReadFailed { id: id.into() }); + .deliver_ereport(&FanRpmReadFailed { name: fan.name }); ringbuf_entry!(Trace::FanReadFailed(id, e)); } Fps::Nominal(_) => { @@ -1649,7 +1653,7 @@ ereports::declare_ereporter! { #[derive(microcbor::EncodeFields)] struct FanInfo { - id: u32, + name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, lo_rpm_lim: u16, hi_rpm_lim: u16, } @@ -1658,14 +1662,14 @@ struct FanInfo { #[derive(Encode)] #[ereport(class = "hw.remove.fan", version = 0)] struct FanRemoved { - id: u32, + name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, } /// An ereport representing a fan being inserted #[derive(Encode)] #[ereport(class = "hw.insert.fan", version = 0)] struct FanInserted { - id: u32, + name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, } /// An ereport representing a fan entering a nominal state @@ -1698,5 +1702,5 @@ struct FanUnderspeed { #[derive(Encode)] #[ereport(class = "hw.fan.rpm.err", version = 0)] struct FanRpmReadFailed { - id: u32, + name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, } From 5fe008f66ce0cf9c5fe2d7d0bb8c8bac491b3110 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 20 Aug 2026 17:37:34 +0200 Subject: [PATCH 09/12] Also include component_id --- task/sensor-api/src/lib.rs | 2 +- task/thermal/src/control.rs | 26 +++++++++++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/task/sensor-api/src/lib.rs b/task/sensor-api/src/lib.rs index 4b6a697e9d..403663844e 100644 --- a/task/sensor-api/src/lib.rs +++ b/task/sensor-api/src/lib.rs @@ -87,7 +87,7 @@ impl SensorId { /// Note that multiple sensor IDs may have the same component ID, when a /// single device exposes multiple measurement channels. #[cfg(feature = "component-id-lookup")] - pub fn component_id( + pub const fn component_id( &self, ) -> fixedstr::FixedStr<'static, { config::MAX_COMPONENT_ID_LEN }> { config::SENSOR_ID_TO_COMPONENT_ID[self.0 as usize] diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index b5db07ab03..8d149cdfd5 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -76,7 +76,8 @@ use microcbor::Encode; use ringbuf::ringbuf_entry_root as ringbuf_entry; use task_packrat_api::Packrat; use task_sensor_api::{ - NoData, Sensor as SensorApi, SensorId, config::MAX_SENSOR_NAME_LEN, + NoData, Sensor as SensorApi, SensorId, + config::{MAX_COMPONENT_ID_LEN, MAX_SENSOR_NAME_LEN}, }; use task_thermal_api::{ FanProperties, SensorReadError, ThermalAutoState, ThermalProperties, @@ -244,6 +245,7 @@ pub struct Fan { pub bsp_data: D, /// Parameter model for this fan pub model: FanProperties, + pub component_id: fixedstr::FixedStr<'static, MAX_COMPONENT_ID_LEN>, pub name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, } @@ -263,6 +265,7 @@ impl Fan { bsp_data, model, name: rpm_sensor_id.name(), + component_id: rpm_sensor_id.component_id(), } } @@ -1572,11 +1575,17 @@ fn report_fan_state( match fan.cur_state { Fs::NotPresent => { ringbuf_entry!(Trace::FanRemoved(id)); - _ = ereporter.deliver_ereport(&FanRemoved { name: fan.name }); + _ = ereporter.deliver_ereport(&FanRemoved { + name: fan.name, + component_id: fan.component_id, + }); } Fs::Present(_) => { ringbuf_entry!(Trace::FanAdded(id)); - _ = ereporter.deliver_ereport(&FanInserted { name: fan.name }) + _ = ereporter.deliver_ereport(&FanInserted { + name: fan.name, + component_id: fan.component_id, + }) } }; fan.presence_acked = true; @@ -1608,13 +1617,16 @@ fn report_fan_state( if !fan.state_acked { let fan_info = || FanInfo { name: fan.name, + component_id: fan.component_id, lo_rpm_lim: fan.model.underspeed_rpm.0, hi_rpm_lim: fan.model.overspeed_rpm.0, }; match pres { Fps::Unresponsive(e) => { - _ = ereporter - .deliver_ereport(&FanRpmReadFailed { name: fan.name }); + _ = ereporter.deliver_ereport(&FanRpmReadFailed { + name: fan.name, + component_id: fan.component_id, + }); ringbuf_entry!(Trace::FanReadFailed(id, e)); } Fps::Nominal(_) => { @@ -1654,6 +1666,7 @@ ereports::declare_ereporter! { #[derive(microcbor::EncodeFields)] struct FanInfo { name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, + component_id: fixedstr::FixedStr<'static, MAX_COMPONENT_ID_LEN>, lo_rpm_lim: u16, hi_rpm_lim: u16, } @@ -1663,6 +1676,7 @@ struct FanInfo { #[ereport(class = "hw.remove.fan", version = 0)] struct FanRemoved { name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, + component_id: fixedstr::FixedStr<'static, MAX_COMPONENT_ID_LEN>, } /// An ereport representing a fan being inserted @@ -1670,6 +1684,7 @@ struct FanRemoved { #[ereport(class = "hw.insert.fan", version = 0)] struct FanInserted { name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, + component_id: fixedstr::FixedStr<'static, MAX_COMPONENT_ID_LEN>, } /// An ereport representing a fan entering a nominal state @@ -1703,4 +1718,5 @@ struct FanUnderspeed { #[ereport(class = "hw.fan.rpm.err", version = 0)] struct FanRpmReadFailed { name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, + component_id: fixedstr::FixedStr<'static, MAX_COMPONENT_ID_LEN>, } From 99b585e36f5038564fbc452668358734530c785a Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 20 Aug 2026 17:58:27 +0200 Subject: [PATCH 10/12] Ram adjustment --- app/grapefruit/rev-a-ruby.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/grapefruit/rev-a-ruby.toml b/app/grapefruit/rev-a-ruby.toml index 42da2e65f2..7366715729 100644 --- a/app/grapefruit/rev-a-ruby.toml +++ b/app/grapefruit/rev-a-ruby.toml @@ -33,7 +33,7 @@ task-slots = ["sys", "packrat"] name = "task-thermal" features = ["grapefruit"] priority = 5 -max-sizes = {flash = 32768, ram = 4096 } +max-sizes = {flash = 32768, ram = 5120 } stacksize = 1536 start = true task-slots = ["i2c_driver", "sensor", "packrat"] From df409fbc72c284bd14e8d0464789bb74d0c8656d Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 20 Aug 2026 20:28:48 +0200 Subject: [PATCH 11/12] Rework to include raw response code in FanRpmReadFailed --- task/thermal/src/bsp/common/emc2305.rs | 4 +- task/thermal/src/bsp/common/max31790.rs | 4 +- task/thermal/src/bsp/cosmo_ab.rs | 6 +-- task/thermal/src/bsp/gimlet_bcdef.rs | 6 +-- task/thermal/src/bsp/grapefruit.rs | 6 +-- task/thermal/src/bsp/sidecar_bcd.rs | 10 ++-- task/thermal/src/control.rs | 67 +++++++++++++++---------- task/thermal/src/main.rs | 3 ++ 8 files changed, 54 insertions(+), 52 deletions(-) diff --git a/task/thermal/src/bsp/common/emc2305.rs b/task/thermal/src/bsp/common/emc2305.rs index 754a4b6391..32b04c00dd 100644 --- a/task/thermal/src/bsp/common/emc2305.rs +++ b/task/thermal/src/bsp/common/emc2305.rs @@ -111,9 +111,7 @@ pub(crate) const fn make_consecutive_nonremovable_fans( while idx < N { out[idx].rpm_sensor_id = sensors[idx]; out[idx].bsp_data = EmcFan::new_const(idx as u8); - out[idx].cur_state = FanState::Present(FanPresentState::Unresponsive( - SensorReadError::NoData, - )); + out[idx].cur_state = FanState::Present(FanPresentState::Unpolled); out[idx].presence_acked = true; idx += 1; } diff --git a/task/thermal/src/bsp/common/max31790.rs b/task/thermal/src/bsp/common/max31790.rs index e526fe7ab7..0475193a88 100644 --- a/task/thermal/src/bsp/common/max31790.rs +++ b/task/thermal/src/bsp/common/max31790.rs @@ -118,9 +118,7 @@ pub(crate) const fn make_consecutive_nonremovable_fans( while idx < N { out[idx].rpm_sensor_id = sensors[idx]; out[idx].bsp_data = MaxFan::new_const(idx as u8); - out[idx].cur_state = FanState::Present(FanPresentState::Unresponsive( - SensorReadError::NoData, - )); + out[idx].cur_state = FanState::Present(FanPresentState::Unpolled); out[idx].presence_acked = true; idx += 1; } diff --git a/task/thermal/src/bsp/cosmo_ab.rs b/task/thermal/src/bsp/cosmo_ab.rs index ab9f7eb52a..db6339fd69 100644 --- a/task/thermal/src/bsp/cosmo_ab.rs +++ b/task/thermal/src/bsp/cosmo_ab.rs @@ -15,7 +15,7 @@ pub use drv_cpu_seq_api::SeqError; use drv_cpu_seq_api::{PowerState, Sequencer, StateChangeReason}; use drv_i2c_devices::max31790::I2cWatchdog; use task_sensor_api::{Sensor, SensorId}; -use task_thermal_api::{SensorReadError, ThermalError, ThermalProperties}; +use task_thermal_api::{ThermalError, ThermalProperties}; use userlib::{ TaskId, task_slot, units::{Celsius, PWMDuty}, @@ -123,9 +123,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl.try_initialize() { for fan in self.fans.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(|| { - fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) - }); + fan.poll_rpm_with(|| fctl.fan_rpm(bsp_data)); } } diff --git a/task/thermal/src/bsp/gimlet_bcdef.rs b/task/thermal/src/bsp/gimlet_bcdef.rs index 0c69b2cae9..27a4f93275 100644 --- a/task/thermal/src/bsp/gimlet_bcdef.rs +++ b/task/thermal/src/bsp/gimlet_bcdef.rs @@ -15,7 +15,7 @@ pub use drv_cpu_seq_api::SeqError; use drv_cpu_seq_api::{PowerState, Sequencer, StateChangeReason}; use drv_i2c_devices::max31790::I2cWatchdog; use task_sensor_api::{Sensor, SensorId}; -use task_thermal_api::{SensorReadError, ThermalError, ThermalProperties}; +use task_thermal_api::{ThermalError, ThermalProperties}; use userlib::{ TaskId, task_slot, units::{Celsius, PWMDuty}, @@ -168,9 +168,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl.try_initialize() { for fan in self.fans.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(|| { - fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) - }); + fan.poll_rpm_with(|| fctl.fan_rpm(bsp_data)); } } diff --git a/task/thermal/src/bsp/grapefruit.rs b/task/thermal/src/bsp/grapefruit.rs index decd40d1ee..78bfda56f0 100644 --- a/task/thermal/src/bsp/grapefruit.rs +++ b/task/thermal/src/bsp/grapefruit.rs @@ -8,7 +8,7 @@ use crate::control::{ActiveInputState, MiscSensorPollingOutcome}; use crate::control::{ChannelType, PidConfig}; use drv_i2c_devices::max31790::I2cWatchdog; use task_sensor_api::SensorId; -use task_thermal_api::{SensorReadError, ThermalError, ThermalProperties}; +use task_thermal_api::{ThermalError, ThermalProperties}; use userlib::TaskId; use userlib::units::{Celsius, PWMDuty}; @@ -88,9 +88,7 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl.try_initialize() { for fan in self.fans.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(|| { - fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) - }); + fan.poll_rpm_with(|| fctl.fan_rpm(bsp_data)); } } diff --git a/task/thermal/src/bsp/sidecar_bcd.rs b/task/thermal/src/bsp/sidecar_bcd.rs index 12d4000839..b0a9162684 100644 --- a/task/thermal/src/bsp/sidecar_bcd.rs +++ b/task/thermal/src/bsp/sidecar_bcd.rs @@ -16,8 +16,8 @@ pub use drv_sidecar_seq_api::SeqError; use drv_sidecar_seq_api::{Sequencer, TofinoSeqState, TofinoSequencerPolicy}; use ringbuf::ringbuf_entry_root; use task_sensor_api::SensorId; +use task_thermal_api::ThermalProperties; use task_thermal_api::{SANYO_DENKI_FAN_PROPERTIES, ThermalError}; -use task_thermal_api::{SensorReadError, ThermalProperties}; use userlib::{TaskId, task_slot, units::Celsius}; include!(concat!(env!("OUT_DIR"), "/i2c_config.rs")); @@ -154,17 +154,13 @@ impl crate::control::BspInterface for Bsp { if let Ok(fctl) = self.fctrl_east.try_initialize() { for fan in east.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(|| { - fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) - }); + fan.poll_rpm_with(|| fctl.fan_rpm(bsp_data)); } } if let Ok(fctl) = self.fctrl_west.try_initialize() { for fan in west.iter_mut() { let bsp_data = fan.bsp_data; - fan.poll_rpm_with(|| { - fctl.fan_rpm(bsp_data).map_err(SensorReadError::I2cError) - }); + fan.poll_rpm_with(|| fctl.fan_rpm(bsp_data)); } } diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index 8d149cdfd5..77631f4d28 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -213,8 +213,10 @@ pub enum FanState { #[allow(dead_code)] // Not all bsps have fans! #[derive(Copy, Clone, PartialEq)] pub enum FanPresentState { - /// The fan is physically present, but is unresponsive to RPM queries - Unresponsive(SensorReadError), + /// The fan is present but has not yet been polled + Unpolled, + /// The fan is present, but is unresponsive to RPM queries + I2cReadError(drv_i2c_api::ResponseCode), /// The fan is present and at a reasonable speed Nominal(Rpm), /// The fan is present, but is overspeed @@ -289,9 +291,7 @@ impl Fan { pub(crate) fn update_presence(&mut self, is_present: bool) { match (is_present, self.cur_state) { (true, FanState::NotPresent) => { - self.update_state(FanState::Present( - FanPresentState::Unresponsive(SensorReadError::NoData), - )) + self.update_state(FanState::Present(FanPresentState::Unpolled)) } (true, _) => {} (false, _) => { @@ -324,31 +324,27 @@ impl Fan { } // Present -> Present (Fs::Present(cur), Fs::Present(newp)) => match (cur, newp) { - // Same -> Same, just take state (Fps::Nominal(_), Fps::Nominal(_)) | (Fps::TooFast(_), Fps::TooFast(_)) | (Fps::TooSlow(_), Fps::TooSlow(_)) - | (Fps::Unresponsive(_), Fps::Unresponsive(_)) => { + | (Fps::I2cReadError(_), Fps::I2cReadError(_)) + | (Fps::Unpolled, Fps::Unpolled) => { self.cur_state = new; } - // Any of the following: - // - // - Nominal -> Deviant - // - Deviant -> Nominal - // - Deviant -> Deviant - // - // Take: - // - // - New state - // - Status ack state (Fps::Nominal(_), _) | (_, Fps::Nominal(_)) - | (Fps::TooFast(_), Fps::Unresponsive(_)) + | (Fps::TooFast(_), Fps::Unpolled) | (Fps::TooFast(_), Fps::TooSlow(_)) - | (Fps::TooSlow(_), Fps::Unresponsive(_)) + | (Fps::TooFast(_), Fps::I2cReadError(_)) + | (Fps::TooSlow(_), Fps::Unpolled) | (Fps::TooSlow(_), Fps::TooFast(_)) - | (Fps::Unresponsive(_), Fps::TooFast(_)) - | (Fps::Unresponsive(_), Fps::TooSlow(_)) => { + | (Fps::TooSlow(_), Fps::I2cReadError(_)) + | (Fps::Unpolled, Fps::TooFast(_)) + | (Fps::Unpolled, Fps::TooSlow(_)) + | (Fps::Unpolled, Fps::I2cReadError(_)) + | (Fps::I2cReadError(_), Fps::Unpolled) + | (Fps::I2cReadError(_), Fps::TooFast(_)) + | (Fps::I2cReadError(_), Fps::TooSlow(_)) => { self.cur_state = new; self.state_acked = false; } @@ -358,9 +354,9 @@ impl Fan { /// Update the RPM of a present fan with the given closure, which should /// retrieve the RPM. Used to share logic across different fan controllers - pub(crate) fn poll_rpm_with>( + pub(crate) fn poll_rpm_with( &mut self, - poll_rpm: impl FnOnce() -> Result, + poll_rpm: impl FnOnce() -> Result, ) { // If this fan is not present, then do not attempt to poll it. Presence // is only restored via presence polling. @@ -386,7 +382,7 @@ impl Fan { Err(e) => { // No good, mark as unresponsive self.update_state(FanState::Present( - FanPresentState::Unresponsive(e.into()), + FanPresentState::I2cReadError(e), )); } } @@ -1604,7 +1600,7 @@ fn report_fan_state( }; match pres { // If the fan is unresponsive, clear the data from the sensor API - Fps::Unresponsive(_) => { + Fps::Unpolled | Fps::I2cReadError(_) => { sensor_api.nodata(id, NoData::DeviceUnavailable, now_ms); } // If we have valid RPM data, report it immediately. @@ -1622,12 +1618,16 @@ fn report_fan_state( hi_rpm_lim: fan.model.overspeed_rpm.0, }; match pres { - Fps::Unresponsive(e) => { + Fps::I2cReadError(e) => { _ = ereporter.deliver_ereport(&FanRpmReadFailed { name: fan.name, component_id: fan.component_id, + raw_response_code: e as u8, }); - ringbuf_entry!(Trace::FanReadFailed(id, e)); + ringbuf_entry!(Trace::FanReadFailed( + id, + SensorReadError::I2cError(e) + )); } Fps::Nominal(_) => { _ = ereporter.deliver_ereport(&FanNominal { info: fan_info() }); @@ -1647,6 +1647,13 @@ fn report_fan_state( }); ringbuf_entry!(Trace::FanUnderspeed(id, rpm)); } + Fps::Unpolled => { + // This is likely a bug, this means that a BSP failed to call + // `poll_rpm_with`. Don't panic, because that's worse than just + // not monitoring the fan at all, but ringbuf so we can catch + // it while developing. + ringbuf_entry!(Trace::FanUnpolled(id)); + } }; fan.state_acked = true; } @@ -1719,4 +1726,10 @@ struct FanUnderspeed { struct FanRpmReadFailed { name: fixedstr::FixedStr<'static, MAX_SENSOR_NAME_LEN>, component_id: fixedstr::FixedStr<'static, MAX_COMPONENT_ID_LEN>, + /// The raw I2C driver code reported when this query failed. This value + /// is not stable across versions of the SP firmware, and should only + /// be logged or used for interactive or post-mortem debugging. + /// Requires knowledge of the exact firmware revision to meaningfully + /// decode. + raw_response_code: u8, } diff --git a/task/thermal/src/main.rs b/task/thermal/src/main.rs index 6fe7b90ec7..93c95dc3fb 100644 --- a/task/thermal/src/main.rs +++ b/task/thermal/src/main.rs @@ -127,6 +127,7 @@ enum Trace { /// because an entry with two u64s doubles the size of the ringbuf. #[count(skip)] CriticalFor(u64), + /// Fan is present and read failed FanReadFailed(SensorId, SensorReadError), MiscReadFailed(SensorId, SensorReadError), SensorReadFailed(SensorId, SensorReadError), @@ -160,6 +161,8 @@ enum Trace { FanOverspeed(SensorId, Rpm), /// Fan is present and underspeed FanUnderspeed(SensorId, Rpm), + /// Fan is present but BSP didn't poll it + FanUnpolled(SensorId), } counted_ringbuf!(Trace, 32, Trace::None); From d36cdf360946fe6bb98c0038d00be8e07d073852 Mon Sep 17 00:00:00 2001 From: James Munns Date: Thu, 20 Aug 2026 20:31:17 +0200 Subject: [PATCH 12/12] Fix comment --- task/thermal/src/control.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/task/thermal/src/control.rs b/task/thermal/src/control.rs index 77631f4d28..9e21363780 100644 --- a/task/thermal/src/control.rs +++ b/task/thermal/src/control.rs @@ -1720,7 +1720,7 @@ struct FanUnderspeed { rpm: u16, } -/// An ereport representing a failure to remove a fan +/// An ereport representing a failure to read from a fan #[derive(Encode)] #[ereport(class = "hw.fan.rpm.err", version = 0)] struct FanRpmReadFailed {