> On Nov 11, 2025, at 8:12 PM, Alexandre Courbot <[email protected]> wrote: > > On Wed Nov 12, 2025 at 7:06 AM JST, Joel Fernandes wrote: >> >> >> On 11/10/2025 8:39 AM, Alexandre Courbot wrote: >>>> + // GSP sequencer delay payload structure. >>>> + GSP_SEQ_BUF_PAYLOAD_DELAY_US, >>>> + >>>> + // GSP sequencer register payload structures. >>>> + GSP_SEQ_BUF_PAYLOAD_REG_MODIFY, >>>> + GSP_SEQ_BUF_PAYLOAD_REG_POLL, >>>> + GSP_SEQ_BUF_PAYLOAD_REG_STORE, >>>> + GSP_SEQ_BUF_PAYLOAD_REG_WRITE, // >>>> These ones are a bit trickier to abstract. Since they ever only use >>> `bar` from the sequencer, I guess we can have their semantics in the >>> `fw` module, exposed through a method that receives the `bar`? That way >>> the sequencer won't have to access their members which are private to >>> it. >> >> The sequencer does need access to the private fields, because the logic of >> what >> to write to the bar should be in the sequencer, and that logic depends on the >> fields. >> >> Example: >> >> impl GspSeqCmdRunner for fw::GSP_SEQ_BUF_PAYLOAD_REG_MODIFY { >> fn run(&self, sequencer: &GspSequencer<'_>) -> Result { >> let addr = self.addr as usize; >> if let Ok(temp) = sequencer.bar.try_read32(addr) { >> let _ = sequencer >> .bar >> .try_write32((temp & !self.mask) | self.val, addr); >> } >> Ok(()) >> } >> } >> >> Here, the sequencer needs access to `.addr`, `.mask` and `.val` to craft the >> address and the value to write. >> >> I could expose access to those fields as functions, but I think we should not >> move sequencer logic to fw.rs, that should live in the sequencer. > > Yeah although I floated the idea I have to admit I am not a big fan of > that either. So I guess we could have accessor functions for the fields, > so the `GspSeqCmdRunner` implementation stays in the sequencer? > > It will at least provide the level of abstraction we require against the > firmware types' internal structure.
Yes, I will provide accessors for the fields for v4. Thanks! - Joel
