1
0
Fork 0
Commit Graph

110 Commits (0a4e36125451165847c6d4e7d5633d92e53f5c69)

Author SHA1 Message Date
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
Finn Thain 9903fa914a ncr5380: Remove LIST and REMOVE macros
Printing command pointers can be useful when debugging queues. Other than
that, the LIST and REMOVE macros are just clutter. These macros are
redundant now that NDEBUG_QUEUES causes pointers to be printed, so remove
them.

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 0d3d9a423c ncr5380: Use dsprintk() for queue debugging
Print the command pointers in the log messages for debugging queue data
structures. The LIST and REMOVE macros can then be removed.

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:06 -05:00
Finn Thain e8a6014442 ncr5380: Use shost_priv helper
Make use of the shost_priv() helper. Remove HOSTDATA and SETUP_HOSTDATA
macros because they harm readability.

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:06 -05:00
Finn Thain dbb6b35069 ncr5380: Remove H_NO macro and introduce dsprintk
Replace all H_NO and some HOSTNO macros (both peculiar to atari_NCR5380.c)
with a new dsprintk macro that's more useful and more consistent. The new
macro avoids a lot of boilerplate in new code in subsequent patches. Keep
NCR5380.c in sync. Remaining HOSTNO macros are removed as side-effects
of subsequent patches.

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:05 -05:00
Finn Thain e5c3fddfaa ncr5380: Remove command list debug code
Some NCR5380 hosts offer a .show_info method to access the contents of
the various command list data structures from a procfs file. When NDEBUG
is set, the same information is sent to the console during EH.

The two core drivers, atari_NCR5380.c and NCR5380.c differ here. Because
it is just for debugging, the easiest way to fix the discrepancy is
simply remove this code.

The only remaining users of NCR5380_show_info() and NCR5380_write_info()
are drivers that define PSEUDO_DMA. The others have no use for the
.show_info method, so don't initialize it.

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:05 -05:00
Finn Thain 11d2f63b9c ncr5380: Change instance->host_lock to hostdata->lock
NCR5380.c presently uses the instance->host_lock spin lock. Convert this
to a new spin lock that protects the NCR5380_hostdata struct.

atari_NCR5380.c previously used local_irq_save/restore() rather than a
spin lock. Convert this to hostdata->lock in irq mode. For SMP platforms,
the interrupt handler now also acquires the spin lock.

This brings all locking in the two core drivers into agreement.

Adding this locking also means that a bunch of volatile qualifiers can be
removed from the members of the NCR5380_hostdata struct. This is done in
a subsequent patch.

Proper locking will allow the abort handler to locate a command being
aborted. This is presently impossible if the abort handler is invoked when
the command has been moved from a queue to a pointer on the stack. (If
eh_abort_handler can't determine whether a command has been completed
or is still being processed then it can't decide whether to return
success or failure.)

The hostdata spin lock is now held when calling NCR5380_select() and
NCR5380_information_transfer(). Where possible, the lock is dropped for
polling and PIO transfers.

Clean up the now-redundant SELECT_ENABLE_REG writes, that used to provide
limited mutual exclusion between information_transfer() and reselect().

Accessing hostdata->connected without data races means taking the lock;
cleanup these accesses.

The new spin lock falls away for m68k and other UP builds, so this should
have little impact there. In the SMP case the new lock should be
uncontested even when the SCSI bus is contested.

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:04 -05:00
Finn Thain be3f4121aa ncr5380: Remove redundant ICR_ARBITRATION_LOST test and eliminate FLAG_DTC3181E
Remove FLAG_DTC3181E. It was used to suppress a final Arbitration Lost
(SEL asserted) test that isn't actually needed. The test was suppressed
because it causes problems for DTC436 and DTC536 chips. It takes place
after the host wins arbitration, so SEL has been asserted. These chips
can't seem to tell whether it was the host or another bus device that
did so.

This questionable final test appears in a flow chart in an early NCR5380
datasheet. It was removed from later documents like the DP5380 datasheet.

