From 18f43226d24e664b66a4ff5d6753e1c2619e64be Mon Sep 17 00:00:00 2001 From: Yuan Yao Date: Wed, 9 Oct 2024 03:19:14 +0000 Subject: [PATCH] devices: bat: get power property before first read Previously, before powerd monitor is set up and receives first broadcast signal from powerd, the read() function will return default initial power property, which does not reflect the real host battery. This Cl makes goldfish battery device request to get power property before first time read. It is achieved by sending GetPowerSupplyProperties dbus request to powerd. BUG=b:361281568 TEST=deploy to DUT & test Change-Id: I5044ded5efc744525dfe87fe81370f202f0a43fb Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5918906 Commit-Queue: Yuan Yao Reviewed-by: Keiichi Watanabe --- arch/src/sys/linux.rs | 13 ++++++--- devices/src/bat.rs | 62 ++++++++++++++++++++++++++++++++++++++++ power_monitor/src/lib.rs | 15 ++++++++-- 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/arch/src/sys/linux.rs b/arch/src/sys/linux.rs index 13960546cc..d7dc5ad3e7 100644 --- a/arch/src/sys/linux.rs +++ b/arch/src/sys/linux.rs @@ -59,13 +59,17 @@ pub fn add_goldfish_battery( Tube::pair().map_err(DeviceRegistrationError::CreateTube)?; #[cfg(feature = "power-monitor-powerd")] - let create_monitor = Some( - Box::new(power_monitor::powerd::monitor::DBusMonitor::connect) - as Box, + let (create_monitor, create_client) = ( + Some( + Box::new(power_monitor::powerd::monitor::DBusMonitor::connect) + as Box, + ), + Some(Box::new(power_monitor::powerd::client::DBusClient::connect) + as Box), ); #[cfg(not(feature = "power-monitor-powerd"))] - let create_monitor = None; + let (create_monitor, create_client) = (None, None); let irq_evt = devices::IrqLevelEvent::new().map_err(DeviceRegistrationError::EventCreate)?; @@ -77,6 +81,7 @@ pub fn add_goldfish_battery( .map_err(DeviceRegistrationError::EventClone)?, response_tube, create_monitor, + create_client, ) .map_err(DeviceRegistrationError::RegisterBattery)?; goldfish_bat.to_aml_bytes(amls); diff --git a/devices/src/bat.rs b/devices/src/bat.rs index 0deaaa1a0a..ab81a728a5 100644 --- a/devices/src/bat.rs +++ b/devices/src/bat.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use acpi_tables::aml; use acpi_tables::aml::Aml; +use anyhow::bail; use anyhow::Context; use base::error; use base::warn; @@ -17,6 +18,7 @@ use base::Tube; use base::WaitContext; use base::WorkerThread; use power_monitor::BatteryStatus; +use power_monitor::CreatePowerClientFn; use power_monitor::CreatePowerMonitorFn; use remain::sorted; use serde::Deserialize; @@ -62,6 +64,7 @@ struct GoldfishBatteryState { current: u32, charge_counter: u32, charge_full: u32, + initialized: bool, } macro_rules! create_battery_func { @@ -117,6 +120,7 @@ pub struct GoldfishBattery { monitor_thread: Option>, tube: Option, create_power_monitor: Option>, + create_powerd_client: Option>, } #[derive(Serialize, Deserialize)] @@ -323,6 +327,7 @@ impl GoldfishBattery { irq_evt: IrqLevelEvent, tube: Tube, create_power_monitor: Option>, + create_powerd_client: Option>, ) -> Result { if mmio_base + GOLDFISHBAT_MMIO_LEN - 1 > u32::MAX as u64 { return Err(BatteryError::Non32BitMmioAddress); @@ -339,6 +344,7 @@ impl GoldfishBattery { current: 0, charge_counter: 0, charge_full: 0, + initialized: false, })); Ok(GoldfishBattery { @@ -350,6 +356,7 @@ impl GoldfishBattery { monitor_thread: None, tube: Some(tube), create_power_monitor, + create_powerd_client, }) } @@ -383,6 +390,46 @@ impl GoldfishBattery { self.activated = true; } } + + fn initialize_battery_state(&mut self) -> anyhow::Result<()> { + let mut power_client = match &self.create_powerd_client { + Some(f) => match f() { + Ok(c) => c, + Err(e) => bail!("failed to connect to the powerd: {:#}", e), + }, + None => return Ok(()), + }; + match power_client.get_power_data() { + Ok(data) => { + let mut bat_state = self.state.lock(); + bat_state.set_ac_online(data.ac_online.into()); + + match data.battery { + Some(battery_data) => { + bat_state.set_capacity(battery_data.percent); + let battery_status = match battery_data.status { + BatteryStatus::Unknown => BATTERY_STATUS_VAL_UNKNOWN, + BatteryStatus::Charging => BATTERY_STATUS_VAL_CHARGING, + BatteryStatus::Discharging => BATTERY_STATUS_VAL_DISCHARGING, + BatteryStatus::NotCharging => BATTERY_STATUS_VAL_NOT_CHARGING, + }; + bat_state.set_status(battery_status); + bat_state.set_voltage(battery_data.voltage); + bat_state.set_current(battery_data.current); + bat_state.set_charge_counter(battery_data.charge_counter); + bat_state.set_charge_full(battery_data.charge_full); + } + None => { + bat_state.set_present(0); + } + } + Ok(()) + } + Err(e) => { + bail!("failed to get response from powerd: {:#}", e); + } + } + } } impl Drop for GoldfishBattery { @@ -412,6 +459,20 @@ impl BusDevice for GoldfishBattery { return; } + // Before first read, we try to ask powerd the actual power data to initialize `self.state`. + if !self.state.lock().initialized { + match self.initialize_battery_state() { + Ok(()) => self.state.lock().initialized = true, + Err(e) => { + error!( + "{}: failed to get power data and update: {:#}", + self.debug_label(), + e + ); + } + } + } + let val = match info.offset as u32 { BATTERY_INT_STATUS => { // read to clear the interrupt status @@ -547,6 +608,7 @@ mod tests { IrqLevelEvent::new().unwrap(), Tube::pair().unwrap().1, None, + None, ).unwrap(), modify_device } diff --git a/power_monitor/src/lib.rs b/power_monitor/src/lib.rs index a4c44fab1f..8cd7fe0c4e 100644 --- a/power_monitor/src/lib.rs +++ b/power_monitor/src/lib.rs @@ -16,12 +16,13 @@ pub trait PowerClient { fn get_power_data(&mut self) -> std::result::Result>; } +#[derive(Debug)] pub struct PowerData { pub ac_online: bool, pub battery: Option, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub struct BatteryData { pub status: BatteryStatus, pub percent: u32, @@ -35,7 +36,7 @@ pub struct BatteryData { pub charge_full: u32, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug)] pub enum BatteryStatus { Unknown, Charging, @@ -53,6 +54,16 @@ impl CreatePowerMonitorFn for T where { } +pub trait CreatePowerClientFn: + Send + Fn() -> std::result::Result, Box> +{ +} + +impl CreatePowerClientFn for T where + T: Send + Fn() -> std::result::Result, Box> +{ +} + #[cfg(feature = "powerd")] pub mod powerd;