Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set host flash Mux back to SP once host is booted #1575

Merged
merged 1 commit into from
Dec 12, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 43 additions & 6 deletions task/host-sp-comms/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#[cfg(any(feature = "stm32h743", feature = "stm32h753"))]
use drv_stm32h7_usart as drv_usart;

use drv_gimlet_hf_api::{HfDevSelect, HostFlash};
use drv_gimlet_hf_api::{HfDevSelect, HfMuxState, HostFlash};
use drv_gimlet_seq_api::{PowerState, SeqError, Sequencer};
use drv_stm32xx_sys_api as sys_api;
use drv_usart::Usart;
Expand Down Expand Up @@ -97,6 +97,10 @@ enum Trace {
now: u64,
state: PowerState,
},
HfMux {
now: u64,
state: Option<HfMuxState>,
},
JefeNotification {
now: u64,
state: PowerState,
Expand All @@ -118,7 +122,7 @@ enum Trace {
},
}

ringbuf!(Trace, 16, Trace::None);
ringbuf!(Trace, 20, Trace::None);

#[derive(Debug, Clone, Copy, PartialEq)]
enum TimerDisposition {
Expand Down Expand Up @@ -252,6 +256,7 @@ struct ServerImpl {
packrat: Packrat,
reboot_state: Option<RebootState>,
host_kv_storage: HostKeyValueStorage,
hf_mux_state: Option<HfMuxState>,
}

impl ServerImpl {
Expand Down Expand Up @@ -283,6 +288,7 @@ impl ServerImpl {
packrat: Packrat::from(PACKRAT.get_task_id()),
reboot_state: None,
host_kv_storage: HostKeyValueStorage::claim_static_resources(),
hf_mux_state: None,
}
}

Expand All @@ -299,6 +305,23 @@ impl ServerImpl {
}
}

fn update_hf_mux_state(&mut self) {
self.hf_mux_state = self.hf.get_mux().ok();
ringbuf_entry!(Trace::HfMux {
now: sys_get_timer().now,
state: self.hf_mux_state,
});
}

fn set_hf_mux_to_sp(&mut self) {
if self.hf_mux_state != Some(HfMuxState::SP) {
// This can only fail if the `hf` task panics, in which case the
// MUX will be set to the SP when it restarts.
let _ = self.hf.set_mux(HfMuxState::SP);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to ask what we could or should do if this fails (e.g., should we retry setting the mux to the SP on every valid IPCC message), but it looks like this can't actually fail unless the hf task crashes outright. If hf crashes and restarts, do we know what happens with the mux?

Copy link
Contributor Author

@citrus-it citrus-it Dec 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If hf crashes and restarts, the mux will be set to the SP - that's its default disposition and it explicitly resets the pin. Testing bears this out. I've added a comment, thanks.

self.update_hf_mux_state();
}
}

/// Power off the host (i.e., transition to A2).
///
/// If `reboot` is true and we successfully instruct the sequencer to
Expand Down Expand Up @@ -384,6 +407,7 @@ impl ServerImpl {
fn handle_jefe_notification(&mut self, state: PowerState) {
let now = sys_get_timer().now;
ringbuf_entry!(Trace::JefeNotification { now, state });
self.update_hf_mux_state();
// If we're rebooting and jefe has notified us that we're now in A2,
// move to A0. Otherwise, ignore this notification.
match state {
Expand Down Expand Up @@ -770,10 +794,21 @@ impl ServerImpl {
}
Some(SpToHost::Ack)
}
HostToSp::GetStatus => Some(SpToHost::Status {
status: self.status,
startup: self.packrat.get_next_boot_host_startup_options(),
}),
HostToSp::GetStatus => {
// This status request is the first IPCC command that the OS
// kernel sends once it has started. When we receive it, we
// know that we're far enough along that the host no longer
// needs access to the flash.
// Set the mux back to the SP to remove the host's access,
// which has the added benefit of enabling host flash updates
// while the host OS is running.
action = Some(Action::HfMuxToSP);

Some(SpToHost::Status {
status: self.status,
startup: self.packrat.get_next_boot_host_startup_options(),
})
}
HostToSp::AckSpStart => {
action =
Some(Action::ClearStatusBits(Status::SP_TASK_RESTARTED));
Expand Down Expand Up @@ -858,6 +893,7 @@ impl ServerImpl {
Action::ClearStatusBits(to_clear) => {
self.set_status_impl(self.status.difference(to_clear))
}
Action::HfMuxToSP => self.set_hf_mux_to_sp(),
}
}

Expand Down Expand Up @@ -1205,6 +1241,7 @@ enum Action {
RebootHost,
PowerOffHost,
ClearStatusBits(Status),
HfMuxToSP,
}

#[cfg(any(feature = "stm32h743", feature = "stm32h753"))]
Expand Down