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 <yuanyaogoog@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
This commit is contained in:
Yuan Yao 2024-10-09 03:19:14 +00:00 committed by crosvm LUCI
parent f36fc40bbe
commit 18f43226d2
3 changed files with 84 additions and 6 deletions

View file

@ -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<dyn power_monitor::CreatePowerMonitorFn>,
let (create_monitor, create_client) = (
Some(
Box::new(power_monitor::powerd::monitor::DBusMonitor::connect)
as Box<dyn power_monitor::CreatePowerMonitorFn>,
),
Some(Box::new(power_monitor::powerd::client::DBusClient::connect)
as Box<dyn power_monitor::CreatePowerClientFn>),
);
#[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);

View file

@ -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<WorkerThread<()>>,
tube: Option<Tube>,
create_power_monitor: Option<Box<dyn CreatePowerMonitorFn>>,
create_powerd_client: Option<Box<dyn CreatePowerClientFn>>,
}
#[derive(Serialize, Deserialize)]
@ -323,6 +327,7 @@ impl GoldfishBattery {
irq_evt: IrqLevelEvent,
tube: Tube,
create_power_monitor: Option<Box<dyn CreatePowerMonitorFn>>,
create_powerd_client: Option<Box<dyn CreatePowerClientFn>>,
) -> Result<Self> {
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
}

View file

@ -16,12 +16,13 @@ pub trait PowerClient {
fn get_power_data(&mut self) -> std::result::Result<PowerData, Box<dyn Error>>;
}
#[derive(Debug)]
pub struct PowerData {
pub ac_online: bool,
pub battery: Option<BatteryData>,
}
#[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<T> CreatePowerMonitorFn for T where
{
}
pub trait CreatePowerClientFn:
Send + Fn() -> std::result::Result<Box<dyn PowerClient>, Box<dyn Error>>
{
}
impl<T> CreatePowerClientFn for T where
T: Send + Fn() -> std::result::Result<Box<dyn PowerClient>, Box<dyn Error>>
{
}
#[cfg(feature = "powerd")]
pub mod powerd;