DRAFT: RAA229620A phase check and open pin support

Cargo.toml

@@ -247,8 +247,8 @@ serde-xml-rs = "0.5.1"
 sha2 = "0.10.1"
 splitty = "0.1.0"
 srec = "0.2"
-strum = "0.22"
-strum_macros = "0.22"
+strum = "0.25"
+strum_macros = "0.25"
 syn = "1.0"
 tempfile = "3.3"
 termimad = "0.21"

cmd/rendmp/Cargo.toml

@@ -14,6 +14,8 @@ num-derive.workspace = true
 num-traits.workspace = true
 parse_int.workspace = true
 pmbus.workspace = true
+strum.workspace = true
+strum_macros.workspace = true
 zerocopy.workspace = true
 
 humility-cli.workspace = true

cmd/rendmp/src/lib.rs

@@ -164,8 +164,11 @@ use std::fs::{self, OpenOptions};
 use std::io::BufReader;
 use std::io::Write;
 use std::io::prelude::*;
+use std::str::FromStr as _;
 use std::thread;
 use std::time::{Duration, Instant};
+use strum::VariantNames;
+use strum_macros::{Display, EnumString, EnumVariantNames};
 use zerocopy::{AsBytes, FromBytes};
 
 mod blackbox;
@@ -949,10 +952,14 @@ fn rendmp_ingest(subargs: &RendmpArgs) -> Result<()> {
 }
 
 /// A device which supports open-pin detection and other advanced debug
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+#[derive(
+    Debug, Copy, Clone, Eq, PartialEq, Display, EnumString, EnumVariantNames,
+)]
+#[strum(ascii_case_insensitive)]
 enum SupportedDevice {
     ISL68224,
     RAA229618,
+    RAA229620A,
 }
 
 impl SupportedDevice {
@@ -961,6 +968,7 @@ impl SupportedDevice {
         match self {
             SupportedDevice::ISL68224 => 3,
             SupportedDevice::RAA229618 => 2,
+            SupportedDevice::RAA229620A => 2,
         }
     }
 
@@ -987,20 +995,24 @@ impl SupportedDevice {
                 }
                 phases
             }
+            SupportedDevice::RAA229620A => {
+                let mut phases = vec![];
+                for phase in 0..12 {
+                    phases.push((phase.to_string(), phase));
+                }
+                for i in 0..2 {
+                    // The 20 here looks like a copy paste mistake from the
+                    // '618, but is deliberate; we believe the bit offset here
+                    // is the same as the '618 despite the smaller phase count.
+                    // (TODO)
+                    phases.push((format!("VSEN{i}"), i + 20));
+                }
+                phases
+            }
         }
     }
 }
 
-impl std::fmt::Display for SupportedDevice {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        let s = match self {
-            SupportedDevice::ISL68224 => "ISL68224",
-            SupportedDevice::RAA229618 => "RAA229618",
-        };
-        write!(f, "{s}")
-    }
-}
-
 /// Checks that the address provided is valid
 ///
 /// This checks that the address uniquely points to an RAA229618 or ISL68224.
