• Finn Thain's avatar
    ncr5380: Rework disconnect versus poll logic · 686f3990
    Finn Thain authored
    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 37cd23b4 ("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: default avatarFinn Thain <fthain@telegraphics.com.au>
    Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
    Tested-by: default avatarOndrej Zary <linux@rainbow-software.org>
    Tested-by: default avatarMichael Schmitz <schmitzmic@gmail.com>
    Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
    686f3990
NCR5380.c 81.2 KB