ADM1273 support (#2250)

This change is largely a renaming of ADM1272 to ADM127x since the
ADM1273 is a drop-in replacement and has the same behavior. The only
thing of substance that was modified was how we Validate this device.
The current behavior is the read the MFG_MODEL register and expect it to
be `ADM1272-2A`. I think this is a bit fragile as the die revision is
tacked on there (`-2A`) and could change without us knowing (the
orderable PN here is actually still `ADM1272-1A` even though
`ADM1272-2A` is being delivered). Then there is the additional
complication that with the ADM1273 we would see something like
`ADM1273-$A`. Thus, I've changed the behavior of the `adm127x::validate`
no longer use the common `pmbus_validate` but to use its own version
which will look at the first 7 bytes of the MFG_MODEL and make sure it is
either `ADM1272` or `ADM1273`.

fixes #2247

Author: Aaron-Hartwig

Cargo.lock

@@ -1223,7 +1223,7 @@ refdes = "U73"
 [[config.i2c.devices]]
 bus = "rear"
 address = 0x14
-device = "adm1272"
+device = "adm127x"
 description = "Sled hot swap controller"
 power = { rails = [ "V54P5_IBC_A3" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }

app/cosmo/base.toml

@@ -263,7 +263,7 @@ refdes = "U501"
 # a development or evaluation vehicle
 #
 [[config.i2c.devices]]
-device = "adm1272"
+device = "adm127x"
 controller = 4
 port = "F"
 mux = 1

app/gemini-bu/app.toml

@@ -994,7 +994,7 @@ refdes = "U352"
 [[config.i2c.devices]]
 bus = "rear"
 address = 0x10
-device = "adm1272"
+device = "adm127x"
 description = "Fan hot swap controller"
 power = { rails = [ "V54_FAN" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }
@@ -1003,7 +1003,7 @@ refdes = "U419"
 [[config.i2c.devices]]
 bus = "rear"
 address = 0x14
-device = "adm1272"
+device = "adm127x"
 description = "Sled hot swap controller"
 power = { rails = [ "V54_HS_OUTPUT" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }

app/gimlet/base.toml

@@ -248,7 +248,7 @@ refdes = "U12"
 [[config.i2c.devices]]
 bus = "power"
 address = 0b0010_001
-device = "adm1272"
+device = "adm127x"
 description = "VBUS_SLED hot swap controller"
 power = { rails = [ "VBUS_SLED" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }
@@ -257,7 +257,7 @@ refdes = "U1"
 [[config.i2c.devices]]
 bus = "power"
 address = 0b0010_000
-device = "adm1272"
+device = "adm127x"
 description = "VBUS_SYS hot swap controller"
 power = { rails = [ "VBUS_SYS" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }

app/minibar/app.toml

@@ -483,7 +483,7 @@ af = 4
 [[config.i2c.devices]]
 bus = "northeast0"
 address = 0b0010_000
-device = "adm1272"
+device = "adm127x"
 description = "Fan 1 hot swap controller"
 power = { rails = [ "V54_FAN1" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }
@@ -531,7 +531,7 @@ refdes = "U92"
 [[config.i2c.devices]]
 bus = "northeast1"
 address = 0b0010_011
-device = "adm1272"
+device = "adm127x"
 description = "Fan 0 hot swap controller"
 power = { rails = [ "V54_FAN0" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }
@@ -559,7 +559,7 @@ removable = true
 [[config.i2c.devices]]
 bus = "northwest0"
 address = 0b0010_110
-device = "adm1272"
+device = "adm127x"
 description = "54V hot swap controller"
 power = { rails = [ "V54_HSC" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }
@@ -616,7 +616,7 @@ refdes = "U12"
 [[config.i2c.devices]]
 bus = "northwest1"
 address = 0b0010_011
-device = "adm1272"
+device = "adm127x"
 description = "Fan 2 hot swap controller"
 power = { rails = [ "V54_FAN2" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }
@@ -625,7 +625,7 @@ refdes = "U8"
 [[config.i2c.devices]]
 bus = "northwest1"
 address = 0b0010_000
-device = "adm1272"
+device = "adm127x"
 description = "Fan 3 hot swap controller"
 power = { rails = [ "V54_FAN3" ] }
 sensors = { temperature = 1, voltage = 1, current = 1 }

app/sidecar/base.toml

@@ -2,13 +2,12 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-//! Driver for the ADM1272 hot-swap controller
+//! Driver for the ADM1272 and ADM1273 hot-swap controller
 
 use core::cell::Cell;
 
 use crate::{
-    pmbus_validate, BadValidation, CurrentSensor, TempSensor, Validate,
-    VoltageSensor,
+    BadValidation, CurrentSensor, TempSensor, Validate, VoltageSensor,
 };
 use drv_i2c_api::*;
 use num_traits::float::FloatCore;
@@ -62,34 +61,34 @@ struct Coefficients {
     power: pmbus::Coefficients,
 }
 
-pub struct Adm1272 {
+pub struct Adm127X {
     /// Underlying I2C device
     device: I2cDevice,
     /// Value of the rsense resistor, in milliohms
     rsense: i32,
     /// Our (cached) coefficients
     coefficients: Cell<Option<Coefficients>>,
     /// Our (cached) configuration
-    config: Cell<Option<adm1272::PMON_CONFIG::CommandData>>,
+    config: Cell<Option<adm127x::PMON_CONFIG::CommandData>>,
 }
 
-impl core::fmt::Display for Adm1272 {
+impl core::fmt::Display for Adm127X {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        write!(f, "adm1272: {}", &self.device)
+        write!(f, "adm127x: {}", &self.device)
     }
 }
 
 #[derive(Copy, Clone, PartialEq)]
 enum Trace {
     None,
     Coefficients(pmbus::Coefficients),
-    Config(adm1272::PMON_CONFIG::CommandData),
-    WriteConfig(adm1272::PMON_CONFIG::CommandData),
+    Config(adm127x::PMON_CONFIG::CommandData),
+    WriteConfig(adm127x::PMON_CONFIG::CommandData),
 }
 
 ringbuf!(Trace, 8, Trace::None);
 
-impl Adm1272 {
+impl Adm127X {
     pub fn new(device: &I2cDevice, rsense: Ohms) -> Self {
         Self {
             device: *device,
@@ -99,12 +98,12 @@ impl Adm1272 {
         }
     }
 
-    fn read_config(&self) -> Result<adm1272::PMON_CONFIG::CommandData, Error> {
+    fn read_config(&self) -> Result<adm127x::PMON_CONFIG::CommandData, Error> {
         if let Some(ref config) = self.config.get() {
             return Ok(*config);
         }
 
-        let config = pmbus_read!(self.device, adm1272::PMON_CONFIG)?;
+        let config = pmbus_read!(self.device, adm127x::PMON_CONFIG)?;
         ringbuf_entry!(Trace::Config(config));
         self.config.set(Some(config));
 
@@ -113,10 +112,10 @@ impl Adm1272 {
 
     fn write_config(
         &self,
-        config: adm1272::PMON_CONFIG::CommandData,
+        config: adm127x::PMON_CONFIG::CommandData,
     ) -> Result<(), Error> {
         ringbuf_entry!(Trace::WriteConfig(config));
-        let out = pmbus_write!(self.device, adm1272::PMON_CONFIG, config);
+        let out = pmbus_write!(self.device, adm127x::PMON_CONFIG, config);
         if out.is_err() {
             // If the write fails, invalidate the cache, since we don't
             // know exactly what state the remote system ended up in.
@@ -127,11 +126,11 @@ impl Adm1272 {
 
     //
     // Unlike many/most PMBus devices that have one set of coefficients, the
-    // coefficients for the ADM1272 depends on the mode of the device.  We
+    // coefficients for the ADM127x depends on the mode of the device.  We
     // therefore determine these dynamically -- but cache the results.
     //
     fn load_coefficients(&self) -> Result<Coefficients, Error> {
-        use adm1272::PMON_CONFIG::*;
+        use adm127x::PMON_CONFIG::*;
 
         if let Some(coefficients) = self.coefficients.get() {
             return Ok(coefficients);
@@ -143,7 +142,7 @@ impl Adm1272 {
         let irange = config.get_i_range().ok_or(Error::InvalidConfig)?;
 
         //
-        // From Table 10 (columns 1 and 2) of the ADM1272 datasheet.
+        // From Table 10 (columns 1 and 2) of the ADM1272 and ADM1273 datasheets.
         //
         let voltage = match vrange {
             VRange::Range100V => pmbus::Coefficients {
@@ -161,7 +160,7 @@ impl Adm1272 {
         ringbuf_entry!(Trace::Coefficients(voltage));
 
         //
-        // From Table 10 (columns 3 and 4) of the ADM1272 datasheet.
+        // From Table 10 (columns 3 and 4) of the ADM1272 and ADM1273 datasheets.
         //
         let current = match irange {
             IRange::Range30mV => pmbus::Coefficients {
@@ -179,7 +178,7 @@ impl Adm1272 {
         ringbuf_entry!(Trace::Coefficients(current));
 
         //
-        // From Table 10 (columns 5 through 8) of the ADM1272 datasheet.
+        // From Table 10 (columns 5 through 8) of the ADM1272 and ADM1273 datasheet.
         //
         let power = match (irange, vrange) {
             (IRange::Range15mV, VRange::Range60V) => pmbus::Coefficients {
@@ -215,7 +214,7 @@ impl Adm1272 {
     }
 
     fn enable_vin_sampling(&self) -> Result<(), Error> {
-        use adm1272::PMON_CONFIG::*;
+        use adm127x::PMON_CONFIG::*;
         let mut config = self.read_config()?;
 
         match config.get_v_in_enable() {
@@ -229,7 +228,7 @@ impl Adm1272 {
     }
 
     fn enable_vout_sampling(&self) -> Result<(), Error> {
-        use adm1272::PMON_CONFIG::*;
+        use adm127x::PMON_CONFIG::*;
         let mut config = self.read_config()?;
 
         match config.get_v_out_enable() {
@@ -243,7 +242,7 @@ impl Adm1272 {
     }
 
     fn enable_temp1_sampling(&self) -> Result<(), Error> {
-        use adm1272::PMON_CONFIG::*;
+        use adm127x::PMON_CONFIG::*;
         let mut config = self.read_config()?;
 
         match config.get_temp_1_enable() {
@@ -258,12 +257,12 @@ impl Adm1272 {
 
     pub fn read_vin(&self) -> Result<Volts, Error> {
         self.enable_vin_sampling()?;
-        let vin = pmbus_read!(self.device, adm1272::READ_VIN)?;
+        let vin = pmbus_read!(self.device, adm127x::READ_VIN)?;
         Ok(Volts(vin.get(&self.load_coefficients()?.voltage)?.0))
     }
 
     pub fn peak_iout(&self) -> Result<Amperes, Error> {
-        let iout = pmbus_read!(self.device, adm1272::PEAK_IOUT)?;
+        let iout = pmbus_read!(self.device, adm127x::PEAK_IOUT)?;
         Ok(Amperes(iout.get(&self.load_coefficients()?.current)?.0))
     }
 
@@ -272,33 +271,42 @@ impl Adm1272 {
     }
 }
 
-impl Validate<Error> for Adm1272 {
+impl Validate<Error> for Adm127X {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
-        let expected = b"ADM1272-2A";
-        pmbus_validate(device, CommandCode::MFR_MODEL, expected)
-            .map_err(Into::into)
+        // We don't use the usual `pmbus_validate` here as the ADM127x case is special in that
+        // multiple device models and die revisions may be supported. We read 10 bytes but only
+        // look at the first 7 since the final three are a dash byte and two bytes of die rev.
+        // E.g., ADM1272-2A or ADM1273-1A
+        let cmd = CommandCode::MFR_MODEL as u8;
+        let mut model = [0u8; 10];
+        let allowed = [b"ADM1272", b"ADM1273"];
+        match device.read_block(cmd, &mut model) {
+            Ok(size) => Ok(size == model.len()
+                && allowed.iter().any(|&m| model.starts_with(m))),
+            Err(code) => Err(Error::from(BadValidation { cmd, code })),
+        }
     }
 }
 
-impl TempSensor<Error> for Adm1272 {
+impl TempSensor<Error> for Adm127X {
     fn read_temperature(&self) -> Result<Celsius, Error> {
         self.enable_temp1_sampling()?;
-        let temp = pmbus_read!(self.device, adm1272::READ_TEMPERATURE_1)?;
+        let temp = pmbus_read!(self.device, adm127x::READ_TEMPERATURE_1)?;
         Ok(Celsius(temp.get()?.0))
     }
 }
 
-impl CurrentSensor<Error> for Adm1272 {
+impl CurrentSensor<Error> for Adm127X {
     fn read_iout(&self) -> Result<Amperes, Error> {
-        let iout = pmbus_read!(self.device, adm1272::READ_IOUT)?;
+        let iout = pmbus_read!(self.device, adm127x::READ_IOUT)?;
         Ok(Amperes(iout.get(&self.load_coefficients()?.current)?.0))
     }
 }
 
-impl VoltageSensor<Error> for Adm1272 {
+impl VoltageSensor<Error> for Adm127X {
     fn read_vout(&self) -> Result<Volts, Error> {
         self.enable_vout_sampling()?;
-        let vout = pmbus_read!(self.device, adm1272::READ_VOUT)?;
+        let vout = pmbus_read!(self.device, adm127x::READ_VOUT)?;
         Ok(Volts(vout.get(&self.load_coefficients()?.voltage)?.0))
     }
 }

drv/i2c-devices/src/adm1272.rs renamed to drv/i2c-devices/src/adm127x.rs

@@ -6,7 +6,7 @@
 //!
 //! This crate contains (generally) all I2C device drivers, including:
 //!
-//! - [`adm1272`]: ADM1272 hot swap controller
+//! - [`adm127x`]: ADM1272 or ADM1273 hot swap controller
 //! - [`adt7420`]: ADT7420 temperature sensor
 //! - [`at24csw080`]: AT24CSW080 serial EEPROM
 //! - [`ds2482`]: DS2482-100 1-wire initiator
@@ -233,7 +233,7 @@ pub trait Validate<T: core::convert::Into<drv_i2c_api::ResponseCode>> {
     }
 }
 
-pub mod adm1272;
+pub mod adm127x;
 pub mod adt7420;
 pub mod at24csw080;
 pub mod bmr491;

drv/i2c-devices/src/lib.rs

@@ -414,7 +414,7 @@ pub enum InventoryData {
         fans: [Identity; 3],
     },
 
-    Adm1272 {
+    Adm127x {
         /// MFR_ID (PMBus operation 0x99)
         mfr_id: [u8; 3],
         /// MFR_MODEL (PMBus operation 0x9A)
@@ -1192,7 +1192,7 @@ mod tests {
             b = &b[106..];
         }
 
-        let d = InventoryData::Adm1272 {
+        let d = InventoryData::Adm127x {
             mfr_id: [1, 2, 3],
             mfr_model: [9, 8, 7, 6, 5, 4, 3, 2, 1, 0],
             mfr_revision: [0, 10],

lib/host-sp-messages/src/lib.rs

@@ -345,9 +345,9 @@ impl ServerImpl {
                 })
             }
             33 => {
-                let (dev, sensors) = by_refdes!(U79, adm1272);
+                let (dev, sensors) = by_refdes!(U79, adm127x);
                 let name = dev.component_id().as_bytes();
-                *self.scratch = InventoryData::Adm1272 {
+                *self.scratch = InventoryData::Adm127x {
                     mfr_id: [0u8; 3],
                     mfr_model: [0u8; 10],
                     mfr_revision: [0u8; 2],
@@ -359,7 +359,7 @@ impl ServerImpl {
                 };
                 self.tx_buf.try_encode_inventory(sequence, name, || {
                     use pmbus::commands::tps546b24a::CommandCode;
-                    let InventoryData::Adm1272 {
+                    let InventoryData::Adm127x {
                         mfr_id,
                         mfr_model,
                         mfr_revision,