Improve PMBus handling for PSC (#252)

When bringing up the PSC firmware, I ran into a few issues.

Both Hubris and Humility assume that for an I2C device with a
```toml
power = { rails = [ ... ] }
```
field, all sensors (voltage / current / temperature / etc) correspond one-to-one
with power rails.

This is problematic for the power shelves, which have two rails (54V and 12V),
two fans (uncorrelated to voltage rails), and _three_ temperature sensors (also
unrelated to rails!).

This PR adds support for an optional `sensors = [ ... ]` field within power, e.g.
```toml
[[config.i2c.devices]]
bus = "backplane"
name = "psu0mcu"
address = 0b1011_000
device = "mwocp68"
description = "PSU 0 MCU"
power = { rails = [ "V54_PSU0", "V12_PSU0" ], sensors = ["voltage", "current"] }
sensors = { voltage = 2, current = 2, temperature = 3, speed = 2 }
```

The `sensors` field indicates that only `voltage` and `current` correspond
one-to-one with rails; other sensor types are treated as independent.

If the `sensors` field is blank (i.e. all of our existing TOML files), then
every sensor corresponds one-to-one with rails, matching the current behavior.

In addition, `humility pmbus --summarize` is modified to not `bail!` on an
error; instead, it prints the error and continues.  This makes working with the
PSC nicer, because not all 6 PSUs will be populated at all times.

Cargo.toml

@@ -17,7 +17,7 @@ name = "humility"
 #
 # Be sure to check in and push all of the files that change.  Happy versioning!
 #
-version = "0.9.2"
+version = "0.9.3"
 authors = ["Bryan Cantrill <bryan@oxide.computer>"]
 edition = "2018"
 license = "MPL-2.0"

cmd/pmbus/src/lib.rs

@@ -759,18 +759,24 @@ fn summarize(
     let results = context.run(core, ops.as_slice(), None)?;
     let mut base = 0;
 
-    print!("{:13} {:16} {:3} {:4}", "DEVICE", "RAIL", "PG?", "#FLT");
+    print!(
+        "{:13} {:16} {:3} {:4}",
+        "DEVICE".bold(),
+        "RAIL".bold(),
+        "PG?".bold(),
+        "#FLT".bold()
+    );
 
     for (_, header) in commands.iter() {
         if let Some(header) = header {
-            print!(" {:>width$}", header, width = width);
+            print!(" {:>width$}", header.bold(), width = width);
         }
     }
 
     println!();
 
     for (device, driver, rail, calls) in &work {
-        summarize_rail(
+        if let Err(e) = summarize_rail(
             subargs,
             device,
             driver,
@@ -779,7 +785,14 @@ fn summarize(
             &results[base..base + calls.len()],
             func,
             width,
-        )?;
+        ) {
+            println!(
+                " {0}  {1} {2}  {0} ",
+                "--".dimmed(),
+                "error:".yellow(),
+                e,
+            );
+        }
 
         base += calls.len();
     }

humility-core/src/hubris.rs

@@ -36,7 +36,7 @@ const OXIDE_NT_BASE: u32 = 0x1de << 20;
 const OXIDE_NT_HUBRIS_ARCHIVE: u32 = OXIDE_NT_BASE + 1;
 const OXIDE_NT_HUBRIS_REGISTERS: u32 = OXIDE_NT_BASE + 2;
 
-const MAX_HUBRIS_VERSION: u32 = 3;
+const MAX_HUBRIS_VERSION: u32 = 4;
 
 #[derive(Default, Debug)]
 pub struct HubrisManifest {
@@ -116,6 +116,14 @@ struct HubrisConfigI2cPower {
     rails: Option<Vec<String>>,
     #[serde(default = "HubrisConfigI2cPower::default_pmbus")]
     pmbus: bool,
+
+    /// Lists which sensor types have a one-to-one association with power rails
+    ///
+    /// When `None`, we assume that all sensor types are mapped one-to-one with
+    /// rails.  Otherwise, *only* the listed sensor types are associated with
+    /// rails (which is the case in systems with independent temperature sensor
+    /// and power rails).
+    sensors: Option<Vec<HubrisSensorKind>>,
 }
 
 impl HubrisConfigI2cPower {
@@ -208,7 +216,8 @@ pub struct HubrisI2cDevice {
     pub removable: bool,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Copy, Clone, Deserialize, Debug, PartialEq, Eq, Hash)]
+#[serde(rename_all = "lowercase")]
 pub enum HubrisSensorKind {
     Temperature,
     Power,
@@ -2218,7 +2227,8 @@ impl HubrisArchive {
         }
 
         let sensor_name = |d: &HubrisConfigI2cDevice,
-                           idx: usize|
+                           idx: usize,
+                           kind: HubrisSensorKind|
          -> Result<String> {
             if let Some(pmbus) = &d.pmbus {
                 if let Some(rails) = &pmbus.rails {
@@ -2228,10 +2238,15 @@ impl HubrisArchive {
                         bail!("sensor count exceeds rails for {:?}", d);
                     }
                 }
-            }
-
-            if let Some(power) = &d.power {
-                if let Some(rails) = &power.rails {
+            } else if d.power.is_some()
+                && d.power
+                    .as_ref()
+                    .unwrap()
+                    .sensors
+                    .as_ref()
+                    .map_or(true, |s| s.contains(&kind))
+            {
+                if let Some(rails) = &d.power.as_ref().unwrap().rails {
                     if idx < rails.len() {
                         return Ok(rails[idx].clone());
                     } else {
@@ -2262,6 +2277,14 @@ impl HubrisArchive {
                 Ok(format!("{}#{}", d.device, idx))
             }
         };
+        let get_sensor = |d: &HubrisConfigI2cDevice,
+                          i: usize,
+                          ndx: usize,
+                          kind: HubrisSensorKind|
+         -> Result<HubrisSensor> {
+            let name = sensor_name(d, i, kind)?;
+            Ok(HubrisSensor { name, kind, device: ndx })
+        };
 
         if let Some(ref devices) = i2c.devices {
             for device in devices {
@@ -2298,41 +2321,46 @@ impl HubrisArchive {
                     let ndx = self.manifest.i2c_devices.len();
 
                     for i in 0..sensors.temperature {
-                        self.manifest.sensors.push(HubrisSensor {
-                            name: sensor_name(device, i)?,
-                            kind: HubrisSensorKind::Temperature,
-                            device: ndx,
-                        });
+                        self.manifest.sensors.push(get_sensor(
+                            device,
+                            i,
+                            ndx,
+                            HubrisSensorKind::Temperature,
+                        )?);
                     }
 
                     for i in 0..sensors.power {
-                        self.manifest.sensors.push(HubrisSensor {
-                            name: sensor_name(device, i)?,
-                            kind: HubrisSensorKind::Power,
-                            device: ndx,
-                        });
+                        self.manifest.sensors.push(get_sensor(
+                            device,
+                            i,
+                            ndx,
+                            HubrisSensorKind::Power,
+                        )?);
                     }
                     for i in 0..sensors.current {
-                        self.manifest.sensors.push(HubrisSensor {
-                            name: sensor_name(device, i)?,
-                            kind: HubrisSensorKind::Current,
-                            device: ndx,
-                        });
+                        self.manifest.sensors.push(get_sensor(
+                            device,
+                            i,
+                            ndx,
+                            HubrisSensorKind::Current,
+                        )?);
                     }
                     for i in 0..sensors.voltage {
-                        self.manifest.sensors.push(HubrisSensor {
-                            name: sensor_name(device, i)?,
-                            kind: HubrisSensorKind::Voltage,
-                            device: ndx,
-                        });
+                        self.manifest.sensors.push(get_sensor(
+                            device,
+                            i,
+                            ndx,
+                            HubrisSensorKind::Voltage,
+                        )?);
                     }
 
                     for i in 0..sensors.speed {
-                        self.manifest.sensors.push(HubrisSensor {
-                            name: sensor_name(device, i)?,
-                            kind: HubrisSensorKind::Speed,
-                            device: ndx,
-                        });
+                        self.manifest.sensors.push(get_sensor(
+                            device,
+                            i,
+                            ndx,
+                            HubrisSensorKind::Speed,
+                        )?);
                     }
                 }
 

tests/cmd/chip.trycmd

@@ -13,7 +13,7 @@ For more information try --help
 
 ```
 $ humility --chip this-can-be-anything -V
-humility 0.9.2
+humility 0.9.3
 
 ```
 
@@ -28,7 +28,7 @@ For more information try --help
 
 ```
 $ humility -c apx432 -V
-humility 0.9.2
+humility 0.9.3
 
 ```
 

tests/cmd/version.trycmd

@@ -2,14 +2,14 @@ Long version flag:
 
 ```
 $ humility --version
-humility 0.9.2
+humility 0.9.3
 
 ```
 
 Short version flag:
 
 ```
 $ humility -V
-humility 0.9.2
+humility 0.9.3
 
 ```