Make pmbus_validate a function instead of a macro (oxidecomputer#952)

This allows us to size the amount of data we're reading based on the expected value, which fixes a failure to validate mwocp68 (which had an ID length of 17, one larger than the max size assumed by the macro).
ThreeFx · Nov 22, 2022 · e25acf0 · e25acf0
1 parent 7ffac61
commit e25acf0
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 47 deletions.
diff --git a/drv/i2c-devices/src/adm1272.rs b/drv/i2c-devices/src/adm1272.rs
@@ -6,7 +6,10 @@
 
 use core::cell::Cell;
 
-use crate::{CurrentSensor, TempSensor, Validate, VoltageSensor};
+use crate::{
+    pmbus_validate, BadValidation, CurrentSensor, TempSensor, Validate,
+    VoltageSensor,
+};
 use drv_i2c_api::*;
 use num_traits::float::FloatCore;
 use pmbus::commands::*;
@@ -23,6 +26,15 @@ pub enum Error {
     InvalidConfig,
 }
 
+impl From<BadValidation> for Error {
+    fn from(value: BadValidation) -> Self {
+        Self::BadValidation {
+            cmd: value.cmd,
+            code: value.code,
+        }
+    }
+}
+
 impl From<pmbus::Error> for Error {
     fn from(err: pmbus::Error) -> Self {
         Error::InvalidData { err }
@@ -256,8 +268,9 @@ impl Adm1272 {
 
 impl Validate<Error> for Adm1272 {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
-        let id = [0x41, 0x44, 0x4d, 0x31, 0x32, 0x37, 0x32, 0x2d, 0x32, 0x41];
-        pmbus_validate!(device, MFR_MODEL, id)
+        let expected = b"ADM1272-2A";
+        pmbus_validate(device, CommandCode::MFR_MODEL, expected)
+            .map_err(Into::into)
     }
 }
 

diff --git a/drv/i2c-devices/src/bmr491.rs b/drv/i2c-devices/src/bmr491.rs
@@ -6,7 +6,10 @@
 
 use core::cell::Cell;
 
-use crate::{CurrentSensor, TempSensor, Validate, VoltageSensor};
+use crate::{
+    pmbus_validate, BadValidation, CurrentSensor, TempSensor, Validate,
+    VoltageSensor,
+};
 use drv_i2c_api::*;
 use pmbus::commands::*;
 use userlib::units::*;
@@ -25,6 +28,15 @@ pub enum Error {
     InvalidData { err: pmbus::Error },
 }
 
+impl From<BadValidation> for Error {
+    fn from(value: BadValidation) -> Self {
+        Self::BadValidation {
+            cmd: value.cmd,
+            code: value.code,
+        }
+    }
+}
+
 impl From<pmbus::Error> for Error {
     fn from(err: pmbus::Error) -> Self {
         Error::InvalidData { err }
@@ -69,8 +81,9 @@ impl Bmr491 {
 
 impl Validate<Error> for Bmr491 {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
-        let expected = [0x46, 0x6c, 0x65, 0x78];
-        pmbus_validate!(device, MFR_ID, expected)
+        let expected = b"Flex";
+        pmbus_validate(device, CommandCode::MFR_ID, expected)
+            .map_err(Into::into)
     }
 }
 

diff --git a/drv/i2c-devices/src/isl68224.rs b/drv/i2c-devices/src/isl68224.rs
@@ -4,7 +4,10 @@
 
 use core::cell::Cell;
 
-use crate::{CurrentSensor, TempSensor, Validate, VoltageSensor};
+use crate::{
+    pmbus_validate, BadValidation, CurrentSensor, TempSensor, Validate,
+    VoltageSensor,
+};
 use drv_i2c_api::*;
 use pmbus::commands::isl68224::*;
 use pmbus::commands::CommandCode;
@@ -32,6 +35,15 @@ pub enum Error {
     InvalidData { err: pmbus::Error },
 }
 
+impl From<BadValidation> for Error {
+    fn from(value: BadValidation) -> Self {
+        Self::BadValidation {
+            cmd: value.cmd,
+            code: value.code,
+        }
+    }
+}
+
 impl From<Error> for ResponseCode {
     fn from(err: Error) -> Self {
         match err {
@@ -91,8 +103,9 @@ impl Isl68224 {
 
 impl Validate<Error> for Isl68224 {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
-        let expected = [0x00, 0x52, 0xd2, 0x49];
-        pmbus_validate!(device, IC_DEVICE_ID, expected)
+        let expected = &[0x00, 0x52, 0xd2, 0x49];
+        pmbus_validate(device, CommandCode::IC_DEVICE_ID, expected)
+            .map_err(Into::into)
     }
 }
 

diff --git a/drv/i2c-devices/src/lib.rs b/drv/i2c-devices/src/lib.rs
@@ -33,6 +33,9 @@
 
 #![no_std]
 
+use drv_i2c_api::{I2cDevice, ResponseCode};
+use pmbus::commands::CommandCode;
+
 macro_rules! pmbus_read {
     ($device:expr, $cmd:ident) => {
         match $cmd::CommandData::from_slice(&match $device
@@ -101,36 +104,23 @@ macro_rules! pmbus_write {
     }};
 }
 
-macro_rules! pmbus_validate {
-    ($device:expr, $dev:ident::$cmd:ident, $expected:ident) => {{
-        let mut id = [0u8; 16];
-
-        match $device.read_block::<u8>($dev::CommandCode::$cmd as u8, &mut id) {
-            Ok(size) => {
-                Ok(size == $expected.len()
-                    && id[0..$expected.len()] == $expected)
-            }
-            Err(code) => Err(Error::BadValidation {
-                cmd: CommandCode::$cmd as u8,
-                code,
-            }),
-        }
-    }};
-
-    ($device:expr, $cmd:ident, $expected:ident) => {{
-        let mut id = [0u8; 16];
+struct BadValidation {
+    cmd: u8,
+    code: ResponseCode,
+}
 
-        match $device.read_block::<u8>(CommandCode::$cmd as u8, &mut id) {
-            Ok(size) => {
-                Ok(size == $expected.len()
-                    && id[0..$expected.len()] == $expected)
-            }
-            Err(code) => Err(Error::BadValidation {
-                cmd: CommandCode::$cmd as u8,
-                code,
-            }),
-        }
-    }};
+fn pmbus_validate<const N: usize>(
+    device: &I2cDevice,
+    cmd: CommandCode,
+    expected: &[u8; N],
+) -> Result<bool, BadValidation> {
+    let mut id = [0u8; N];
+    let cmd = cmd as u8;
+
+    match device.read_block(cmd, &mut id) {
+        Ok(size) => Ok(size == N && id == *expected),
+        Err(code) => Err(BadValidation { cmd, code }),
+    }
 }
 
 pub trait TempSensor<T: core::convert::Into<drv_i2c_api::ResponseCode>> {

diff --git a/drv/i2c-devices/src/mwocp68.rs b/drv/i2c-devices/src/mwocp68.rs
@@ -4,7 +4,9 @@
 
 //! MWOCP68-3600 Murata power shelf
 
-use crate::{CurrentSensor, Validate, VoltageSensor};
+use crate::{
+    pmbus_validate, BadValidation, CurrentSensor, Validate, VoltageSensor,
+};
 use core::cell::Cell;
 use drv_i2c_api::*;
 use pmbus::commands::mwocp68::*;
@@ -32,6 +34,15 @@ pub enum Error {
     InvalidData { err: pmbus::Error },
 }
 
+impl From<BadValidation> for Error {
+    fn from(value: BadValidation) -> Self {
+        Self::BadValidation {
+            cmd: value.cmd,
+            code: value.code,
+        }
+    }
+}
+
 impl From<Error> for ResponseCode {
     fn from(err: Error) -> Self {
         match err {
@@ -105,8 +116,9 @@ impl Mwocp68 {
 
 impl Validate<Error> for Mwocp68 {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
-        let expected = *b"MWOCP68-3600-D-RM";
-        pmbus_validate!(device, MFR_MODEL, expected)
+        let expected = b"MWOCP68-3600-D-RM";
+        pmbus_validate(device, CommandCode::MFR_MODEL, expected)
+            .map_err(Into::into)
     }
 }
 

diff --git a/drv/i2c-devices/src/raa229618.rs b/drv/i2c-devices/src/raa229618.rs
@@ -4,7 +4,10 @@
 
 use core::cell::Cell;
 
-use crate::{CurrentSensor, TempSensor, Validate, VoltageSensor};
+use crate::{
+    pmbus_validate, BadValidation, CurrentSensor, TempSensor, Validate,
+    VoltageSensor,
+};
 use drv_i2c_api::*;
 use pmbus::commands::raa229618::*;
 use pmbus::commands::CommandCode;
@@ -32,6 +35,15 @@ pub enum Error {
     InvalidData { err: pmbus::Error },
 }
 
+impl From<BadValidation> for Error {
+    fn from(value: BadValidation) -> Self {
+        Self::BadValidation {
+            cmd: value.cmd,
+            code: value.code,
+        }
+    }
+}
+
 impl From<Error> for ResponseCode {
     fn from(err: Error) -> Self {
         match err {
@@ -104,8 +116,9 @@ impl Raa229618 {
 
 impl Validate<Error> for Raa229618 {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
-        let expected = [0x00, 0x99, 0xd2, 0x49];
-        pmbus_validate!(device, IC_DEVICE_ID, expected)
+        let expected = &[0x00, 0x99, 0xd2, 0x49];
+        pmbus_validate(device, CommandCode::IC_DEVICE_ID, expected)
+            .map_err(Into::into)
     }
 }
 

diff --git a/drv/i2c-devices/src/tps546b24a.rs b/drv/i2c-devices/src/tps546b24a.rs
@@ -6,7 +6,10 @@
 
 use core::cell::Cell;
 
-use crate::{CurrentSensor, TempSensor, Validate, VoltageSensor};
+use crate::{
+    pmbus_validate, BadValidation, CurrentSensor, TempSensor, Validate,
+    VoltageSensor,
+};
 use drv_i2c_api::*;
 use pmbus::commands::*;
 use userlib::units::*;
@@ -25,6 +28,15 @@ pub enum Error {
     InvalidData { err: pmbus::Error },
 }
 
+impl From<BadValidation> for Error {
+    fn from(value: BadValidation) -> Self {
+        Self::BadValidation {
+            cmd: value.cmd,
+            code: value.code,
+        }
+    }
+}
+
 impl From<pmbus::Error> for Error {
     fn from(err: pmbus::Error) -> Self {
         Error::InvalidData { err }
@@ -64,8 +76,9 @@ impl Tps546B24A {
 
 impl Validate<Error> for Tps546B24A {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
-        let expected = [0x54, 0x49, 0x54, 0x6B, 0x24, 0x41];
-        pmbus_validate!(device, IC_DEVICE_ID, expected)
+        let expected = &[0x54, 0x49, 0x54, 0x6B, 0x24, 0x41];
+        pmbus_validate(device, CommandCode::IC_DEVICE_ID, expected)
+            .map_err(Into::into)
     }
 }