diff mbox series

[v2,09/42] esp: introduce esp_get_tc() and esp_set_tc()

Message ID 20210209193018.31339-10-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series esp: consolidate PDMA transfer buffers and other fixes | expand

Commit Message

Mark Cave-Ayland Feb. 9, 2021, 7:29 p.m. UTC 
This simplifies reading and writing the TC register value without having to
manually shift each individual 8-bit value.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/scsi/esp.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

Comments

Laurent Vivier March 1, 2021, 9:24 p.m. UTC | #1 
Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> This simplifies reading and writing the TC register value without having to
> manually shift each individual 8-bit value.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/scsi/esp.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index e82e75d490..3a39450930 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -98,6 +98,24 @@ void esp_request_cancelled(SCSIRequest *req)
>      }
>  }
>  
> +static uint32_t esp_get_tc(ESPState *s)
> +{
> +    uint32_t dmalen;
> +
> +    dmalen = s->rregs[ESP_TCLO];
> +    dmalen |= s->rregs[ESP_TCMID] << 8;
> +    dmalen |= s->rregs[ESP_TCHI] << 16;
> +
> +    return dmalen;
> +}
> +
> +static void esp_set_tc(ESPState *s, uint32_t dmalen)
> +{
> +    s->rregs[ESP_TCLO] = dmalen & 0xff;

The "& 0xff" is not needed as rregs is uint8_t.

> +    s->rregs[ESP_TCMID] = dmalen >> 8;
> +    s->rregs[ESP_TCHI] = dmalen >> 16;
> +}
> +
>  static void set_pdma(ESPState *s, enum pdma_origin_id origin,
>                       uint32_t index, uint32_t len)
>  {
> @@ -157,9 +175,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t buflen)
>  
>      target = s->wregs[ESP_WBUSID] & BUSID_DID;
>      if (s->dma) {
> -        dmalen = s->rregs[ESP_TCLO];
> -        dmalen |= s->rregs[ESP_TCMID] << 8;
> -        dmalen |= s->rregs[ESP_TCHI] << 16;
> +        dmalen = esp_get_tc(s);
>          if (dmalen > buflen) {
>              return 0;
>          }
> @@ -348,9 +364,7 @@ static void esp_dma_done(ESPState *s)
>      s->rregs[ESP_RINTR] = INTR_BS;
>      s->rregs[ESP_RSEQ] = 0;
>      s->rregs[ESP_RFLAGS] = 0;
> -    s->rregs[ESP_TCLO] = 0;
> -    s->rregs[ESP_TCMID] = 0;
> -    s->rregs[ESP_TCHI] = 0;
> +    esp_set_tc(s, 0);
>      esp_raise_irq(s);
>  }
>  
> @@ -536,9 +550,7 @@ static void handle_ti(ESPState *s)
>          return;
>      }
>  
> -    dmalen = s->rregs[ESP_TCLO];
> -    dmalen |= s->rregs[ESP_TCMID] << 8;
> -    dmalen |= s->rregs[ESP_TCHI] << 16;
> +    dmalen = esp_get_tc(s);
>      if (dmalen == 0) {
>          dmalen = 0x10000;
>      }
> @@ -889,9 +901,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
>  
>      trace_esp_pdma_write(size);
>  
> -    dmalen = s->rregs[ESP_TCLO];
> -    dmalen |= s->rregs[ESP_TCMID] << 8;
> -    dmalen |= s->rregs[ESP_TCHI] << 16;
> +    dmalen = esp_get_tc(s);
>      if (dmalen == 0 || s->pdma_len == 0) {
>          return;
>      }
> @@ -908,9 +918,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
>          dmalen -= 2;
>          break;
>      }
> -    s->rregs[ESP_TCLO] = dmalen & 0xff;
> -    s->rregs[ESP_TCMID] = dmalen >> 8;
> -    s->rregs[ESP_TCHI] = dmalen >> 16;
> +    esp_set_tc(s, dmalen);
>      if (s->pdma_len == 0 && s->pdma_cb) {
>          esp_lower_drq(s);
>          s->pdma_cb(s);
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Mark Cave-Ayland March 3, 2021, 8:35 a.m. UTC | #2 
On 01/03/2021 21:24, Laurent Vivier wrote:

> Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
>> This simplifies reading and writing the TC register value without having to
>> manually shift each individual 8-bit value.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/scsi/esp.c | 38 +++++++++++++++++++++++---------------
>>   1 file changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
>> index e82e75d490..3a39450930 100644
>> --- a/hw/scsi/esp.c
>> +++ b/hw/scsi/esp.c
>> @@ -98,6 +98,24 @@ void esp_request_cancelled(SCSIRequest *req)
>>       }
>>   }
>>   
>> +static uint32_t esp_get_tc(ESPState *s)
>> +{
>> +    uint32_t dmalen;
>> +
>> +    dmalen = s->rregs[ESP_TCLO];
>> +    dmalen |= s->rregs[ESP_TCMID] << 8;
>> +    dmalen |= s->rregs[ESP_TCHI] << 16;
>> +
>> +    return dmalen;
>> +}
>> +
>> +static void esp_set_tc(ESPState *s, uint32_t dmalen)
>> +{
>> +    s->rregs[ESP_TCLO] = dmalen & 0xff;
> 
> The "& 0xff" is not needed as rregs is uint8_t.

That's true - I think it's just done that way since that was how it was originally 
written in the section below where I copied it from. I'll remove it in the next version.

>> +    s->rregs[ESP_TCMID] = dmalen >> 8;
>> +    s->rregs[ESP_TCHI] = dmalen >> 16;
>> +}
>> +
>>   static void set_pdma(ESPState *s, enum pdma_origin_id origin,
>>                        uint32_t index, uint32_t len)
>>   {
>> @@ -157,9 +175,7 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t buflen)
>>   
>>       target = s->wregs[ESP_WBUSID] & BUSID_DID;
>>       if (s->dma) {
>> -        dmalen = s->rregs[ESP_TCLO];
>> -        dmalen |= s->rregs[ESP_TCMID] << 8;
>> -        dmalen |= s->rregs[ESP_TCHI] << 16;
>> +        dmalen = esp_get_tc(s);
>>           if (dmalen > buflen) {
>>               return 0;
>>           }
>> @@ -348,9 +364,7 @@ static void esp_dma_done(ESPState *s)
>>       s->rregs[ESP_RINTR] = INTR_BS;
>>       s->rregs[ESP_RSEQ] = 0;
>>       s->rregs[ESP_RFLAGS] = 0;
>> -    s->rregs[ESP_TCLO] = 0;
>> -    s->rregs[ESP_TCMID] = 0;
>> -    s->rregs[ESP_TCHI] = 0;
>> +    esp_set_tc(s, 0);
>>       esp_raise_irq(s);
>>   }
>>   
>> @@ -536,9 +550,7 @@ static void handle_ti(ESPState *s)
>>           return;
>>       }
>>   
>> -    dmalen = s->rregs[ESP_TCLO];
>> -    dmalen |= s->rregs[ESP_TCMID] << 8;
>> -    dmalen |= s->rregs[ESP_TCHI] << 16;
>> +    dmalen = esp_get_tc(s);
>>       if (dmalen == 0) {
>>           dmalen = 0x10000;
>>       }
>> @@ -889,9 +901,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
>>   
>>       trace_esp_pdma_write(size);
>>   
>> -    dmalen = s->rregs[ESP_TCLO];
>> -    dmalen |= s->rregs[ESP_TCMID] << 8;
>> -    dmalen |= s->rregs[ESP_TCHI] << 16;
>> +    dmalen = esp_get_tc(s);
>>       if (dmalen == 0 || s->pdma_len == 0) {
>>           return;
>>       }
>> @@ -908,9 +918,7 @@ static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
>>           dmalen -= 2;
>>           break;
>>       }
>> -    s->rregs[ESP_TCLO] = dmalen & 0xff;
>> -    s->rregs[ESP_TCMID] = dmalen >> 8;
>> -    s->rregs[ESP_TCHI] = dmalen >> 16;
>> +    esp_set_tc(s, dmalen);
>>       if (s->pdma_len == 0 && s->pdma_cb) {
>>           esp_lower_drq(s);
>>           s->pdma_cb(s);
>>
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index e82e75d490..3a39450930 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -98,6 +98,24 @@  void esp_request_cancelled(SCSIRequest *req)
     }
 }
 
