From: Chalapathi V <chalapathi.v@linux.ibm.com>
To: qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org, fbarrat@linux.ibm.com, npiggin@gmail.com,
clg@kaod.org, calebs@linux.ibm.com, chalapathi.v@ibm.com,
chalapathi.v@linux.ibm.com, saif.abrar@linux.ibm.com,
dantan@linux.vnet.ibm.com, milesg@linux.ibm.com,
philmd@linaro.org, alistair@alistair23.me
Subject: [PATCH v5 2/4] hw/ssi/pnv_spi: Coverity CID 1558827: Use local var seq_index instead of get_seq_index().
Date: Fri, 3 Jan 2025 10:18:22 -0600 [thread overview]
Message-ID: <20250103161824.22469-3-chalapathi.v@linux.ibm.com> (raw)
In-Reply-To: <20250103161824.22469-1-chalapathi.v@linux.ibm.com>
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);
/*
* There are only 8 possible operation IDs to iterate through though
* some operations may cause more than one frame to be sequenced.
*/
- while (get_seq_index(s) < NUM_SEQ_OPS) {
- opcode = s->seq_op[get_seq_index(s)];
+ while (seq_index < NUM_SEQ_OPS) {
+ opcode = s->seq_op[seq_index];
/* Set sequencer state to decode */
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_DECODE);
/*
@@ -672,7 +662,7 @@ static void operation_sequencer(PnvSpi *s)
case SEQ_OP_STOP:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
/* A stop operation in any position stops the sequencer */
- trace_pnv_spi_sequencer_op("STOP", get_seq_index(s));
+ trace_pnv_spi_sequencer_op("STOP", seq_index);
stop = true;
s->status = SETFIELD(SPI_STS_SHIFTER_FSM, s->status, FSM_IDLE);
@@ -683,7 +673,7 @@ static void operation_sequencer(PnvSpi *s)
case SEQ_OP_SELECT_SLAVE:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
- trace_pnv_spi_sequencer_op("SELECT_SLAVE", get_seq_index(s));
+ trace_pnv_spi_sequencer_op("SELECT_SLAVE", seq_index);
/*
* This device currently only supports a single responder
* connection at position 0. De-selecting a responder is fine
@@ -694,8 +684,7 @@ static void operation_sequencer(PnvSpi *s)
if (s->responder_select == 0) {
trace_pnv_spi_shifter_done();
qemu_set_irq(s->cs_line[0], 1);
- s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status,
- (get_seq_index(s) + 1));
+ seq_index++;
s->status = SETFIELD(SPI_STS_SHIFTER_FSM, s->status, FSM_DONE);
} else if (s->responder_select != 1) {
qemu_log_mask(LOG_GUEST_ERROR, "Slave selection other than 1 "
@@ -720,13 +709,15 @@ static void operation_sequencer(PnvSpi *s)
* applies once a valid responder select has occurred.
*/
s->shift_n1_done = false;
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status,
+ SEQ_STATE_INDEX_INCREMENT);
}
break;
case SEQ_OP_SHIFT_N1:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
- trace_pnv_spi_sequencer_op("SHIFT_N1", get_seq_index(s));
+ trace_pnv_spi_sequencer_op("SHIFT_N1", seq_index);
/*
* Only allow a shift_n1 when the state is not IDLE or DONE.
* In either of those two cases the sequencer is not in a proper
@@ -758,8 +749,9 @@ static void operation_sequencer(PnvSpi *s)
* transmission to the responder without requiring a refill of
* the TDR between the two operations.
*/
- if (PNV_SPI_MASKED_OPCODE(s->seq_op[get_seq_index(s) + 1])
- == SEQ_OP_SHIFT_N2) {
+ if ((seq_index != 7) &&
+ PNV_SPI_MASKED_OPCODE(s->seq_op[(seq_index + 1)]) ==
+ SEQ_OP_SHIFT_N2) {
send_n1_alone = false;
}
s->status = SETFIELD(SPI_STS_SHIFTER_FSM, s->status, FSM_SHIFT_N1);
@@ -779,9 +771,8 @@ static void operation_sequencer(PnvSpi *s)
if (GETFIELD(SPI_STS_TDR_UNDERRUN, s->status)) {
s->shift_n1_done = true;
s->status = SETFIELD(SPI_STS_SHIFTER_FSM, s->status,
- FSM_SHIFT_N2);
- s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status,
- (get_seq_index(s) + 1));
+ FSM_SHIFT_N2);
+ seq_index++;
} else {
/*
* This is case (1) or (2) so the sequencer needs to
@@ -792,14 +783,16 @@ static void operation_sequencer(PnvSpi *s)
} else {
/* Ok to move on to the next index */
s->shift_n1_done = true;
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status,
+ SEQ_STATE_INDEX_INCREMENT);
}
}
break;
case SEQ_OP_SHIFT_N2:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
- trace_pnv_spi_sequencer_op("SHIFT_N2", get_seq_index(s));
+ trace_pnv_spi_sequencer_op("SHIFT_N2", seq_index);
if (!s->shift_n1_done) {
qemu_log_mask(LOG_GUEST_ERROR, "Shift_N2 is not allowed if a "
"Shift_N1 is not done, shifter state = 0x%llx",
@@ -824,14 +817,16 @@ static void operation_sequencer(PnvSpi *s)
s->status = SETFIELD(SPI_STS_SHIFTER_FSM, s->status, FSM_WAIT);
} else {
/* Ok to move on to the next index */
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status,
+ SEQ_STATE_INDEX_INCREMENT);
}
}
break;
case SEQ_OP_BRANCH_IFNEQ_RDR:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
- trace_pnv_spi_sequencer_op("BRANCH_IFNEQ_RDR", get_seq_index(s));
+ trace_pnv_spi_sequencer_op("BRANCH_IFNEQ_RDR", seq_index);
/*
* The memory mapping register RDR match value is compared against
* the 16 rightmost bytes of the RDR (potentially with masking).
@@ -847,15 +842,16 @@ static void operation_sequencer(PnvSpi *s)
if (rdr_matched) {
trace_pnv_spi_RDR_match("success");
/* A match occurred, increment the sequencer index. */
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status,
+ SEQ_STATE_INDEX_INCREMENT);
} else {
trace_pnv_spi_RDR_match("failed");
/*
* Branch the sequencer to the index coded into the op
* code.
*/
- s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status,
- PNV_SPI_OPCODE_LO_NIBBLE(opcode));
+ seq_index = PNV_SPI_OPCODE_LO_NIBBLE(opcode);
}
/*
* Regardless of where the branch ended up we want the
@@ -874,12 +870,13 @@ static void operation_sequencer(PnvSpi *s)
case SEQ_OP_TRANSFER_TDR:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
qemu_log_mask(LOG_GUEST_ERROR, "Transfer TDR is not supported\n");
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_INDEX_INCREMENT);
break;
case SEQ_OP_BRANCH_IFNEQ_INC_1:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
- trace_pnv_spi_sequencer_op("BRANCH_IFNEQ_INC_1", get_seq_index(s));
+ trace_pnv_spi_sequencer_op("BRANCH_IFNEQ_INC_1", seq_index);
/*
* The spec says the loop should execute count compare + 1 times.
* However we learned from engineering that we really only loop
@@ -893,19 +890,21 @@ static void operation_sequencer(PnvSpi *s)
* mask off all but the first three bits so we don't try to
* access beyond the sequencer_operation_reg boundary.
*/
- s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status,
- PNV_SPI_OPCODE_LO_NIBBLE(opcode));
+ seq_index = PNV_SPI_OPCODE_LO_NIBBLE(opcode);
s->loop_counter_1++;
} else {
/* Continue to next index if loop counter is reached */
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status,
+ SEQ_STATE_INDEX_INCREMENT);
}
break;
case SEQ_OP_BRANCH_IFNEQ_INC_2:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
- trace_pnv_spi_sequencer_op("BRANCH_IFNEQ_INC_2", get_seq_index(s));
- uint8_t condition2 = GETFIELD(SPI_CTR_CFG_CMP2, s->regs[SPI_CTR_CFG_REG]);
+ trace_pnv_spi_sequencer_op("BRANCH_IFNEQ_INC_2", seq_index);
+ uint8_t condition2 = GETFIELD(SPI_CTR_CFG_CMP2,
+ s->regs[SPI_CTR_CFG_REG]);
/*
* The spec says the loop should execute count compare + 1 times.
* However we learned from engineering that we really only loop
@@ -918,19 +917,21 @@ static void operation_sequencer(PnvSpi *s)
* mask off all but the first three bits so we don't try to
* access beyond the sequencer_operation_reg boundary.
*/
- s->status = SETFIELD(SPI_STS_SEQ_INDEX,
- s->status, PNV_SPI_OPCODE_LO_NIBBLE(opcode));
+ seq_index = PNV_SPI_OPCODE_LO_NIBBLE(opcode);
s->loop_counter_2++;
} else {
/* Continue to next index if loop counter is reached */
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status,
+ SEQ_STATE_INDEX_INCREMENT);
}
break;
default:
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_EXECUTE);
/* Ignore unsupported operations. */
- next_sequencer_fsm(s);
+ seq_index++;
+ s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_INDEX_INCREMENT);
break;
} /* end of switch */
/*
@@ -938,10 +939,10 @@ static void operation_sequencer(PnvSpi *s)
* we need to go ahead and end things as if there was a STOP at the
* end.
*/
- if (get_seq_index(s) == NUM_SEQ_OPS) {
+ if (seq_index == NUM_SEQ_OPS) {
/* All 8 opcodes completed, sequencer idling */
s->status = SETFIELD(SPI_STS_SHIFTER_FSM, s->status, FSM_IDLE);
- s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status, 0);
+ seq_index = 0;
s->loop_counter_1 = 0;
s->loop_counter_2 = 0;
s->status = SETFIELD(SPI_STS_SEQ_FSM, s->status, SEQ_STATE_IDLE);
@@ -952,6 +953,8 @@ static void operation_sequencer(PnvSpi *s)
break;
}
} /* end of while */
+ /* Update sequencer index field in status.*/
+ s->status = SETFIELD(SPI_STS_SEQ_INDEX, s->status, seq_index);
return;
} /* end of operation_sequencer() */
--
2.39.5
next prev parent reply other threads:[~2025-01-03 16:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-03 16:18 [PATCH v5 0/4] hw/ssi/pnv_spi: Remove PnvXferBuffer and fix CID 1558827 Chalapathi V
2025-01-03 16:18 ` [PATCH v5 1/4] hw/ssi/pnv_spi: Replace PnvXferBuffer with Fifo8 structure Chalapathi V
2025-02-27 1:39 ` Nicholas Piggin
2025-02-28 2:59 ` Chalapathi V
2025-01-03 16:18 ` Chalapathi V [this message]
2025-02-27 1:43 ` [PATCH v5 2/4] hw/ssi/pnv_spi: Coverity CID 1558827: Use local var seq_index instead of get_seq_index() Nicholas Piggin
2025-02-27 1:44 ` Nicholas Piggin
2025-01-03 16:18 ` [PATCH v5 3/4] hw/ssi/pnv_spi: Make bus names distinct for each controllers of a socket Chalapathi V
2025-02-27 1:54 ` Nicholas Piggin
2025-02-28 3:03 ` Chalapathi V
2025-02-28 7:45 ` Cédric Le Goater
2025-02-28 11:00 ` Chalapathi V
2025-02-28 16:15 ` Cédric Le Goater
2025-01-03 16:18 ` [PATCH v5 4/4] hw/ssi/pnv_spi: Put a limit to RDR match failures Chalapathi V
2025-02-27 1:56 ` Nicholas Piggin
2025-02-28 3:04 ` Chalapathi V
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250103161824.22469-3-chalapathi.v@linux.ibm.com \
--to=chalapathi.v@linux.ibm.com \
--cc=alistair@alistair23.me \
--cc=calebs@linux.ibm.com \
--cc=chalapathi.v@ibm.com \
--cc=clg@kaod.org \
--cc=dantan@linux.vnet.ibm.com \
--cc=fbarrat@linux.ibm.com \
--cc=milesg@linux.ibm.com \
--cc=npiggin@gmail.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=saif.abrar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.