* [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs
@ 2022-08-23 6:11 Wilfred Mallawa
2022-08-23 6:12 ` [PATCH v4 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2022-08-23 6:11 UTC (permalink / raw)
To: Alistair.Francis, qemu-riscv; +Cc: qemu-devel, Wilfred Mallawa
From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Patch V4 fixes up:
- Fixup missing register field clearing on tx/rx_fifo_reset() 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 | 174 ++++++++++++++++++++-------------
include/hw/ssi/ibex_spi_host.h | 4 +-
2 files changed, 106 insertions(+), 72 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host
2022-08-23 6:11 [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa
@ 2022-08-23 6:12 ` Wilfred Mallawa
2022-08-23 6:12 ` [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2022-08-23 6:12 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] 11+ messages in thread
* [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue
2022-08-23 6:11 [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa
2022-08-23 6:12 ` [PATCH v4 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa
@ 2022-08-23 6:12 ` Wilfred Mallawa
2022-08-30 9:11 ` Alistair Francis
2022-09-08 11:30 ` Alistair Francis
2022-08-23 6:12 ` [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality Wilfred Mallawa
` (2 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2022-08-23 6:12 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 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: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
hw/ssi/ibex_spi_host.c | 132 +++++++++++++++++++++--------------------
1 file changed, 68 insertions(+), 64 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 601041d719..d52b193a1a 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -108,18 +108,22 @@ 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, RXFULL, 0);
+ 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, TXFULL, 0);
+ data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
+ s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_host_reset(DeviceState *dev)
@@ -162,37 +166,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 +209,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 +234,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 +265,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 +342,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 +399,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 +456,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 +468,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] 11+ messages in thread
* [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
2022-08-23 6:11 [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa
2022-08-23 6:12 ` [PATCH v4 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa
2022-08-23 6:12 ` [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa
@ 2022-08-23 6:12 ` Wilfred Mallawa
2022-08-30 12:33 ` Philippe Mathieu-Daudé
2022-08-23 6:12 ` [PATCH v4 4/4] hw/ssi: ibex_spi: update reg addr Wilfred Mallawa
2022-09-08 9:08 ` [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Alistair Francis
4 siblings, 1 reply; 11+ messages in thread
From: Wilfred Mallawa @ 2022-08-23 6:12 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 d52b193a1a..40d401ad47 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -352,7 +352,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:
@@ -495,7 +505,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] 11+ messages in thread
* [PATCH v4 4/4] hw/ssi: ibex_spi: update reg addr
2022-08-23 6:11 [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa
` (2 preceding siblings ...)
2022-08-23 6:12 ` [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality Wilfred Mallawa
@ 2022-08-23 6:12 ` Wilfred Mallawa
2022-09-08 9:08 ` [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Alistair Francis
4 siblings, 0 replies; 11+ messages in thread
From: Wilfred Mallawa @ 2022-08-23 6:12 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 40d401ad47..0becad34e0 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] 11+ messages in thread
* Re: [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue
2022-08-23 6:12 ` [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa
@ 2022-08-30 9:11 ` Alistair Francis
2022-09-08 11:30 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-08-30 9:11 UTC (permalink / raw)
To: Wilfred Mallawa
Cc: Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers, Wilfred Mallawa, Andrew Jones
On Tue, Aug 23, 2022 at 8:13 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: Alistair Francis <alistair.francis@wdc.com>
I didn't review the previous version, please don't add tags unless
they are explicitly stated
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
For this patch though:
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> hw/ssi/ibex_spi_host.c | 132 +++++++++++++++++++++--------------------
> 1 file changed, 68 insertions(+), 64 deletions(-)
>
> diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> index 601041d719..d52b193a1a 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -108,18 +108,22 @@ 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, RXFULL, 0);
> + 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, TXFULL, 0);
> + data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> + s->regs[IBEX_SPI_HOST_STATUS] = data;
> }
>
> static void ibex_spi_host_reset(DeviceState *dev)
> @@ -162,37 +166,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 +209,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 +234,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 +265,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 +342,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 +399,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 +456,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 +468,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] 11+ messages in thread
* Re: [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
2022-08-23 6:12 ` [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality Wilfred Mallawa
@ 2022-08-30 12:33 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-08-30 12:33 UTC (permalink / raw)
To: Wilfred Mallawa, Alistair.Francis, qemu-riscv; +Cc: qemu-devel, Wilfred Mallawa
On 23/8/22 08:12, Wilfred Mallawa wrote:
> 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 d52b193a1a..40d401ad47 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -352,7 +352,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:
> @@ -495,7 +505,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);
> + }
Alistair, does this call to add some FIELD_1CLEAR() API?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
@ 2022-08-30 12:33 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-08-30 12:33 UTC (permalink / raw)
To: Wilfred Mallawa, Alistair.Francis, qemu-riscv; +Cc: qemu-devel, Wilfred Mallawa
On 23/8/22 08:12, Wilfred Mallawa wrote:
> 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 d52b193a1a..40d401ad47 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -352,7 +352,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:
> @@ -495,7 +505,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);
> + }
Alistair, does this call to add some FIELD_1CLEAR() API?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
2022-08-30 12:33 ` Philippe Mathieu-Daudé
(?)
@ 2022-08-30 15:50 ` Alistair Francis
-1 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-08-30 15:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Wilfred Mallawa, Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers, Wilfred Mallawa
On Tue, Aug 30, 2022 at 2:37 PM Philippe Mathieu-Daudé via
<qemu-devel@nongnu.org> wrote:
>
> On 23/8/22 08:12, Wilfred Mallawa wrote:
> > 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 d52b193a1a..40d401ad47 100644
> > --- a/hw/ssi/ibex_spi_host.c
> > +++ b/hw/ssi/ibex_spi_host.c
> > @@ -352,7 +352,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:
> > @@ -495,7 +505,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);
> > + }
>
> Alistair, does this call to add some FIELD_1CLEAR() API?
Yeah, that's probably something useful to add! Good idea
Alistair
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs
2022-08-23 6:11 [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa
` (3 preceding siblings ...)
2022-08-23 6:12 ` [PATCH v4 4/4] hw/ssi: ibex_spi: update reg addr Wilfred Mallawa
@ 2022-09-08 9:08 ` Alistair Francis
4 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-09-08 9:08 UTC (permalink / raw)
To: Wilfred Mallawa
Cc: Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers, Wilfred Mallawa
On Tue, Aug 23, 2022 at 8:12 AM Wilfred Mallawa
<wilfred.mallawa@opensource.wdc.com> wrote:
>
> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>
> Patch V4 fixes up:
> - Fixup missing register field clearing on tx/rx_fifo_reset() 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
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> hw/ssi/ibex_spi_host.c | 174 ++++++++++++++++++++-------------
> include/hw/ssi/ibex_spi_host.h | 4 +-
> 2 files changed, 106 insertions(+), 72 deletions(-)
>
> --
> 2.37.2
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue
2022-08-23 6:12 ` [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa
2022-08-30 9:11 ` Alistair Francis
@ 2022-09-08 11:30 ` Alistair Francis
1 sibling, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2022-09-08 11:30 UTC (permalink / raw)
To: Wilfred Mallawa
Cc: Alistair Francis, open list:RISC-V,
qemu-devel@nongnu.org Developers, Wilfred Mallawa, Andrew Jones
On Tue, Aug 23, 2022 at 8:13 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: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> hw/ssi/ibex_spi_host.c | 132 +++++++++++++++++++++--------------------
> 1 file changed, 68 insertions(+), 64 deletions(-)
>
> diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> index 601041d719..d52b193a1a 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -108,18 +108,22 @@ 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, RXFULL, 0);
> + 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, TXFULL, 0);
> + data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> + s->regs[IBEX_SPI_HOST_STATUS] = data;
> }
>
> static void ibex_spi_host_reset(DeviceState *dev)
> @@ -162,37 +166,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 +209,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 +234,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 +265,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 +342,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;
data is unused.
It's a good idea to run a full build without --disable-werror to
ensure there are no warnings
Alistair
> uint8_t txqd_len;
>
> trace_ibex_spi_host_write(addr, size, val64);
> @@ -397,22 +399,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 +456,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 +468,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] 11+ messages in thread
end of thread, other threads:[~2022-09-08 11:34 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-23 6:11 [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Wilfred Mallawa
2022-08-23 6:12 ` [PATCH v4 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host Wilfred Mallawa
2022-08-23 6:12 ` [PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue Wilfred Mallawa
2022-08-30 9:11 ` Alistair Francis
2022-09-08 11:30 ` Alistair Francis
2022-08-23 6:12 ` [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality Wilfred Mallawa
2022-08-30 12:33 ` Philippe Mathieu-Daudé via
2022-08-30 12:33 ` Philippe Mathieu-Daudé
2022-08-30 15:50 ` Alistair Francis
2022-08-23 6:12 ` [PATCH v4 4/4] hw/ssi: ibex_spi: update reg addr Wilfred Mallawa
2022-09-08 9:08 ` [PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs Alistair Francis
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.