Skip to content

Commit

Permalink
Allow customizing platform if not matched
Browse files Browse the repository at this point in the history
If the platform couldnd't be matched by SMBIOS, now you can select the
platform config manually on the commandline.
This is not usually needed, just for advanced users.

For example:

```
framework_tool --test --driver portio --pd-addrs 8 64 --pd-ports 6 6 --has-mec true

framework_tool --test --driver portio --pd-addrs 8 64 --pd-ports 6 7 --has-mec true

framework_tool --test --driver portio --pd-addrs 66 64 --pd-ports 1 2 --has-mec false
```

Signed-off-by: Daniel Schaefer <dhs@frame.work>
  • Loading branch information
JohnAZoidberg committed May 12, 2024
1 parent b96a342 commit f5bd38e
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 21 deletions.
15 changes: 9 additions & 6 deletions framework_lib/src/ccgx/device.rs
Original file line number Diff line number Diff line change
@@ -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.

Expand Down Expand Up @@ -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,
}
Expand All @@ -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!(
Expand Down
5 changes: 5 additions & 0 deletions framework_lib/src/chromium_ec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions framework_lib/src/commandline/clap_std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
27 changes: 22 additions & 5 deletions framework_lib/src/commandline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down
66 changes: 65 additions & 1 deletion framework_lib/src/commandline/uefi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 18 additions & 2 deletions framework_lib/src/smbios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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");
Expand All @@ -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),
Expand Down
33 changes: 26 additions & 7 deletions framework_lib/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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")]
Expand All @@ -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
Expand Down

0 comments on commit f5bd38e

Please sign in to comment.