feat(hil): ftdi cmd: list + eeprom read/write

Refactored FTDI serial terminology - Clear distinction between:
- `--usb-serial` (physical USB device, e.g., FT7ABC12)
- `--ftdi-serial` (channel-specific, e.g., FT7ABC12C)
- `--desc` (e.g., FT4232H C)
Added FtdiChannel enum with FromStr impl for parsing A/B/C/D
Updated commands (button-ctrl, reboot, ftdi read/write) to use the new
terminology
Added `ftdi list` command to enumerate all FTDI devices/channels
Fixed serial comparison in destroy_helper() to strip channel suffix
when comparing with USB serial

Author: fouge

hil/src/boot.rs

@@ -1,11 +1,11 @@
 use std::time::Duration;
 
-use crate::ftdi::{FtdiGpio, FtdiId, OutputState};
+use crate::ftdi::{FtdiChannel, FtdiGpio, FtdiId, OutputState};
 use color_eyre::{eyre::WrapErr as _, Result};
 use tracing::{debug, info};
 
-pub const BUTTON_PIN: crate::ftdi::Pin = FtdiGpio::CTS_PIN;
-pub const RECOVERY_PIN: crate::ftdi::Pin = FtdiGpio::RTS_PIN;
+pub const BUTTON_PIN: crate::ftdi::Pin = FtdiGpio::DTR_PIN;
+pub const RECOVERY_PIN: crate::ftdi::Pin = FtdiGpio::CTS_PIN;
 pub const NVIDIA_VENDOR_ID: u16 = 0x0955;
 pub const NVIDIA_USB_ETHERNET: u16 = 0x7035;
 
@@ -20,15 +20,28 @@ pub async fn is_recovery_mode_detected() -> Result<bool> {
     Ok(num_nvidia_devices > 0)
 }
 
