ncr5380: Rework disconnect versus poll logic
authorFinn Thain <fthain@telegraphics.com.au>
Sun, 3 Jan 2016 05:05:26 +0000 (16:05 +1100)
committerMartin K. Petersen <martin.petersen@oracle.com>
Thu, 7 Jan 2016 02:42:56 +0000 (21:42 -0500)
commit686f3990e6a9111f97f2d385f4d1c1a5b0628c15
tree172b0b65d29f6dc4f0a187b17c17631c015095c1
parentff3d4578840fd96a50558edf02ca0178b9ebb652
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>
drivers/scsi/NCR5380.c
drivers/scsi/NCR5380.h
drivers/scsi/atari_NCR5380.c
drivers/scsi/g_NCR5380.c