By the time this final test takes place, the driver has already tested
the Arbitration Lost bit several times. The first test happens 3 us after
BUS FREE (or longer due to register access delays). The protocol requires
that a device stop signalling within 1.8 us after BUS FREE unless it won
arbitration, in which case it must assert SEL, which is detected 1.2 us
later by the first Arbitration Lost test.

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:04 -05:00
Finn Thain bf1a0c6f8f ncr5380: Fix NDEBUG_NO_DATAOUT flag
NDEBUG_NO_DATAOUT should not disable DATA IN phases too. Fix this.
(This bug has long been fixed in atari_NCR5380.c.)

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Tested-by: Ondrej Zary <linux@rainbow-software.org>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:43:02 -05:00
Finn Thain 161c0059a2 ncr5380: Cleanup #include directives
Remove unused includes (stat.h, signal.h, proc_fs.h) and move includes
needed by the core drivers into the common header (delay.h etc).

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:02 -05:00
Finn Thain e0783ed366 ncr5380: Fix off-by-one bug in extended_msg[] bounds check
Fix the array bounds check when transferring an extended message from the
target.

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:02 -05:00
Finn Thain 72064a783b ncr5380: Standardize reselection handling
Bring the two NCR5380_reselect() implementations into agreement.

Replace infinite loops in atari_NCR5380.c with timeouts, as per NCR5380.c.

Remove 'abort' flag in NCR5380.c as per atari_NCR5380.c -- if reselection
fails, there may be no MESSAGE IN phase so don't attempt data transfer.

During selection, don't interfere with the chip registers after a
reselection interrupt intervenes.

Clean up some trivial issues with code style, comments and printk.

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:01 -05:00
Finn Thain 9db6024e55 ncr5380: Replace READ_OVERRUNS macro with FLAG_NO_DMA_FIXUPS
The workarounds for chip errata appear twice, in slightly different
forms. One is used when defined(REAL_DMA) || defined(REAL_DMA_POLL), the
other when defined(PSEUDO_DMA). In the PDMA case, the workarounds have
been made conditional on FLAG_NO_DMA_FIXUPS. Do the same for the DMA case,
to eliminate the READ_OVERRUNS macro.

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:01 -05:00
Finn Thain 55181be8ce ncr5380: Replace redundant flags with FLAG_NO_DMA_FIXUP
The flags DMA_WORKS_RIGHT, FLAG_NCR53C400 and FLAG_HAS_LAST_BYTE_SENT
all mean the same thing, i.e. the chip is not a 538[01]. (More recent
devices such as the 53C80 have a 'Last Byte Sent' bit in the Target
Command Register as well as other fixes for End-of-DMA errata.)

These flags have no additional meanings since previous cleanup patches
eliminated the NCR53C400 macro, moved g_NCR5380-specific code out of the
core driver and standardized interrupt handling.

Use the FLAG_NO_DMA_FIXUP flag to suppress End-of-DMA errata workarounds,
for those cards and drivers that make use of the TCR_LAST_BYTE_SENT bit.
Remove the old flags.

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:01 -05:00
Finn Thain b32ade1244 ncr5380: Introduce NCR5380_poll_politely2
SCSI bus protocol sometimes requires monitoring two related conditions
simultaneously. Enhance NCR5380_poll_politely() for this purpose, and
put it to use in the arbitration algorithm. It will also find use in
pseudo 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:01 -05:00
Finn Thain cd400825c9 ncr5380: Standardize interrupt handling
Because interrupt handling is crucial to the core driver(s), all wrapper
drivers need to agree on this code. This patch removes discrepancies.

NCR5380_intr() in NCR5380.c has the following pointless loop that differs
from the code in atari_NCR5380.c.

	done = 1;
	do {
		/* ... */
	} while (!done);

The 'done' flag gets cleared when a reconnected command is to be processed
from the work queue. But in NCR5380.c, the flag is also used to cause the
interrupt conditions to be re-examined. Perhaps this was because
NCR5380_reselect() was expected to cause another interrupt, or perhaps
the remaining present interrupt conditions need to be handled after the
NCR5380_reselect() call?

