From f481a76a3f39fb8070c18d839b0f1a6000d68cf1 Mon Sep 17 00:00:00 2001 From: Tim Crawford Date: Mon, 30 Dec 2024 15:47:41 -0700 Subject: [PATCH 1/2] Rename `FAN_{GET,SET}` to `FAN_{GET,SET}_PWM` Make the command and function names explicit that they are for controlling fan PWM duty. Signed-off-by: Tim Crawford --- src/board/system76/common/smfi.c | 12 ++++++------ src/common/include/common/command.h | 8 ++++---- tool/src/ec.rs | 12 ++++++------ tool/src/main.rs | 16 ++++++++-------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/board/system76/common/smfi.c b/src/board/system76/common/smfi.c index 225675aa8..938db59b1 100644 --- a/src/board/system76/common/smfi.c +++ b/src/board/system76/common/smfi.c @@ -123,7 +123,7 @@ static enum Result cmd_print(void) { return RES_OK; } -static enum Result cmd_fan_get(void) { +static enum Result cmd_fan_get_pwm(void) { switch (smfi_cmd[SMFI_CMD_DATA]) { case 1: smfi_cmd[SMFI_CMD_DATA + 1] = fan1_pwm_actual; @@ -139,7 +139,7 @@ static enum Result cmd_fan_get(void) { return RES_ERR; } -static enum Result cmd_fan_set(void) { +static enum Result cmd_fan_set_pwm(void) { switch (smfi_cmd[SMFI_CMD_DATA]) { case 1: // Set duty cycle of FAN1 @@ -384,11 +384,11 @@ void smfi_event(void) { case CMD_PRINT: smfi_cmd[SMFI_CMD_RES] = cmd_print(); break; - case CMD_FAN_GET: - smfi_cmd[SMFI_CMD_RES] = cmd_fan_get(); + case CMD_FAN_GET_PWM: + smfi_cmd[SMFI_CMD_RES] = cmd_fan_get_pwm(); break; - case CMD_FAN_SET: - smfi_cmd[SMFI_CMD_RES] = cmd_fan_set(); + case CMD_FAN_SET_PWM: + smfi_cmd[SMFI_CMD_RES] = cmd_fan_set_pwm(); break; case CMD_KEYMAP_GET: smfi_cmd[SMFI_CMD_RES] = cmd_keymap_get(); diff --git a/src/common/include/common/command.h b/src/common/include/common/command.h index 15c1ad31e..13b2e8730 100644 --- a/src/common/include/common/command.h +++ b/src/common/include/common/command.h @@ -20,10 +20,10 @@ enum Command { CMD_SPI = 5, // Reset EC CMD_RESET = 6, - // Get fan speeds - CMD_FAN_GET = 7, - // Set fan speeds - CMD_FAN_SET = 8, + // Get fan PWM duty + CMD_FAN_GET_PWM = 7, + // Set fan PWM duty + CMD_FAN_SET_PWM = 8, // Get keyboard map index CMD_KEYMAP_GET = 9, // Set keyboard map index diff --git a/tool/src/ec.rs b/tool/src/ec.rs index ae2111954..31a0ad72d 100644 --- a/tool/src/ec.rs +++ b/tool/src/ec.rs @@ -16,8 +16,8 @@ enum Cmd { Print = 4, Spi = 5, Reset = 6, - FanGet = 7, - FanSet = 8, + FanGetPwm = 7, + FanSetPwm = 8, KeymapGet = 9, KeymapSet = 10, LedGetValue = 11, @@ -176,16 +176,16 @@ impl Ec { } /// Read fan duty cycle by fan index - pub unsafe fn fan_get(&mut self, index: u8) -> Result { + pub unsafe fn fan_get_pwm(&mut self, index: u8) -> Result { let mut data = [index, 0]; - self.command(Cmd::FanGet, &mut data)?; + self.command(Cmd::FanGetPwm, &mut data)?; Ok(data[1]) } /// Set fan duty cycle by fan index - pub unsafe fn fan_set(&mut self, index: u8, duty: u8) -> Result<(), Error> { + pub unsafe fn fan_set_pwm(&mut self, index: u8, duty: u8) -> Result<(), Error> { let mut data = [index, duty]; - self.command(Cmd::FanSet, &mut data) + self.command(Cmd::FanSetPwm, &mut data) } /// Read keymap data by layout, output pin, and input pin diff --git a/tool/src/main.rs b/tool/src/main.rs index b49747761..8c3555dca 100644 --- a/tool/src/main.rs +++ b/tool/src/main.rs @@ -270,15 +270,15 @@ unsafe fn print(ec: &mut Ec>, message: &[u8]) -> Result<(), Erro Ok(()) } -unsafe fn fan_get(ec: &mut Ec>, index: u8) -> Result<(), Error> { - let duty = ec.fan_get(index)?; +unsafe fn fan_get_pwm(ec: &mut Ec>, index: u8) -> Result<(), Error> { + let duty = ec.fan_get_pwm(index)?; println!("{}", duty); Ok(()) } -unsafe fn fan_set(ec: &mut Ec>, index: u8, duty: u8) -> Result<(), Error> { - ec.fan_set(index, duty) +unsafe fn fan_set_pwm(ec: &mut Ec>, index: u8, duty: u8) -> Result<(), Error> { + ec.fan_set_pwm(index, duty) } unsafe fn keymap_get( @@ -330,7 +330,7 @@ fn parse_color(s: &str) -> Result<(u8, u8, u8), String> { #[clap(rename_all = "snake_case")] enum SubCommand { Console, - Fan { + FanPwm { index: u8, duty: Option, }, @@ -448,15 +448,15 @@ fn main() { process::exit(1); } }, - SubCommand::Fan { index, duty } => match duty { - Some(duty) => match unsafe { fan_set(&mut ec, index, duty) } { + SubCommand::FanPwm { index, duty } => match duty { + Some(duty) => match unsafe { fan_set_pwm(&mut ec, index, duty) } { Ok(()) => (), Err(err) => { eprintln!("failed to set fan {} to {}: {:X?}", index, duty, err); process::exit(1); } }, - None => match unsafe { fan_get(&mut ec, index) } { + None => match unsafe { fan_get_pwm(&mut ec, index) } { Ok(()) => (), Err(err) => { eprintln!("failed to get fan {}: {:X?}", index, err); From 066a93fcaab97a9f2a2e9e0e5d23706c5fc99de3 Mon Sep 17 00:00:00 2001 From: Tim Crawford Date: Thu, 2 Jan 2025 13:38:02 -0700 Subject: [PATCH 2/2] Add manual fan control Allow fan target duties to be set via ACPI or SMFI command. This will allow system firmware or OS to set fan duty, which can be used for testing or implementing user-defined fan tables. Setting PWM via SMFI already existed, but would not work as the value would simply be overwritten by the EC. RPM target is not supported. Signed-off-by: Tim Crawford --- src/board/system76/common/acpi.c | 27 ++++++++++ src/board/system76/common/fan.c | 9 ++++ src/board/system76/common/include/board/fan.h | 12 +++++ src/board/system76/common/main.c | 5 +- src/board/system76/common/smfi.c | 29 +++++++++++ src/common/include/common/command.h | 5 +- tool/src/ec.rs | 52 +++++++++++++++++++ tool/src/lib.rs | 2 +- tool/src/main.rs | 30 +++++++++++ 9 files changed, 168 insertions(+), 3 deletions(-) diff --git a/src/board/system76/common/acpi.c b/src/board/system76/common/acpi.c index 132dd6cdc..eac2f8046 100644 --- a/src/board/system76/common/acpi.c +++ b/src/board/system76/common/acpi.c @@ -177,6 +177,10 @@ uint8_t acpi_read(uint8_t addr) { ACPI_16(0xD2, fan2_rpm); #endif // FAN2_PWM + case 0xD4: + data = fan_get_mode(); + break; + #if HAVE_LED_AIRPLANE_N // Airplane mode LED case 0xD9: @@ -224,6 +228,29 @@ void acpi_write(uint8_t addr, uint8_t data) { (void)battery_save_thresholds(); break; + case 0xCE: + if (fan_get_mode() == FAN_MODE_PWM) { + fan1_pwm_target = data; + } + break; + +#ifdef FAN2_PWM + case 0xCF: + if (fan_get_mode() == FAN_MODE_PWM) { + fan2_pwm_target = data; + } + break; +#endif + + case 0xD4: + switch (data) { + case FAN_MODE_AUTO: + case FAN_MODE_PWM: + fan_set_mode(data); + break; + } + break; + #if HAVE_LED_AIRPLANE_N // Airplane mode LED case 0xD9: diff --git a/src/board/system76/common/fan.c b/src/board/system76/common/fan.c index 17462c862..15f005ae4 100644 --- a/src/board/system76/common/fan.c +++ b/src/board/system76/common/fan.c @@ -12,6 +12,7 @@ #endif bool fan_max = false; +static enum FanMode fan_mode = FAN_MODE_AUTO; uint8_t fan1_pwm_actual = 0; uint8_t fan1_pwm_target = 0; @@ -261,3 +262,11 @@ void fan_update_duty(void) { fan2_rpm = fan_get_tach1_rpm(); #endif } + +enum FanMode fan_get_mode(void) { + return fan_mode; +} + +void fan_set_mode(enum FanMode mode) { + fan_mode = mode; +} diff --git a/src/board/system76/common/include/board/fan.h b/src/board/system76/common/include/board/fan.h index d06171528..25a84bc75 100644 --- a/src/board/system76/common/include/board/fan.h +++ b/src/board/system76/common/include/board/fan.h @@ -21,6 +21,15 @@ struct Fan { uint8_t pwm_min; }; +enum FanMode { + // EC control + FAN_MODE_AUTO = 0, + // Host control via target PWM + FAN_MODE_PWM = 1, + // Host control via target RPM + FAN_MODE_RPM = 2, +}; + extern bool fan_max; extern uint8_t fan1_pwm_actual; @@ -34,4 +43,7 @@ void fan_reset(void); void fan_update_duty(void); void fan_update_target(void); +enum FanMode fan_get_mode(void); +void fan_set_mode(enum FanMode); + #endif // _BOARD_FAN_H diff --git a/src/board/system76/common/main.c b/src/board/system76/common/main.c index 5c9240a18..4d79566ce 100644 --- a/src/board/system76/common/main.c +++ b/src/board/system76/common/main.c @@ -173,7 +173,10 @@ void main(void) { last_time_1sec = time; battery_event(); - fan_update_target(); + + if (fan_get_mode() == FAN_MODE_AUTO) { + fan_update_target(); + } } // Idle until next timer interrupt diff --git a/src/board/system76/common/smfi.c b/src/board/system76/common/smfi.c index 938db59b1..c2ece637f 100644 --- a/src/board/system76/common/smfi.c +++ b/src/board/system76/common/smfi.c @@ -140,6 +140,10 @@ static enum Result cmd_fan_get_pwm(void) { } static enum Result cmd_fan_set_pwm(void) { + if (fan_get_mode() != FAN_MODE_PWM) { + return RES_ERR; + } + switch (smfi_cmd[SMFI_CMD_DATA]) { case 1: // Set duty cycle of FAN1 @@ -157,6 +161,24 @@ static enum Result cmd_fan_set_pwm(void) { return RES_ERR; } +static enum Result cmd_fan_get_mode(void) { + smfi_cmd[SMFI_CMD_DATA] = fan_get_mode(); + return RES_OK; +} + +static enum Result cmd_fan_set_mode(void) { + enum FanMode mode = smfi_cmd[SMFI_CMD_DATA]; + + switch (mode) { + case FAN_MODE_AUTO: + case FAN_MODE_PWM: + fan_set_mode(mode); + return RES_OK; + } + + return RES_ERR; +} + static enum Result cmd_keymap_get(void) { int16_t layer = smfi_cmd[SMFI_CMD_DATA]; int16_t output = smfi_cmd[SMFI_CMD_DATA + 1]; @@ -421,6 +443,13 @@ void smfi_event(void) { break; #endif // CONFIG_SECURITY + case CMD_FAN_GET_MODE: + smfi_cmd[SMFI_CMD_RES] = cmd_fan_get_mode(); + break; + case CMD_FAN_SET_MODE: + smfi_cmd[SMFI_CMD_RES] = cmd_fan_set_mode(); + break; + #endif // !defined(__SCRATCH__) case CMD_SPI: smfi_cmd[SMFI_CMD_RES] = cmd_spi(); diff --git a/src/common/include/common/command.h b/src/common/include/common/command.h index 13b2e8730..0e17e51f4 100644 --- a/src/common/include/common/command.h +++ b/src/common/include/common/command.h @@ -50,7 +50,10 @@ enum Command { CMD_SECURITY_GET = 20, // Set security state CMD_SECURITY_SET = 21, - //TODO + // Get fan control mode + CMD_FAN_GET_MODE = 22, + // Set fan control mode + CMD_FAN_SET_MODE = 23, }; enum Result { diff --git a/tool/src/ec.rs b/tool/src/ec.rs index 31a0ad72d..f7cc7dc09 100644 --- a/tool/src/ec.rs +++ b/tool/src/ec.rs @@ -3,6 +3,7 @@ #[cfg(not(feature = "std"))] use alloc::{boxed::Box, vec}; use core::convert::TryFrom; +use core::fmt; use crate::{Access, Error, Spi, SpiTarget}; @@ -31,6 +32,8 @@ enum Cmd { SetNoInput = 19, SecurityGet = 20, SecuritySet = 21, + FanGetMode = 22, + FanSetMode = 23, } const CMD_SPI_FLAG_READ: u8 = 1 << 0; @@ -65,6 +68,42 @@ impl TryFrom for SecurityState { } } +#[derive(Clone, Copy, Default, Debug, Eq, PartialEq)] +#[cfg_attr(feature = "std", derive(clap::ValueEnum))] +#[repr(u8)] +pub enum FanMode { + /// EC control + #[default] + Auto = 0, + /// Host control via target PWM + Pwm = 1, + /// Host control via target RPM + Rpm = 2, +} + +impl fmt::Display for FanMode { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::Auto => write!(f, "auto"), + Self::Pwm => write!(f, "pwm"), + Self::Rpm => write!(f, "rpm"), + } + } +} + +impl TryFrom for FanMode { + type Error = Error; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(Self::Auto), + 1 => Ok(Self::Pwm), + 2 => Ok(Self::Rpm), + _ => Err(Error::Verify), + } + } +} + /// Run EC commands using a provided access method pub struct Ec { access: A, @@ -275,6 +314,19 @@ impl Ec { self.command(Cmd::SecuritySet, &mut data) } + /// Get fan control mode. + pub unsafe fn fan_get_mode(&mut self) -> Result { + let mut data = [0]; + self.command(Cmd::FanGetMode, &mut data)?; + FanMode::try_from(data[0]) + } + + /// Set fan control mode. + pub unsafe fn fan_set_mode(&mut self, mode: FanMode) -> Result<(), Error> { + let mut data = [mode as u8]; + self.command(Cmd::FanSetMode, &mut data) + } + pub fn into_dyn(self) -> Ec> where A: 'static, diff --git a/tool/src/lib.rs b/tool/src/lib.rs index 72bd0df64..8b999bfdf 100644 --- a/tool/src/lib.rs +++ b/tool/src/lib.rs @@ -24,7 +24,7 @@ extern crate alloc; pub use self::access::*; mod access; -pub use self::ec::{Ec, SecurityState}; +pub use self::ec::{Ec, FanMode, SecurityState}; mod ec; pub use self::error::Error; diff --git a/tool/src/main.rs b/tool/src/main.rs index 8c3555dca..88e6af3af 100644 --- a/tool/src/main.rs +++ b/tool/src/main.rs @@ -281,6 +281,17 @@ unsafe fn fan_set_pwm(ec: &mut Ec>, index: u8, duty: u8) -> Resu ec.fan_set_pwm(index, duty) } +unsafe fn fan_get_mode(ec: &mut Ec>) -> Result<(), Error> { + let mode = ec.fan_get_mode()?; + println!("{}", mode); + + Ok(()) +} + +unsafe fn fan_set_mode(ec: &mut Ec>, mode: ectool::FanMode) -> Result<(), Error> { + ec.fan_set_mode(mode) +} + unsafe fn keymap_get( ec: &mut Ec>, layer: u8, @@ -334,6 +345,9 @@ enum SubCommand { index: u8, duty: Option, }, + FanMode { + mode: Option, + }, Flash { path: String, }, @@ -448,6 +462,22 @@ fn main() { process::exit(1); } }, + SubCommand::FanMode { mode } => match mode { + Some(mode) => match unsafe { fan_set_mode(&mut ec, mode) } { + Ok(()) => (), + Err(err) => { + eprintln!("failed to set fan mode {}: {:X?}", mode, err); + process::exit(1); + } + }, + None => match unsafe { fan_get_mode(&mut ec) } { + Ok(()) => (), + Err(err) => { + eprintln!("failed to get fan mode: {:X?}", err); + process::exit(1); + } + }, + }, SubCommand::FanPwm { index, duty } => match duty { Some(duty) => match unsafe { fan_set_pwm(&mut ec, index, duty) } { Ok(()) => (),