Re: [PATCH v2 11/42] esp: apply transfer length adjustment when STC is zero at TC load time

[Mark Cave-Ayland]

On 01/03/2021 21:35, Laurent Vivier wrote:

> Le 09/02/2021 à 20:29, Mark Cave-Ayland a écrit :
> > Perform the length adjustment whereby a value of 0 in the STC represents
> > a transfer length of 0x10000 at the point where the TC is loaded at the
> > start of a DMA command rather than just when a TI (Transfer Information)
> > command is executed. This better matches the description as given in the
> > datasheet.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/scsi/esp.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> > index a1acc2c9bd..02b7876394 100644
> > --- a/hw/scsi/esp.c
> > +++ b/hw/scsi/esp.c
> > @@ -562,9 +562,6 @@ static void handle_ti(ESPState *s)
> >       }
> >   
> >       dmalen = esp_get_tc(s);
> > -    if (dmalen == 0) {
> > -        dmalen = 0x10000;
> > -    }
> >       s->dma_counter = dmalen;
> >   
> >       if (s->do_cmd) {
> > @@ -699,7 +696,11 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t 
> > val)
> >           if (val & CMD_DMA) {
> >               s->dma = 1;
> >               /* Reload DMA counter.  */
> > -            esp_set_tc(s, esp_get_stc(s));
> > +            if (esp_get_stc(s) == 0) {
> > +                esp_set_tc(s, 0x10000);
> > +            } else {
> > +                esp_set_tc(s, esp_get_stc(s));
> > +            }
>
> More fun?
>
>      esp_set_tc(s, esp_get_stc(s) ?: 0x10000);

I've just tried this, and it feels less intuitive to read when skimming over the 
code. I'd prefer to keep this in its current form and just accept that my coding 
style is more functional than artistic :/

> >           } else {
> >               s->dma = 0;
> >           }
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>


ATB,

Mark.