This simplifies reading the STC register value without having to manually shift
each individual 8-bit value.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/esp.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 3a39450930..a1acc2c9bd 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -116,6 +116,17 @@ static void esp_set_tc(ESPState *s, uint32_t dmalen)
     s->rregs[ESP_TCHI] = dmalen >> 16;
 }
 
+static uint32_t esp_get_stc(ESPState *s)
+{
+    uint32_t dmalen;
+
+    dmalen = s->wregs[ESP_TCLO];
+    dmalen |= s->wregs[ESP_TCMID] << 8;
+    dmalen |= s->wregs[ESP_TCHI] << 16;
+
+    return dmalen;
+}
+
 static void set_pdma(ESPState *s, enum pdma_origin_id origin,
                      uint32_t index, uint32_t len)
 {
@@ -688,9 +699,7 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val)
         if (val & CMD_DMA) {
             s->dma = 1;
             /* Reload DMA counter.  */
-            s->rregs[ESP_TCLO] = s->wregs[ESP_TCLO];
-            s->rregs[ESP_TCMID] = s->wregs[ESP_TCMID];
-            s->rregs[ESP_TCHI] = s->wregs[ESP_TCHI];
+            esp_set_tc(s, esp_get_stc(s));
         } else {
             s->dma = 0;
         }
-- 
2.20.1

On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
> This simplifies reading the STC register value without having to manually shift
> each individual 8-bit value.

If possible repeat the subject so the sentence is easier to understand.

> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/scsi/esp.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

On 10/02/2021 22:33, Philippe Mathieu-Daudé wrote:

> On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
>> This simplifies reading the STC register value without having to manually shift
>> each individual 8-bit value.
> 
> If possible repeat the subject so the sentence is easier to understand.

I've always read commit messages as summary followed detail, so I've tended to avoid 
repetition if the context is obvious from the summary (a quick glance through my 
inbox suggest that quite a few authors also do the same).

Perhaps adding in the word "function" would help readability here, e.g. "This 
function simplifies reading the STC register value..."?

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


ATB,

Mark.

On 2/11/21 8:53 AM, Mark Cave-Ayland wrote:
> On 10/02/2021 22:33, Philippe Mathieu-Daudé wrote:
> 
>> On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
>>> This simplifies reading the STC register value without having to
>>> manually shift
>>> each individual 8-bit value.
>>
>> If possible repeat the subject so the sentence is easier to understand.
> 
> I've always read commit messages as summary followed detail, so I've
> tended to avoid repetition if the context is obvious from the summary (a
> quick glance through my inbox suggest that quite a few authors also do
> the same).

Not because other do it means it is a good practice ;)

I suppose it depends on personal review workflow and email client used
or gitk layout. In the one I use the commit description is displayed
first, so I have to look elsewhere to prepend the patch subject which
is using another font.
I'm using Thunderbird on Fedora and X1 carbon but had to do some config
change in the default config because the font was too small, unreadable,
then I added some 1.5x zoom factor. It always looked ugly. Apparently
checking that again it seems the Fedora I used was not supporting these
now displays well but now it should work OK, but I'm still using this
old config. Well, my bad. Not in mood to reinstall. Forget my comment
about having "atomic" commit descriptions then.

> 
> Perhaps adding in the word "function" would help readability here, e.g.
> "This function simplifies reading the STC register value..."?

Sounds better to me :)

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

On 11/02/2021 10:07, Philippe Mathieu-Daudé wrote:
> On 2/11/21 8:53 AM, Mark Cave-Ayland wrote:
>> On 10/02/2021 22:33, Philippe Mathieu-Daudé wrote:
>>
>>> On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
>>>> This simplifies reading the STC register value without having to
>>>> manually shift
>>>> each individual 8-bit value.
>>>
>>> If possible repeat the subject so the sentence is easier to understand.
>>
>> I've always read commit messages as summary followed detail, so I've
>> tended to avoid repetition if the context is obvious from the summary (a
>> quick glance through my inbox suggest that quite a few authors also do
>> the same).
> 
> Not because other do it means it is a good practice ;)
> 
> I suppose it depends on personal review workflow and email client used
> or gitk layout. In the one I use the commit description is displayed
> first, so I have to look elsewhere to prepend the patch subject which
> is using another font.
> I'm using Thunderbird on Fedora and X1 carbon but had to do some config
> change in the default config because the font was too small, unreadable,
> then I added some 1.5x zoom factor. It always looked ugly. Apparently
> checking that again it seems the Fedora I used was not supporting these
> now displays well but now it should work OK, but I'm still using this
> old config. Well, my bad. Not in mood to reinstall. Forget my comment
> about having "atomic" commit descriptions then.
> 
>>
>> Perhaps adding in the word "function" would help readability here, e.g.
>> "This function simplifies reading the STC register value..."?
> 
> Sounds better to me :)

Okay - I've made this change, along with a similar change to the patch before which 
had similar wording.

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


ATB,

Mark.

Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> This simplifies reading the STC register value without having to manually shift
> each individual 8-bit value.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/scsi/esp.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 3a39450930..a1acc2c9bd 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -116,6 +116,17 @@ static void esp_set_tc(ESPState *s, uint32_t dmalen)
>      s->rregs[ESP_TCHI] = dmalen >> 16;
>  }
>  
> +static uint32_t esp_get_stc(ESPState *s)
> +{
> +    uint32_t dmalen;
> +
> +    dmalen = s->wregs[ESP_TCLO];
> +    dmalen |= s->wregs[ESP_TCMID] << 8;
> +    dmalen |= s->wregs[ESP_TCHI] << 16;
> +
> +    return dmalen;
> +}
> +
>  static void set_pdma(ESPState *s, enum pdma_origin_id origin,
>                       uint32_t index, uint32_t len)
>  {
> @@ -688,9 +699,7 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t val)
>          if (val & CMD_DMA) {
>              s->dma = 1;
>              /* Reload DMA counter.  */
> -            s->rregs[ESP_TCLO] = s->wregs[ESP_TCLO];
> -            s->rregs[ESP_TCMID] = s->wregs[ESP_TCMID];
> -            s->rregs[ESP_TCHI] = s->wregs[ESP_TCHI];
> +            esp_set_tc(s, esp_get_stc(s));
>          } else {
>              s->dma = 0;
>          }
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>