Commit Graph

151 Commits

Author SHA1 Message Date
Finn Thain e5d55d1abc ncr5380: Remove BOARD_REQUIRES_NO_DELAY macro
The io_recovery_delay macro is intended to insert a microsecond delay
between the chip register accesses that begin a DMA operation. This
is reportedly needed for some ISA boards.

Reverse the sense of the macro test so that in the common case,
where no delay is required, drivers need not define the macro.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-04-11 16:57:09 -04:00
Finn Thain f825e40b23 ncr5380: Remove PSEUDO_DMA macro
For those wrapper drivers which only implement Programmed IO, have
NCR5380_dma_xfer_len() evaluate to zero. That allows PDMA to be easily
disabled at run-time and so the PSEUDO_DMA macro is no longer needed.

Also remove the spin counters used for debugging pseudo DMA drivers.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-04-11 16:57:09 -04:00
Finn Thain 1bb4600245 ncr5380: Disable the DMA errata workaround flag by default
The only chip that needs the workarounds enabled is an early NMOS
device. That means that the common case is to disable them.

Unfortunately the sense of the flag is such that it has to be set
for the common case.

Rename the flag so that zero can be used to mean "no errata workarounds
needed". This simplifies the code.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-04-11 16:57:09 -04:00
Finn Thain e4dec6806a ncr5380: Remove REAL_DMA and REAL_DMA_POLL macros
For the NCR5380.c core driver, these macros are never used.
If REAL_DMA were to be defined, compilation would fail.

For the atari_NCR5380.c core driver, REAL_DMA is always defined.

Hence these macros are pointless.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-04-11 16:57:09 -04:00
Finn Thain 7e9ec8d9cc ncr5380: Remove FLAG_NO_PSEUDO_DMA where possible
Drivers that define PSEUDO_DMA also define NCR5380_dma_xfer_len.
The core driver must call NCR5380_dma_xfer_len which means
FLAG_NO_PSEUDO_DMA can be eradicated from the core driver.

dmx3191d doesn't define PSEUDO_DMA and has no use for FLAG_NO_PSEUDO_DMA,
so remove it there also.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-04-11 16:57:09 -04:00
Finn Thain 8d5dbec3bc ncr5380: Call scsi_eh_prep_cmnd() and scsi_eh_restore_cmnd() as and when appropriate
This bug causes the wrong command to have its sense pointer overwritten,
which sometimes leads to a NULL pointer deref. Fix this by checking which
command is being requeued before restoring the scsi_eh_save data.

It turns out that some targets will disconnect a REQUEST SENSE command.
The autosense algorithm doesn't anticipate this. Hence multiple commands
can end up undergoing autosense simultaneously, and they will all try to
use the same scsi_eh_save struct, which won't work. Defer autosense when
the scsi_eh_save storage is in use by another command.

Fixes: f27db8eb98 ("ncr5380: Fix autosense bugs")
Reported-and-tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: <stable@vger.kernel.org> # 4.5
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-03-01 09:38:58 -05:00
Finn Thain ccf6efd783 ncr5380: Fix NCR5380_select() EH checks and result handling
Add missing checks for EH abort during arbitration and selection.
Rework the handling of NCR5380_select() result to improve clarity.

