On Sat Jan 4, 2025 at 2:18 AM AEST, Chalapathi V wrote: > Use a local variable seq_index instead of repeatedly calling > get_seq_index() method and open-code next_sequencer_fsm(). > > Signed-off-by: Chalapathi V <chalapath...@linux.ibm.com> > --- > hw/ssi/pnv_spi.c | 93 +++++++++++++++++++++++++----------------------- > 1 file changed, 48 insertions(+), 45 deletions(-) > > diff --git a/hw/ssi/pnv_spi.c b/hw/ssi/pnv_spi.c > index 63d298980d..87eac666bb 100644 > --- a/hw/ssi/pnv_spi.c > +++ b/hw/ssi/pnv_spi.c > @@ -212,18 +212,6 @@ static void transfer(PnvSpi *s) > fifo8_reset(&s->rx_fifo); > } > > -static inline uint8_t get_seq_index(PnvSpi *s) > -{ > - return GETFIELD(SPI_STS_SEQ_INDEX, s->status); > -} > - > -static inline void next_sequencer_fsm(PnvSpi *s) > -{ > - uint8_t seq_index = get_seq_index(s); > - s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status, (seq_index + 1)); > - s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, > SEQ_STATE_INDEX_INCREMENT); > -} > - > /* > * Calculate the N1 counters based on passed in opcode and > * internal register values. > @@ -637,6 +625,7 @@ static void operation_sequencer(PnvSpi *s) > bool stop = false; /* Flag to stop the sequencer */ > uint8_t opcode = 0; > uint8_t masked_opcode = 0; > + uint8_t seq_index; > > /* > * Clear the sequencer FSM error bit - general_SPI_status[3] > @@ -650,12 +639,13 @@ static void operation_sequencer(PnvSpi *s) > if (GETFIELD(SPI_STS_SEQ_FSM, s->status) == SEQ_STATE_IDLE) { > s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status, 0); > } > + seq_index = GETFIELD(SPI_STS_SEQ_INDEX, s->status);
You could add a comment that this field is kept in seq_index and not updated in place until the end of the function. Otherwise looks good. Reviewed-by: Nicholas Piggin <npig...@gmail.com>