qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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é via
  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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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é via
  2022-08-30 15:50     ` Alistair Francis
  0 siblings, 1 reply; 10+ 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] 10+ messages in thread

* Re: [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
  2022-08-30 12:33   ` Philippe Mathieu-Daudé via
@ 2022-08-30 15:50     ` Alistair Francis
  0 siblings, 0 replies; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread

end of thread, other threads:[~2022-09-08 11:34 UTC | newest]

Thread overview: 10+ 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).