Merge pull request #31 from FrameworkComputer/custom-platform

Allow customizing platform if not matched

Author: JohnAZoidberg

framework_lib/src/ccgx/device.rs

@@ -1,4 +1,4 @@
-//! Communicate with CCGX (CCG5, CCG6) PD controllers
+//! Communicate with CCGX (CCG5, CCG6, CCG8) PD controllers
 //!
 //! The current implementation talks to them by tunneling I2C through EC host commands.
 
@@ -41,9 +41,12 @@ impl PdPort {
         let platform = &(*config).as_ref().unwrap().platform;
 
         match (platform, self) {
+            (Platform::GenericFramework((left, _), _, _), PdPort::Left01) => *left,
+            (Platform::GenericFramework((_, right), _, _), PdPort::Right23) => *right,
+            // Framework AMD Platforms (CCG8)
             (Platform::Framework13Amd | Platform::Framework16, PdPort::Left01) => 0x42,
             (Platform::Framework13Amd | Platform::Framework16, PdPort::Right23) => 0x40,
-            // Intel Platforms
+            // Framework Intel Platforms (CCG5 and CCG6)
             (_, PdPort::Left01) => 0x08,
             (_, PdPort::Right23) => 0x40,
         }
@@ -55,11 +58,11 @@ impl PdPort {
         let platform = &(*config).as_ref().unwrap().platform;
 
         Ok(match (platform, self) {
+            (Platform::GenericFramework(_, (left, _), _), PdPort::Left01) => *left,
+            (Platform::GenericFramework(_, (_, right), _), PdPort::Right23) => *right,
             (Platform::IntelGen11, _) => 6,
-            (Platform::IntelGen12, PdPort::Left01) => 6,
-            (Platform::IntelGen12, PdPort::Right23) => 7,
-            (Platform::IntelGen13, PdPort::Left01) => 6,
-            (Platform::IntelGen13, PdPort::Right23) => 7,
+            (Platform::IntelGen12 | Platform::IntelGen13, PdPort::Left01) => 6,
+            (Platform::IntelGen12 | Platform::IntelGen13, PdPort::Right23) => 7,
             (Platform::Framework13Amd | Platform::Framework16, PdPort::Left01) => 1,
             (Platform::Framework13Amd | Platform::Framework16, PdPort::Right23) => 2,
             // (_, _) => Err(EcError::DeviceError(format!(

framework_lib/src/chromium_ec/mod.rs

@@ -120,6 +120,11 @@ pub enum EcResponseStatus {
 }
 
 pub fn has_mec() -> bool {
+    let platform = smbios::get_platform().unwrap();
+    if let Platform::GenericFramework(_, _, has_mec) = platform {
+        return has_mec;
+    }
+
     !matches!(
         smbios::get_platform().unwrap(),
         Platform::Framework13Amd | Platform::Framework16

framework_lib/src/commandline/clap_std.rs

@@ -140,6 +140,21 @@ struct ClapCli {
     #[arg(long)]
     driver: Option<CrosEcDriverType>,
 
+    /// Specify I2C addresses of the PD chips (Advanced)
+    #[clap(number_of_values = 2, requires("pd_ports"), requires("has_mec"))]
+    #[arg(long)]
+    pd_addrs: Vec<u16>,
+
+    /// Specify I2C ports of the PD chips (Advanced)
+    #[clap(number_of_values = 2, requires("pd_addrs"), requires("has_mec"))]
+    #[arg(long)]
+    pd_ports: Vec<u8>,
+
+    /// Specify the type of EC chip (MEC/MCHP or other)
+    #[clap(requires("pd_addrs"), requires("pd_ports"))]
+    #[arg(long)]
+    has_mec: Option<bool>,
+
     /// Run self-test to check if interaction with EC is possible
     #[arg(long, short)]
     test: bool,
@@ -149,6 +164,31 @@ struct ClapCli {
 pub fn parse(args: &[String]) -> Cli {
     let args = ClapCli::parse_from(args);
 
+    let pd_addrs = match args.pd_addrs.len() {
+        2 => Some((args.pd_addrs[0], args.pd_addrs[1])),
+        0 => None,
+        _ => {
+            // Actually unreachable, checked by clap
+            println!(
+                "Must provide exactly to PD Addresses. Provided: {:?}",
+                args.pd_addrs
+            );
+            std::process::exit(1);
+        }
+    };
+    let pd_ports = match args.pd_ports.len() {
+        2 => Some((args.pd_ports[0], args.pd_ports[1])),
+        0 => None,
+        _ => {
+            // Actually unreachable, checked by clap
+            println!(
+                "Must provide exactly to PD Ports. Provided: {:?}",
+                args.pd_ports
+            );
+            std::process::exit(1);
+        }
+    };
+
     Cli {
         verbosity: args.verbosity.log_level_filter(),
         versions: args.versions,
@@ -199,6 +239,9 @@ pub fn parse(args: &[String]) -> Cli {
         reboot_ec: args.reboot_ec,
         hash: args.hash.map(|x| x.into_os_string().into_string().unwrap()),
         driver: args.driver,
+        pd_addrs,
+        pd_ports,
+        has_mec: args.has_mec,
         test: args.test,
         // TODO: Set help. Not very important because Clap handles this by itself
         help: false,

framework_lib/src/commandline/mod.rs

@@ -47,6 +47,7 @@ use crate::smbios::{dmidecode_string_val, get_smbios, is_framework};
 #[cfg(feature = "uefi")]
 use crate::uefi::enable_page_break;
 use crate::util;
+use crate::util::Config;
 #[cfg(not(feature = "uefi"))]
 use hidapi::HidApi;
 use sha2::{Digest, Sha256, Sha384, Sha512};
@@ -147,6 +148,9 @@ pub struct Cli {
     pub console: Option<ConsoleArg>,
     pub reboot_ec: Option<RebootEcArg>,
     pub hash: Option<String>,
+    pub pd_addrs: Option<(u16, u16)>,
+    pub pd_ports: Option<(u8, u8)>,
+    pub has_mec: Option<bool>,
     pub help: bool,
     pub info: bool,
     // UEFI only
@@ -469,6 +473,16 @@ pub fn run_with_args(args: &Cli, _allupdate: bool) -> i32 {
             .init();
     }
 
+    // Must be run before any application code to set the config
+    if args.pd_addrs.is_some() && args.pd_ports.is_some() && args.has_mec.is_some() {
+        let platform = util::Platform::GenericFramework(
+            args.pd_addrs.unwrap(),
+            args.pd_ports.unwrap(),
+            args.has_mec.unwrap(),
+        );
+        Config::set(platform);
+    }
+
     let ec = if let Some(driver) = args.driver {
         if let Some(driver) = CrosEc::with(driver) {
             driver
@@ -836,11 +850,14 @@ fn hash(data: &[u8]) {
 }
 
 fn selftest(ec: &CrosEc) -> Option<()> {
-    println!(
-        "  SMBIOS Platform:     {:?}",
-        smbios::get_platform().unwrap()
-    );
-    println!("  SMBIOS is_framework: {}", smbios::is_framework());
+    if let Some(platform) = smbios::get_platform() {
+        println!("  SMBIOS Platform:     {:?}", platform);
+    } else {
+        println!("  SMBIOS Platform:     Unknown");
+        println!();
+        println!("Specify custom platform parameters with --pd-ports --pd-addrs --has-mec");
+        return None;
+    };
 
     println!("  Dump EC memory region");
     if let Some(mem) = ec.dump_mem_region() {

framework_lib/src/commandline/uefi.rs

@@ -86,6 +86,9 @@ pub fn parse(args: &[String]) -> Cli {
         hash: None,
         // This is the only driver that works on UEFI
         driver: Some(CrosEcDriverType::Portio),
+        pd_addrs: None,
+        pd_ports: None,
+        has_mec: None,
         test: false,
         help: false,
         allupdate: false,
@@ -169,7 +172,7 @@ pub fn parse(args: &[String]) -> Cli {
                     Some(Some(percent))
                 } else {
                     println!(
-                        "Invalid value for --charge_limit: '{}'. Must be integer < 100.",
+                        "Invalid value for --charge-limit: '{}'. Must be integer < 100.",
                         args[i + 1]
                     );
                     None
@@ -340,11 +343,72 @@ pub fn parse(args: &[String]) -> Cli {
                 None
             };
             found_an_option = true;
+        } else if arg == "--pd-addrs" {
+            cli.pd_addrs = if args.len() > i + 2 {
+                let left = args[i + 1].parse::<u16>();
+                let right = args[i + 2].parse::<u16>();
+                if left.is_ok() && right.is_ok() {
+                    Some((left.unwrap(), right.unwrap()))
+                } else {
+                    println!(
+                        "Invalid values for --pd-addrs: '{} {}'. Must be u16 integers.",
+                        args[i + 1],
+                        args[i + 2]
+                    );
+                    None
+                }
+            } else {
+                println!("--pd-addrs requires two arguments, one for each address");
+                None
+            };
+            found_an_option = true;
+        } else if arg == "--pd-ports" {
+            cli.pd_ports = if args.len() > i + 2 {
+                let left = args[i + 1].parse::<u8>();
+                let right = args[i + 2].parse::<u8>();
+                if left.is_ok() && right.is_ok() {
+                    Some((left.unwrap(), right.unwrap()))
+                } else {
+                    println!(
+                        "Invalid values for --pd-ports: '{} {}'. Must be u16 integers.",
+                        args[i + 1],
+                        args[i + 2]
+                    );
+                    None
+                }
+            } else {
+                println!("--pd-ports requires two arguments, one for each port");
+                None
+            };
+            found_an_option = true;
+        } else if arg == "--has-mec" {
+            cli.has_mec = if args.len() > i + 1 {
+                if let Ok(b) = args[i + 1].parse::<bool>() {
+                    Some(b)
+                } else {
+                    println!(
+                        "Invalid value for --has-mec: '{}'. Must be 'true' or 'false'.",
+                        args[i + 1]
+                    );
+                    None
+                }
+            } else {
+                println!("--has-mec requires extra boolean argument.");
+                None
+            };
+            found_an_option = true;
         } else if arg == "--raw-command" {
             cli.raw_command = args[1..].to_vec();
         }
     }
 
+    let custom_platform = cli.pd_addrs.is_some() && cli.pd_ports.is_some() && cli.has_mec.is_some();
+    let no_customization =
+        cli.pd_addrs.is_none() && cli.pd_ports.is_none() && cli.has_mec.is_none();
+    if !(custom_platform || no_customization) {
+        println!("To customize the platform you need to provide all of --pd-addrs, --pd-ports and --has-mec");
+    }
+
     if args.len() == 1 && cli.paginate {
         cli.help = true;
         found_an_option = true;

framework_lib/src/smbios.rs

@@ -5,7 +5,7 @@ use std::prelude::v1::*;
 #[cfg(not(feature = "uefi"))]
 use std::io::ErrorKind;
 
-use crate::util::Platform;
+use crate::util::{Config, Platform};
 use num_derive::FromPrimitive;
 use smbioslib::*;
 #[cfg(feature = "uefi")]
@@ -40,6 +40,12 @@ pub enum ConfigDigit0 {
 
 /// Check whether the manufacturer in the SMBIOS says Framework
 pub fn is_framework() -> bool {
+    if matches!(
+        get_platform(),
+        Some(Platform::GenericFramework((_, _), (_, _), _))
+    ) {
+        return true;
+    }
     let smbios = if let Some(smbios) = get_smbios() {
         smbios
     } else {
@@ -104,6 +110,16 @@ pub fn get_platform() -> Option<Platform> {
         return platform;
     }
 
+    if Config::is_set() {
+        // Config::get() recursively calls get_platform.
+        // Except if it's a GenericFramework platform
+        let config = Config::get();
+        let platform = &(*config).as_ref().unwrap().platform;
+        if matches!(platform, Platform::GenericFramework((_, _), (_, _), _)) {
+            return Some(*platform);
+        }
+    }
+
     let smbios = get_smbios();
     if smbios.is_none() {
         println!("Failed to find SMBIOS");
@@ -123,7 +139,7 @@ pub fn get_platform() -> Option<Platform> {
                 }
             }
             if let Some(family) = dmidecode_string_val(&data.family()) {
-                // Actually "Laptop" and "16in Laptop"
+                // Actually "Laptop", "13in Laptop", and "16in Laptop"
                 match family.as_str() {
                     // TGL Mainboard (I don't this ever appears in family)
                     "FRANBMCP" => return Some(Platform::IntelGen11),

framework_lib/src/util.rs

@@ -27,6 +27,9 @@ pub enum Platform {
     Framework13Amd,
     /// Framework 16
     Framework16,
+    /// Generic Framework device
+    /// pd_addrs, pd_ports, has_mec
+    GenericFramework((u16, u16), (u8, u8), bool),
 }
 
 #[derive(Debug)]
@@ -36,12 +39,27 @@ pub struct Config {
 }
 
 impl Config {
-    fn new() -> Self {
-        Config {
-            verbose: false,
-            platform: Platform::IntelGen11,
+    pub fn set(platform: Platform) {
+        #[cfg(feature = "std")]
+        let mut config = CONFIG.lock().unwrap();
+        #[cfg(not(feature = "std"))]
+        let mut config = CONFIG.lock();
+
+        if (*config).is_none() {
+            *config = Some(Config {
+                verbose: false,
+                platform,
+            });
         }
     }
+    pub fn is_set() -> bool {
+        #[cfg(feature = "std")]
+        let config = CONFIG.lock().unwrap();
+        #[cfg(not(feature = "std"))]
+        let config = CONFIG.lock();
+
+        (*config).is_some()
+    }
 
     pub fn get() -> MutexGuard<'static, Option<Self>> {
         #[cfg(feature = "std")]
@@ -50,12 +68,13 @@ impl Config {
         let mut config = CONFIG.lock();
 
         if (*config).is_none() {
-            let mut cfg = Config::new();
             if let Some(platform) = smbios::get_platform() {
                 // TODO: Perhaps add Qemu or NonFramework as a platform
-                cfg.platform = platform;
+                *config = Some(Config {
+                    verbose: false,
+                    platform,
+                });
             }
-            *config = Some(cfg);
         }
 
         // TODO: See if we can map the Option::unwrap before returning