Fixes: 707d62b37f ("ncr5380: Fix EH during arbitration and selection")
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: <stable@vger.kernel.org> # 4.5
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-03-01 09:38:50 -05:00
Finn Thain dc18396528 ncr5380: Forget aborted commands
The list structures and related logic used in the NCR5380 driver mean that
a command cannot be queued twice (i.e. can't appear on more than one queue
and can't appear on the same queue more than once).

The abort handler must forget the command so that the mid-layer can re-use
it. E.g. the ML may send it back to the LLD via via scsi_eh_get_sense().

Fix this and also fix two error paths, so that commands get forgotten iff
completed.

Fixes: 8b00c3d5d4 ("ncr5380: Implement new eh_abort_handler")
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: <stable@vger.kernel.org> # 4.5
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-03-01 09:38:25 -05:00
Finn Thain 71a00593ec ncr5380: Dont re-enter NCR5380_select()
Calling NCR5380_select() from the abort handler causes various problems.
Firstly, it means potentially re-entering NCR5380_select(). Secondly, it
means that the lock is released, which permits the EH handlers to be
re-entered. The combination results in crashes. Don't do it.

Fixes: 8b00c3d5d4 ("ncr5380: Implement new eh_abort_handler")
Reported-and-tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: <stable@vger.kernel.org> # 4.5
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-03-01 09:38:14 -05:00
Finn Thain 1678847ec9 ncr5380: Dont release lock for PIO transfer
The calls to NCR5380_transfer_pio() for DATA IN and DATA OUT phases will
modify cmd->SCp.this_residual, cmd->SCp.ptr and cmd->SCp.buffer. That
works as long as EH does not intervene, which became possible in
atari_NCR5380.c when I changed the locking to bring it closer to
NCR5380.c.

If error recovery aborts the command, the scsi_cmnd in question and its
buffer will be returned to the mid-layer. So the transfer has to cease,
but it can't be stopped by the initiator because the target controls the
bus phase.

The problem does not arise if the lock is not released. That was fine for
atari_scsi, because it implements DMA. For the other drivers, we have to
release the lock and re-enable interrupts for long PIO data transfers.

The solution is to split the transfer into small chunks. In between chunks
the main loop releases the lock and re-enables interrupts. Thus interrupts
can be serviced and eh_bus_reset_handler can intervene if need be.

This fixes an oops in NCR5380_transfer_pio() that can happen when the EH
abort handler is invoked during DATA IN or DATA OUT phase.

Fixes: 11d2f63b9c ("ncr5380: Change instance->host_lock to hostdata->lock")
Reported-and-tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: <stable@vger.kernel.org> # 4.5
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-03-01 09:37:53 -05:00
Finn Thain 1884c2838f ncr5380: Correctly clear command pointers and lists after bus reset
Commands subject to exception handling are to be returned to the scsi
mid-layer. Make sure that the various command pointers and command lists
in the low-level driver are correctly cleansed of affected commands.

This fixes some bugs that I accidentally introduced in v4.5-rc1 including
the removal of INIT_LIST_HEAD for the 'autosense' and 'disconnected'
command lists, and the possible NULL pointer dereference in
NCR5380_bus_reset() that was reported by Dan Carpenter.

hostdata->sensing may also point to an affected command so this pointer
also has to be cleared. The abort handler calls complete_cmd() to take
care of this; let's have the bus reset handler do the same.

The issue queue may also contain an affected command. If so, remove it.
This also follows the abort handler logic.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: 62717f537e ("ncr5380: Implement new eh_bus_reset_handler")
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Cc: <stable@vger.kernel.org> # 4.5
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-03-01 09:37:11 -05:00
Finn Thain 3d07d22b3d ncr5380: Cleanup whitespace and parentheses
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:10 -05:00
Finn Thain 0d2cf867e4 ncr5380: Merge changes from atari_NCR5380.c
In the past, NCR5380.c was overlooked by those working on atari_NCR5380.c
and this caused needless divergence. All of the changes in this patch were
taken from atari_NCR5380.c.

This removes some unimportant discrepancies between the two core driver
forks so that 'diff' can be used to reveal the important ones, to
facilitate reunification.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:09 -05:00
Finn Thain 594d4ba36c ncr5380: Fix whitespace in comments using regexp
Hanging indentation was a poor choice for the text inside comments. It
has been used in the wrong places and done badly elsewhere. There is
little consistency within any file. One fork of the core driver uses
tabs for this indentation while the other uses spaces. Better to use
flush-left alignment throughout.

This patch is the result of the following substitution. It replaces tabs
and spaces at the start of a comment line with a single space.

perl -i -pe 's,^(\t*[/ ]\*)[ \t]+,$1 ,' drivers/scsi/{atari_,}NCR5380.c

This removes some unimportant discrepancies between the two core driver
forks so that the important ones become obvious, to facilitate
reunification.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:09 -05:00
Finn Thain aff0cf9a7e ncr5380: Fix trailing whitespace using regexp
This patch is the result of the following substitution. It removes any
tabs and spaces at the end of a line.

perl -i -pe 's,[\t ]+$,,' drivers/scsi/{atari_,}NCR5380.c

This removes some unimportant discrepancies between the two core driver
forks so that the important ones become obvious, to facilitate
reunification.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:09 -05:00
Finn Thain c16df32e5f ncr5380: Cleanup comments
The CVS revision log is not nearly as useful as the history/history.git
repo, so remove it. Roman's commentary at the top of his driver repeats
the same information elsewhere in the file so remove it. Also remove
some other redundant or obsolete comments.

Both the driver and the datasheets confusingly refer to a DMA access
for a SCSI WRITE command as a "DMA write". Similarly a SCSI READ command
is called a "DMA read". This is the opposite of the usual convention.
Thankfully, the chip documentation and driver code also use "DMA send" and
"DMA receive", so adopt this terminology.

This removes some unimportant discrepancies between the two core driver
forks so that 'diff' can be used to reveal the important ones, to
facilitate reunification.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:09 -05:00
Finn Thain 0a4e361254 ncr5380: Fix soft lockups
Because of the rudimentary design of the chip, it is necessary to poll the
SCSI bus signals during PIO and this tends to hog the CPU. The driver will
accept new commands while others execute, and this causes a soft lockup
because the workqueue item will not terminate until the issue queue is
emptied.

When exercising dmx3191d using sequential IO from dd, the driver is sent
512 KiB WRITE commands and 128 KiB READs. For a PIO transfer, the rate is
is only about 300 KiB/s, so these are long-running commands. And although
PDMA may run at several MiB/s, interrupts are disabled for the duration
of the transfer.

Fix the unresponsiveness and soft lockup issues by calling cond_resched()
after each command is completed and by limiting max_sectors for drivers
that don't implement real DMA.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:09 -05:00
Finn Thain b746545f8b atari_NCR5380: Eliminate HOSTNO macro
Keep the two core driver forks in sync.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:08 -05:00
Finn Thain 6a6ff4ac0d atari_NCR5380: Remove HOSTNO macro from printk() and seq_printf() calls
Remove the HOSTNO macro that is peculiar to atari_NCR5380.c and
contributes to the problem of divergence of the NCR5380 core drivers.
Keep NCR5380.c in sync.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:08 -05:00
Finn Thain 62717f537e ncr5380: Implement new eh_bus_reset_handler
NCR5380.c lacks a sane eh_bus_reset_handler. The atari_NCR5380.c code is
much better but it should not throw out the issue queue (that would be
a host reset) and it neglects to set the result code for commands that it
throws out. Fix these bugs and keep the two core drivers in sync.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:08 -05:00
Finn Thain 707d62b37f ncr5380: Fix EH during arbitration and selection
During arbitration and selection, the relevant command is invisible to
exception handlers and can be found only in a pointer on the stack of a
different thread.

When eh_abort_handler can't find a given command, it can't decide whether
that command was completed already or is still in arbitration or selection
phase. But it must return either SUCCESS (e.g. command completed earlier)
or FAILED (could not abort the nexus, try bus reset).

The solution is to make sure all commands belonging to the LLD are always
visible to exception handlers. Add another scsi_cmnd pointer to the
hostdata struct to track the command in arbitration or selection phase.

Replace 'retain_dma_irq' with the new 'selecting' pointer, to bring
atari_NCR5380.c into line with NCR5380.c.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:08 -05:00
Finn Thain 8b00c3d5d4 ncr5380: Implement new eh_abort_handler
Introduce a new eh_abort_handler implementation. This one attempts to
follow all of the rules relating to EH handlers. There is still a known
bug: during selection, a command becomes invisible to the EH handlers
because it only appears in a pointer on the stack of a different thread.
This bug is addressed in a subsequent patch.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:08 -05:00
Finn Thain f27db8eb98 ncr5380: Fix autosense bugs
NCR5380_information_transfer() may re-queue a command for autosense,
after calling scsi_eh_prep_cmnd(). This creates several possibilities:

1. Reselection may intervene before the re-queued command gets processed.
   If the reconnected command then undergoes autosense, this causes the
   scsi_eh_save data from the previous command to be overwritten.

2. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(),
   a new REQUEST SENSE command may arrive. This would be queued ahead
   of any command already undergoing autosense, which means the
   scsi_eh_save data might be restored to the wrong command.

3. After NCR5380_information_transfer() calls scsi_eh_prep_cmnd(),
   eh_abort_handler() may abort the command. But the scsi_eh_save data is
   not discarded, which means the scsi_eh_save data might be incorrectly
   restored to the next REQUEST SENSE command issued.

This patch adds a new autosense list so that commands that are re-queued
because of a CHECK CONDITION result can be kept apart from the REQUEST
SENSE commands that arrive via queuecommand.

This patch also adds a function dedicated to dequeueing and preparing the
next command for processing. By refactoring the main loop in this way,
scsi_eh_save takes place when an autosense command is dequeued rather
than when re-queued.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:08 -05:00
Finn Thain 677e01947e ncr5380: Refactor command completion
Implement a 'complete_cmd' function to complete commands. This is needed
by the following patch; the new function provides a site for the logic
needed to correctly handle REQUEST SENSE commands.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:07 -05:00
Finn Thain 32b26a1042 ncr5380: Use standard list data structure
The NCR5380 drivers have a home-spun linked list implementation for
scsi_cmnd structs that uses cmd->host_scribble as a 'next' pointer. Adopt
the standard list_head data structure and list operations instead. Remove
the eh_abort_handler rather than convert it. Doing the conversion would
only be churn because the existing EH handlers don't work and get replaced
in a subsequent patch.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:07 -05:00