Re: [PATCH v5 2/4] hw/ssi/pnv_spi: Coverity CID 1558827: Use local var seq_index instead of get_seq_index().

["Nicholas Piggin" <npiggin@gmail.com>]

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 <chalapathi.v@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 <npiggin@gmail.com>