diff mbox series

[v3,11/17] esp.c: rework esp_cdb_length() into esp_cdb_ready()

Message ID 20240324191707.623175-12-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series esp: avoid explicit setting of DRQ within ESP state machine | expand

Commit Message

Mark Cave-Ayland March 24, 2024, 7:17 p.m. UTC 
The esp_cdb_length() function is only used as part of a calculation to determine
whether the cmdfifo contains an entire SCSI CDB. Rework esp_cdb_length() into a
new esp_cdb_ready() function which both enables us to handle the case where
scsi_cdb_length() returns -1, plus simplify the logic for its callers.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/esp.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

Comments

Philippe Mathieu-Daudé April 2, 2024, 11:38 a.m. UTC | #1 
On 24/3/24 20:17, Mark Cave-Ayland wrote:
> The esp_cdb_length() function is only used as part of a calculation to determine
> whether the cmdfifo contains an entire SCSI CDB. Rework esp_cdb_length() into a
> new esp_cdb_ready() function which both enables us to handle the case where
> scsi_cdb_length() returns -1, plus simplify the logic for its callers.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/scsi/esp.c | 30 ++++++++++++++----------------
>   1 file changed, 14 insertions(+), 16 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff mbox series

Patch

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f3aa5364cf..f47abc36d6 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -425,20 +425,20 @@  static void write_response(ESPState *s)
     }
 }
 
-static int esp_cdb_length(ESPState *s)
+static bool esp_cdb_ready(ESPState *s)
 {
+    int len = fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset;
     const uint8_t *pbuf;
-    int cmdlen, len;
+    int cdblen;
 
-    cmdlen = fifo8_num_used(&s->cmdfifo);
-    if (cmdlen < s->cmdfifo_cdb_offset) {
-        return 0;
+    if (len <= 0) {
+        return false;
     }
 
-    pbuf = fifo8_peek_buf(&s->cmdfifo, cmdlen, NULL);
-    len = scsi_cdb_length((uint8_t *)&pbuf[s->cmdfifo_cdb_offset]);
+    pbuf = fifo8_peek_buf(&s->cmdfifo, len, NULL);
+    cdblen = scsi_cdb_length((uint8_t *)&pbuf[s->cmdfifo_cdb_offset]);
 
-    return len;
+    return cdblen < 0 ? false : (len >= cdblen);
 }
 
 static void esp_dma_ti_check(ESPState *s)
@@ -806,10 +806,9 @@  static void esp_do_nodma(ESPState *s)
             trace_esp_handle_ti_cmd(cmdlen);
 
             /* CDB may be transferred in one or more TI commands */
-            if (esp_cdb_length(s) && esp_cdb_length(s) ==
-                fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset) {
-                    /* Command has been received */
-                    do_cmd(s);
+            if (esp_cdb_ready(s)) {
+                /* Command has been received */
+                do_cmd(s);
             } else {
                 /*
                  * If data was transferred from the FIFO then raise bus
@@ -832,10 +831,9 @@  static void esp_do_nodma(ESPState *s)
             fifo8_push_all(&s->cmdfifo, buf, len);
 
             /* Handle when DMA transfer is terminated by non-DMA FIFO write */
-            if (esp_cdb_length(s) && esp_cdb_length(s) ==
-                fifo8_num_used(&s->cmdfifo) - s->cmdfifo_cdb_offset) {
-                    /* Command has been received */
-                    do_cmd(s);
+            if (esp_cdb_ready(s)) {
+                /* Command has been received */
+                do_cmd(s);
             }
             break;