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>

1 files changed, 8 insertions, 16 deletions

diff --git a/drivers/scsi/atari_NCR5380.c b/drivers/scsi/atari_NCR5380.c
index 59b378057198..dc283f50e4ac 100644
--- a/drivers/scsi/atari_NCR5380.c
+++ b/drivers/scsi/atari_NCR5380.c
@@ -126,17 +126,7 @@
  * piece of hardware that requires you to sit in a loop polling for
  * the REQ signal as long as you are connected.  Some devices are
  * brain dead (ie, many TEXEL CD ROM drives) and won't disconnect
- * while doing long seek operations.
- *
- * The workaround for this is to keep track of devices that have
- * disconnected.  If the device hasn't disconnected, for commands that
- * should disconnect, we do something like
- *
- * while (!REQ is asserted) { sleep for N usecs; poll for M usecs }
- *
- * Some tweaking of N and M needs to be done.  An algorithm based
- * on "time to data" would give the best results as long as short time
- * to datas (ie, on the same track) were considered, however these
+ * while doing long seek operations. [...] These
  * broken devices are the exception rather than the rule and I'd rather
  * spend my time optimizing for the normal case.
  *
@@ -1740,13 +1730,14 @@ static int NCR5380_transfer_pio(struct Scsi_Host *instance,
 		 * Wait for assertion of REQ, after which the phase bits will be
 		 * valid
 		 */
-		while (!((tmp = NCR5380_read(STATUS_REG)) & SR_REQ))
-			;
+
+		if (NCR5380_poll_politely(instance, STATUS_REG, SR_REQ, SR_REQ, HZ) < 0)
+			break;
 
 		dprintk(NDEBUG_HANDSHAKE, "scsi%d: REQ detected\n", HOSTNO);
 
 		/* Check for phase mismatch */
-		if ((tmp & PHASE_MASK) != p) {
+		if ((NCR5380_read(STATUS_REG) & PHASE_MASK) != p) {
 			dprintk(NDEBUG_PIO, "scsi%d: phase mismatch\n", HOSTNO);
 			NCR5380_dprint_phase(NDEBUG_PIO, instance);
 			break;
@@ -2399,7 +2390,6 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 					/* Accept message by clearing ACK */
 					NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE);
 					local_irq_save(flags);
-					cmd->device->disconnect = 1;
 					LIST(cmd,hostdata->disconnected_queue);
 					SET_NEXT(cmd, hostdata->disconnected_queue);
 					hostdata->connected = NULL;
@@ -2564,7 +2554,9 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 				printk("scsi%d: unknown phase\n", HOSTNO);
 				NCR5380_dprint(NDEBUG_ANY, instance);
 			} /* switch(phase) */
-		} /* if (tmp * SR_REQ) */
+		} else {
+			NCR5380_poll_politely(instance, STATUS_REG, SR_REQ, SR_REQ, HZ);
+		}
 	} /* while (1) */
 }