-/// If `device` is `None`, will get the first available device.
+/// The default channel used for button/recovery control on the HIL.
+pub const DEFAULT_CHANNEL: FtdiChannel = FtdiChannel::C;
+
+/// If `device` is `None`, will get the first available device, or fall back to
+/// the default channel (FT4232H C) if multiple devices are found.
 #[tracing::instrument]
 pub async fn reboot(recovery: bool, device: Option<&FtdiId>) -> Result<()> {
     fn make_ftdi(device: Option<FtdiId>) -> Result<FtdiGpio> {
         let builder = FtdiGpio::builder();
         let builder = match &device {
             Some(FtdiId::Description(desc)) => builder.with_description(desc),
-            Some(FtdiId::SerialNumber(serial)) => builder.with_serial_number(serial),
-            None => builder.with_default_device(),
+            Some(FtdiId::FtdiSerial(serial)) => builder.with_ftdi_serial(serial),
+            None => match builder.with_default_device() {
+                Ok(b) => return b.configure().wrap_err("failed to configure ftdi"),
+                Err(_) => {
+                    // Fall back to default channel when multiple devices exist
+                    return FtdiGpio::builder()
+                        .with_description(DEFAULT_CHANNEL.description_suffix())
+                        .and_then(|b| b.configure())
+                        .wrap_err("failed to create ftdi device with default channel");
+                }
+            },
         };
         builder
             .and_then(|b| b.configure())

hil/src/commands/button_ctrl.rs

@@ -2,37 +2,112 @@ use clap::Parser;
 use color_eyre::{eyre::WrapErr as _, Result};
 use humantime::parse_duration;
 use std::time::Duration;
-use tracing::info;
+use tracing::{debug, info};
 
-use crate::boot::BUTTON_PIN;
-use crate::ftdi::{FtdiGpio, OutputState};
+use crate::boot::{BUTTON_PIN, DEFAULT_CHANNEL};
+use crate::ftdi::{FtdiChannel, FtdiGpio, OutputState};
 
+/// Control the orb button over the debug board
 #[derive(Debug, Parser)]
 pub struct ButtonCtrl {
-    ///Button press duration (e.g., "1s", "500ms")
+    /// Button press duration (e.g., "1s", "500ms")
     #[arg(long, default_value = "1s", value_parser = parse_duration)]
     press_duration: Duration,
+
+    /// The USB serial number of the FTDI chip (e.g., "FT7ABC12").
+    /// Will use the default channel (C) for this chip.
+    #[arg(long, conflicts_with_all = ["ftdi_serial", "desc"])]
+    usb_serial: Option<String>,
+
+    /// The FTDI serial number including channel (e.g., "FT7ABC12C").
+    /// This is the USB serial + channel letter (A/B/C/D).
+    #[arg(long, conflicts_with_all = ["usb_serial", "desc"])]
+    ftdi_serial: Option<String>,
+
+    /// The FTDI description (e.g., "FT4232H C").
+    #[arg(long, conflicts_with_all = ["usb_serial", "ftdi_serial"])]
+    desc: Option<String>,
+
+    /// The channel to use when --usb-serial is provided (A, B, C, or D).
+    /// Defaults to C.
+    #[arg(long, default_value = "C", requires = "usb_serial")]
+    channel: FtdiChannel,
 }
 
 impl ButtonCtrl {
     pub async fn run(self) -> Result<()> {
-        fn make_ftdi() -> Result<FtdiGpio> {
-            FtdiGpio::builder()
-                .with_default_device()
-                .and_then(|b| b.configure())
-                .wrap_err("failed to create ftdi device")
-        }
+        let usb_serial = self.usb_serial.clone();
+        let ftdi_serial = self.ftdi_serial.clone();
+        let desc = self.desc.clone();
+        let channel = self.channel;
+
+        let make_ftdi = move || -> Result<FtdiGpio> {
+            let builder = FtdiGpio::builder();
+            match (usb_serial.as_ref(), ftdi_serial.as_ref(), desc.as_ref()) {
+                (Some(usb_serial), None, None) => {
+                    debug!(
+                        "using FTDI device with USB serial: {usb_serial}, channel: {:?}",
+                        channel
+                    );
+                    builder
+                        .with_usb_serial(usb_serial, channel)
+                        .and_then(|b| b.configure())
+                        .wrap_err("failed to create ftdi device with USB serial")
+                }
+                (None, Some(ftdi_serial), None) => {
+                    debug!("using FTDI device with FTDI serial: {ftdi_serial}");
+                    builder
+                        .with_ftdi_serial(ftdi_serial)
+                        .and_then(|b| b.configure())
+                        .wrap_err("failed to create ftdi device with FTDI serial")
+                }
+                (None, None, Some(desc)) => {
+                    debug!("using FTDI device with description: {desc}");
+                    builder
+                        .with_description(desc)
+                        .and_then(|b| b.configure())
+                        .wrap_err("failed to create ftdi device with description")
+                }
+                (None, None, None) => {
+                    // Try default device first, fall back to default channel description
+                    match builder.with_default_device() {
+                        Ok(b) => {
+                            b.configure().wrap_err("failed to configure ftdi device")
+                        }
+                        Err(e) => {
+                            debug!("default device selection failed: {e}");
+                            let desc_suffix = DEFAULT_CHANNEL.description_suffix();
+                            debug!(
+                                "attempting to find device with description '{desc_suffix}'"
+                            );
+                            FtdiGpio::builder()
+                                .with_description(desc_suffix)
+                                .and_then(|b| b.configure())
+                                .wrap_err_with(|| {
+                                    format!(
+                                        "failed to open FTDI device with description \
+                                         '{desc_suffix}'"
+                                    )
+                                })
+                        }
+                    }
+                }
+                _ => unreachable!(),
+            }
+        };
 
         info!(
             "Holding button for {} seconds",
             self.press_duration.as_secs_f32()
         );
+        let press_duration = self.press_duration;
         tokio::task::spawn_blocking(move || -> Result<(), color_eyre::Report> {
             let mut ftdi = make_ftdi()?;
             ftdi.set_pin(BUTTON_PIN, OutputState::Low)?;
-            std::thread::sleep(self.press_duration);
+            std::thread::sleep(press_duration);
             ftdi.set_pin(BUTTON_PIN, OutputState::High)?;
             ftdi.destroy().wrap_err("failed to destroy ftdi")?;
+
             Ok(())
         })
         .await

hil/src/commands/ftdi.rs

@@ -1,4 +1,4 @@
-//! FTDI EEPROM read/write operations for FT4232H chips.
+//! FTDI device operations for FT4232H chips.
 
 use camino::Utf8PathBuf;
 use clap::{Parser, Subcommand};
@@ -11,7 +11,9 @@ use nusb::MaybeFuture;
 use serde::{Deserialize, Serialize};
 use tracing::{debug, info};
 
-/// FTDI EEPROM operations
+use crate::ftdi::{detach_all_ftdi_kernel_drivers, FtdiGpio};
+
+/// FTDI device operations
 #[derive(Debug, Parser)]
 pub struct FtdiCmd {
     #[command(subcommand)]
@@ -20,23 +22,34 @@ pub struct FtdiCmd {
 
 #[derive(Debug, Subcommand)]
 enum FtdiSubcommand {
+    /// List all connected FTDI devices/channels
+    List(ListCmd),
     /// Read EEPROM content and dump to a file
     Read(ReadCmd),
     /// Write EEPROM content from a file
     Write(WriteCmd),
 }
 
+/// List all connected FTDI devices
+#[derive(Debug, Parser)]
+struct ListCmd;
+
 /// Read EEPROM content to a file or stdout
 #[derive(Debug, Parser)]
 struct ReadCmd {
     /// Output file path (JSON format). If not specified, prints to stdout.
     #[arg(long, short)]
     file: Option<Utf8PathBuf>,
-    /// The serial number of the FTDI device to use
+
+    /// The USB serial number of the FTDI chip (e.g., "FT7ABC12").
+    /// Note: EEPROM is shared across all channels of a chip, so any channel
+    /// works for reading/writing EEPROM.
     #[arg(long, conflicts_with = "desc")]
-    serial_num: Option<String>,
-    /// The description of the FTDI device to use
-    #[arg(long, conflicts_with = "serial_num")]
+    usb_serial: Option<String>,
+
+    /// The FTDI description (e.g., "FT4232H A").
+    /// Any channel description works since EEPROM is chip-wide.
+    #[arg(long, conflicts_with = "usb_serial")]
     desc: Option<String>,
 }
 
@@ -45,11 +58,16 @@ struct ReadCmd {
 struct WriteCmd {
     /// Input file path (JSON format)
     input: Utf8PathBuf,
-    /// The serial number of the FTDI device to use
+
+    /// The USB serial number of the FTDI chip (e.g., "FT7ABC12").
+    /// Note: EEPROM is shared across all channels of a chip, so any channel
+    /// works for reading/writing EEPROM.
     #[arg(long, conflicts_with = "desc")]
-    serial_num: Option<String>,
-    /// The description of the FTDI device to use
-    #[arg(long, conflicts_with = "serial_num")]
+    usb_serial: Option<String>,
+
+    /// The FTDI description (e.g., "FT4232H A").
+    /// Any channel description works since EEPROM is chip-wide.
+    #[arg(long, conflicts_with = "usb_serial")]
     desc: Option<String>,
 }
 
@@ -130,20 +148,52 @@ impl Ft4232hEepromData {
 impl FtdiCmd {
     pub async fn run(self) -> Result<()> {
         match self.command {
+            FtdiSubcommand::List(cmd) => cmd.run().await,
             FtdiSubcommand::Read(cmd) => cmd.run().await,
             FtdiSubcommand::Write(cmd) => cmd.run().await,
         }
     }
 }
 
+impl ListCmd {
+    async fn run(self) -> Result<()> {
+        tokio::task::spawn_blocking(|| -> Result<()> {
+            detach_all_ftdi_kernel_drivers();
+
+            let devices: Vec<_> = FtdiGpio::list_devices()
+                .wrap_err("failed to list FTDI devices")?
+                .collect();
+
+            if devices.is_empty() {
+                println!("No FTDI devices found.");
+                return Ok(());
+            }
+
+            println!("Found {} FTDI device(s)/channel(s):\n", devices.len());
+            for (i, device) in devices.iter().enumerate() {
+                println!("Device {}:", i + 1);
+                println!("  Description:  {}", device.description);
+                println!("  FTDI Serial:  {}", device.serial_number);
+                println!("  Vendor ID:    0x{:04X}", device.vendor_id);
+                println!("  Product ID:   0x{:04X}", device.product_id);
+                println!();
+            }
+
+            Ok(())
+        })
+        .await
+        .wrap_err("task panicked")?
+    }
+}
+
 impl ReadCmd {
     async fn run(self) -> Result<()> {
         let output_path = self.file.clone();
-        let serial_num = self.serial_num.clone();
+        let usb_serial = self.usb_serial.clone();
         let desc = self.desc.clone();
 
         tokio::task::spawn_blocking(move || -> Result<()> {
-            let mut ft4232h = open_ft4232h(serial_num.as_deref(), desc.as_deref())?;
+            let mut ft4232h = open_ft4232h(usb_serial.as_deref(), desc.as_deref())?;
 
             info!("Reading EEPROM from FT4232H device...");
             let (eeprom, strings) = ft4232h
@@ -174,7 +224,7 @@ impl ReadCmd {
 impl WriteCmd {
     async fn run(self) -> Result<()> {
         let input_path = self.input.clone();
-        let serial_num = self.serial_num.clone();
+        let usb_serial = self.usb_serial.clone();
         let desc = self.desc.clone();
 
         tokio::task::spawn_blocking(move || -> Result<()> {
@@ -191,7 +241,7 @@ impl WriteCmd {
 
             let (eeprom, strings) = data.to_eeprom()?;
 
-            let mut ft4232h = open_ft4232h(serial_num.as_deref(), desc.as_deref())?;
+            let mut ft4232h = open_ft4232h(usb_serial.as_deref(), desc.as_deref())?;
 
             ft4232h
                 .eeprom_program(eeprom, strings)
@@ -207,14 +257,17 @@ impl WriteCmd {
     }
 }
 
-/// Opens an FT4232H device with optional serial number or description filter.
-fn open_ft4232h(serial_num: Option<&str>, desc: Option<&str>) -> Result<Ft4232h> {
-    match (serial_num, desc) {
-        (Some(serial), None) => open_ft4232h_by_serial(serial),
+/// Opens an FT4232H device with optional USB serial or description filter.
+///
+/// Note: For USB serial, we append 'A' to get the FTDI serial of the first channel,
+/// since EEPROM operations work the same on any channel of the same chip.
+fn open_ft4232h(usb_serial: Option<&str>, desc: Option<&str>) -> Result<Ft4232h> {
+    match (usb_serial, desc) {
+        (Some(usb_serial), None) => open_ft4232h_by_usb_serial(usb_serial),
         (None, Some(desc)) => open_ft4232h_by_description(desc),
         (None, None) => open_default_ft4232h(),
         (Some(_), Some(_)) => {
-            bail!("cannot specify both serial number and description")
+            bail!("cannot specify both USB serial and description")
         }
     }
 }
@@ -231,7 +284,7 @@ fn open_default_ft4232h() -> Result<Ft4232h> {
     }
     if usb_device_infos.len() > 1 {
         bail!(
-            "multiple FTDI devices found, please specify --serial-num or --desc to select one"
+            "multiple FTDI devices found, please specify --usb-serial or --desc to select one"
         );
     }
 
@@ -252,14 +305,14 @@ fn open_default_ft4232h() -> Result<Ft4232h> {
     Ok(ft4232h)
 }
 
-fn open_ft4232h_by_serial(serial: &str) -> Result<Ft4232h> {
-    ensure!(!serial.is_empty(), "serial number cannot be empty");
+fn open_ft4232h_by_usb_serial(usb_serial: &str) -> Result<Ft4232h> {
+    ensure!(!usb_serial.is_empty(), "USB serial cannot be empty");
 
     let usb_device_info = nusb::list_devices()
         .wait()
         .wrap_err("failed to enumerate devices")?
-        .find(|d| d.serial_number() == Some(serial))
-        .ok_or_else(|| eyre!("no device with serial number \"{serial}\" found"))?;
+        .find(|d| d.serial_number() == Some(usb_serial))
+        .ok_or_else(|| eyre!("no device with USB serial \"{usb_serial}\" found"))?;
 
     // Detach kernel drivers if needed
     if let Ok(usb_device) = usb_device_info.open().wait() {
@@ -268,8 +321,11 @@ fn open_ft4232h_by_serial(serial: &str) -> Result<Ft4232h> {
         }
     }
 
-    let ftdi = Ftdi::with_serial_number(serial).map_err(|e| {
-        eyre!("failed to open FTDI device with serial \"{serial}\": {e:?}")
+    // Use channel A (append 'A') to get the FTDI serial.
+    // EEPROM is shared across all channels, so any channel works.
+    let ftdi_serial = format!("{usb_serial}A");
+    let ftdi = Ftdi::with_serial_number(&ftdi_serial).map_err(|e| {
+        eyre!("failed to open FTDI device with FTDI serial \"{ftdi_serial}\": {e:?}")
     })?;
     let ft4232h: Ft4232h = ftdi
         .try_into()