Actually, both possibilities are bogus, as is the loop itself. It seems
have been overlooked in the hit-and-miss removal of scsi host instance
list iteration many years ago; see history/history.git commit 491447e1fcff
("[PATCH] next NCR5380 updates") and commit 69e1a9482e57 ("[PATCH] fix up
NCR5380 private data"). See also my earlier patch, "Always retry
arbitration and selection".

The datasheet says, "IRQ can be reset simply by reading the Reset
Parity/Interrupt Register". So don't treat the chip IRQ like a
level-triggered interrupt. Of the conditions that set the IRQ flag,
some are level-triggered and some are edge-triggered, which means IRQ
itself must be edge-triggered.

Some interrupt conditions are latched and some are not. Before clearing
the chip IRQ flag, clear all state that may cause it to be raised. That
means clearing the DMA Mode and Busy Monitor bits in the Mode Register
and clearing the host ID in the Select Enable register.

Also clean up some printk's and some comments. Keep atari_NCR5380.c and
NCR5380.c in agreement.

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:00 -05:00
Finn Thain d1af9c7f4a ncr5380: Remove UNSAFE macro
Configuring core drivers using macros like this one prevents re-unifying
the core driver forks, and prevents implementing the core driver as a
library or a platform driver.

The UNSAFE macro in particular is a poor workaround for the problem of
interrupt latency. Releasing the locks complicates things because then we
would have to handle the possibility of EH handler invocation during a
PDMA transfer.

The comments say that instead of using this macro, "you're going to be
better off twiddling with transfersize". I agree. Remove this stuff.

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:00 -05:00
Finn Thain 8d8601a757 ncr5380: Use work_struct instead of delayed_work
Each host instance now has it's own work queue so the main() work item can
sleep when necessary. That means we can use a simple work item rather than
a delayed work item. This brings NCR5380.c closer to atari_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>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
2016-01-06 21:42:59 -05:00
Finn Thain 401e79fe8d ncr5380: Dont wait for BUS FREE after disconnect
When there is a queued command and no connected command, NCR5380_select()
is called and arbitration begins. The chip waits for BUS FREE once the
MR_ARBITRATE bit in the mode register is enabled. That means there is
no need to wait for BUS FREE after disconnecting.

There is presently no polling for BUS FREE after sending an ABORT or
other message that might lead to disconnection. It only happens after
COMMAND COMPLETE or DISCONNECT messages, which seems inconsistent.
Remove the polling for !BSY in the COMMAND COMPLETE and DISCONNECT
cases.

BTW, the comments say "avoid nasty timeouts" and perhaps BUS FREE polling
was somehow helpful back in Linux v0.99.14u, when it was introduced.
The relevant timeout is presently 1 second (for bus arbitration).

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:42:59 -05:00
Finn Thain 55500d9b08 atari_NCR5380: Use arbitration timeout
Allow target selection to fail with a timeout instead of waiting in
infinite loops. This gets rid of the unused NCR_TIMEOUT macro, it is more
defensive and has proved helpful in debugging.

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:42:58 -05:00
Finn Thain 80d3eb6df4 atari_NCR5380: Set do_abort() timeouts
Use timeouts in do_abort() in atari_NCR5380.c instead of infinite loops.
Also fix the kernel-doc comment. 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:42:58 -05:00
Finn Thain f35d34744c ncr5380: Fix bus phase in do_abort()
NCR5380_poll_politely() returns either 0 (success) or -ETIMEDOUT. However,
in do_abort(), the return value is incorrectly taken to be the status
register value. This means that the bus is put into DATA OUT phase instead
of MESSAGE OUT. Fix this.

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:42:58 -05:00
Finn Thain 1cc160e1ba ncr5380: Fix !REQ timeout in do_abort()
NCR5380_poll_politely() never returns -1. That means do_abort() can fail
to handle a timeout after waiting for the target to negate REQ. Fix this
and cleanup other NCR5380_poll_politely() call sites.

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:42:57 -05:00
Finn Thain 340b96126d ncr5380: Add missing break after case MESSAGE_REJECT
MESSAGE REJECT does not imply DISCONNECT: the target is about to enter
MESSAGE IN or MESSAGE OUT phase.

This bug fix comes from atari_NCR5380.c. Unfortunately it never made it
into the original NCR5380.c core driver.

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:42:57 -05:00
Hannes Reinecke 2f10e47ccf ncr5380: Remove references to linked commands
Some old drivers partially implemented support for linked commands using
a "proposed" next_link pointer in struct scsi_cmnd that never actually
existed. Remove this code.

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:42:57 -05:00
Finn Thain 1bb40589ea ncr5380: Drop DEF_SCSI_QCMD macro
Remove the DEF_SCSI_QCMD macro (already removed from atari_NCR5380.c). The
lock provided by DEF_SCSI_QCMD is only needed for queue data structures.

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:42:57 -05:00
Finn Thain 9dafbd939a ncr5380: Add missing lock in eh_abort_handler
The host spin lock needs to be acquired by NCR5380_abort() before it calls
NCR5380_select(). This patch doesn't actually fix the EH issues in this
driver but it does avoid this:

BUG: spinlock already unlocked on CPU#0, kworker/u4:1/14
 lock: 0xc0c0f834, .magic: dead4ead, .owner: <none>/-1, .owner_cpu: -1
 CPU: 0 PID: 14 Comm: kworker/u4:1 Not tainted 3.15.5 #5
 Workqueue: scsi_tmf_4 scmd_eh_abort_handler
 Call Trace:
 [ef885d70] [c0008acc] show_stack+0x70/0x1bc (unreliable)
 [ef885db0] [c0492a00] dump_stack+0x84/0x684
 [ef885dc0] [c006f314] spin_dump+0xd0/0xe8
 [ef885dd0] [c006f460] do_raw_spin_unlock+0xd4/0xd8
 [ef885df0] [c0491c8c] _raw_spin_unlock_irq+0x10/0x3c
 [ef885e00] [f381fe3c] NCR5380_select+0x3e4/0x6e8 [dmx3191d]
 [ef885e40] [f382026c] NCR5380_abort+0x12c/0x190 [dmx3191d]
 [ef885e60] [c02fec9c] scmd_eh_abort_handler+0x100/0x460
 [ef885e80] [c0046470] process_one_work+0x16c/0x420
 [ef885ea0] [c0046870] worker_thread+0x14c/0x430
 [ef885ed0] [c004e4f4] kthread+0xd8/0xec
 [ef885f40] [c00124d4] ret_from_kernel_thread+0x5c/0x64

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:42:57 -05:00
Finn Thain a2edc4a63b ncr5380: Fix NCR5380_transfer_pio() result
According to the SCSI-2 draft revision 10L, atari_NCR5380.c is correct
when it says that the phase lines are valid up until ACK is negated
following the transmission of the last byte in MESSAGE IN phase. This is
true for all information transfer phases, from target to initiator.

Sample the phase bits in STATUS_REG so that NCR5380_transfer_pio() can
return the correct result. The return value is presently unused (perhaps
because of bugs like this) but this change at least fixes the caller's
phase variable, which is passed by reference.

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:42:57 -05:00
Finn Thain 686f3990e6 ncr5380: Rework disconnect versus poll logic
The atari_NCR5380.c and NCR5380.c core drivers differ in their handling of
target disconnection. This is partly because atari_NCR5380.c had all of
the polling and sleeping removed to become entirely interrupt-driven, and
it is partly because of damage done to NCR5380.c after atari_NCR5380.c was
forked. See commit 37cd23b44929 ("Linux 2.1.105") in history/history.git.

The polling changes that were made in v2.1.105 are questionable at best:
if REQ is not already asserted when NCR5380_transfer_pio() is invoked, and
if the expected phase is DATA IN or DATA OUT, the function will schedule
main() to execute after USLEEP_SLEEP jiffies and then return. The problems
here are the expected REQ timing and the sleep interval*. Avoid this issue
by using NCR5380_poll_politely() instead of scheduling main().

The atari_NCR5380.c core driver requires the use of the chip interrupt and
always permits target disconnection. It sets the cmd->device->disconnect
flag when a device disconnects, but never tests this flag.

The NCR5380.c core driver permits disconnection only when
instance->irq != NO_IRQ. It sets the cmd->device->disconnect flag when
a device disconnects and it tests this flag in a couple of places:

1. During NCR5380_information_transfer(), following COMMAND OUT phase,
   if !cmd->device->disconnect, the initiator will take a guess as to
   whether or not the target will then choose to go to MESSAGE IN phase
   and disconnect. If the driver guesses "yes", it will schedule main()
   to execute after USLEEP_SLEEP jiffies and then return there.

   Unfortunately the driver may guess "yes" even after it has denied
   the target the disconnection privilege. When the target does not
   disconnect, the sleep can be beneficial, assuming the sleep interval
   is appropriate (mostly it is not*).

   And even if the driver guesses "yes" correctly, and the target would
   then disconnect, the driver still has to go through the MESSAGE IN
   phase in order to get to BUS FREE phase. The main loop can do nothing
   useful until BUS FREE, and sleeping just delays the phase transition.

2. If !cmd->device->disconnect and REQ is not already asserted when
   NCR5380_information_transfer() is invoked, the function polls for REQ
   for USLEEP_POLL jiffies. If REQ is not asserted, it then schedules
   main() to execute after USLEEP_SLEEP jiffies and returns.

   The idea is apparently to yeild the CPU while waiting for REQ.
   This is conditional upon !cmd->device->disconnect, but there seems
   to be no rhyme or reason for that. For example, the flag may be
   unset because disconnection privilege was denied because the driver
   has no IRQ. Or the flag may be unset because the device has never
   needed to disconnect before. Or if the flag is set, disconnection
   may have no relevance to the present bus phase.

Another deficiency of the existing algorithm is as follows. When the
driver has no IRQ, it prevents disconnection, and generally polls and
sleeps more than it would normally. Now, if the driver is going to poll
anyway, why not allow the target to disconnect? That way the driver can do
something useful with the bus instead of polling unproductively!

Avoid this pointless latency, complexity and guesswork by using
NCR5380_poll_politely() instead of scheduling main().

* For g_NCR5380, the time intervals for USLEEP_SLEEP and USLEEP_POLL are
  200 ms and 10 ms, respectively. They are 20 ms and 200 ms respectively
  for the other NCR5380 drivers. There doesn't seem to be any reason for
  this discrepancy. The timing seems to have no relation to the type of
  adapter. Bizarrely, the timing in g_NCR5380 seems to relate only to one
  particular type of target device. This patch attempts to solve the
  problem for all NCR5380 drivers and all target devices.

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:42:56 -05:00
Finn Thain ff3d457884 ncr5380: Implement NCR5380_dma_xfer_len and remove LIMIT_TRANSFERSIZE macro
Follow the example of the atari_NCR5380.c core driver and adopt the
NCR5380_dma_xfer_len() hook. Implement NCR5380_dma_xfer_len() for dtc.c
and g_NCR5380.c to take care of the limitations of these cards. Keep the
default for drivers using PSEUDO_DMA.

Eliminate the unused macro LIMIT_TRANSFERSIZE.

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:42:56 -05:00
Finn Thain 1d3db59d59 ncr5380: Always retry arbitration and selection
If NCR5380_select() returns -1, it means arbitration was lost or selection
failed and should be retried. If the main loop simply terminates when there
are still commands on the issue queue, they will remain queued until they
expire.

Fix this by clearing the 'done' flag after selection failure or lost
arbitration.

The "else break" clause in NCR5380_main() that gets removed here appears
to be a vestige of a long-gone loop that iterated over host instances.
See commit 491447e1fcff ("[PATCH] next NCR5380 updates") in
history/history.git.

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:42:56 -05:00
Finn Thain ae753a3387 ncr5380: Eliminate selecting state
Linux v2.1.105 changed the algorithm for polling for the BSY signal
in NCR5380_select() and NCR5380_main().

Presently, this code has a bug. Back then, NCR5380_set_timer(hostdata, 1)
meant reschedule main() after sleeping for 10 ms. Repeated 25 times this
provided the recommended 250 ms selection time-out delay. This got broken
when HZ became configurable.

We could fix this but there's no need to reschedule the main loop. This
BSY polling presently happens when the NCR5380_main() work queue item
calls NCR5380_select(), which in turn schedules NCR5380_main(), which
calls NCR5380_select() again, and so on.

This algorithm is a deviation from the simpler one in atari_NCR5380.c.
The extra complexity and state is pointless. There's no reason to
stop selection half-way and return to to the main loop when the main
loop can do nothing useful until selection completes.

So just poll for BSY. We can sleep while polling now that we have a
suitable workqueue.

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:42:56 -05:00
Finn Thain 2f854b82b0 ncr5380: Sleep when polling, if possible
When in process context, sleep during polling if doing so won't add
significant latency. In interrupt context or if the lock is held, poll
briefly then give up. Keep both core drivers in sync.

Calibrate busy-wait iterations to allow for variation in chip register
access times between different 5380 hardware implementations.

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:42:56 -05:00
Finn Thain 0ad0eff98f ncr5380: Introduce unbound workqueue
Allocate a work queue that will permit busy waiting and sleeping. This
means NCR5380_init() can potentially fail, so add this error path.

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:42:56 -05:00
Finn Thain 4d029e9ae9 ncr5380: Eliminate USLEEP_WAITLONG delay
Linux 2.1.105 introduced the USLEEP_WAITLONG delay, apparently "needed for
Mustek scanners". It is intended to stall the issue queue for 5 seconds.
There are a number of problems with this.

1. Only g_NCR5380 enables the delay, which implies that the other five
   drivers using the NCR5380.c core driver remain incompatible with
   Mustek scanners.

2. The delay is not implemented by atari_NCR5380.c, which is problematic
   for re-unifying the two core driver forks.

3. The delay is implemented using NCR5380_set_timer() which makes it
   unreliable. A new command queued by the mid-layer cancels the delay.

4. The delay is applied indiscriminately in several situations in which
   NCR5380_select() returns -1. These are-- reselection by the target,
   failure of the target to assert BSY, and failure of the target to
   assert REQ. It's clear from the comments that USLEEP_WAITLONG is not
   relevant to the reselection case. And reportedly, these scanners do
   not disconnect.

5. atari_NCR5380.c was forked before Linux 2.1.105, so it was spared some
   of the damage done to NCR5380.c. In this case, the atari_NCR5380.c core
   driver was more standard-compliant and may not have needed any
   workaround like the USLEEP_WAITLONG kludge. The compliance issue was
   addressed in the previous patch.

If these scanners still don't work, we need a better solution. Retrying
selection until EH aborts a command offers equivalent robustness. Bugs in
the existing driver prevent EH working correctly but this is addressed in
a subsequent patch. Remove USLEEP_WAITLONG.

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:42:55 -05:00
Finn Thain cf13b0837d ncr5380: Keep BSY asserted when entering SELECTION phase
NCR5380.c is not compliant with the SCSI-2 standard (at least, not with
the draft revision 10L that I have to refer to). The selection algorithm
in atari_NCR5380.c is correct, so use that.

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:42:55 -05:00
Finn Thain 1f1b0c74df ncr5380: Proceed with next command after NCR5380_select() calls scsi_done
If a target disappears from the SCSI bus, NCR5380_select() may
subsequently fail with a time-out. In this situation, scsi_done is
called and NCR5380_select() returns 0. Both hostdata->connected and
hostdata->selecting are NULL and the main loop should proceed with
the next command in the issue queue. Clarify this logic.

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:42:55 -05:00
Finn Thain ac9399d095 ncr5380: Always escalate bad target time-out in NCR5380_select()
Remove the restart_select and targets_present variables introduced in
Linux v1.1.38. The former was used only for a questionable debug printk
and the latter "so we can call a select failure a retryable condition".
Well, retrying select failure in general is a different problem to a
target that doesn't assert BSY. We need to handle these two cases
differently; the latter case can be left to the SCSI ML.

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:42:54 -05:00
Finn Thain 6323876f55 ncr5380: Use return instead of goto in NCR5380_select()
The "failed" label in NCR5380_select() is not helpful. Some failures
return 0, others -1. Use return instead of goto to improve clarity and
brevity, like atari_NCR5380.c does. Fix the relevant comments.

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:42:54 -05:00
Finn Thain 206cc031ad ncr5380: Remove redundant register writes
Remove the duplicate write to the Select Enable Register that appeared
in v1.1.38.

Also remove the redundant write to Initiator Command Register prior to
calling do_abort().

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:42:54 -05:00
Finn Thain 9b68d3415b ncr5380: Remove unused hostdata->aborted flag
The aborted flag was introduced in v1.1.38 but never used. Remove it.

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:42:54 -05:00
Finn Thain 3be1b3ea6d ncr5380: Simplify bus reset handlers
Make use of do_reset() in the bus reset handler in atari_NCR5380.c. The
version in NCR5380.c already does so. Keep them 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:42:54 -05:00