@@ -1011,20 +1023,17 @@ fn check_addr(
     subargs: &RendmpArgs,
     hubris: &HubrisArchive,
 ) -> Result<(u8, SupportedDevice)> {
-    const ISL_DEV_NAME: &str = "isl68224";
-    const RAA_DEV_NAME: &str = "raa229618";
-
     if let Some(rail) = &subargs.dev.rail {
         for d in &hubris.manifest.i2c_devices {
             if let HubrisI2cDeviceClass::Pmbus { rails } = &d.class
                 && rails.iter().any(|r| r.name == *rail)
             {
-                let dev = match d.device.as_str() {
-                    ISL_DEV_NAME => SupportedDevice::ISL68224,
-                    RAA_DEV_NAME => SupportedDevice::RAA229618,
-                    _ => {
-                        bail!("rail {rail} is not a supported device");
-                    }
+                let Ok(dev) = SupportedDevice::from_str(&d.device) else {
+                    let supported = SupportedDevice::VARIANTS.join(", ");
+                    bail!(
+                        "rail {rail} is not a supported device; \
+                        expected one of: {supported}"
+                    );
                 };
 
                 return Ok((d.address, dev));
@@ -1039,12 +1048,12 @@ fn check_addr(
     };
     let addr: u8 = parse_int::parse(addr).context("failed to parse address")?;
     let mut iter = hubris.manifest.i2c_devices.iter().filter(|dev| {
-        matches!(dev.device.as_str(), RAA_DEV_NAME | ISL_DEV_NAME)
-            && dev.address == addr
+        SupportedDevice::from_str(&dev.device).is_ok() && dev.address == addr
     });
     let Some(dev) = iter.next() else {
+        let supported = SupportedDevice::VARIANTS.join(", ");
         bail!(
-            "no RAA229618 or ISL68224 with address {addr}; \
+            "no supported device ({supported}) with address {addr}; \
              use `humility pmbus -l` to list devices"
         );
     };
@@ -1054,10 +1063,8 @@ fn check_addr(
              this should not be possible on an Oxide board"
         )
     }
-    let dev = match dev.device.as_str() {
-        ISL_DEV_NAME => SupportedDevice::ISL68224,
-        RAA_DEV_NAME => SupportedDevice::RAA229618,
-        _ => unreachable!(), // checked above
+    let Ok(dev) = SupportedDevice::from_str(&dev.device) else {
+        unreachable!() // checked above
     };
 
     Ok((addr, dev))
@@ -1142,6 +1149,11 @@ fn get_pin_states(
             0x00BE, 0x00BF, // open-pin
             0xE904, 0xE905, // mask
         ],
+        SupportedDevice::RAA229620A => &[
+            // TODO unverified
+            0x00BE, 0x00BF, // open-pin
+            0xE904, 0xE905, // mask
+        ],
     };
     let mut ops = vec![];
     for &r in regs {
@@ -1168,7 +1180,8 @@ fn get_pin_states(
     // experiments and discussion with Renesas.
     let (open, mask) = match dev {
         SupportedDevice::ISL68224 => (values[0] as u64, values[1] as u64),
-        SupportedDevice::RAA229618 => {
+        // TODO: RAA229620A support is untested/unverified
+        SupportedDevice::RAA229618 | SupportedDevice::RAA229620A => {
             let open = values[0] as u64 | ((values[1] as u64) << 32);
             let mask = values[2] as u64 | ((values[3] as u64) << 32);
             (open, mask)
@@ -1708,6 +1721,7 @@ fn rendmp_phase_check<'a>(
     let disable_fault_reg: u16 = match dev {
         SupportedDevice::ISL68224 => 0xE952,
         SupportedDevice::RAA229618 => 0xE932,
+        SupportedDevice::RAA229620A => 0xE932, // TODO may have changed
     };
 
     ////////////////////////////////////////////////////////////////////////////
@@ -1734,7 +1748,13 @@ fn rendmp_phase_check<'a>(
         worker.read_word32(index, true, LOOPCFG as u8)?;
         worker.read_word(index, true, PEAK_OCUC_COUNT as u8)?;
 
-        // Set PMBus command codes 0xD0 and 0xD1 to 0x8000 (disable VMon)
+        // Set the appropriate device-specific PMBus command codes to input
+        // voltage thresholding, by setting the thresholds to a very negative
+        // value. This should ensure that it always thinks the input value is
+        // high enough.
+        //
+        // This is a 16-bit two's-complement register, so 0x8000 is the most
+        // negative value.
         match dev {
             SupportedDevice::ISL68224 => {
                 use pmbus::commands::isl68224::CommandCode;
@@ -1769,6 +1789,25 @@ fn rendmp_phase_check<'a>(
                 let reg = (0xEA5B + rail * 0x80) as u16;
                 worker.read_dma(addr, reg)?;
             }
+            SupportedDevice::RAA229620A => {
+                use pmbus::commands::raa229620a::CommandCode;
+                worker.write_word(
+                    index,
+                    true,
+                    CommandCode::VIN_ON as u8,
+                    0x8000,
+                )?;
+                worker.write_word(
+                    index,
+                    true,
+                    CommandCode::VIN_OFF as u8,
+                    0x8000,
+                )?;
+
+                // TODO: we do this undocumented read of 0xEA5B on the '618.
+                let reg = (0xEA5B + rail * 0x80) as u16;
+                worker.read_dma(addr, reg)?;
+            }
         }
 
         worker.read_dma(addr, disable_fault_reg + rail as u16)?;
@@ -1861,6 +1900,26 @@ fn rendmp_phase_check<'a>(
                     bail!("failed to set VIN_OFF for {rail}: {e}",);
                 }
 
+                // Clear bit 0 of DMA register EA5B and write it back
+                // (the name part_fast_add comes from Power Navigator)
+                let mut part_fast_add = match next()? {
+                    Ok(v) => v.expect_read_dma()?,
+                    Err(e) => {
+                        bail!("worker.failed to read EA5B for rail {rail}: {e}")
+                    }
+                };
+                part_fast_add &= !1; // clear bit 0
+                let reg = (0xEA5B + rail * 0x80) as u16;
+                worker.write_dma(addr, reg, part_fast_add)?;
+            }
+            SupportedDevice::RAA229620A => {
+                if let Err(e) = next()? {
+                    bail!("failed to set VIN_ON for {rail}: {e}",);
+                }
+                if let Err(e) = next()? {
+                    bail!("failed to set VIN_OFF for {rail}: {e}",);
+                }
+
                 // Clear bit 0 of DMA register EA5B and write it back
                 // (the name part_fast_add comes from Power Navigator)
                 let mut part_fast_add = match next()? {
@@ -1931,6 +1990,12 @@ fn rendmp_phase_check<'a>(
                     bail!("failed to modify EA5B for rail {rail}: {e}")
                 }
             },
+            SupportedDevice::RAA229620A => match next()? {
+                Ok(v) => v.expect_write_dma()?,
+                Err(e) => {
+                    bail!("failed to modify EA5B for rail {rail}: {e}")
+                }
+            },
             SupportedDevice::ISL68224 => {
                 // no changes were made specifically for the ISL68224
             }
@@ -2091,6 +2156,7 @@ fn rendmp_phase_check<'a>(
         };
         let device = match dev {
             SupportedDevice::RAA229618 => pmbus::Device::Raa229618,
+            SupportedDevice::RAA229620A => pmbus::Device::Raa229620A,
             SupportedDevice::ISL68224 => pmbus::Device::Isl68224,
         };