Skip to content

Commit

Permalink
thermal: move Max31790State into control
Browse files Browse the repository at this point in the history
Seemed nicer to have it be in the `control` module. I also added some
comments.
  • Loading branch information
hawkw committed May 21, 2024
1 parent e93a640 commit 53fe03f
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 83 deletions.
12 changes: 5 additions & 7 deletions task/thermal/src/bsp/gimlet_bcdef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@

use crate::{
control::{
ChannelType, Device, FanControl, Fans, InputChannel, PidConfig,
TemperatureSensor,
ChannelType, ControllerInitError, Device, FanControl, Fans,
InputChannel, Max31790State, PidConfig, TemperatureSensor,
},
i2c_config::{devices, sensors},
ControllerInitError,
};
pub use drv_gimlet_seq_api::SeqError;
use drv_gimlet_seq_api::{PowerState, Sequencer};
Expand Down Expand Up @@ -63,7 +62,7 @@ pub(crate) struct Bsp {
pub misc_sensors: &'static [TemperatureSensor],

/// Fan control IC
fctrl: crate::Max31790State,
fctrl: Max31790State,

/// Handle to the sequencer task, to query power state
seq: Sequencer,
Expand Down Expand Up @@ -167,9 +166,8 @@ impl Bsp {

pub fn new(i2c_task: TaskId) -> Self {
// Initializes and build a handle to the fan controller IC
let fctrl = crate::Max31790State::new(Max31790::new(
&devices::max31790(i2c_task)[0],
));
let fctrl =
Max31790State::new(Max31790::new(&devices::max31790(i2c_task)[0]));

// Handle for the sequencer task, which we check for power state
let seq = Sequencer::from(SEQ.get_task_id());
Expand Down
17 changes: 7 additions & 10 deletions task/thermal/src/bsp/sidecar_bcd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@

//! BSP for Sidecar

use crate::{
control::{
ChannelType, Device, FanControl, Fans, InputChannel, PidConfig,
TemperatureSensor,
},
ControllerInitError,
use crate::control::{
ChannelType, ControllerInitError, Device, FanControl, Fans, InputChannel,
Max31790State, PidConfig, TemperatureSensor,
};
use drv_i2c_devices::max31790::Max31790;
use drv_i2c_devices::tmp451::*;
Expand Down Expand Up @@ -68,8 +65,8 @@ pub(crate) struct Bsp {
pub misc_sensors: &'static [TemperatureSensor],

/// Our two fan controllers: east for 0/1 and west for 1/2
fctrl_east: crate::Max31790State,
fctrl_west: crate::Max31790State,
fctrl_east: Max31790State,
fctrl_west: Max31790State,

seq: Sequencer,

Expand Down Expand Up @@ -177,10 +174,10 @@ impl Bsp {
// fan presence
let seq = Sequencer::from(SEQUENCER.get_task_id());

let fctrl_east = crate::Max31790State::new(Max31790::new(
let fctrl_east = Max31790State::new(Max31790::new(
&devices::max31790_east(i2c_task),
));
let fctrl_west = crate::Max31790State::new(Max31790::new(
let fctrl_west = Max31790State::new(Max31790::new(
&devices::max31790_west(i2c_task),
));

Expand Down
78 changes: 78 additions & 0 deletions task/thermal/src/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,84 @@ impl ThermalSensorErrors {

////////////////////////////////////////////////////////////////////////////////

/// Tracks whether a MAX31790 fan controller has been initialized, and
/// initializes it on demand when accessed, if necessary.
///
/// Because initializing the fan controller can fail due to a transient bus
/// error, we don't panic if an initial attempt to initialize it as soon as the
/// `thermal` task starts fails. Because the fan controller's I2C watchdog will
/// simply run the fans at 100% if we aren't able to talk to it right away, the
/// `thermal` task should keep running, publishing sensor measurements, and
/// periodically trying to reach the fan controller until we're able to
/// initialize it successfully. Thus, we wrap it in this struct to track whether
/// it's been successfully initialized yet.
pub(crate) struct Max31790State {
max31790: Max31790,
initialized: bool,
}

impl Max31790State {
pub(crate) fn new(max31790: Max31790) -> Self {
let mut this = Self {
max31790,
initialized: false,
};
// When we first start up, try to initialize the fan controller a few
// times, in case there's a transient I2C error.
for remaining in (0..3).rev() {
if this.initialize().is_ok() {
break;
}
ringbuf_entry!(Trace::FanControllerInitRetry { remaining });
}
this
}

/// Access the fan controller, attempting to initialize it if it has not yet
/// been initialized.
#[inline]
pub(crate) fn try_initialize(
&mut self,
) -> Result<&mut Max31790, ControllerInitError> {
if self.initialized {
return Ok(&mut self.max31790);
}

self.initialize()
}

// Slow path that actually performs initialization. This is "outlined" so
// that we can avoid pushing a stack frame in the case where we just need to
// check a bool and return a pointer.
#[inline(never)]
fn initialize(&mut self) -> Result<&mut Max31790, ControllerInitError> {
self.max31790.initialize().map_err(|e| {
ringbuf_entry!(Trace::FanControllerInitError(e));
ControllerInitError(e)
})?;

self.initialized = true;
ringbuf_entry!(Trace::FanControllerInitialized);
Ok(&mut self.max31790)
}
}

pub(crate) struct ControllerInitError(ResponseCode);

impl From<ControllerInitError> for ThermalError {
fn from(_: ControllerInitError) -> Self {
ThermalError::FanControllerUninitialized
}
}

impl From<ControllerInitError> for SensorReadError {
fn from(ControllerInitError(code): ControllerInitError) -> Self {
SensorReadError::I2cError(code)
}
}

////////////////////////////////////////////////////////////////////////////////

/// The thermal control loop.
///
/// This object uses slices of sensors and fans, which must be owned
Expand Down
67 changes: 1 addition & 66 deletions task/thermal/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::{
control::ThermalControl,
};
use drv_i2c_api::ResponseCode;
use drv_i2c_devices::max31790::{I2cWatchdog, Max31790};
use drv_i2c_devices::max31790::I2cWatchdog;
use idol_runtime::{NotificationHandler, RequestError};
use ringbuf::*;
use task_sensor_api::{Sensor as SensorApi, SensorId};
Expand Down Expand Up @@ -93,71 +93,6 @@ counted_ringbuf!(Trace, 32, Trace::None);

////////////////////////////////////////////////////////////////////////////////

pub(crate) struct Max31790State {
max31790: Max31790,
initialized: bool,
}

impl Max31790State {
pub(crate) fn new(max31790: Max31790) -> Self {
let mut this = Self {
max31790,
initialized: false,
};
// When we first start up, try to initialize the fan controller a few
// times, in case there's a transient I2C error.
for remaining in (0..3).rev() {
if this.initialize().is_ok() {
break;
}
ringbuf_entry!(Trace::FanControllerInitRetry { remaining });
}
this
}

/// Access the fan controller, attempting to initialize it if it has not yet
/// been initialized.
#[inline]
pub(crate) fn try_initialize(
&mut self,
) -> Result<&mut Max31790, ControllerInitError> {
if self.initialized {
return Ok(&mut self.max31790);
}

self.initialize()
}

// Slow path that actually performs initialization. This is "outlined" so
// that we can avoid pushing a stack frame in the case where we just need to
// check a bool and return a pointer.
#[inline(never)]
fn initialize(&mut self) -> Result<&mut Max31790, ControllerInitError> {
self.max31790.initialize().map_err(|e| {
ringbuf_entry!(Trace::FanControllerInitError(e));
ControllerInitError(e)
})?;

self.initialized = true;
ringbuf_entry!(Trace::FanControllerInitialized);
Ok(&mut self.max31790)
}
}

pub(crate) struct ControllerInitError(ResponseCode);

impl From<ControllerInitError> for ThermalError {
fn from(_: ControllerInitError) -> Self {
ThermalError::FanControllerUninitialized
}
}

impl From<ControllerInitError> for SensorReadError {
fn from(ControllerInitError(code): ControllerInitError) -> Self {
SensorReadError::I2cError(code)
}
}

struct ServerImpl<'a> {
mode: ThermalMode,
control: ThermalControl<'a>,
Expand Down

0 comments on commit 53fe03f

Please sign in to comment.