commandline: Remove --has-mec

Now we can autodetect, we don't need it anymore, yay!

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

Author: JohnAZoidberg

README.md

@@ -202,8 +202,6 @@ Options:
           Specify I2C addresses of the PD chips (Advanced)
       --pd-ports <PD_PORTS> <PD_PORTS>
           Specify I2C ports of the PD chips (Advanced)
-      --has-mec <HAS_MEC>
-          Specify the type of EC chip (MEC/MCHP or other) [possible values: true, false]
   -t, --test                        Run self-test to check if interaction with EC is possible
   -h, --help                        Print help information
 ```

completions/bash/framework_tool

@@ -50,7 +50,6 @@ _framework_tool() {
         "--driver"
         "--pd-addrs"
         "--pd-ports"
-        "--has-mec"
         "-t" "--test"
         "-h" "--help"
     )
@@ -59,7 +58,6 @@ _framework_tool() {
     local inputdeck_modes=("auto" "off" "on")
     local console_modes=("recent" "follow")
     local drivers=("portio" "cros-ec" "windows")
-    local has_mec_options=("true" "false")
     local brightness_options=("high" "medium" "low" "ultra-low")
 
     local current_word prev_word
@@ -77,8 +75,6 @@ _framework_tool() {
         COMPREPLY=( $(compgen -W "${console_modes[*]}" -- "$current_word") )
     elif [[ $prev_word == "--driver" ]]; then
         COMPREPLY=( $(compgen -W "${drivers[*]}" -- "$current_word") )
-    elif [[ $prev_word == "--has-mec" ]]; then
-        COMPREPLY=( $(compgen -W "${has_mec_options[*]}" -- "$current_word") )
     elif [[ $prev_word == "--fp-brightness" ]]; then
         COMPREPLY=( $(compgen -W "${brightness_options[*]}" -- "$current_word") )
     fi

completions/zsh/_framework_tool

@@ -48,7 +48,6 @@ options=(
   '--driver[Select which driver is used]:driver:(portio cros-ec windows)'
   '--pd-addrs[Specify I2C addresses of the PD chips (Advanced)]:pd_addrs'
   '--pd-ports[Specify I2C ports of the PD chips (Advanced)]:pd_ports'
-  '--has-mec[Specify the type of EC chip (MEC/MCHP or other)]:has_mec:(true false)'
   '-t[Run self-test to check if interaction with EC is possible]'
   '-h[Print help]'
 )

framework_lib/src/ccgx/device.rs

@@ -41,8 +41,8 @@ impl PdPort {
         let platform = &(*config).as_ref().unwrap().platform;
 
         match (platform, self) {
-            (Platform::GenericFramework((left, _), _, _), PdPort::Left01) => *left,
-            (Platform::GenericFramework((_, right), _, _), PdPort::Right23) => *right,
+            (Platform::GenericFramework((left, _), _), PdPort::Left01) => *left,
+            (Platform::GenericFramework((_, right), _), PdPort::Right23) => *right,
             // Framework AMD Platforms (CCG8)
             (
                 Platform::Framework13Amd7080
@@ -70,8 +70,8 @@ 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::GenericFramework(_, (left, _)), PdPort::Left01) => *left,
+            (Platform::GenericFramework(_, (_, right)), PdPort::Right23) => *right,
             (Platform::IntelGen11, _) => 6,
             (Platform::IntelGen12 | Platform::IntelGen13, PdPort::Left01) => 6,
             (Platform::IntelGen12 | Platform::IntelGen13, PdPort::Right23) => 7,

framework_lib/src/chromium_ec/mod.rs

@@ -201,24 +201,6 @@ const BOARD_VERSION_NPC_DB: [i32; BOARD_VERSION_COUNT] = [
     100, 311, 521, 721, 931, 1131, 1341, 1551, 1751, 1961, 2171, 2370, 2580, 2780, 2990, 3200,
 ];
 
-pub fn has_mec() -> bool {
-    let platform = smbios::get_platform().unwrap();
-    if let Platform::GenericFramework(_, _, has_mec) = platform {
-        return has_mec;
-    }
-
-    // TODO: Should turn this around
-    !matches!(
-        smbios::get_platform().unwrap(),
-        Platform::Framework13Amd7080
-            | Platform::Framework16Amd7080
-            | Platform::IntelCoreUltra1
-            | Platform::Framework13AmdAi300
-            | Platform::Framework12IntelGen13
-            | Platform::FrameworkDesktopAmdAiMax300
-    )
-}
-
 pub trait CrosEcDriver {
     fn read_memory(&self, offset: u16, length: u16) -> Option<Vec<u8>>;
     fn send_command(&self, command: u16, command_version: u8, data: &[u8]) -> EcResult<Vec<u8>>;
@@ -953,30 +935,25 @@ impl CrosEc {
         // Everything before is probably a header.
         // TODO: I don't think there are magic bytes on zephyr firmware
         //
-        if has_mec() {
-            println!("    Check MCHP magic byte at start of firmware code.");
-            // Make sure we can read at an offset and with arbitrary length
-            let data = self.read_ec_flash(FLASH_PROGRAM_OFFSET, 16).unwrap();
-            debug!("Expecting beginning with 50 48 43 4D ('PHCM' in ASCII)");
-            debug!("{:02X?}", data);
-            println!(
-                "      {:02X?} ASCII:{:?}",
-                &data[..4],
-                core::str::from_utf8(&data[..4])
-            );
-
-            if data[0..4] != [0x50, 0x48, 0x43, 0x4D] {
-                println!("      INVALID: {:02X?}", &data[0..3]);
-                res = Err(EcError::DeviceError(format!(
-                    "INVALID: {:02X?}",
-                    &data[0..3]
-                )));
-            }
+        debug!("    Check MCHP magic bytes at start of firmware code.");
+        // Make sure we can read at an offset and with arbitrary length
+        let data = self.read_ec_flash(FLASH_PROGRAM_OFFSET, 16).unwrap();
+        debug!("Expecting beginning with 50 48 43 4D ('PHCM' in ASCII)");
+        debug!("{:02X?}", data);
+        debug!(
+            "      {:02X?} ASCII:{:?}",
+            &data[..4],
+            core::str::from_utf8(&data[..4])
+        );
+
+        let has_mec = data[0..4] == [0x50, 0x48, 0x43, 0x4D];
+        if has_mec {
+            debug!("    Found MCHP magic bytes at start of firmware code.");
         }
 
         // ===== Test 4 =====
         println!("    Read flash flags");
-        let data = if has_mec() {
+        let data = if has_mec {
             self.read_ec_flash(MEC_FLASH_FLAGS, 0x80).unwrap()
         } else {
             self.read_ec_flash(NPC_FLASH_FLAGS, 0x80).unwrap()

framework_lib/src/commandline/clap_std.rs

@@ -228,20 +228,15 @@ struct ClapCli {
     driver: Option<CrosEcDriverType>,
 
     /// Specify I2C addresses of the PD chips (Advanced)
-    #[clap(number_of_values = 2, requires("pd_ports"), requires("has_mec"))]
+    #[clap(number_of_values = 2, requires("pd_ports"))]
     #[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"))]
+    #[clap(number_of_values = 2, requires("pd_addrs"))]
     #[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,
@@ -408,7 +403,6 @@ pub fn parse(args: &[String]) -> Cli {
         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

@@ -191,7 +191,6 @@ pub struct Cli {
     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,
     pub flash_gpu_descriptor: Option<(u8, String)>,
@@ -701,12 +700,8 @@ pub fn run_with_args(args: &Cli, _allupdate: bool) -> i32 {
     }
 
     // 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 = Platform::GenericFramework(
-            args.pd_addrs.unwrap(),
-            args.pd_ports.unwrap(),
-            args.has_mec.unwrap(),
-        );
+    if args.pd_addrs.is_some() && args.pd_ports.is_some() {
+        let platform = Platform::GenericFramework(args.pd_addrs.unwrap(), args.pd_ports.unwrap());
         Config::set(platform);
     }
 
@@ -1169,7 +1164,7 @@ fn selftest(ec: &CrosEc) -> Option<()> {
     } else {
         println!("  SMBIOS Platform:     Unknown");
         println!();
-        println!("Specify custom platform parameters with --pd-ports --pd-addrs --has-mec");
+        println!("Specify custom platform parameters with --pd-ports --pd-addrs");
         return None;
     };
 

framework_lib/src/commandline/uefi.rs

@@ -106,7 +106,6 @@ pub fn parse(args: &[String]) -> Cli {
         driver: Some(CrosEcDriverType::Portio),
         pd_addrs: None,
         pd_ports: None,
-        has_mec: None,
         test: false,
         help: false,
         flash_gpu_descriptor: None,
@@ -610,22 +609,6 @@ pub fn parse(args: &[String]) -> Cli {
                 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();
         } else if arg == "--compare-version" {
@@ -699,11 +682,10 @@ pub fn parse(args: &[String]) -> Cli {
         }
     }
 
-    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();
+    let custom_platform = cli.pd_addrs.is_some() && cli.pd_ports.is_some();
+    let no_customization = cli.pd_addrs.is_none() && cli.pd_ports.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");
+        println!("To customize the platform you need to provide all of --pd-addrs, and --pd-ports");
     }
 
     if args.len() == 1 && cli.paginate {

framework_lib/src/smbios.rs

@@ -46,7 +46,7 @@ pub enum ConfigDigit0 {
 pub fn is_framework() -> bool {
     if matches!(
         get_platform(),
-        Some(Platform::GenericFramework((_, _), (_, _), _))
+        Some(Platform::GenericFramework((_, _), (_, _)))
     ) {
         return true;
     }
@@ -252,7 +252,7 @@ pub fn get_platform() -> Option<Platform> {
         // Except if it's a GenericFramework platform
         let config = Config::get();
         let platform = &(*config).as_ref().unwrap().platform;
-        if matches!(platform, Platform::GenericFramework((_, _), (_, _), _)) {
+        if matches!(platform, Platform::GenericFramework((_, _), (_, _))) {
             return Some(*platform);
         }
     }

framework_lib/src/util.rs

@@ -36,8 +36,8 @@ pub enum Platform {
     /// Framework Desktop - AMD Ryzen AI Max 300
     FrameworkDesktopAmdAiMax300,
     /// Generic Framework device
-    /// pd_addrs, pd_ports, has_mec
-    GenericFramework((u16, u16), (u8, u8), bool),
+    /// pd_addrs, pd_ports
+    GenericFramework((u16, u16), (u8, u8)),
 }
 
 #[derive(Debug, PartialEq, Clone, Copy)]

rgbkbd.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 # Example invocation in fish shell
 # cargo build && sudo ./target/debug/framework_tool \
-#   --driver portio --has-mec false --pd-ports 1 1 --pd-addrs 64 64 \
+#   --driver portio --pd-ports 1 1 --pd-addrs 64 64 \
 #   (./rgbkbd.py | string split ' ')
 
 BRIGHTNESS = 1