I2C codegen dedupe

build/i2c/src/lib.rs

@@ -1386,31 +1386,14 @@ impl ConfigGenerator {
             for (rail, (device, index)) in &all {
                 let raw_bank = index.unwrap_or(0);
 
-                // ----
-                // First accessor, returns `(I2cDevice, u8)`
-
-                // if we update this code to be more clever than just
-                // to-lowercase'ing the rail names, you might need to go update
-                // the mapping in `control-plane-agent`!
-                write!(
-                    &mut self.output,
-                    r##"
-        #[allow(dead_code)]
-        pub fn {}(task: TaskId) -> (I2cDevice, u8) {{"##,
-                    rail.to_lowercase(),
-                )?;
-
-                let out = self.generate_device(device, 16);
-                writeln!(&mut self.output, "({out}, {raw_bank})\n        }}")?;
-
                 // ---
-                // Second accessor, returns `(I2cDevice, Option<u8>)`
+                // Accessor, returns `(I2cDevice, Option<u8>)`
 
                 write!(
                     &mut self.output,
                     r##"
         #[allow(dead_code)]
-        pub fn {}_with_opt_page_idx(task: TaskId)"##,
+        pub fn {}(task: TaskId)"##,
                     rail.to_lowercase(),
                 )?;
                 write!(&mut self.output, " -> (I2cDevice, Option<u8>) {{")?;

drv/cosmo-seq-server/src/main.rs

@@ -195,7 +195,7 @@ fn main() -> ! {
         use drv_i2c_devices::bmr491::{Bmr491, ExternalInputVoltageProtection};
 
         let dev = i2c_config::devices::bmr491_u80(I2C.get_task_id());
-        let driver = Bmr491::new(&dev, 0);
+        let driver = Bmr491::new(&dev);
 
         // Cosmo provides external undervoltage protection that kicks in at a
         // lower voltage than we'd like to tolerate, so, request additional

drv/cosmo-seq-server/src/vcore.rs

@@ -190,10 +190,12 @@ const VCORE_NSAMPLES: usize = 25;
 
 impl VCore {
     pub fn new(i2c: TaskId) -> Self {
-        let (device, rail) = i2c_config::pmbus::vddcr_cpu0_a0(i2c);
+        let (device, opt_rail) = i2c_config::pmbus::vddcr_cpu0_a0(i2c);
+        let rail = opt_rail.unwrap_or(0);
         let vddcr_cpu0 = Raa229620A::new(&device, rail);
 
-        let (device, rail) = i2c_config::pmbus::vddcr_cpu1_a0(i2c);
+        let (device, opt_rail) = i2c_config::pmbus::vddcr_cpu1_a0(i2c);
+        let rail = opt_rail.unwrap_or(0);
         let vddcr_cpu1 = Raa229620A::new(&device, rail);
 
         Self {

drv/gimlet-seq-server/src/main.rs

@@ -462,7 +462,7 @@ impl<S: SpiServer + Clone> ServerImpl<S> {
             };
 
             let dev = i2c_config::devices::bmr491_u431(I2C.get_task_id());
-            let driver = Bmr491::new(&dev, 0);
+            let driver = Bmr491::new(&dev);
 
             // Gimlet provides external undervoltage protection that is better
             // than what we'd get from the 491, so we rely on that.
@@ -528,7 +528,9 @@ impl<S: SpiServer + Clone> ServerImpl<S> {
         // Turn on the chassis LED once we reach A2
         sys.gpio_set(CHASSIS_LED);
 
-        let (device, rail) = i2c_config::pmbus::vdd_vcore(I2C.get_task_id());
+        let (device, opt_rail) =
+            i2c_config::pmbus::vdd_vcore(I2C.get_task_id());
+        let rail = opt_rail.unwrap_or(0);
 
         let mut server = Self {
             state: PowerState::A2,
@@ -1578,10 +1580,12 @@ cfg_if::cfg_if! {
             use drv_i2c_devices::raa229618::Raa229618;
             let i2c = I2C.get_task_id();
 
-            let (device, rail) = i2c_config::pmbus::vdd_vcore(i2c);
+            let (device, opt_rail) = i2c_config::pmbus::vdd_vcore(i2c);
+            let rail = opt_rail.unwrap_or(0);
             let mut vdd_vcore = Raa229618::new(&device, rail);
 
-            let (device, rail) = i2c_config::pmbus::vddcr_soc(i2c);
+            let (device, opt_rail) = i2c_config::pmbus::vddcr_soc(i2c);
+            let rail = opt_rail.unwrap_or(0);
             let mut vddcr_soc = Raa229618::new(&device, rail);
 
             retry_i2c_txn(I2cTxn::VCoreOff, || vdd_vcore.turn_off())?;
@@ -1593,10 +1597,12 @@ cfg_if::cfg_if! {
             use drv_i2c_devices::raa229618::Raa229618;
             let i2c = I2C.get_task_id();
 
-            let (device, rail) = i2c_config::pmbus::vdd_vcore(i2c);
+            let (device, opt_rail) = i2c_config::pmbus::vdd_vcore(i2c);
+            let rail = opt_rail.unwrap_or(0);
             let mut vdd_vcore = Raa229618::new(&device, rail);
 
-            let (device, rail) = i2c_config::pmbus::vddcr_soc(i2c);
+            let (device, opt_rail) = i2c_config::pmbus::vddcr_soc(i2c);
+            let rail = opt_rail.unwrap_or(0);
             let mut vddcr_soc = Raa229618::new(&device, rail);
 
             retry_i2c_txn(I2cTxn::VCoreOn, || vdd_vcore.turn_on())?;
@@ -1620,8 +1626,8 @@ cfg_if::cfg_if! {
 
             let i2c = I2C.get_task_id();
 
-            let (device, rail) = i2c_config::pmbus::v3p3_sys_a0(i2c);
-            let v3p3_sys_a0 = Tps546B24A::new(&device, rail);
+            let (device, _rail) = i2c_config::pmbus::v3p3_sys_a0(i2c);
+            let v3p3_sys_a0 = Tps546B24A::new(&device);
 
             ringbuf_entry!(
                 Trace::V3P3SysA0VOut(v3p3_sys_a0.read_vout().unwrap_lite())

drv/i2c-devices/src/bmr491.rs

@@ -133,7 +133,7 @@ pub struct MitigationFailure {
 }
 
 impl Bmr491 {
-    pub fn new(device: &I2cDevice, _rail: u8) -> Self {
+    pub fn new(device: &I2cDevice) -> Self {
         Bmr491 {
             device: *device,
             mode: Cell::new(None),

drv/i2c-devices/src/tps546b24a.rs

@@ -55,7 +55,7 @@ impl From<Error> for ResponseCode {
 }
 
 impl Tps546B24A {
-    pub fn new(device: &I2cDevice, _rail: u8) -> Self {
+    pub fn new(device: &I2cDevice) -> Self {
         Tps546B24A {
             device: *device,
             mode: Cell::new(None),

drv/psc-seq-server/src/bsp/observer_a.rs

@@ -3,9 +3,7 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use super::{PSU_COUNT, i2c_config, notifications};
-use drv_i2c_api::I2cDevice;
 use drv_stm32xx_sys_api as sys_api;
-use userlib::*;
 
 pub use drv_i2c_devices::mwocp6x::Mwocp67 as Mwocp6x;
 
@@ -49,8 +47,11 @@ pub const PSU_PWR_OK_NOTIF: [u32; PSU_COUNT] = [
     notifications::PSU_PWR_OK_5_MASK,
 ];
 
+/// Type returned by generated pmbus rail functions
+pub type SummonFn = fn(userlib::TaskId) -> (drv_i2c_api::I2cDevice, Option<u8>);
+
 /// In order to get the PMBus devices by PSU index, we need a little lookup table.
-pub const PSU_PMBUS_DEVS: [fn(TaskId) -> (I2cDevice, u8); PSU_COUNT] = [
+pub const PSU_PMBUS_DEVS: [SummonFn; PSU_COUNT] = [
     i2c_config::pmbus::v50_main_psu0,
     i2c_config::pmbus::v50_main_psu1,
     i2c_config::pmbus::v50_main_psu2,

drv/psc-seq-server/src/bsp/psc_bc.rs

@@ -3,9 +3,7 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use super::{PSU_COUNT, i2c_config, notifications};
-use drv_i2c_api::I2cDevice;
 use drv_stm32xx_sys_api as sys_api;
-use userlib::*;
 
 pub use drv_i2c_devices::mwocp6x::Mwocp68 as Mwocp6x;
 
@@ -51,8 +49,11 @@ pub const PSU_PWR_OK_NOTIF: [u32; PSU_COUNT] = [
     notifications::PSU_PWR_OK_6_MASK,
 ];
 
+/// Type returned by generated pmbus rail functions
+pub type SummonFn = fn(userlib::TaskId) -> (drv_i2c_api::I2cDevice, Option<u8>);
+
 /// In order to get the PMBus devices by PSU index, we need a little lookup table.
-pub const PSU_PMBUS_DEVS: [fn(TaskId) -> (I2cDevice, u8); PSU_COUNT] = [
+pub const PSU_PMBUS_DEVS: [SummonFn; PSU_COUNT] = [
     i2c_config::pmbus::v54_psu0,
     i2c_config::pmbus::v54_psu1,
     i2c_config::pmbus::v54_psu2,

drv/psc-seq-server/src/main.rs

@@ -506,7 +506,8 @@ fn main() -> ! {
         let dev = {
             let i2c = I2C.get_task_id();
             let make_dev = bsp::PSU_PMBUS_DEVS[i];
-            let (dev, rail) = make_dev(i2c);
+            let (dev, opt_rail) = make_dev(i2c);
+            let rail = opt_rail.unwrap_or(0);
             bsp::Mwocp6x::new(&dev, rail)
         };
         let slot = PSU_SLOTS[i];

drv/sidecar-seq-server/src/main.rs

@@ -903,7 +903,7 @@ fn main() -> ! {
         use drv_i2c_devices::bmr491::{Bmr491, ExternalInputVoltageProtection};
 
         let dev = i2c_config::devices::bmr491_u12(I2C.get_task_id());
-        let driver = Bmr491::new(&dev, 0);
+        let driver = Bmr491::new(&dev);
 
         // Sidecar provides external undervoltage protection that is better than
         // what we'd get from the 491, so we rely on that.

drv/sidecar-seq-server/src/tofino.rs

@@ -18,7 +18,9 @@ pub(crate) struct Tofino {
 
 impl Tofino {
     pub fn new(i2c_task: userlib::TaskId) -> Self {
-        let (i2c_device, rail) = i2c_config::pmbus::v0p8_tf2_vdd_core(i2c_task);
+        let (i2c_device, opt_rail) =
+            i2c_config::pmbus::v0p8_tf2_vdd_core(i2c_task);
+        let rail = opt_rail.unwrap_or(0);
         let vddcore = Raa229618::new(&i2c_device, rail);
         Self {
             policy: TofinoSequencerPolicy::Disabled,

task/control-plane-agent/build.rs

@@ -111,7 +111,7 @@ fn do_pmbus() -> Result<()> {
         // build_i2c *also* only to-lowercases the rail names to make functions
         write!(
             file,
-            "summon_fn: crate::i2c_config::pmbus::{}_with_opt_page_idx, ",
+            "summon_fn: crate::i2c_config::pmbus::{}, ",
             rail.to_lowercase()
         )?;
         write!(file, "status_bits: Capabilities(0x{:08x}) ", caps.0)?;
@@ -184,7 +184,7 @@ fn context_create_file(path: &Path) -> Result<File> {
 
 /// Look at the `pmbus` crate metadata to see if a specific command is "Illegal"
 /// and set the capability bit if not.
-macro_rules! check_if_marked_as_illegal_by_pmbus_metadata {
+macro_rules! set_if_pmbus_read_illegal {
     ($out:ident, $module:ident, $cmd:ident) => {{
         use drv_i2c_types::pmbus_status::Capabilities;
         use pmbus::{Command, Operation};
@@ -204,54 +204,16 @@ macro_rules! generator {
     ($name:literal, $module:ident) => {
         ($name, || {
             let mut out = 0u32;
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_WORD
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_VOUT
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_IOUT
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_TEMPERATURE
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out, $module, STATUS_CML
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_OTHER
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_INPUT
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_MFR_SPECIFIC
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_FANS_1_2
-            );
-            check_if_marked_as_illegal_by_pmbus_metadata!(
-                out,
-                $module,
-                STATUS_FANS_3_4
-            );
+            set_if_pmbus_read_illegal!(out, $module, STATUS_WORD);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_VOUT);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_IOUT);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_TEMPERATURE);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_CML);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_OTHER);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_INPUT);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_MFR_SPECIFIC);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_FANS_1_2);
+            set_if_pmbus_read_illegal!(out, $module, STATUS_FANS_3_4);
             Capabilities(out)
         })
     };

task/power/src/bsp/cosmo_ab.rs

@@ -344,7 +344,9 @@ impl State {
         .iter()
         .enumerate()
         {
-            let (dev, rail) = (builder)(i2c_task);
+            let (dev, opt_rail) = (builder)(i2c_task);
+            // If there is no rail index, then there must be a single rail
+            let rail = opt_rail.unwrap_or(0);
             let m = Max5970::new(&dev, rail, Ohms(0.005), true);
             if let Err(err) = m.set_dac_fast(0x99) {
                 ringbuf_entry!(Trace::Max5970ConfigFailed { u2_index: i, err });