+static uint32_t esp_get_tc(ESPState *s)
+{
+    uint32_t dmalen;
+
+    dmalen = s->rregs[ESP_TCLO];
+    dmalen |= s->rregs[ESP_TCMID] << 8;
+    dmalen |= s->rregs[ESP_TCHI] << 16;
+
+    return dmalen;
+}
+
+static void esp_set_tc(ESPState *s, uint32_t dmalen)
+{
+    s->rregs[ESP_TCLO] = dmalen & 0xff;
+    s->rregs[ESP_TCMID] = dmalen >> 8;
+    s->rregs[ESP_TCHI] = dmalen >> 16;
+}
+
 static void set_pdma(ESPState *s, enum pdma_origin_id origin,
                      uint32_t index, uint32_t len)
 {
@@ -157,9 +175,7 @@  static uint32_t get_cmd(ESPState *s, uint8_t *buf, uint8_t buflen)
 
     target = s->wregs[ESP_WBUSID] & BUSID_DID;
     if (s->dma) {
-        dmalen = s->rregs[ESP_TCLO];
-        dmalen |= s->rregs[ESP_TCMID] << 8;
-        dmalen |= s->rregs[ESP_TCHI] << 16;
+        dmalen = esp_get_tc(s);
         if (dmalen > buflen) {
             return 0;
         }
@@ -348,9 +364,7 @@  static void esp_dma_done(ESPState *s)
     s->rregs[ESP_RINTR] = INTR_BS;
     s->rregs[ESP_RSEQ] = 0;
     s->rregs[ESP_RFLAGS] = 0;
-    s->rregs[ESP_TCLO] = 0;
-    s->rregs[ESP_TCMID] = 0;
-    s->rregs[ESP_TCHI] = 0;
+    esp_set_tc(s, 0);
     esp_raise_irq(s);
 }
 
@@ -536,9 +550,7 @@  static void handle_ti(ESPState *s)
         return;
     }
 
