On Thu Mar 5, 2026 at 10:34 AM JST, Eliot Courtney wrote:
> On Wed Mar 4, 2026 at 8:56 PM JST, Alexandre Courbot wrote:
>>> + /// Sends `command` to the GSP and waits for the reply.
>>> + ///
>>> + /// # Errors
>>> + ///
>>> + /// - `ETIMEDOUT` if space does not become available to send the
>>> command, or if the reply is
>>> + /// not received within the timeout.
>>> + /// - `EIO` if the variable payload requested by the command has not
>>> been entirely
>>> + /// written to by its [`CommandToGsp::init_variable_payload`] method.
>>> + ///
>>> + /// Error codes returned by the command and reply initializers are
>>> propagated as-is.
>>> + pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) ->
>>> Result<M::Reply>
>>> + where
>>> + M: CommandToGsp,
>>> + M::Reply: MessageFromGsp,
>>> + Error: From<M::InitError>,
>>> + Error: From<<M::Reply as MessageFromGsp>::InitError>,
>>> + {
>>> + self.send_command_internal(bar, command)?;
>>> +
>>> + loop {
>>> + match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
>>> + Ok(reply) => break Ok(reply),
>>> + Err(ERANGE) => continue,
>>> + Err(e) => break Err(e),
>>> + }
>>> + }
>>
>> There is an opportunity for factorize some more code here.
>>
>> Notice how the other callers of `receive_msg` (`wait_gsp_init_done` and
>> `GspSequencer::run`) both use the same kind of loop, down to the same
>> error handling. We could move that loop logic here and do it in a single
>> place.
>>
>> In the future, we will probably want to add handlers for
>> unexpected messages from the GSP and it will be easier if we receive all
>> messages from a single place.
>>
>> This can be a separate patch from this one, but I think it makes sense
>> to have that in this series.
>>
>> I expect the last patch to change a bit as a consequence of that - maybe
>> we will need a `receive_msg_loop` or something in `CmdqInner`.
>
> I agree we should migrate all callers and make Cmdq responsible for
> draining / handling spontaneous messages from the GSP, but I was
> planning on doing it in a separate patch series until now. I can put it
> into this one though if you want though no worries.
If it ends up being convulated, let's do that afterwards but since it
looks like a quick and easy win I thought it would make sense to have it
here. Your call though.
