Merge pull request #157 from FrameworkComputer/pd-version-v2

--versions: Show any number of PD versions

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,8 @@
 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;
 use num_derive::FromPrimitive;
@@ -234,23 +236,32 @@ 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
 #[derive(Debug)]
-pub struct MainPdVersions {
-    pub controller01: ControllerVersion,
-    pub controller23: ControllerVersion,
+pub enum MainPdVersions {
+    RightLeft((ControllerVersion, ControllerVersion)),
+    Single(ControllerVersion),
+    Many(Vec<ControllerVersion>),
 }
 
 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/chromium_ec/commands.rs

@@ -987,18 +987,38 @@ impl EcRequest<EcResponseChassisIntrusionControl> for EcRequestChassisIntrusionC
 }
 
 #[repr(C, packed)]
-pub struct EcRequestReadPdVersion {}
+pub struct EcRequestReadPdVersionV0 {}
 
 #[repr(C, packed)]
-pub struct _EcResponseReadPdVersion {
+pub struct _EcResponseReadPdVersionV0 {
     pub controller01: [u8; 8],
     pub controller23: [u8; 8],
 }
 
-impl EcRequest<_EcResponseReadPdVersion> for EcRequestReadPdVersion {
+impl EcRequest<_EcResponseReadPdVersionV0> for EcRequestReadPdVersionV0 {
     fn command_id() -> EcCommands {
         EcCommands::ReadPdVersion
     }
+    fn command_version() -> u8 {
+        0
+    }
+}
+
+#[repr(C, packed)]
+pub struct EcRequestReadPdVersionV1 {}
+#[repr(C, packed)]
+pub struct _EcResponseReadPdVersionV1 {
+    pub pd_chip_count: u8,
+    pub pd_controllers: [u8; 0],
+}
+
+impl EcRequest<_EcResponseReadPdVersionV1> for EcRequestReadPdVersionV1 {
+    fn command_id() -> EcCommands {
+        EcCommands::ReadPdVersion
+    }
+    fn command_version() -> u8 {
+        1
+    }
 }
 
 #[repr(C, packed)]

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, SiliconId::*};
+use crate::ccgx::{self, MainPdVersions, PdVersions, SiliconId::*};
 use crate::chromium_ec;
 use crate::chromium_ec::commands::DeckStateMode;
 use crate::chromium_ec::commands::FpLedBrightnessLevel;
@@ -59,8 +59,7 @@ use crate::touchpad::print_touchpad_fw_ver;
 use crate::touchscreen;
 #[cfg(feature = "uefi")]
 use crate::uefi::enable_page_break;
-use crate::util;
-use crate::util::{Config, Platform, PlatformFamily};
+use crate::util::{self, Config, Platform, PlatformFamily};
 #[cfg(feature = "hidapi")]
 use hidapi::HidApi;
 use sha2::{Digest, Sha256, Sha384, Sha512};
@@ -398,11 +397,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,10 +472,43 @@ 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
-        println!("  Right (01):     {}", pd_versions.controller01.app);
-        println!("  Left  (23):     {}", pd_versions.controller23.app);
+        debug!("  Fallback to PD Host command");
+        match pd_versions {
+            MainPdVersions::RightLeft((controller01, controller23)) => {
+                if let Some(Platform::IntelGen11) = smbios::get_platform() {
+                    println!("  Right (01):     {}", controller01.base);
+                    println!("  Left  (23):     {}", controller23.base);
+                } else {
+                    println!("  Right (01):     {}", controller01.app);
+                    println!("  Left  (23):     {}", controller23.app);
+                }
+            }
+            MainPdVersions::Single(version) => {
+                println!("  Version:        {}", version.app);
+            }
+            MainPdVersions::Many(versions) => {
+                for (i, version) in versions.into_iter().enumerate() {
+                    println!("  PD {}:          {}", i, version.app);
+                }
+            }
+        }
     } else {
         println!("  Unknown")
     }
@@ -527,7 +556,7 @@ fn print_versions(ec: &CrosEc) {
     }
 
     #[cfg(target_os = "linux")]
