cosmo_seq: mask STATUS_CML bit 1 to not get annoying pmbus alerts about it

drv/cosmo-seq-server/src/vcore.rs

@@ -18,6 +18,7 @@ use drv_i2c_api::ResponseCode;
 use drv_i2c_devices::raa229620a::{self, Raa229620A};
 use ereports::pwr::{PmbusAlert, PmbusStatus};
 use fixedstr::FixedStr;
+use pmbus::commands::raa229620a::STATUS_CML;
 use pmbus::commands::raa229620a::STATUS_IOUT;
 use pmbus::commands::raa229620a::STATUS_WORD;
 use ringbuf::*;
@@ -43,6 +44,7 @@ pub(crate) enum Rail {
 enum PmbusCmd {
     LoadLimit,
     SetStatusIoutMask,
+    SetStatusCmlMask,
     ClearFaults,
     ReadVin,
     Status,
@@ -59,6 +61,9 @@ enum Trace {
     StatusIoutMaskSet {
         all_ok: bool,
     },
+    StatusCmlMaskSet {
+        all_ok: bool,
+    },
     PmbusAlert {
         timestamp: u64,
         alerted: Vrms,
@@ -219,15 +224,10 @@ impl VCore {
         // if possible.
 
         // Set our warn limit
-        let mut all_ok = true;
-        all_ok &= retry_i2c_txn(Rail::VddcrCpu0, PmbusCmd::LoadLimit, || {
-            self.vddcr_cpu0.set_vin_uv_warn_limit(VCORE_UV_WARN_LIMIT)
-        })
-        .is_ok();
-        all_ok &= retry_i2c_txn(Rail::VddcrCpu1, PmbusCmd::LoadLimit, || {
-            self.vddcr_cpu1.set_vin_uv_warn_limit(VCORE_UV_WARN_LIMIT)
-        })
-        .is_ok();
+        let all_ok = self
+            .set_alert_config_on_both_vrms(PmbusCmd::LoadLimit, |vrm| {
+                vrm.set_vin_uv_warn_limit(VCORE_UV_WARN_LIMIT)
+            });
         ringbuf_entry!(Trace::LimitsLoaded { all_ok });
 
         let iout_mask = {
@@ -250,19 +250,33 @@ impl VCore {
             );
             mask
         };
-        let mut all_ok = true;
-        all_ok &=
-            retry_i2c_txn(Rail::VddcrCpu0, PmbusCmd::SetStatusIoutMask, || {
-                self.vddcr_cpu0.set_status_iout_smbalert_mask(iout_mask)
-            })
-            .is_ok();
-        all_ok &=
-            retry_i2c_txn(Rail::VddcrCpu1, PmbusCmd::SetStatusIoutMask, || {
-                self.vddcr_cpu1.set_status_iout_smbalert_mask(iout_mask)
-            })
-            .is_ok();
+        let all_ok = self.set_alert_config_on_both_vrms(
+            PmbusCmd::SetStatusIoutMask,
+            |vrm| vrm.set_status_iout_smbalert_mask(iout_mask),
+        );
         ringbuf_entry!(Trace::StatusIoutMaskSet { all_ok });
 
+        let cml_mask = {
+            let mut mask = STATUS_CML::CommandData(0);
+            // Mask out SMBus alerts for STATUS_CML bit 1. This bit, "other
+            // fault", is basically set when the PMBus sees something happen on
+            // the I2C bus that makes it feel weird. Unfortunately, it turns out
+            // that "I2C things that make you feel kinda weird" can happen a lot
+            // in an otherwise healthy system. While we are thankful for the
+            // RAA229620A for setting the status bit that says it saw something
+            // weird, we would really rather not get an IRQ about it every time
+            // there's I2C weather. So let's not get alerts for this one.
+            mask.set_other_communication_error(
+                STATUS_CML::OtherCommunicationError::Error,
+            );
+            mask
+        };
+        let all_ok = self
+            .set_alert_config_on_both_vrms(PmbusCmd::SetStatusCmlMask, |vrm| {
+                vrm.set_status_cml_smbalert_mask_on_all_rails(cml_mask)
+            });
+        ringbuf_entry!(Trace::StatusCmlMaskSet { all_ok });
+
         // Clear our faults
         self.try_to_clear_faults(Vrms {
             pwr_cont1: true,
@@ -275,6 +289,29 @@ impl VCore {
         ringbuf_entry!(Trace::Initialized);
     }
 
+    fn set_alert_config_on_both_vrms(
+        &self,
+        which: PmbusCmd,
+        txn: impl Fn(&Raa229620A) -> Result<(), raa229620a::Error>,
+    ) -> bool {
+        let mut all_ok = true;
+        all_ok &=
+            retry_i2c_txn(
+                Rail::VddcrCpu0,
+                which,
+                &mut || txn(&self.vddcr_cpu0),
+            )
+            .is_ok();
+        all_ok &=
+            retry_i2c_txn(
+                Rail::VddcrCpu1,
+                which,
+                &mut || txn(&self.vddcr_cpu1),
+            )
+            .is_ok();
+        all_ok
+    }
+
     pub fn can_we_unmask_any_vrm_irqs_again(&mut self) -> Vrms {
         self.try_to_clear_faults(self.faulted);
         Vrms {

drv/i2c-devices/src/lib.rs

@@ -178,6 +178,42 @@ macro_rules! pmbus_rail_write {
     }};
 }
 
+/// Write the mask `$mask` to the `SMBALERT_MASK` register for `$reg`, where
+/// `$reg` is a status register, and `$mask` is a `CommandData` value for that
+/// register.
+///
+/// Importantly, `$reg` must be a PMBus `STATUS_<whatever>` register. This macro
+/// cannot stop you from providing any `CommandCode` as the value of `$reg` and
+/// any `CommandData` as the value of `$mask`, but, uh, don't do that. On the
+/// other hand, the macro *does* at least ensure that `$mask` is a `CommandData`.
+/// for the same register as `$reg`.
+macro_rules! pmbus_smbalert_mask_write {
+    ($device:expr, $rail:expr, $reg:ident, $mask:expr) => {{
+        // This assignment is just a type assertion that `$mask` is a
+        // `CommandData` for the same register as `$reg`.
+        let mask: $reg::CommandData = $mask;
+        let rpayload = [PAGE::CommandData::code(), $rail];
+        // N.B. that the status register *should* always be a single byte, but
+        // we'll do this "properly" just in case.
+        let mut payload = [0u8; $reg::CommandData::len() + 2];
+        // 0               7               15              23
+        // +---------------+---------------+---------------+
+        // | SMBALERT_MASK | register code | mask byte     |
+        // +---------------+---------------+---------------+
+        payload[0] = CommandCode::SMBALERT_MASK as u8;
+        payload[1] = $reg::CommandData::code();
+        mask.to_slice(&mut payload[2..]);
+
+        match $device.write_write(&rpayload, &payload) {
+            Err(code) => Err(Error::BadWrite {
+                cmd: CommandCode::SMBALERT_MASK as u8,
+                code,
+            }),
+            Ok(_) => Ok(()),
+        }
+    }};
+}
+
 struct BadValidation {
     cmd: u8,
     code: ResponseCode,

drv/i2c-devices/src/raa229620a.rs

@@ -151,26 +151,23 @@ impl Raa229620A {
         &self,
         mask: STATUS_IOUT::CommandData,
     ) -> Result<(), Error> {
-        // Unfortunately, SMBALERT_MASK is kinda hard to use with the
-        // `pmbus_write!` macro family, due to not having its own `CommandData`
-        // type, and because it requires writing both the name of the status
-        // register being masked *and* the value of that register (as the mask).
-        // It's a bit odd. Probably it deserves its own
-        // `pmbus_smbalert_mask_write!` macro or something, but for now, we'll
-        // just do it manually.
-        self.device
-            .write_write(
-                &[PAGE::CommandData::code(), self.rail],
-                &[
-                    CommandCode::SMBALERT_MASK as u8,
-                    CommandCode::STATUS_IOUT as u8,
-                    mask.0,
-                ],
-            )
-            .map_err(|code| Error::BadWrite {
-                cmd: CommandCode::SMBALERT_MASK as u8,
-                code,
-            })
+        pmbus_smbalert_mask_write!(self.device, self.rail, STATUS_IOUT, mask)
+    }
+
+    /// Set the `SMBALERT_MASK` for the `STATUS_CML` register, sending page
+    /// 0xFF. Though I couldn't find explicit confirmation of this in the PMBus
+    /// standard, one must kind of assume that `STATUS_CML` bits, which are not
+    /// specific to a particular output rail, are probably set on all PMBus
+    /// pages when a CML event happens, and thus we must mask them out on all
+    /// pages to stop SMBus alerts from being generated?
+    ///
+    /// Any bits set in `mask` will be masked, suppressing SMBus alerts when
+    /// those bits in `STATUS_CML` become set.
+    pub fn set_status_cml_smbalert_mask_on_all_rails(
+        &self,
+        mask: STATUS_CML::CommandData,
+    ) -> Result<(), Error> {
+        pmbus_smbalert_mask_write!(self.device, 0xff, STATUS_CML, mask)
     }
 
     pub fn read_vin(&self) -> Result<Volts, Error> {