Re: [PATCH 5/7] scsi: mac_scsi: Fix pseudo DMA implementation, take 2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 2 Jun 2019, Geert Uytterhoeven wrote:

Hi Finn,

On Sun, Jun 2, 2019 at 3:29 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> 
wrote:
A system bus error during a PDMA transfer can mess up the calculation 
of the transfer residual (the PDMA handshaking hardware lacks a byte 
counter). This results in data corruption.

The algorithm in this patch anticipates a bus error by starting each 
transfer with a MOVE.B instruction. If a bus error is caught the 
transfer will be retried. If a bus error is caught later in the 
transfer (for a MOVE.W instruction) the transfer gets failed and 
subsequent requests for that target will use PIO instead of PDMA.

This avoids the "!REQ and !ACK" error so the severity level of that 
message is reduced to KERN_DEBUG.

Cc: Michael Schmitz <schmitzmic@xxxxxxxxx>
Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx # v4.14+
Fixes: 3a0f64bfa907 ("mac_scsi: Fix pseudo DMA implementation")
Reported-by: Chris Jones <chris@xxxxxxxxxxxxxxxx>
Tested-by: Stan Johnson <userm57@xxxxxxxxx>
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx>

Thanks for your patch!

---
 arch/m68k/include/asm/mac_pdma.h | 179 +++++++++++++++++++++++++++
 drivers/scsi/mac_scsi.c          | 201 ++++++++-----------------------

Why have you moved the PDMA implementation to a header file under 
arch/m68k/? Do you intend to reuse it by other drivers?


There are a couple of reasons: the mac_esp driver also uses PDMA and the 
NuBus PowerMac port also uses mac_scsi.c. OTOH, the NuBus PowerMac port is 
still out-of-tree, and it is unclear whether the mac_esp driver will ever 
benefit from this code.

If not, please keep it in the driver, so (a) you don't need an ack from 
me ;-), and (b) your change may be easier to review.


I take your wink to mean that you don't want to ask the SCSI maintainers 
to review m68k asm. Putting aside the code review process for a moment, do 
you have an opinion on the most logical way to organise this sort of code, 
from the point-of-view of maintainability, re-usability, readability etc.?

Thanks.

-- 

Thanks!

Gr{oetje,eeting}s,

                        Geert





[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux