Re: [PATCH 09/88] esp: update TC check logic in do_dma_pdma_cb() to check for TC == 0

[Paolo Bonzini]

On Fri, Jan 12, 2024 at 1:55 PM Mark Cave-Ayland
<mark.cave-ayl...@ilande.co.uk> wrote:
>
> Invert the logic so that the end of DMA transfer check becomes one that checks
> for TC == 0 in the from device path in do_dma_pdma_cb().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> ---
>  hw/scsi/esp.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index fecfef7c89..63c828c1b2 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -602,21 +602,19 @@ static void do_dma_pdma_cb(ESPState *s)
>              return;
>          }
>
> -        if (esp_get_tc(s) != 0) {
> -            /* Copy device data to FIFO */
> -            len = MIN(s->async_len, esp_get_tc(s));
> -            len = MIN(len, fifo8_num_free(&s->fifo));
> -            fifo8_push_all(&s->fifo, s->async_buf, len);
> -            s->async_buf += len;
> -            s->async_len -= len;
> -            s->ti_size -= len;
> -            esp_set_tc(s, esp_get_tc(s) - len);
> -            return;
> +        if (esp_get_tc(s) == 0) {
> +            esp_lower_drq(s);
> +            esp_dma_done(s);
>          }

I'm only doing a cursory review, but shouldn't there be a return here?

Paolo

>
> -        /* Partially filled a scsi buffer. Complete immediately.  */
> -        esp_lower_drq(s);
> -        esp_dma_done(s);
> +        /* Copy device data to FIFO */
> +        len = MIN(s->async_len, esp_get_tc(s));
> +        len = MIN(len, fifo8_num_free(&s->fifo));
> +        fifo8_push_all(&s->fifo, s->async_buf, len);
> +        s->async_buf += len;
> +        s->async_len -= len;
> +        s->ti_size -= len;
> +        esp_set_tc(s, esp_get_tc(s) - len);
>      }
>  }
>
> --
> 2.39.2
>