don't have IPC methods call each other

oxidecomputer · Dec 18, 2024 · 0939d49 · 0939d49
1 parent 1088947
commit 0939d49
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 31 deletions.
diff --git a/drv/gimlet-seq-server/src/main.rs b/drv/gimlet-seq-server/src/main.rs
@@ -1046,10 +1046,11 @@ impl<S: SpiServer> idl::InOrderSequencerImpl for ServerImpl<S> {
 
     fn set_state(
         &mut self,
-        msg: &RecvMessage,
+        _: &RecvMessage,
         state: PowerState,
     ) -> Result<(), RequestError<SeqError>> {
-        self.set_state_with_reason(msg, state, StateChangeReason::Other)
+        self.set_state_internal(state, reason)
+            .map_err(RequestError::from)
     }
 
     fn set_state_with_reason(

diff --git a/drv/grapefruit-seq-server/src/main.rs b/drv/grapefruit-seq-server/src/main.rs
@@ -269,6 +269,17 @@ impl<S: SpiServer + Clone> ServerImpl<S> {
     fn set_state_impl(&self, state: PowerState) {
         self.jefe.set_state(state as u32);
     }
+
+    fn validate_state_change(&self, state: PowerState) -> Result<(), SeqError> {
+        match (self.get_state_impl(), state) {
+            (PowerState::A2, PowerState::A0)
+            | (PowerState::A0, PowerState::A2)
+            | (PowerState::A0PlusHP, PowerState::A2)
+            | (PowerState::A0Thermtrip, PowerState::A2) => Ok(()),
+
+            _ => Err(SeqError::IllegalTransition),
+        }
+    }
 }
 
 // The `Sequencer` implementation for Grapefruit is copied from
@@ -286,29 +297,21 @@ impl<S: SpiServer + Clone> idl::InOrderSequencerImpl for ServerImpl<S> {
         &mut self,
         msg: &RecvMessage,
         state: PowerState,
-    ) -> Result<(), RequestError<drv_cpu_seq_api::SeqError>> {
-        self.set_state_with_reason(msg, state, StateChangeReason::Other)
+    ) -> Result<(), RequestError<SeqError>> {
+        self.validate_state_change(state)?;
+        self.set_state_impl(state);
+        Ok(())
     }
 
     fn set_state_with_reason(
         &mut self,
         _: &RecvMessage,
         state: PowerState,
         _: StateChangeReason,
-    ) -> Result<(), RequestError<drv_cpu_seq_api::SeqError>> {
-        match (self.get_state_impl(), state) {
-            (PowerState::A2, PowerState::A0)
-            | (PowerState::A0, PowerState::A2)
-            | (PowerState::A0PlusHP, PowerState::A2)
-            | (PowerState::A0Thermtrip, PowerState::A2) => {
-                self.set_state_impl(state);
-                Ok(())
-            }
-
-            _ => Err(RequestError::Runtime(
-                drv_cpu_seq_api::SeqError::IllegalTransition,
-            )),
-        }
+    ) -> Result<(), RequestError<SeqError>> {
+        self.validate_state_change(state)?;
+        self.set_state_impl(state);
+        Ok(())
     }
 
     fn send_hardware_nmi(

diff --git a/drv/mock-gimlet-seq-server/src/main.rs b/drv/mock-gimlet-seq-server/src/main.rs
@@ -44,6 +44,17 @@ impl ServerImpl {
     fn set_state_impl(&self, state: PowerState) {
         self.jefe.set_state(state as u32);
     }
+
+    fn validate_state_change(&self, state: PowerState) -> Result<(), SeqError> {
+        match (self.get_state_impl(), state) {
+            (PowerState::A2, PowerState::A0)
+            | (PowerState::A0, PowerState::A2)
+            | (PowerState::A0PlusHP, PowerState::A2)
+            | (PowerState::A0Thermtrip, PowerState::A2) => Ok(()),
+
+            _ => Err(SeqError::IllegalTransition),
+        }
+    }
 }
 
 impl idl::InOrderSequencerImpl for ServerImpl {
@@ -56,10 +67,12 @@ impl idl::InOrderSequencerImpl for ServerImpl {
 
     fn set_state(
         &mut self,
-        msg: &RecvMessage,
+        _: &RecvMessage,
         state: PowerState,
     ) -> Result<(), RequestError<SeqError>> {
-        self.set_state_with_reason(msg, state, StateChangeReason::Other)
+        self.validate_state_change(state)?;
+        self.set_state_impl(state);
+        Ok(())
     }
 
     fn set_state_with_reason(
@@ -68,17 +81,9 @@ impl idl::InOrderSequencerImpl for ServerImpl {
         state: PowerState,
         _: StateChangeReason,
     ) -> Result<(), RequestError<SeqError>> {
-        match (self.get_state_impl(), state) {
-            (PowerState::A2, PowerState::A0)
-            | (PowerState::A0, PowerState::A2)
-            | (PowerState::A0PlusHP, PowerState::A2)
-            | (PowerState::A0Thermtrip, PowerState::A2) => {
-                self.set_state_impl(state);
-                Ok(())
-            }
-
-            _ => Err(RequestError::Runtime(SeqError::IllegalTransition)),
-        }
+        self.validate_state_change(state)?;
+        self.set_state_impl(state);
+        Ok(())
     }
 
     fn send_hardware_nmi(