-    dmalen = s->rregs[ESP_TCLO];
-    dmalen |= s->rregs[ESP_TCMID] << 8;
-    dmalen |= s->rregs[ESP_TCHI] << 16;
+    dmalen = esp_get_tc(s);
     if (dmalen == 0) {
         dmalen = 0x10000;
     }
@@ -889,9 +901,7 @@  static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
 
     trace_esp_pdma_write(size);
 
-    dmalen = s->rregs[ESP_TCLO];
-    dmalen |= s->rregs[ESP_TCMID] << 8;
-    dmalen |= s->rregs[ESP_TCHI] << 16;
+    dmalen = esp_get_tc(s);
     if (dmalen == 0 || s->pdma_len == 0) {
         return;
     }
@@ -908,9 +918,7 @@  static void sysbus_esp_pdma_write(void *opaque, hwaddr addr,
         dmalen -= 2;
         break;
     }
-    s->rregs[ESP_TCLO] = dmalen & 0xff;
-    s->rregs[ESP_TCMID] = dmalen >> 8;
-    s->rregs[ESP_TCHI] = dmalen >> 16;
+    esp_set_tc(s, dmalen);
     if (s->pdma_len == 0 && s->pdma_cb) {
         esp_lower_drq(s);
         s->pdma_cb(s);