drv-i2c-devices: make TMP117 driver nicer

drv/i2c-devices/src/tmp117.rs

@@ -40,8 +40,8 @@ pub struct Tmp117 {
     device: I2cDevice,
 }
 
-fn convert(raw: (u8, u8)) -> Celsius {
-    Celsius(f32::from(i16::from(raw.0) << 8 | i16::from(raw.1)) / 128.0)
+fn convert(raw: u16) -> Celsius {
+    Celsius(f32::from(raw as i16) / 128.0)
 }
 
 impl core::fmt::Display for Tmp117 {
@@ -55,27 +55,36 @@ impl Tmp117 {
         Self { device: *device }
     }
 
-    fn read_reg(&self, reg: Register) -> Result<(u8, u8), Error> {
-        match self.device.read_reg::<u8, [u8; 2]>(reg as u8) {
-            Ok(buf) => Ok((buf[0], buf[1])),
-            Err(code) => Err(Error::BadRegisterRead { reg, code }),
-        }
+    /// Read a 16-bit TMP117 register value as a `u16`.
+    pub fn read_reg(&self, reg: Register) -> Result<u16, Error> {
+        self.read_reg_bytes(reg).map(u16::from_be_bytes)
+    }
+
+    /// Read a raw 16-bit register as a raw array of two bytes.
+    ///
+    /// The TMP117 transmits a register's value over I2C in big-endian order, so
+    /// the first byte in the array is the most-significant byte and the second
+    /// is the least-significant byte.
+    pub fn read_reg_bytes(&self, reg: Register) -> Result<[u8; 2], Error> {
+        self.device
+            .read_reg::<u8, [u8; 2]>(reg as u8)
+            .map_err(|code| Error::BadRegisterRead { reg, code })
     }
 
     pub fn read_eeprom(&self) -> Result<[u8; 6], Error> {
-        let ee1 = self.read_reg(Register::EEPROM1)?;
-        let ee2 = self.read_reg(Register::EEPROM2)?;
-        let ee3 = self.read_reg(Register::EEPROM3)?;
+        let [ee1_0, ee1_1] = self.read_reg_bytes(Register::EEPROM1)?;
+        let [ee2_0, ee2_1] = self.read_reg_bytes(Register::EEPROM2)?;
+        let [ee3_0, ee3_1] = self.read_reg_bytes(Register::EEPROM3)?;
 
-        Ok([ee1.0, ee1.1, ee2.0, ee2.1, ee3.0, ee3.1])
+        Ok([ee1_0, ee1_1, ee2_0, ee2_1, ee3_0, ee3_1])
     }
 }
 
 impl Validate<Error> for Tmp117 {
     fn validate(device: &I2cDevice) -> Result<bool, Error> {
         let id = Tmp117::new(device).read_reg(Register::DeviceID)?;
 
-        Ok(id.0 == 0x1 && id.1 == 0x17)
+        Ok(id == 0x0117)
     }
 }
 

task/host-sp-comms/src/bsp/cosmo_ab.rs

@@ -429,32 +429,7 @@ impl ServerImpl {
                     5 => by_refdes!(J49_U1, tmp117),
                     _ => unreachable!(),
                 };
-
-                let name = dev.component_id().as_bytes();
-                *self.scratch = InventoryData::Tmp117 {
-                    id: 0,
-                    eeprom1: 0,
-                    eeprom2: 0,
-                    eeprom3: 0,
-                    temp_sensor: sensors.temperature.into(),
-                };
-                self.tx_buf.try_encode_inventory(sequence, name, || {
-                    let InventoryData::Tmp117 {
-                        id,
-                        eeprom1,
-                        eeprom2,
-                        eeprom3,
-                        temp_sensor: _,
-                    } = self.scratch
-                    else {
-                        unreachable!();
-                    };
-                    *id = dev.read_reg(0x0Fu8)?;
-                    *eeprom1 = dev.read_reg(0x05u8)?;
-                    *eeprom2 = dev.read_reg(0x06u8)?;
-                    *eeprom3 = dev.read_reg(0x08u8)?;
-                    Ok(self.scratch)
-                })
+                self.read_tmp117(sequence, dev, sensors.temperature)
             }
             43 => {
                 let spi = drv_spi_api::Spi::from(SPI.get_task_id());

task/host-sp-comms/src/bsp/gimlet_bcde.rs

@@ -402,31 +402,7 @@ impl ServerImpl {
                     5 => by_refdes!(J199_U1, tmp117),
                     _ => unreachable!(),
                 };
-                let name = dev.component_id().as_bytes();
-                *self.scratch = InventoryData::Tmp117 {
-                    id: 0,
-                    eeprom1: 0,
-                    eeprom2: 0,
-                    eeprom3: 0,
-                    temp_sensor: sensors.temperature.into(),
-                };
-                self.tx_buf.try_encode_inventory(sequence, name, || {
-                    let InventoryData::Tmp117 {
-                        id,
-                        eeprom1,
-                        eeprom2,
-                        eeprom3,
-                        temp_sensor: _,
-                    } = self.scratch
-                    else {
-                        unreachable!();
-                    };
-                    *id = dev.read_reg(0x0Fu8)?;
-                    *eeprom1 = dev.read_reg(0x05u8)?;
-                    *eeprom2 = dev.read_reg(0x06u8)?;
-                    *eeprom3 = dev.read_reg(0x08u8)?;
-                    Ok(self.scratch)
-                })
+                self.read_tmp117(sequence, dev, sensors.temperature)
             }
 
             58 => {

task/host-sp-comms/src/inventory/compute_sled_common.rs

@@ -180,6 +180,72 @@ impl crate::ServerImpl {
             Ok(self.scratch)
         });
     }
