Re: [PATCH] esp_scsi: Clear Transfer Count registers before PIO transfers

From: Finn Thain
Date: Sun Nov 17 2019 - 18:13:57 EST

On Sun, 17 Nov 2019, Kars de Jong wrote:

> Hi Finn,
> Op za 16 nov. 2019 om 04:36 schreef Finn Thain
> <fthain@xxxxxxxxxxxxxxxxxxx>:
> >
> > The zorro_esp driver uses both PIO and DMA transfers. If a failed DMA
> > transfer happened to be followed by a PIO transfer, the TCLOW and
> > TCMED registers would not get cleared. It is theoretically possible
> > that the stale value from the transfer counter or the TCLOW/TCMED
> > registers could then be used by the controller and the driver. Avoid
> > that by clearing these registers before each PIO transfer.
> Are you sure this is really needed?

No. I think it improves robustness and correctness.

I would be interested to know whether there is any measurable performance
impact on zorro_esp.

> The only [time when] the driver reads these registers is after a data
> transfer. These are done using DMA on all Zorro boards, so I don't think
> there's a risk of stale values from a PIO transfer there.

I'm not entirely sure that the chip is unaffected by stale counter values.

(Stale transfer counter values are distinct from stale transfer count
register values. Both are addressed by the patch.)

If there are DMA controllers out there that can't do very short transfers
then this objection would seem to be invalid, because the "DMA length is
zero!" issue could be tackled using PIO.

> The only place the controller reads these registers is when a DMA
> command is issued. The only place where that is done is in the zorro_esp
> send_dma_command() functions.

Aren't you overlooking all of the ESP_CMD_DMA flags in the core driver?

Thanks for your review.


> These all set both registers explicitly before issuing the DMA command
> to the controller, so I don't think there's a risk there either.
> Kind regards,
> Kars.