mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Finn Thain <fthain@telegraphics.com.au>
To: Kars de Jong <jongk@linux-m68k.org>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Michael Schmitz <schmitzmic@gmail.com>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] esp_scsi: Clear Transfer Count registers before PIO transfers
Date: Mon, 18 Nov 2019 10:13:08 +1100 (AEDT)	[thread overview]
Message-ID: <alpine.LNX.2.21.1.1911180947020.8@nippy.intranet> (raw)
In-Reply-To: <CACz-3rjHAyi6kMQ6j9YALLm1ApYrsqKiTnGNPUhxqqEuRJ9TjQ@mail.gmail.com>

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

> Hi Finn,
> 
> Op za 16 nov. 2019 om 04:36 schreef Finn Thain 
> <fthain@telegraphics.com.au>:
> >
> > 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.
> 

  parent reply	other threads:[~2019-11-17 23:13 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-16  3:36 Finn Thain
2019-11-16  4:30 ` Michael Schmitz
2019-11-16 23:02   ` Finn Thain
2019-11-17  6:26 ` Kars de Jong
2019-11-17  6:29   ` Kars de Jong
2019-11-17 23:13   ` Finn Thain [this message]
2019-11-17 23:26     ` Finn Thain
2019-11-18  8:13     ` Kars de Jong
2019-11-18 23:04       ` Finn Thain
2019-11-19 23:12         ` Finn Thain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LNX.2.21.1.1911180947020.8@nippy.intranet \
    --to=fthain@telegraphics.com.au \
    --cc=jejb@linux.ibm.com \
    --cc=jongk@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=schmitzmic@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®