trace MAX5970 peaks + resets (#1580)

Traces peaks (recorded minimum and maximum) from the MAX5970, as well
as attempts to infer and trace resets (albeit with a mechanism that
can result in false negatives), along with some refactoring to get
board-specific code (e.g., MAX5970 instrumentation) in the board
support package.  Also includes an Idol interface to clear the
BMR491's event buffer.

Co-authored-by: Andy Fiddaman <[email protected]>

app/gimlet/base.toml

@@ -125,7 +125,7 @@ name = "task-power"
 features = ["gimlet"]
 priority = 6
 max-sizes = {flash = 65536, ram = 8192 }
-stacksize = 1504
+stacksize = 2504
 start = true
 task-slots = ["i2c_driver", "sensor", "gimlet_seq"]
 notifications = ["timer"]

drv/i2c-devices/src/max5970.rs

@@ -289,6 +289,10 @@ impl Max5970 {
         self.device.read_reg::<u8, u8>(reg as u8)
     }
 
+    fn write_reg(&self, reg: Register, value: u8) -> Result<(), ResponseCode> {
+        self.device.write(&[reg as u8, value])
+    }
+
     pub fn i2c_device(&self) -> &I2cDevice {
         &self.device
     }
@@ -327,37 +331,80 @@ impl Max5970 {
         Ok(Amperes(delta / self.rsense as f32))
     }
 
+    fn peak_vout(
+        &self,
+        msb_reg: Register,
+        lsb_reg: Register,
+    ) -> Result<Volts, ResponseCode> {
+        Ok(self.convert_volts(
+            MonRange(self.read_reg(Register::mon_range)?),
+            self.read_reg(msb_reg)?,
+            self.read_reg(lsb_reg)?,
+        ))
+    }
+
     pub fn max_vout(&self) -> Result<Volts, ResponseCode> {
         let (msb_reg, lsb_reg) = if self.rail == 0 {
             (Register::max_chx_mon_msb_ch1, Register::max_chx_mon_lsb_ch1)
         } else {
             (Register::max_chx_mon_msb_ch2, Register::max_chx_mon_lsb_ch2)
         };
 
-        Ok(self.convert_volts(
-            MonRange(self.read_reg(Register::mon_range)?),
-            self.read_reg(msb_reg)?,
-            self.read_reg(lsb_reg)?,
-        ))
+        self.peak_vout(msb_reg, lsb_reg)
     }
 
-    pub fn max_iout(&self) -> Result<Amperes, ResponseCode> {
+    pub fn min_vout(&self) -> Result<Volts, ResponseCode> {
         let (msb_reg, lsb_reg) = if self.rail == 0 {
-            (Register::max_chx_cs_msb_ch1, Register::max_chx_cs_lsb_ch1)
+            (Register::min_chx_mon_msb_ch1, Register::min_chx_mon_lsb_ch1)
         } else {
-            (Register::max_chx_cs_msb_ch2, Register::max_chx_cs_lsb_ch2)
+            (Register::min_chx_mon_msb_ch2, Register::min_chx_mon_lsb_ch2)
         };
 
+        self.peak_vout(msb_reg, lsb_reg)
+    }
+
+    fn peak_iout(
+        &self,
+        msb_reg: Register,
+        lsb_reg: Register,
+    ) -> Result<Amperes, ResponseCode> {
         self.convert_current(
             Status2(self.read_reg(Register::status2)?),
             self.read_reg(msb_reg)?,
             self.read_reg(lsb_reg)?,
         )
     }
 
+    pub fn max_iout(&self) -> Result<Amperes, ResponseCode> {
+        let (msb_reg, lsb_reg) = if self.rail == 0 {
+            (Register::max_chx_cs_msb_ch1, Register::max_chx_cs_lsb_ch1)
+        } else {
+            (Register::max_chx_cs_msb_ch2, Register::max_chx_cs_lsb_ch2)
+        };
+
+        self.peak_iout(msb_reg, lsb_reg)
+    }
+
+    pub fn min_iout(&self) -> Result<Amperes, ResponseCode> {
+        let (msb_reg, lsb_reg) = if self.rail == 0 {
+            (Register::min_chx_cs_msb_ch1, Register::min_chx_cs_lsb_ch1)
+        } else {
+            (Register::min_chx_cs_msb_ch2, Register::min_chx_cs_lsb_ch2)
+        };
+
+        self.peak_iout(msb_reg, lsb_reg)
+    }
+
     pub fn status0(&self) -> Result<u8, ResponseCode> {
         self.read_reg(Register::status0)
     }
+
+    pub fn clear_peaks(&self) -> Result<(), ResponseCode> {
+        let rst = if self.rail == 0 { 0b00_11 } else { 0b11_00 };
+
+        self.write_reg(Register::peak_log_rst, rst)?;
+        self.write_reg(Register::peak_log_rst, 0)
+    }
 }
 
 impl Validate<ResponseCode> for Max5970 {

idl/power.idol

@@ -46,7 +46,7 @@ Interface(
                 err: CLike("ResponseCode"),
             ),
             idempotent: true,
-        ), 
+        ),
         "bmr491_event_log_read": (
             doc: "reads an event from the BMR491's combined fault and lifecycle event log",
             args: {
@@ -58,6 +58,14 @@ Interface(
             ),
             idempotent: true,
         ),
+        "bmr491_fault_log_clear": (
+            doc: "clears the BMR491's persistent fault log",
+            reply: Result(
+                ok: "()",
+                err: CLike("ResponseCode"),
+            ),
+            idempotent: true,
+        ),
         "bmr491_max_fault_event_index": (
             doc: "returns the index of the most recent fault event in the BMR491's event log",
             reply: Result(

task/power/src/bsp/gimlet_bcde.rs

@@ -3,11 +3,17 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use crate::{
-    i2c_config, i2c_config::sensors, DeviceType, Ohms, PowerControllerConfig,
-    PowerState,
+    i2c_config, i2c_config::sensors, Device, DeviceType, Ohms,
+    PowerControllerConfig, PowerState, SensorId,
 };
 
+use drv_i2c_devices::max5970::*;
+use ringbuf::*;
+use userlib::units::*;
+
 pub(crate) const CONTROLLER_CONFIG_LEN: usize = 37;
+const MAX5970_CONFIG_LEN: usize = 22;
+
 pub(crate) static CONTROLLER_CONFIG: [PowerControllerConfig;
     CONTROLLER_CONFIG_LEN] = [
     rail_controller!(IBC, bmr491, v12_sys_a2, A2),
@@ -71,8 +77,178 @@ pub(crate) fn get_state() -> PowerState {
     }
 }
 
-pub fn preinit() {
-    // Nothing to do here
+#[derive(Copy, Clone, PartialEq)]
+enum Trace {
+    Now(u32),
+    Max5970 {
+        sensor: SensorId,
+        last_bounce_detected: Option<u32>,
+        status0: u8,
+        status1: u8,
+        status3: u8,
+        fault0: u8,
+        fault1: u8,
+        fault2: u8,
+        min_iout: f32,
+        max_iout: f32,
+        min_vout: f32,
+        max_vout: f32,
+    },
+    None,
+}
+
+ringbuf!(Trace, 64, Trace::None);
+
+fn trace_max5970(
+    dev: &Max5970,
+    sensor: SensorId,
+    peaks: &mut Max5970Peaks,
+    now: u32,
+) {
+    let max_vout = match dev.max_vout() {
+        Ok(Volts(v)) => v,
+        _ => return,
+    };
+
+    let min_vout = match dev.min_vout() {
+        Ok(Volts(v)) => v,
+        _ => return,
+    };
+
+    let max_iout = match dev.max_iout() {
+        Ok(Amperes(a)) => a,
+        _ => return,
+    };
+
+    let min_iout = match dev.min_iout() {
+        Ok(Amperes(a)) => a,
+        _ => return,
+    };
+
+    if peaks.iout.bounced(min_iout, max_iout)
+        || peaks.vout.bounced(min_vout, max_vout)
+    {
+        peaks.last_bounce_detected = Some(now);
+    }
+
+    ringbuf_entry!(Trace::Max5970 {
+        sensor,
+        last_bounce_detected: peaks.last_bounce_detected,
+        status0: match dev.read_reg(Register::status0) {
+            Ok(reg) => reg,
+            _ => return,
+        },
+        status1: match dev.read_reg(Register::status1) {
+            Ok(reg) => reg,
+            _ => return,
+        },
+        status3: match dev.read_reg(Register::status3) {
+            Ok(reg) => reg,
+            _ => return,
+        },
+        fault0: match dev.read_reg(Register::fault0) {
+            Ok(reg) => reg,
+            _ => return,
+        },
+        fault1: match dev.read_reg(Register::fault1) {
+            Ok(reg) => reg,
+            _ => return,
+        },
+        fault2: match dev.read_reg(Register::fault2) {
+            Ok(reg) => reg,
+            _ => return,
+        },
+        min_iout,
+        max_iout,
+        min_vout,
+        max_vout,
+    });
+}
+
+#[derive(Copy, Clone)]
+struct Max5970Peak {
+    min: f32,
+    max: f32,
+}
+
+impl Default for Max5970Peak {
+    fn default() -> Self {
+        Self {
+            min: f32::MAX,
+            max: f32::MIN,
+        }
+    }
+}
+
+impl Max5970Peak {
+    ///
+    /// If we see the drives lose power, it is helpful to disambiguate PDN issues
+    /// from the power being explicitly disabled via system software (e.g., via
+    /// CEM_TO_PCIEHP_PWREN on Sharkfin).  The MAX5970 doesn't have a way of
+    /// recording power cycles, but we know that if we see the peaks travel in
+    /// the wrong direction (that is, a max that is less than the previous max
+    /// or a minimum that is greater than our previous minimum) then there must
+    /// have been a power cycle.  This can clearly yield false negatives, but
+    /// it will not yield false positives:  if [`bounced`] returns true, one can
+    /// know with confidence that the power has been cycled.
+    ///
+    fn bounced(&mut self, min: f32, max: f32) -> bool {
+        let bounced = min > self.min || max < self.max;
+        self.min = min;
+        self.max = max;
+        bounced
+    }
+}
+
+#[derive(Copy, Clone, Default)]
+struct Max5970Peaks {
+    iout: Max5970Peak,
+    vout: Max5970Peak,
+    last_bounce_detected: Option<u32>,
+}
+
+pub(crate) struct State {
+    fired: u32,
+    peaks: [Max5970Peaks; MAX5970_CONFIG_LEN],
+}
+
+impl State {
+    pub(crate) fn init() -> Self {
+        Self {
+            fired: 0,
+            peaks: [Max5970Peaks::default(); MAX5970_CONFIG_LEN],
+        }
+    }
+
+    pub(crate) fn handle_timer_fired(
+        &mut self,
+        devices: &[Device],
+        state: PowerState,
+    ) {
+        //
+        // Trace the detailed state every ten seconds, provided that we are in A0.
+        //
+        if state == PowerState::A0 && self.fired % 10 == 0 {
+            ringbuf_entry!(Trace::Now(self.fired));
+
+            for ((dev, sensor), peak) in CONTROLLER_CONFIG
+                .iter()
+                .zip(devices.iter())
+                .filter_map(|(c, dev)| {
+                    if let Device::Max5970(dev) = dev {
+                        Some((dev, c.current))
+                    } else {
+                        None
+                    }
+                })
+                .zip(self.peaks.iter_mut())
+            {
+                trace_max5970(dev, sensor, peak, self.fired);
+            }
+        }
+
+        self.fired += 1;
+    }
 }
 
 pub const HAS_RENDMP_BLACKBOX: bool = true;

task/power/src/bsp/gimletlet_2.rs

@@ -19,8 +19,19 @@ pub(crate) fn get_state() -> PowerState {
     PowerState::A2
 }
 
-pub fn preinit() {
-    // Nothing to do here
+pub(crate) struct State(());
+
+impl State {
+    pub(crate) fn init() -> Self {
+        State(())
+    }
+
+    pub(crate) fn handle_timer_fired(
+        &self,
+        _devices: &[crate::Device],
+        _state: PowerState,
+    ) {
+    }
 }
 
 pub const HAS_RENDMP_BLACKBOX: bool = false;