-    {
+    if smbios::get_platform().and_then(Platform::which_cpu_vendor) != Some(util::CpuVendor::Amd) {
         println!("CSME");
         if let Ok(csme) = csme::csme_from_sysfs() {
             info!("  Enabled:          {}", csme.enabled);
@@ -638,8 +667,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) {
@@ -650,8 +679,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) {

framework_lib/src/power.rs

@@ -1,7 +1,7 @@
 //! Get information about system power (battery, AC, PD ports)
 
 use alloc::format;
-use alloc::string::String;
+use alloc::string::{String, ToString};
 use alloc::vec;
 use alloc::vec::Vec;
 use core::convert::TryInto;
@@ -798,6 +798,11 @@ pub fn is_charging(ec: &CrosEc) -> EcResult<(bool, bool)> {
     Ok((port0 || port1, port2 || port3))
 }
 
+fn parse_pd_ver_slice(data: &[u8]) -> ControllerVersion {
+    parse_pd_ver(&[
+        data[0], data[1], data[2], data[3], data[4], data[5], data[6], data[7],
+    ])
+}
 fn parse_pd_ver(data: &[u8; 8]) -> ControllerVersion {
     ControllerVersion {
         base: BaseVersion {
@@ -815,15 +820,37 @@ fn parse_pd_ver(data: &[u8; 8]) -> ControllerVersion {
     }
 }
 
-// NOTE: Only works on ADL at the moment!
-// TODO: Not on TGL, need to check if RPL and later have it.
+// NOTE: TGL (hx20) does not have this host command
 pub fn read_pd_version(ec: &CrosEc) -> EcResult<MainPdVersions> {
-    let info = EcRequestReadPdVersion {}.send_command(ec)?;
+    let info = EcRequestReadPdVersionV1 {}.send_command_vec(ec);
 
-    Ok(MainPdVersions {
-        controller01: parse_pd_ver(&info.controller01),
-        controller23: parse_pd_ver(&info.controller23),
-    })
+    // If v1 not available, fall back
+    if let Err(EcError::Response(EcResponseStatus::InvalidVersion)) = info {
+        let info = EcRequestReadPdVersionV0 {}.send_command(ec)?;
+
+        return Ok(if info.controller23 == [0, 0, 0, 0, 0, 0, 0, 0] {
+            MainPdVersions::Single(parse_pd_ver(&info.controller01))
+        } else {
+            MainPdVersions::RightLeft((
+                parse_pd_ver(&info.controller01),
+                parse_pd_ver(&info.controller23),
+            ))
+        });
+    }
+    // If any other error, exit
+    let info = info?;
+
+    let mut versions = vec![];
+    let pd_count = info[0] as usize;
+    for i in 0..pd_count {
+        // TODO: Is there a safer way to check the range?
+        if info.len() < 1 + 8 * (i + 1) {
+            return Err(EcError::DeviceError("Not enough data returned".to_string()));
+        }
+        versions.push(parse_pd_ver_slice(&info[1 + 8 * i..1 + 8 * (i + 1)]));
+    }
+
+    Ok(MainPdVersions::Many(versions))
 }
 
 pub fn standalone_mode(ec: &CrosEc) -> bool {

framework_lib/src/util.rs

@@ -49,7 +49,28 @@ pub enum PlatformFamily {
     FrameworkDesktop,
 }
 
+#[derive(Debug, PartialEq, Clone, Copy)]
+pub enum CpuVendor {
+    Intel,
+    Amd,
+}
+
 impl Platform {
+    pub fn which_cpu_vendor(self) -> Option<CpuVendor> {
+        match self {
+            Platform::Framework12IntelGen13
+            | Platform::IntelGen11
+            | Platform::IntelGen12
+            | Platform::IntelGen13
+            | Platform::IntelCoreUltra1 => Some(CpuVendor::Intel),
+            Platform::Framework13Amd7080
+            | Platform::Framework13AmdAi300
+            | Platform::Framework16Amd7080
+            | Platform::FrameworkDesktopAmdAiMax300 => Some(CpuVendor::Amd),
+            Platform::GenericFramework(..) => None,
+            Platform::UnknownSystem => None,
+        }
+    }
     pub fn which_family(self) -> Option<PlatformFamily> {
         match self {
             Platform::Framework12IntelGen13 => Some(PlatformFamily::Framework12),