+
+    pub(crate) fn read_tmp117(
+        &mut self,
+        sequence: u64,
+        dev: I2cDevice,
+        sensor: impl Into<host_sp_messages::SensorIndex>,
+    ) {
+        use drv_i2c_devices::tmp117::{Error, Register, Tmp117};
+
+        let name = dev.component_id().as_bytes();
+        *self.scratch = InventoryData::Tmp117 {
+            id: 0,
+            eeprom1: 0,
+            eeprom2: 0,
+            eeprom3: 0,
+            temp_sensor: sensor.into(),
+        };
+        self.tx_buf.try_encode_inventory(sequence, name, || {
+            let InventoryData::Tmp117 {
+                id,
+                eeprom1,
+                eeprom2,
+                eeprom3,
+                temp_sensor: _,
+            } = self.scratch
+            else {
+                unreachable!();
+            };
+
+            let dev = Tmp117::new(&dev);
+            let read_reg =
+                |reg: Register| -> Result<u16, drv_i2c_api::ResponseCode> {
+                    dev.read_reg_bytes(reg)
+                        // Okay, this is a little bit wacky: the TMP117 returns
+                        // register values over I2C in big-endian order. The
+                        // `tmp117` driver's `Tmp117::read_reg` will convert
+                        // these to a u16 in a way that's aware of their wire
+                        // endianness. However, this message was added to the
+                        // IPCC inventory prior to that function being added to
+                        // the driver, and instead, it previously serialized a
+                        // little-endian u16 that was constructed from the
+                        // two-byte register values in the order that they were
+                        // read on the wire...so rather than `0x0117`, we would
+                        // send `0x1701`, and so forth. On the host side, we
+                        // expect them to be sent like this, and libtopo will
+                        // then unscramble the bytes so they are a
+                        // properly-ordered `u64` from the host's side. So, we
+                        // preserve this historical format by reading the raw
+                        // bytes and converting them to a u16 as though they
+                        // were sent little-endian, even though they were
+                        // actually sent on the wire big-endian.
+                        //
+                        // Sigh.
+                        .map(u16::from_le_bytes)
+                        .map_err(|err| match err {
+                            Error::BadRegisterRead { code, .. } => code,
+                        })
+                };
+
+            *id = read_reg(Register::DeviceID)?;
+            *eeprom1 = read_reg(Register::EEPROM1)?;
+            *eeprom2 = read_reg(Register::EEPROM2)?;
+            *eeprom3 = read_reg(Register::EEPROM3)?;
+            Ok(self.scratch)
+        })
+    }
 }
 
 fn read_fan_barcodes<T>(