--version: Properly fetch single PD controller ver from ccgx

For Framework Desktop. It has only a single PD controller.

Signed-off-by: Daniel Schaefer <[email protected]>

Author: JohnAZoidberg

framework_lib/src/ccgx/device.rs

@@ -26,13 +26,17 @@ enum ControlRegisters {
 pub enum PdPort {
     Left01,
     Right23,
+    Back,
 }
 
 impl PdPort {
     /// SMBUS/I2C Address
     fn i2c_address(&self) -> EcResult<u16> {
         let config = Config::get();
         let platform = &(*config).as_ref().unwrap().platform;
+        let unsupported = Err(EcError::DeviceError(
+            "Controller does not exist on this platform".to_string(),
+        ));
 
         Ok(match (platform, self) {
             (Platform::GenericFramework((left, _), _), PdPort::Left01) => *left,
@@ -50,21 +54,40 @@ impl PdPort {
                 | Platform::Framework16Amd7080,
                 PdPort::Right23,
             ) => 0x40,
-            // TODO: It only has a single PD controller
-            (Platform::FrameworkDesktopAmdAiMax300, _) => 0x08,
+            (Platform::FrameworkDesktopAmdAiMax300, PdPort::Back) => 0x08,
+            (Platform::FrameworkDesktopAmdAiMax300, _) => unsupported?,
+            // Framework Intel Platforms (CCG5 and CCG6)
+            (
+                Platform::Framework12IntelGen13
+                | Platform::IntelGen11
+                | Platform::IntelGen12
+                | Platform::IntelGen13
+                | Platform::IntelCoreUltra1,
+                PdPort::Left01,
+            ) => 0x08,
+            (
+                Platform::Framework12IntelGen13
+                | Platform::IntelGen11
+                | Platform::IntelGen12
+                | Platform::IntelGen13
+                | Platform::IntelCoreUltra1,
+                PdPort::Right23,
+            ) => 0x40,
             (Platform::UnknownSystem, _) => {
                 Err(EcError::DeviceError("Unsupported platform".to_string()))?
             }
-            // Framework Intel Platforms (CCG5 and CCG6)
-            (_, PdPort::Left01) => 0x08,
-            (_, PdPort::Right23) => 0x40,
+            (_, PdPort::Back) => unsupported?,
         })
     }
 
     /// I2C port on the EC
     fn i2c_port(&self) -> EcResult<u8> {
         let config = Config::get();
         let platform = &(*config).as_ref().unwrap().platform;
+        let unsupported = Err(EcError::DeviceError(format!(
+            "Controller {:?}, does not exist on {:?}",
+            self, platform
+        )));
 
         Ok(match (platform, self) {
             (Platform::GenericFramework(_, (left, _)), PdPort::Left01) => *left,
@@ -88,11 +111,12 @@ impl PdPort {
                 | Platform::Framework12IntelGen13,
                 PdPort::Right23,
             ) => 2,
-            // TODO: It only has a single PD controller
-            (Platform::FrameworkDesktopAmdAiMax300, _) => 1,
+            (Platform::FrameworkDesktopAmdAiMax300, PdPort::Back) => 1,
+            (Platform::FrameworkDesktopAmdAiMax300, _) => unsupported?,
             (Platform::UnknownSystem, _) => {
                 Err(EcError::DeviceError("Unsupported platform".to_string()))?
             }
+            (_, PdPort::Back) => unsupported?,
         })
     }
 }

framework_lib/src/ccgx/mod.rs

@@ -3,6 +3,7 @@
 use alloc::format;
 use alloc::string::String;
 use alloc::string::ToString;
+use alloc::vec;
 use alloc::vec::Vec;
 #[cfg(feature = "uefi")]
 use core::prelude::rust_2021::derive;
@@ -235,9 +236,10 @@ impl ControllerFirmwares {
 }
 
 #[derive(Debug, PartialEq)]
-pub struct PdVersions {
-    pub controller01: ControllerFirmwares,
-    pub controller23: ControllerFirmwares,
+pub enum PdVersions {
+    RightLeft((ControllerFirmwares, ControllerFirmwares)),
+    Single(ControllerFirmwares),
+    Many(Vec<ControllerFirmwares>),
 }
 
 /// Same as PdVersions but only the main FW
@@ -249,10 +251,17 @@ pub enum MainPdVersions {
 }
 
 pub fn get_pd_controller_versions(ec: &CrosEc) -> EcResult<PdVersions> {
-    Ok(PdVersions {
-        controller01: PdController::new(PdPort::Left01, ec.clone()).get_fw_versions()?,
-        controller23: PdController::new(PdPort::Right23, ec.clone()).get_fw_versions()?,
-    })
+    let pd01 = PdController::new(PdPort::Left01, ec.clone()).get_fw_versions();
+    let pd23 = PdController::new(PdPort::Right23, ec.clone()).get_fw_versions();
+    let pd_back = PdController::new(PdPort::Back, ec.clone()).get_fw_versions();
+
+    match (pd01, pd23, pd_back) {
+        (Err(_), Err(_), Ok(pd_back)) => Ok(PdVersions::Single(pd_back)),
+        (Ok(pd01), Ok(pd23), Err(_)) => Ok(PdVersions::RightLeft((pd01, pd23))),
+        (Ok(pd01), Ok(pd23), Ok(pd_back)) => Ok(PdVersions::Many(vec![pd01, pd23, pd_back])),
+        (Err(err), _, _) => Err(err),
+        (_, Err(err), _) => Err(err),
+    }
 }
 
 fn parse_metadata_ccg3(buffer: &[u8]) -> Option<(u32, u32)> {

framework_lib/src/commandline/mod.rs

@@ -33,7 +33,7 @@ use crate::capsule_content::{
 use crate::ccgx::device::{FwMode, PdController, PdPort};
 #[cfg(feature = "hidapi")]
 use crate::ccgx::hid::{check_ccg_fw_version, find_devices, DP_CARD_PID, HDMI_CARD_PID};
-use crate::ccgx::{self, MainPdVersions, SiliconId::*};
+use crate::ccgx::{self, MainPdVersions, PdVersions, SiliconId::*};
 use crate::chromium_ec;
 use crate::chromium_ec::commands::DeckStateMode;
 use crate::chromium_ec::commands::FpLedBrightnessLevel;
@@ -398,11 +398,8 @@ fn print_versions(ec: &CrosEc) {
     }
 
     println!("PD Controllers");
-
-    if let Ok(pd_versions) = ccgx::get_pd_controller_versions(ec) {
-        let right = &pd_versions.controller01;
-        let left = &pd_versions.controller23;
-        // let active_mode =
+    let ccgx_pd_vers = ccgx::get_pd_controller_versions(ec);
+    if let Ok(PdVersions::RightLeft((right, left))) = ccgx_pd_vers {
         if let Some(Platform::IntelGen11) = smbios::get_platform() {
             if right.main_fw.base != right.backup_fw.base {
                 println!("  Right (01)");
@@ -476,8 +473,24 @@ fn print_versions(ec: &CrosEc) {
                 left.main_fw.app, left.active_fw
             );
         }
+    } else if let Ok(PdVersions::Single(pd)) = ccgx_pd_vers {
+        if pd.main_fw.app != pd.backup_fw.app {
+            println!(
+                "    Main:         {}{}",
+                pd.main_fw.app,
+                active_mode(&pd.active_fw, FwMode::MainFw)
+            );
+            println!(
+                "    Backup:       {}{}",
+                pd.backup_fw.app,
+                active_mode(&pd.active_fw, FwMode::BackupFw)
+            );
+        } else {
+            println!("  Version:        {} ({:?})", pd.main_fw.app, pd.active_fw);
+        }
     } else if let Ok(pd_versions) = power::read_pd_version(ec) {
         // As fallback try to get it from the EC. But not all EC versions have this command
+        debug!("  Fallback to PD Host command");
         match pd_versions {
             MainPdVersions::RightLeft((controller01, controller23)) => {
                 if let Some(Platform::IntelGen11) = smbios::get_platform() {
@@ -655,8 +668,8 @@ fn compare_version(device: Option<HardwareDeviceType>, version: String, ec: &Cro
             }
         }
         Some(HardwareDeviceType::PD0) => {
-            if let Ok(pd_versions) = ccgx::get_pd_controller_versions(ec) {
-                let ver = pd_versions.controller01.active_fw_ver();
+            if let Ok(PdVersions::RightLeft((pd01, _pd23))) = ccgx::get_pd_controller_versions(ec) {
+                let ver = pd01.active_fw_ver();
                 println!("Comparing PD0 version {:?}", ver);
 
                 if ver.contains(&version) {
@@ -667,8 +680,8 @@ fn compare_version(device: Option<HardwareDeviceType>, version: String, ec: &Cro
             }
         }
         Some(HardwareDeviceType::PD1) => {
-            if let Ok(pd_versions) = ccgx::get_pd_controller_versions(ec) {
-                let ver = pd_versions.controller23.active_fw_ver();
+            if let Ok(PdVersions::RightLeft((_pd01, pd23))) = ccgx::get_pd_controller_versions(ec) {
+                let ver = pd23.active_fw_ver();
                 println!("Comparing PD1 version {:?}", ver);
 
                 if ver.contains(&version) {