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.

Or am I missing something? thanks,

 - Joel



Reply via email to