* [PATCH v3 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs
@ 2022-08-21 23:41 Wilfred Mallawa
2022-08-21 23:41 ` [PATCH v3 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Wilfred Mallawa @ 2022-08-21 23:41 UTC (permalink / raw)
To: Alistair.Francis, qemu-riscv; +Cc: qemu-devel, Wilfred Mallawa
From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
This patch series cleans up the ibex_spi driver,
fixes the specified coverity issue,
implements register rw1c functionality and
updates an incorrect register offset.
Patch V3 fixes up:
- Style errors (excess indentation on multi-line)
- Remove patch note from commit message in [2/4]
Testing:
- Tested with Opentitan unit tests for TockOS...[OK]
Wilfred Mallawa (4):
hw/ssi: ibex_spi: fixup typos in ibex_spi_host
hw/ssi: ibex_spi: fixup coverity issue
hw/ssi: ibex_spi: fixup/add rw1c functionality
hw/ssi: ibex_spi: update reg addr
hw/ssi/ibex_spi_host.c | 172 +++++++++++++++++++--------------
include/hw/ssi/ibex_spi_host.h | 4 +-
2 files changed, 104 insertions(+), 72 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v3 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host 2022-08-21 23:41 [PATCH v3 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa @ 2022-08-21 23:41 ` Wilfred Mallawa 2022-08-21 23:41 ` [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Wilfred Mallawa @ 2022-08-21 23:41 UTC (permalink / raw) To: Alistair.Francis, qemu-riscv Cc: qemu-devel, Wilfred Mallawa, Alistair Francis, Andrew Jones From: Wilfred Mallawa <wilfred.mallawa@wdc.com> This patch fixes up minor typos in ibex_spi_host Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> --- hw/ssi/ibex_spi_host.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c index d14580b409..601041d719 100644 --- a/hw/ssi/ibex_spi_host.c +++ b/hw/ssi/ibex_spi_host.c @@ -172,7 +172,7 @@ static void ibex_spi_host_irq(IbexSPIHostState *s) & R_INTR_STATE_SPI_EVENT_MASK; int err_irq = 0, event_irq = 0; - /* Error IRQ enabled and Error IRQ Cleared*/ + /* Error IRQ enabled and Error IRQ Cleared */ if (error_en && !err_pending) { /* Event enabled, Interrupt Test Error */ if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) { @@ -434,7 +434,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, case IBEX_SPI_HOST_TXDATA: /* * This is a hardware `feature` where - * the first word written TXDATA after init is omitted entirely + * the first word written to TXDATA after init is omitted entirely */ if (s->init_status) { s->init_status = false; @@ -487,7 +487,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, break; case IBEX_SPI_HOST_ERROR_STATUS: /* - * Indicates that any errors that have occurred. + * Indicates any errors that have occurred. * When an error occurs, the corresponding bit must be cleared * here before issuing any further commands */ -- 2.37.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue 2022-08-21 23:41 [PATCH v3 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa 2022-08-21 23:41 ` [PATCH v3 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa @ 2022-08-21 23:41 ` Wilfred Mallawa 2022-08-22 3:42 ` Alistair Francis 2022-08-21 23:42 ` [PATCH v3 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality Wilfred Mallawa 2022-08-21 23:42 ` [PATCH v3 4/4] hw/ssi: ibex_spi: update reg addr Wilfred Mallawa 3 siblings, 1 reply; 7+ messages in thread From: Wilfred Mallawa @ 2022-08-21 23:41 UTC (permalink / raw) To: Alistair.Francis, qemu-riscv; +Cc: qemu-devel, Wilfred Mallawa, Andrew Jones From: Wilfred Mallawa <wilfred.mallawa@wdc.com> This patch addresses the coverity issues specified in [1], as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been implemented to clean up the code. [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html Fixes: Coverity CID 1488107 Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> --- hw/ssi/ibex_spi_host.c | 130 +++++++++++++++++++++-------------------- 1 file changed, 66 insertions(+), 64 deletions(-) diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c index 601041d719..1298664d2b 100644 --- a/hw/ssi/ibex_spi_host.c +++ b/hw/ssi/ibex_spi_host.c @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend) static void ibex_spi_rxfifo_reset(IbexSPIHostState *s) { + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS]; /* Empty the RX FIFO and assert RXEMPTY */ fifo8_reset(&s->rx_fifo); - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK; - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK; + data = FIELD_DP32(data, STATUS, RXEMPTY, 1); + s->regs[IBEX_SPI_HOST_STATUS] = data; } static void ibex_spi_txfifo_reset(IbexSPIHostState *s) { + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS]; /* Empty the TX FIFO and assert TXEMPTY */ fifo8_reset(&s->tx_fifo); - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK; - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK; + data = FIELD_DP32(data, STATUS, TXEMPTY, 1); + s->regs[IBEX_SPI_HOST_STATUS] = data; } static void ibex_spi_host_reset(DeviceState *dev) @@ -162,37 +164,38 @@ static void ibex_spi_host_reset(DeviceState *dev) */ static void ibex_spi_host_irq(IbexSPIHostState *s) { - bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE] - & R_INTR_ENABLE_ERROR_MASK; - bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE] - & R_INTR_ENABLE_SPI_EVENT_MASK; - bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE] - & R_INTR_STATE_ERROR_MASK; - bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE] - & R_INTR_STATE_SPI_EVENT_MASK; + uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST]; + uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE]; + uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE]; + + uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE]; + uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE]; + uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS]; + uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS]; + + + bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR); + bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, SPI_EVENT); + bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, ERROR); + bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, SPI_EVENT); + int err_irq = 0, event_irq = 0; /* Error IRQ enabled and Error IRQ Cleared */ if (error_en && !err_pending) { /* Event enabled, Interrupt Test Error */ - if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) { + if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) { err_irq = 1; - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] - & R_ERROR_ENABLE_CMDBUSY_MASK) && - s->regs[IBEX_SPI_HOST_ERROR_STATUS] - & R_ERROR_STATUS_CMDBUSY_MASK) { + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY) && + FIELD_EX32(err_status_reg, ERROR_STATUS, CMDBUSY)) { /* Wrote to COMMAND when not READY */ err_irq = 1; - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] - & R_ERROR_ENABLE_CMDINVAL_MASK) && - s->regs[IBEX_SPI_HOST_ERROR_STATUS] - & R_ERROR_STATUS_CMDINVAL_MASK) { + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDINVAL) && + FIELD_EX32(err_status_reg, ERROR_STATUS, CMDINVAL)) { /* Invalid command segment */ err_irq = 1; - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] - & R_ERROR_ENABLE_CSIDINVAL_MASK) && - s->regs[IBEX_SPI_HOST_ERROR_STATUS] - & R_ERROR_STATUS_CSIDINVAL_MASK) { + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CSIDINVAL) && + FIELD_EX32(err_status_reg, ERROR_STATUS, CSIDINVAL)) { /* Invalid value for CSID */ err_irq = 1; } @@ -204,22 +207,19 @@ static void ibex_spi_host_irq(IbexSPIHostState *s) /* Event IRQ Enabled and Event IRQ Cleared */ if (event_en && !status_pending) { - if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_SPI_EVENT_MASK) { + if (FIELD_EX32(intr_test_reg, INTR_STATE, SPI_EVENT)) { /* Event enabled, Interrupt Test Event */ event_irq = 1; - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] - & R_EVENT_ENABLE_READY_MASK) && - (s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_READY_MASK)) { + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, READY) && + FIELD_EX32(status_reg, STATUS, READY)) { /* SPI Host ready for next command */ event_irq = 1; - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] - & R_EVENT_ENABLE_TXEMPTY_MASK) && - (s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_TXEMPTY_MASK)) { + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, TXEMPTY) && + FIELD_EX32(status_reg, STATUS, TXEMPTY)) { /* SPI TXEMPTY, TXFIFO drained */ event_irq = 1; - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] - & R_EVENT_ENABLE_RXFULL_MASK) && - (s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_RXFULL_MASK)) { + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, RXFULL) && + FIELD_EX32(status_reg, STATUS, RXFULL)) { /* SPI RXFULL, RXFIFO full */ event_irq = 1; } @@ -232,10 +232,11 @@ static void ibex_spi_host_irq(IbexSPIHostState *s) static void ibex_spi_host_transfer(IbexSPIHostState *s) { - uint32_t rx, tx; + uint32_t rx, tx, data; /* Get num of one byte transfers */ - uint8_t segment_len = ((s->regs[IBEX_SPI_HOST_COMMAND] & R_COMMAND_LEN_MASK) - >> R_COMMAND_LEN_SHIFT); + uint8_t segment_len = FIELD_EX32(s->regs[IBEX_SPI_HOST_COMMAND], + COMMAND, LEN); + while (segment_len > 0) { if (fifo8_is_empty(&s->tx_fifo)) { /* Assert Stall */ @@ -262,22 +263,21 @@ static void ibex_spi_host_transfer(IbexSPIHostState *s) --segment_len; } + data = s->regs[IBEX_SPI_HOST_STATUS]; /* Assert Ready */ - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_READY_MASK; + data = FIELD_DP32(data, STATUS, READY, 1); /* Set RXQD */ - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXQD_MASK; - s->regs[IBEX_SPI_HOST_STATUS] |= (R_STATUS_RXQD_MASK - & div4_round_up(segment_len)); + data = FIELD_DP32(data, STATUS, RXQD, div4_round_up(segment_len)); /* Set TXQD */ - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXQD_MASK; - s->regs[IBEX_SPI_HOST_STATUS] |= (fifo8_num_used(&s->tx_fifo) / 4) - & R_STATUS_TXQD_MASK; + data = FIELD_DP32(data, STATUS, TXQD, fifo8_num_used(&s->tx_fifo) / 4); /* Clear TXFULL */ - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK; - /* Assert TXEMPTY and drop remaining bytes that exceed segment_len */ - ibex_spi_txfifo_reset(s); + data = FIELD_DP32(data, STATUS, TXFULL, 0); /* Reset RXEMPTY */ - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXEMPTY_MASK; + data = FIELD_DP32(data, STATUS, RXEMPTY, 0); + /* Update register status */ + s->regs[IBEX_SPI_HOST_STATUS] = data; + /* Drop remaining bytes that exceed segment_len */ + ibex_spi_txfifo_reset(s); ibex_spi_host_irq(s); } @@ -340,7 +340,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, { IbexSPIHostState *s = opaque; uint32_t val32 = val64; - uint32_t shift_mask = 0xff; + uint32_t shift_mask = 0xff, data = 0, status = 0; uint8_t txqd_len; trace_ibex_spi_host_write(addr, size, val64); @@ -397,22 +397,24 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, s->regs[addr] = val32; /* STALL, IP not enabled */ - if (!(s->regs[IBEX_SPI_HOST_CONTROL] & R_CONTROL_SPIEN_MASK)) { + if (!(FIELD_EX32(s->regs[IBEX_SPI_HOST_CONTROL], + CONTROL, SPIEN))) { return; } /* SPI not ready, IRQ Error */ - if (!(s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_READY_MASK)) { + if (!(FIELD_EX32(s->regs[IBEX_SPI_HOST_STATUS], + STATUS, READY))) { s->regs[IBEX_SPI_HOST_ERROR_STATUS] |= R_ERROR_STATUS_CMDBUSY_MASK; ibex_spi_host_irq(s); return; } + /* Assert Not Ready */ s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_READY_MASK; - if (((val32 & R_COMMAND_DIRECTION_MASK) >> R_COMMAND_DIRECTION_SHIFT) - != BIDIRECTIONAL_TRANSFER) { - qemu_log_mask(LOG_UNIMP, + if (FIELD_EX32(val32, COMMAND, DIRECTION) != BIDIRECTIONAL_TRANSFER) { + qemu_log_mask(LOG_UNIMP, "%s: Rx Only/Tx Only are not supported\n", __func__); } @@ -452,8 +454,8 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, return; } /* Byte ordering is set by the IP */ - if ((s->regs[IBEX_SPI_HOST_STATUS] & - R_STATUS_BYTEORDER_MASK) == 0) { + status = s->regs[IBEX_SPI_HOST_STATUS]; + if (FIELD_EX32(status, STATUS, BYTEORDER) == 0) { /* LE: LSB transmitted first (default for ibex processor) */ shift_mask = 0xff << (i * 8); } else { @@ -464,18 +466,18 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, fifo8_push(&s->tx_fifo, (val32 & shift_mask) >> (i * 8)); } - + status = s->regs[IBEX_SPI_HOST_STATUS]; /* Reset TXEMPTY */ - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXEMPTY_MASK; + status = FIELD_DP32(status, STATUS, TXEMPTY, 0); /* Update TXQD */ - txqd_len = (s->regs[IBEX_SPI_HOST_STATUS] & - R_STATUS_TXQD_MASK) >> R_STATUS_TXQD_SHIFT; + txqd_len = FIELD_EX32(status, STATUS, TXQD); /* Partial bytes (size < 4) are padded, in words. */ txqd_len += 1; - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXQD_MASK; - s->regs[IBEX_SPI_HOST_STATUS] |= txqd_len; + status = FIELD_DP32(status, STATUS, TXQD, txqd_len); /* Assert Ready */ - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_READY_MASK; + status = FIELD_DP32(status, STATUS, READY, 1); + /* Update register status */ + s->regs[IBEX_SPI_HOST_STATUS] = status; break; case IBEX_SPI_HOST_ERROR_ENABLE: s->regs[addr] = val32; -- 2.37.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue 2022-08-21 23:41 ` [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa @ 2022-08-22 3:42 ` Alistair Francis 2022-08-22 3:53 ` Wilfred Mallawa 0 siblings, 1 reply; 7+ messages in thread From: Alistair Francis @ 2022-08-22 3:42 UTC (permalink / raw) To: Wilfred Mallawa Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers, Wilfred Mallawa, Andrew Jones On Mon, Aug 22, 2022 at 9:53 AM Wilfred Mallawa <wilfred.mallawa@opensource.wdc.com> wrote: > > From: Wilfred Mallawa <wilfred.mallawa@wdc.com> > > This patch addresses the coverity issues specified in [1], > as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been > implemented to clean up the code. > > [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html > > Fixes: Coverity CID 1488107 > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> > --- > hw/ssi/ibex_spi_host.c | 130 +++++++++++++++++++++-------------------- > 1 file changed, 66 insertions(+), 64 deletions(-) > > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c > index 601041d719..1298664d2b 100644 > --- a/hw/ssi/ibex_spi_host.c > +++ b/hw/ssi/ibex_spi_host.c > @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend) > > static void ibex_spi_rxfifo_reset(IbexSPIHostState *s) > { > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS]; > /* Empty the RX FIFO and assert RXEMPTY */ > fifo8_reset(&s->rx_fifo); > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK; > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK; > + data = FIELD_DP32(data, STATUS, RXEMPTY, 1); Doesn't this no longer clear the RXFULL bits? > + s->regs[IBEX_SPI_HOST_STATUS] = data; > } > > static void ibex_spi_txfifo_reset(IbexSPIHostState *s) > { > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS]; > /* Empty the TX FIFO and assert TXEMPTY */ > fifo8_reset(&s->tx_fifo); > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK; > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK; > + data = FIELD_DP32(data, STATUS, TXEMPTY, 1); Same here about TXFULL Otherwise the patch looks good Alistair > + s->regs[IBEX_SPI_HOST_STATUS] = data; > } > > static void ibex_spi_host_reset(DeviceState *dev) > @@ -162,37 +164,38 @@ static void ibex_spi_host_reset(DeviceState *dev) > */ > static void ibex_spi_host_irq(IbexSPIHostState *s) > { > - bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE] > - & R_INTR_ENABLE_ERROR_MASK; > - bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE] > - & R_INTR_ENABLE_SPI_EVENT_MASK; > - bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE] > - & R_INTR_STATE_ERROR_MASK; > - bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE] > - & R_INTR_STATE_SPI_EVENT_MASK; > + uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST]; > + uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE]; > + uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE]; > + > + uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE]; > + uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE]; > + uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS]; > + uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS]; > + > + > + bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR); > + bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, SPI_EVENT); > + bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, ERROR); > + bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, SPI_EVENT); > + > int err_irq = 0, event_irq = 0; > > /* Error IRQ enabled and Error IRQ Cleared */ > if (error_en && !err_pending) { > /* Event enabled, Interrupt Test Error */ > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) { > + if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) { > err_irq = 1; > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] > - & R_ERROR_ENABLE_CMDBUSY_MASK) && > - s->regs[IBEX_SPI_HOST_ERROR_STATUS] > - & R_ERROR_STATUS_CMDBUSY_MASK) { > + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY) && > + FIELD_EX32(err_status_reg, ERROR_STATUS, CMDBUSY)) { > /* Wrote to COMMAND when not READY */ > err_irq = 1; > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] > - & R_ERROR_ENABLE_CMDINVAL_MASK) && > - s->regs[IBEX_SPI_HOST_ERROR_STATUS] > - & R_ERROR_STATUS_CMDINVAL_MASK) { > + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDINVAL) && > + FIELD_EX32(err_status_reg, ERROR_STATUS, CMDINVAL)) { > /* Invalid command segment */ > err_irq = 1; > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] > - & R_ERROR_ENABLE_CSIDINVAL_MASK) && > - s->regs[IBEX_SPI_HOST_ERROR_STATUS] > - & R_ERROR_STATUS_CSIDINVAL_MASK) { > + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CSIDINVAL) && > + FIELD_EX32(err_status_reg, ERROR_STATUS, CSIDINVAL)) { > /* Invalid value for CSID */ > err_irq = 1; > } > @@ -204,22 +207,19 @@ static void ibex_spi_host_irq(IbexSPIHostState *s) > > /* Event IRQ Enabled and Event IRQ Cleared */ > if (event_en && !status_pending) { > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_SPI_EVENT_MASK) { > + if (FIELD_EX32(intr_test_reg, INTR_STATE, SPI_EVENT)) { > /* Event enabled, Interrupt Test Event */ > event_irq = 1; > - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] > - & R_EVENT_ENABLE_READY_MASK) && > - (s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_READY_MASK)) { > + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, READY) && > + FIELD_EX32(status_reg, STATUS, READY)) { > /* SPI Host ready for next command */ > event_irq = 1; > - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] > - & R_EVENT_ENABLE_TXEMPTY_MASK) && > - (s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_TXEMPTY_MASK)) { > + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, TXEMPTY) && > + FIELD_EX32(status_reg, STATUS, TXEMPTY)) { > /* SPI TXEMPTY, TXFIFO drained */ > event_irq = 1; > - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] > - & R_EVENT_ENABLE_RXFULL_MASK) && > - (s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_RXFULL_MASK)) { > + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, RXFULL) && > + FIELD_EX32(status_reg, STATUS, RXFULL)) { > /* SPI RXFULL, RXFIFO full */ > event_irq = 1; > } > @@ -232,10 +232,11 @@ static void ibex_spi_host_irq(IbexSPIHostState *s) > > static void ibex_spi_host_transfer(IbexSPIHostState *s) > { > - uint32_t rx, tx; > + uint32_t rx, tx, data; > /* Get num of one byte transfers */ > - uint8_t segment_len = ((s->regs[IBEX_SPI_HOST_COMMAND] & R_COMMAND_LEN_MASK) > - >> R_COMMAND_LEN_SHIFT); > + uint8_t segment_len = FIELD_EX32(s->regs[IBEX_SPI_HOST_COMMAND], > + COMMAND, LEN); > + > while (segment_len > 0) { > if (fifo8_is_empty(&s->tx_fifo)) { > /* Assert Stall */ > @@ -262,22 +263,21 @@ static void ibex_spi_host_transfer(IbexSPIHostState *s) > --segment_len; > } > > + data = s->regs[IBEX_SPI_HOST_STATUS]; > /* Assert Ready */ > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_READY_MASK; > + data = FIELD_DP32(data, STATUS, READY, 1); > /* Set RXQD */ > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXQD_MASK; > - s->regs[IBEX_SPI_HOST_STATUS] |= (R_STATUS_RXQD_MASK > - & div4_round_up(segment_len)); > + data = FIELD_DP32(data, STATUS, RXQD, div4_round_up(segment_len)); > /* Set TXQD */ > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXQD_MASK; > - s->regs[IBEX_SPI_HOST_STATUS] |= (fifo8_num_used(&s->tx_fifo) / 4) > - & R_STATUS_TXQD_MASK; > + data = FIELD_DP32(data, STATUS, TXQD, fifo8_num_used(&s->tx_fifo) / 4); > /* Clear TXFULL */ > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK; > - /* Assert TXEMPTY and drop remaining bytes that exceed segment_len */ > - ibex_spi_txfifo_reset(s); > + data = FIELD_DP32(data, STATUS, TXFULL, 0); > /* Reset RXEMPTY */ > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXEMPTY_MASK; > + data = FIELD_DP32(data, STATUS, RXEMPTY, 0); > + /* Update register status */ > + s->regs[IBEX_SPI_HOST_STATUS] = data; > + /* Drop remaining bytes that exceed segment_len */ > + ibex_spi_txfifo_reset(s); > > ibex_spi_host_irq(s); > } > @@ -340,7 +340,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, > { > IbexSPIHostState *s = opaque; > uint32_t val32 = val64; > - uint32_t shift_mask = 0xff; > + uint32_t shift_mask = 0xff, data = 0, status = 0; > uint8_t txqd_len; > > trace_ibex_spi_host_write(addr, size, val64); > @@ -397,22 +397,24 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, > s->regs[addr] = val32; > > /* STALL, IP not enabled */ > - if (!(s->regs[IBEX_SPI_HOST_CONTROL] & R_CONTROL_SPIEN_MASK)) { > + if (!(FIELD_EX32(s->regs[IBEX_SPI_HOST_CONTROL], > + CONTROL, SPIEN))) { > return; > } > > /* SPI not ready, IRQ Error */ > - if (!(s->regs[IBEX_SPI_HOST_STATUS] & R_STATUS_READY_MASK)) { > + if (!(FIELD_EX32(s->regs[IBEX_SPI_HOST_STATUS], > + STATUS, READY))) { > s->regs[IBEX_SPI_HOST_ERROR_STATUS] |= R_ERROR_STATUS_CMDBUSY_MASK; > ibex_spi_host_irq(s); > return; > } > + > /* Assert Not Ready */ > s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_READY_MASK; > > - if (((val32 & R_COMMAND_DIRECTION_MASK) >> R_COMMAND_DIRECTION_SHIFT) > - != BIDIRECTIONAL_TRANSFER) { > - qemu_log_mask(LOG_UNIMP, > + if (FIELD_EX32(val32, COMMAND, DIRECTION) != BIDIRECTIONAL_TRANSFER) { > + qemu_log_mask(LOG_UNIMP, > "%s: Rx Only/Tx Only are not supported\n", __func__); > } > > @@ -452,8 +454,8 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, > return; > } > /* Byte ordering is set by the IP */ > - if ((s->regs[IBEX_SPI_HOST_STATUS] & > - R_STATUS_BYTEORDER_MASK) == 0) { > + status = s->regs[IBEX_SPI_HOST_STATUS]; > + if (FIELD_EX32(status, STATUS, BYTEORDER) == 0) { > /* LE: LSB transmitted first (default for ibex processor) */ > shift_mask = 0xff << (i * 8); > } else { > @@ -464,18 +466,18 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, > > fifo8_push(&s->tx_fifo, (val32 & shift_mask) >> (i * 8)); > } > - > + status = s->regs[IBEX_SPI_HOST_STATUS]; > /* Reset TXEMPTY */ > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXEMPTY_MASK; > + status = FIELD_DP32(status, STATUS, TXEMPTY, 0); > /* Update TXQD */ > - txqd_len = (s->regs[IBEX_SPI_HOST_STATUS] & > - R_STATUS_TXQD_MASK) >> R_STATUS_TXQD_SHIFT; > + txqd_len = FIELD_EX32(status, STATUS, TXQD); > /* Partial bytes (size < 4) are padded, in words. */ > txqd_len += 1; > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXQD_MASK; > - s->regs[IBEX_SPI_HOST_STATUS] |= txqd_len; > + status = FIELD_DP32(status, STATUS, TXQD, txqd_len); > /* Assert Ready */ > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_READY_MASK; > + status = FIELD_DP32(status, STATUS, READY, 1); > + /* Update register status */ > + s->regs[IBEX_SPI_HOST_STATUS] = status; > break; > case IBEX_SPI_HOST_ERROR_ENABLE: > s->regs[addr] = val32; > -- > 2.37.2 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue 2022-08-22 3:42 ` Alistair Francis @ 2022-08-22 3:53 ` Wilfred Mallawa 0 siblings, 0 replies; 7+ messages in thread From: Wilfred Mallawa @ 2022-08-22 3:53 UTC (permalink / raw) To: wilfred.mallawa, alistair23 Cc: qemu-riscv, Alistair Francis, qemu-devel, ajones On Mon, 2022-08-22 at 13:42 +1000, Alistair Francis wrote: > On Mon, Aug 22, 2022 at 9:53 AM Wilfred Mallawa > <wilfred.mallawa@opensource.wdc.com> wrote: > > > > From: Wilfred Mallawa <wilfred.mallawa@wdc.com> > > > > This patch addresses the coverity issues specified in [1], > > as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been > > implemented to clean up the code. > > > > [1] > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html > > > > Fixes: Coverity CID 1488107 > > > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> > > --- > > hw/ssi/ibex_spi_host.c | 130 +++++++++++++++++++++---------------- > > ---- > > 1 file changed, 66 insertions(+), 64 deletions(-) > > > > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c > > index 601041d719..1298664d2b 100644 > > --- a/hw/ssi/ibex_spi_host.c > > +++ b/hw/ssi/ibex_spi_host.c > > @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t > > dividend) > > > > static void ibex_spi_rxfifo_reset(IbexSPIHostState *s) > > { > > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS]; > > /* Empty the RX FIFO and assert RXEMPTY */ > > fifo8_reset(&s->rx_fifo); > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK; > > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK; > > + data = FIELD_DP32(data, STATUS, RXEMPTY, 1); > > Doesn't this no longer clear the RXFULL bits? > > > + s->regs[IBEX_SPI_HOST_STATUS] = data; > > } > > > > static void ibex_spi_txfifo_reset(IbexSPIHostState *s) > > { > > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS]; > > /* Empty the TX FIFO and assert TXEMPTY */ > > fifo8_reset(&s->tx_fifo); > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK; > > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK; > > + data = FIELD_DP32(data, STATUS, TXEMPTY, 1); > > Same here about TXFULL > > Otherwise the patch looks good > > Alistair > Good catch! I'll fix these up. Wilfred > > + s->regs[IBEX_SPI_HOST_STATUS] = data; > > } > > > > static void ibex_spi_host_reset(DeviceState *dev) > > @@ -162,37 +164,38 @@ static void ibex_spi_host_reset(DeviceState > > *dev) > > */ > > static void ibex_spi_host_irq(IbexSPIHostState *s) > > { > > - bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE] > > - & R_INTR_ENABLE_ERROR_MASK; > > - bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE] > > - & R_INTR_ENABLE_SPI_EVENT_MASK; > > - bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE] > > - & R_INTR_STATE_ERROR_MASK; > > - bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE] > > - & R_INTR_STATE_SPI_EVENT_MASK; > > + uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST]; > > + uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE]; > > + uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE]; > > + > > + uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE]; > > + uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE]; > > + uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS]; > > + uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS]; > > + > > + > > + bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR); > > + bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, > > SPI_EVENT); > > + bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, > > ERROR); > > + bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, > > SPI_EVENT); > > + > > int err_irq = 0, event_irq = 0; > > > > /* Error IRQ enabled and Error IRQ Cleared */ > > if (error_en && !err_pending) { > > /* Event enabled, Interrupt Test Error */ > > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] & > > R_INTR_TEST_ERROR_MASK) { > > + if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) { > > err_irq = 1; > > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] > > - & R_ERROR_ENABLE_CMDBUSY_MASK) && > > - s->regs[IBEX_SPI_HOST_ERROR_STATUS] > > - & R_ERROR_STATUS_CMDBUSY_MASK) { > > + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY) > > && > > + FIELD_EX32(err_status_reg, ERROR_STATUS, > > CMDBUSY)) { > > /* Wrote to COMMAND when not READY */ > > err_irq = 1; > > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] > > - & R_ERROR_ENABLE_CMDINVAL_MASK) && > > - s->regs[IBEX_SPI_HOST_ERROR_STATUS] > > - & R_ERROR_STATUS_CMDINVAL_MASK) { > > + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, > > CMDINVAL) && > > + FIELD_EX32(err_status_reg, ERROR_STATUS, > > CMDINVAL)) { > > /* Invalid command segment */ > > err_irq = 1; > > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE] > > - & R_ERROR_ENABLE_CSIDINVAL_MASK) && > > - s->regs[IBEX_SPI_HOST_ERROR_STATUS] > > - & R_ERROR_STATUS_CSIDINVAL_MASK) { > > + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, > > CSIDINVAL) && > > + FIELD_EX32(err_status_reg, ERROR_STATUS, > > CSIDINVAL)) { > > /* Invalid value for CSID */ > > err_irq = 1; > > } > > @@ -204,22 +207,19 @@ static void > > ibex_spi_host_irq(IbexSPIHostState *s) > > > > /* Event IRQ Enabled and Event IRQ Cleared */ > > if (event_en && !status_pending) { > > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] & > > R_INTR_TEST_SPI_EVENT_MASK) { > > + if (FIELD_EX32(intr_test_reg, INTR_STATE, SPI_EVENT)) { > > /* Event enabled, Interrupt Test Event */ > > event_irq = 1; > > - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] > > - & R_EVENT_ENABLE_READY_MASK) && > > - (s->regs[IBEX_SPI_HOST_STATUS] & > > R_STATUS_READY_MASK)) { > > + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, READY) > > && > > + FIELD_EX32(status_reg, STATUS, READY)) { > > /* SPI Host ready for next command */ > > event_irq = 1; > > - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] > > - & R_EVENT_ENABLE_TXEMPTY_MASK) && > > - (s->regs[IBEX_SPI_HOST_STATUS] & > > R_STATUS_TXEMPTY_MASK)) { > > + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, > > TXEMPTY) && > > + FIELD_EX32(status_reg, STATUS, TXEMPTY)) { > > /* SPI TXEMPTY, TXFIFO drained */ > > event_irq = 1; > > - } else if ((s->regs[IBEX_SPI_HOST_EVENT_ENABLE] > > - & R_EVENT_ENABLE_RXFULL_MASK) && > > - (s->regs[IBEX_SPI_HOST_STATUS] & > > R_STATUS_RXFULL_MASK)) { > > + } else if (FIELD_EX32(event_en_reg, EVENT_ENABLE, RXFULL) > > && > > + FIELD_EX32(status_reg, STATUS, RXFULL)) { > > /* SPI RXFULL, RXFIFO full */ > > event_irq = 1; > > } > > @@ -232,10 +232,11 @@ static void > > ibex_spi_host_irq(IbexSPIHostState *s) > > > > static void ibex_spi_host_transfer(IbexSPIHostState *s) > > { > > - uint32_t rx, tx; > > + uint32_t rx, tx, data; > > /* Get num of one byte transfers */ > > - uint8_t segment_len = ((s->regs[IBEX_SPI_HOST_COMMAND] & > > R_COMMAND_LEN_MASK) > > - >> R_COMMAND_LEN_SHIFT); > > + uint8_t segment_len = FIELD_EX32(s- > > >regs[IBEX_SPI_HOST_COMMAND], > > + COMMAND, LEN); > > + > > while (segment_len > 0) { > > if (fifo8_is_empty(&s->tx_fifo)) { > > /* Assert Stall */ > > @@ -262,22 +263,21 @@ static void > > ibex_spi_host_transfer(IbexSPIHostState *s) > > --segment_len; > > } > > > > + data = s->regs[IBEX_SPI_HOST_STATUS]; > > /* Assert Ready */ > > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_READY_MASK; > > + data = FIELD_DP32(data, STATUS, READY, 1); > > /* Set RXQD */ > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXQD_MASK; > > - s->regs[IBEX_SPI_HOST_STATUS] |= (R_STATUS_RXQD_MASK > > - & div4_round_up(segment_len)); > > + data = FIELD_DP32(data, STATUS, RXQD, > > div4_round_up(segment_len)); > > /* Set TXQD */ > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXQD_MASK; > > - s->regs[IBEX_SPI_HOST_STATUS] |= (fifo8_num_used(&s->tx_fifo) > > / 4) > > - & R_STATUS_TXQD_MASK; > > + data = FIELD_DP32(data, STATUS, TXQD, fifo8_num_used(&s- > > >tx_fifo) / 4); > > /* Clear TXFULL */ > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK; > > - /* Assert TXEMPTY and drop remaining bytes that exceed > > segment_len */ > > - ibex_spi_txfifo_reset(s); > > + data = FIELD_DP32(data, STATUS, TXFULL, 0); > > /* Reset RXEMPTY */ > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXEMPTY_MASK; > > + data = FIELD_DP32(data, STATUS, RXEMPTY, 0); > > + /* Update register status */ > > + s->regs[IBEX_SPI_HOST_STATUS] = data; > > + /* Drop remaining bytes that exceed segment_len */ > > + ibex_spi_txfifo_reset(s); > > > > ibex_spi_host_irq(s); > > } > > @@ -340,7 +340,7 @@ static void ibex_spi_host_write(void *opaque, > > hwaddr addr, > > { > > IbexSPIHostState *s = opaque; > > uint32_t val32 = val64; > > - uint32_t shift_mask = 0xff; > > + uint32_t shift_mask = 0xff, data = 0, status = 0; > > uint8_t txqd_len; > > > > trace_ibex_spi_host_write(addr, size, val64); > > @@ -397,22 +397,24 @@ static void ibex_spi_host_write(void *opaque, > > hwaddr addr, > > s->regs[addr] = val32; > > > > /* STALL, IP not enabled */ > > - if (!(s->regs[IBEX_SPI_HOST_CONTROL] & > > R_CONTROL_SPIEN_MASK)) { > > + if (!(FIELD_EX32(s->regs[IBEX_SPI_HOST_CONTROL], > > + CONTROL, SPIEN))) { > > return; > > } > > > > /* SPI not ready, IRQ Error */ > > - if (!(s->regs[IBEX_SPI_HOST_STATUS] & > > R_STATUS_READY_MASK)) { > > + if (!(FIELD_EX32(s->regs[IBEX_SPI_HOST_STATUS], > > + STATUS, READY))) { > > s->regs[IBEX_SPI_HOST_ERROR_STATUS] |= > > R_ERROR_STATUS_CMDBUSY_MASK; > > ibex_spi_host_irq(s); > > return; > > } > > + > > /* Assert Not Ready */ > > s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_READY_MASK; > > > > - if (((val32 & R_COMMAND_DIRECTION_MASK) >> > > R_COMMAND_DIRECTION_SHIFT) > > - != BIDIRECTIONAL_TRANSFER) { > > - qemu_log_mask(LOG_UNIMP, > > + if (FIELD_EX32(val32, COMMAND, DIRECTION) != > > BIDIRECTIONAL_TRANSFER) { > > + qemu_log_mask(LOG_UNIMP, > > "%s: Rx Only/Tx Only are not > > supported\n", __func__); > > } > > > > @@ -452,8 +454,8 @@ static void ibex_spi_host_write(void *opaque, > > hwaddr addr, > > return; > > } > > /* Byte ordering is set by the IP */ > > - if ((s->regs[IBEX_SPI_HOST_STATUS] & > > - R_STATUS_BYTEORDER_MASK) == 0) { > > + status = s->regs[IBEX_SPI_HOST_STATUS]; > > + if (FIELD_EX32(status, STATUS, BYTEORDER) == 0) { > > /* LE: LSB transmitted first (default for ibex > > processor) */ > > shift_mask = 0xff << (i * 8); > > } else { > > @@ -464,18 +466,18 @@ static void ibex_spi_host_write(void *opaque, > > hwaddr addr, > > > > fifo8_push(&s->tx_fifo, (val32 & shift_mask) >> (i * > > 8)); > > } > > - > > + status = s->regs[IBEX_SPI_HOST_STATUS]; > > /* Reset TXEMPTY */ > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXEMPTY_MASK; > > + status = FIELD_DP32(status, STATUS, TXEMPTY, 0); > > /* Update TXQD */ > > - txqd_len = (s->regs[IBEX_SPI_HOST_STATUS] & > > - R_STATUS_TXQD_MASK) >> R_STATUS_TXQD_SHIFT; > > + txqd_len = FIELD_EX32(status, STATUS, TXQD); > > /* Partial bytes (size < 4) are padded, in words. */ > > txqd_len += 1; > > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXQD_MASK; > > - s->regs[IBEX_SPI_HOST_STATUS] |= txqd_len; > > + status = FIELD_DP32(status, STATUS, TXQD, txqd_len); > > /* Assert Ready */ > > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_READY_MASK; > > + status = FIELD_DP32(status, STATUS, READY, 1); > > + /* Update register status */ > > + s->regs[IBEX_SPI_HOST_STATUS] = status; > > break; > > case IBEX_SPI_HOST_ERROR_ENABLE: > > s->regs[addr] = val32; > > -- > > 2.37.2 > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality 2022-08-21 23:41 [PATCH v3 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa 2022-08-21 23:41 ` [PATCH v3 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa 2022-08-21 23:41 ` [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa @ 2022-08-21 23:42 ` Wilfred Mallawa 2022-08-21 23:42 ` [PATCH v3 4/4] hw/ssi: ibex_spi: update reg addr Wilfred Mallawa 3 siblings, 0 replies; 7+ messages in thread From: Wilfred Mallawa @ 2022-08-21 23:42 UTC (permalink / raw) To: Alistair.Francis, qemu-riscv Cc: qemu-devel, Wilfred Mallawa, Alistair Francis From: Wilfred Mallawa <wilfred.mallawa@wdc.com> This patch adds the `rw1c` functionality to the respective registers. The status fields are cleared when the respective field is set. Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> --- hw/ssi/ibex_spi_host.c | 34 ++++++++++++++++++++++++++++++++-- include/hw/ssi/ibex_spi_host.h | 4 ++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c index 1298664d2b..3809febb0c 100644 --- a/hw/ssi/ibex_spi_host.c +++ b/hw/ssi/ibex_spi_host.c @@ -350,7 +350,17 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, switch (addr) { /* Skipping any R/O registers */ - case IBEX_SPI_HOST_INTR_STATE...IBEX_SPI_HOST_INTR_ENABLE: + case IBEX_SPI_HOST_INTR_STATE: + /* rw1c status register */ + if (FIELD_EX32(val32, INTR_STATE, ERROR)) { + data = FIELD_DP32(data, INTR_STATE, ERROR, 0); + } + if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) { + data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0); + } + s->regs[addr] = data; + break; + case IBEX_SPI_HOST_INTR_ENABLE: s->regs[addr] = val32; break; case IBEX_SPI_HOST_INTR_TEST: @@ -493,7 +503,27 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr, * When an error occurs, the corresponding bit must be cleared * here before issuing any further commands */ - s->regs[addr] = val32; + status = s->regs[addr]; + /* rw1c status register */ + if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) { + status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0); + } + if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) { + status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0); + } + if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) { + status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0); + } + if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) { + status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0); + } + if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) { + status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0); + } + if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) { + status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0); + } + s->regs[addr] = status; break; case IBEX_SPI_HOST_EVENT_ENABLE: /* Controls which classes of SPI events raise an interrupt. */ diff --git a/include/hw/ssi/ibex_spi_host.h b/include/hw/ssi/ibex_spi_host.h index 3fedcb6805..1f6d077766 100644 --- a/include/hw/ssi/ibex_spi_host.h +++ b/include/hw/ssi/ibex_spi_host.h @@ -40,7 +40,7 @@ OBJECT_CHECK(IbexSPIHostState, (obj), TYPE_IBEX_SPI_HOST) /* SPI Registers */ -#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw */ +#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw1c */ #define IBEX_SPI_HOST_INTR_ENABLE (0x04 / 4) /* rw */ #define IBEX_SPI_HOST_INTR_TEST (0x08 / 4) /* wo */ #define IBEX_SPI_HOST_ALERT_TEST (0x0c / 4) /* wo */ @@ -54,7 +54,7 @@ #define IBEX_SPI_HOST_TXDATA (0x28 / 4) #define IBEX_SPI_HOST_ERROR_ENABLE (0x2c / 4) /* rw */ -#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw */ +#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw1c */ #define IBEX_SPI_HOST_EVENT_ENABLE (0x34 / 4) /* rw */ /* FIFO Len in Bytes */ -- 2.37.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 4/4] hw/ssi: ibex_spi: update reg addr 2022-08-21 23:41 [PATCH v3 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa ` (2 preceding siblings ...) 2022-08-21 23:42 ` [PATCH v3 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality Wilfred Mallawa @ 2022-08-21 23:42 ` Wilfred Mallawa 3 siblings, 0 replies; 7+ messages in thread From: Wilfred Mallawa @ 2022-08-21 23:42 UTC (permalink / raw) To: Alistair.Francis, qemu-riscv Cc: qemu-devel, Wilfred Mallawa, Alistair Francis From: Wilfred Mallawa <wilfred.mallawa@wdc.com> Updates the `EVENT_ENABLE` register to offset `0x34` as per OpenTitan spec [1]. [1] https://docs.opentitan.org/hw/ip/spi_host/doc/#Reg_event_enable Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> --- hw/ssi/ibex_spi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c index 3809febb0c..bafff8ca1f 100644 --- a/hw/ssi/ibex_spi_host.c +++ b/hw/ssi/ibex_spi_host.c @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30) FIELD(ERROR_STATUS, CMDINVAL, 3, 1) FIELD(ERROR_STATUS, CSIDINVAL, 4, 1) FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1) -REG32(EVENT_ENABLE, 0x30) +REG32(EVENT_ENABLE, 0x34) FIELD(EVENT_ENABLE, RXFULL, 0, 1) FIELD(EVENT_ENABLE, TXEMPTY, 1, 1) FIELD(EVENT_ENABLE, RXWM, 2, 1) -- 2.37.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-22 3:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-21 23:41 [PATCH v3 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa 2022-08-21 23:41 ` [PATCH v3 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa 2022-08-21 23:41 ` [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa 2022-08-22 3:42 ` Alistair Francis 2022-08-22 3:53 ` Wilfred Mallawa 2022-08-21 23:42 ` [PATCH v3 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality Wilfred Mallawa 2022-08-21 23:42 ` [PATCH v3 4/4] hw/ssi: ibex_spi: update reg addr Wilfred Mallawa
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.