Hi Geert,

some fixes to the Atari NCR5380 SCSI driver: if the ST-DMA lock cannot be taken 
in the queue function, indicate host busy to the block layer. Fix return codes 
of abort and reset functions, and add scsi_unregister to module exit. 
Error handling is still messed up, most likely due to missing ST-DMA lock in 
abort and reset. This is still WIP, but might benefit users with a Falcon less 
prone to SCSI lockups, i.e. more stable SCSI clock signal.

Signed-off-by: Michael Schmitz <schm...@debian.org>

  Michael

---
 drivers/scsi/atari_NCR5380.c |   78 +++++++++++++++++++++++++++--------------
 drivers/scsi/atari_scsi.c    |   47 +++++++++++++++++++++----
 2 files changed, 91 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/atari_NCR5380.c b/drivers/scsi/atari_NCR5380.c
index 0471f88..3ba46d5 100644
--- a/drivers/scsi/atari_NCR5380.c
+++ b/drivers/scsi/atari_NCR5380.c
@@ -966,13 +966,6 @@ static int NCR5380_queue_command(Scsi_Cmnd *cmd, void 
(*done)(Scsi_Cmnd *))
 
        cmd->result = 0;
 
-       /*
-        * Insert the cmd into the issue queue. Note that REQUEST SENSE
-        * commands are added to the head of the queue since any command will
-        * clear the contingent allegiance condition that exists and the
-        * sense data is only guaranteed to be valid while the condition exists.
-        */
-
        local_irq_save(flags);
        /* ++guenther: now that the issue queue is being set up, we can lock 
ST-DMA.
         * Otherwise a running NCR5380_main may steal the lock.
@@ -987,10 +980,32 @@ static int NCR5380_queue_command(Scsi_Cmnd *cmd, void 
(*done)(Scsi_Cmnd *))
         * alter queues and touch the lock.
         */
        if (!IS_A_TT()) {
+               int rv;
+               /* MSch: since we may now get called from softirq context here, 
+                * we cannot always sleep while waiting for the lock.
+                * Use status from falcon_get_lock() to detect if the lock was
+                * successfully taken, and return appropriate status to midlevel
+                * otherwise.
+                * We used to pause the midlevel SCSI timer here; races between 
+                * command timeout and lowlevel error handling should not hurt
+                * because the command isn't in any of the queues yet.
+                */
                /* perhaps stop command timer here */
-               falcon_get_lock();
+               rv = falcon_get_lock();
                /* perhaps restart command timer here */
+               if (rv) {
+                       local_irq_restore(flags);
+                       return SCSI_MLQUEUE_HOST_BUSY;
+               }
        }
+
+       /*
+        * Insert the cmd into the issue queue. Note that REQUEST SENSE
+        * commands are added to the head of the queue since any command will
+        * clear the contingent allegiance condition that exists and the
+        * sense data is only guaranteed to be valid while the condition exists.
+        */
+
        if (!(hostdata->issue_queue) || (cmd->cmnd[0] == REQUEST_SENSE)) {
                LIST(cmd, hostdata->issue_queue);
                SET_NEXT(cmd, hostdata->issue_queue);
@@ -1014,10 +1029,12 @@ static int NCR5380_queue_command(Scsi_Cmnd *cmd, void 
(*done)(Scsi_Cmnd *))
         * If we're not in an interrupt, we can call NCR5380_main()
         * unconditionally, because it cannot be already running.
         */
-       if (in_interrupt() || ((flags >> 8) & 7) >= 6)
+       /* As of 2.6.19, the coroutine has to be put on the task queue instead
+        * of being called directly. It might still be called directly if not 
+        * in softirq, though. Need to test this.  
+        */
                queue_main();
-       else
-               NCR5380_main(NULL);
+
        return 0;
 }
 
@@ -1463,6 +1480,7 @@ static int NCR5380_select(struct Scsi_Host *instance, 
Scsi_Cmnd *cmd, int tag)
        ARB_PRINTK("scsi%d: arbitration complete\n", HOSTNO);
 
        if (hostdata->connected) {
+               
                NCR5380_write(MODE_REG, MR_BASE);
                return -1;
        }
@@ -2065,7 +2083,6 @@ static void NCR5380_information_transfer(struct Scsi_Host 
*instance)
                                /*
                                 * The preferred transfer method is going to be
                                 * PSEUDO-DMA for systems that are strictly PIO,
-                                * since we can let the hardware do the 
handshaking.
                                 *
                                 * For this to work, we need to know the 
transfersize
                                 * ahead of time, since the pseudo-DMA code 
will sit
@@ -2631,7 +2648,7 @@ static void NCR5380_reselect(struct Scsi_Host *instance)
  *     host byte of the result field to, if zero DID_ABORTED is
  *     used.
  *
- * Returns : 0 - success, -1 on failure.
+ * Returns : SUCCESS - success, FAILED on failure.
  *
  * XXX - there is no way to abort the command that is currently
  *      connected, you have to wait for it to complete.  If this is
@@ -2701,11 +2718,12 @@ int NCR5380_abort(Scsi_Cmnd *cmd)
                        local_irq_restore(flags);
                        cmd->scsi_done(cmd);
                        falcon_release_lock_if_possible(hostdata);
-                       return SCSI_ABORT_SUCCESS;
+                       return SUCCESS;
                } else {
-/*                     local_irq_restore(flags); */
+                       /* not sure */
+                       local_irq_restore(flags);
                        printk("scsi%d: abort of connected command failed!\n", 
HOSTNO);
-                       return SCSI_ABORT_ERROR;
+                       return FAILED;
                }
        }
 #endif
@@ -2729,7 +2747,7 @@ int NCR5380_abort(Scsi_Cmnd *cmd)
                         * yet... */
                        tmp->scsi_done(tmp);
                        falcon_release_lock_if_possible(hostdata);
-                       return SCSI_ABORT_SUCCESS;
+                       return SUCCESS;
                }
        }
 
@@ -2747,7 +2765,7 @@ int NCR5380_abort(Scsi_Cmnd *cmd)
        if (hostdata->connected) {
                local_irq_restore(flags);
                ABRT_PRINTK("scsi%d: abort failed, command connected.\n", 
HOSTNO);
-               return SCSI_ABORT_SNOOZE;
+               return FAILED;
        }
 
        /*
@@ -2782,7 +2800,7 @@ int NCR5380_abort(Scsi_Cmnd *cmd)
                        ABRT_PRINTK("scsi%d: aborting disconnected command.\n", 
HOSTNO);
 
                        if (NCR5380_select(instance, cmd, (int)cmd->tag))
-                               return SCSI_ABORT_BUSY;
+                               return FAILED;
 
                        ABRT_PRINTK("scsi%d: nexus reestablished.\n", HOSTNO);
 
@@ -2809,7 +2827,7 @@ int NCR5380_abort(Scsi_Cmnd *cmd)
                                        local_irq_restore(flags);
                                        tmp->scsi_done(tmp);
                                        
falcon_release_lock_if_possible(hostdata);
-                                       return SCSI_ABORT_SUCCESS;
+                                       return SUCCESS;
                                }
                        }
                }
@@ -2835,7 +2853,9 @@ int NCR5380_abort(Scsi_Cmnd *cmd)
         */
        falcon_release_lock_if_possible(hostdata);
 
-       return SCSI_ABORT_NOT_RUNNING;
+       /* NCR5380 has FAILED here */
+       /* return SUCCESS; */
+       return FAILED;
 }
 
 
@@ -2844,16 +2864,18 @@ int NCR5380_abort(Scsi_Cmnd *cmd)
  *
  * Purpose : reset the SCSI bus.
  *
- * Returns : SCSI_RESET_WAKEUP
+ * Returns : SUCCESS
  *
  */
 
+#undef FALCON_RESET_KILL_QUEUES
+
 static int NCR5380_bus_reset(Scsi_Cmnd *cmd)
 {
        SETUP_HOSTDATA(cmd->device->host);
        int i;
        unsigned long flags;
-#if 1
+#if defined(FALCON_RESET_KILL_QUEUES)
        Scsi_Cmnd *connected, *disconnected_queue;
 #endif
 
@@ -2878,7 +2900,8 @@ static int NCR5380_bus_reset(Scsi_Cmnd *cmd)
         * through anymore ... */
        (void)NCR5380_read(RESET_PARITY_INTERRUPT_REG);
 
-#if 1  /* XXX Should now be done by midlevel code, but it's broken XXX */
+#if defined(FALCON_RESET_KILL_QUEUES)  
+       /* XXX Should now be done by midlevel code, but it's broken XXX */
        /* XXX see below                                            XXX */
 
        /* MSch: old-style reset: actually abort all command processing here */
@@ -2934,7 +2957,7 @@ static int NCR5380_bus_reset(Scsi_Cmnd *cmd)
         * the midlevel code that the reset was SUCCESSFUL, and there is no
         * need to 'wake up' the commands by a request_sense
         */
-       return SCSI_RESET_SUCCESS | SCSI_RESET_BUS_RESET;
+       return SUCCESS;
 #else /* 1 */
 
        /* MSch: new-style reset handling: let the mid-level do what it can */
@@ -2982,6 +3005,7 @@ static int NCR5380_bus_reset(Scsi_Cmnd *cmd)
        local_irq_restore(flags);
 
        /* we did no complete reset of all commands, so a wakeup is required */
-       return SCSI_RESET_WAKEUP | SCSI_RESET_BUS_RESET;
-#endif /* 1 */
+       /* The new error handler code implicitly does this for us anyway */
+       return SUCCESS;
+#endif /* defined(FALCON_RESET_KILL_QUEUES) */
 }
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index ad7a23a..9ebfbc6 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -196,7 +196,7 @@ static unsigned long atari_dma_xfer_len(unsigned long 
wanted_len,
 static irqreturn_t scsi_tt_intr(int irq, void *dummy);
 static irqreturn_t scsi_falcon_intr(int irq, void *dummy);
 static void falcon_release_lock_if_possible(struct NCR5380_hostdata *hostdata);
-static void falcon_get_lock(void);
+static int falcon_get_lock(void);
 #ifdef CONFIG_ATARI_SCSI_RESET_BOOT
 static void atari_scsi_reset_boot(void);
 #endif
@@ -498,6 +498,16 @@ static int falcon_dont_release = 0;
  * again (but others waiting longer more probably will win).
  */
 
+/*
+ * MSch 20061228: in 2.6.x, the fairness wait appears to open a race with
+ * concurrent use of the lock by the IDE driver. Once the lock is taken by
+ * IDE, the SCSI queuecmd function can no longer schedule because it is run
+ * from softirq context a lot. 
+ * Disable the fairness wait until I have had time to sort out the lock issues.
+ */
+#undef FALCON_FAIRNESS_WAIT
+#define FALCON_NEVER_SLEEP 1
+
 static void falcon_release_lock_if_possible(struct NCR5380_hostdata *hostdata)
 {
        unsigned long flags;
@@ -519,7 +529,9 @@ static void falcon_release_lock_if_possible(struct 
NCR5380_hostdata *hostdata)
                }
                falcon_got_lock = 0;
                stdma_release();
+#if defined(FALCON_FAIRNESS_WAIT)
                wake_up(&falcon_fairness_wait);
+#endif
        }
 
        local_irq_restore(flags);
@@ -540,21 +552,37 @@ static void falcon_release_lock_if_possible(struct 
NCR5380_hostdata *hostdata)
  * Complicated, complicated.... Sigh...
  */
 
-static void falcon_get_lock(void)
+/* MSch 20061229: in order to avoid sleeping when the lock is not available, 
+ * return the current lock state here. atari_queue_command() will then return
+ * with error, causing the midlevel code to pause request submission.
+ */
+static int falcon_get_lock(void)
 {
        unsigned long flags;
 
        if (IS_A_TT())
-               return;
+               return 0;
 
        local_irq_save(flags);
 
-       while (!in_irq() && falcon_got_lock && stdma_others_waiting())
+#if defined(FALCON_FAIRNESS_WAIT)
+       /* MSch: we may not sleep even while in softirq ! */
+       while (!in_interrupt() && falcon_got_lock && stdma_others_waiting())
                sleep_on(&falcon_fairness_wait);
-
+#endif
        while (!falcon_got_lock) {
                if (in_irq())
                        panic("Falcon SCSI hasn't ST-DMA lock in interrupt");
+               /* we may not sleep in soft interrupts neither, so bail out */
+#if defined(FALCON_NEVER_SLEEP)
+               if (stdma_islocked()) {
+#else
+               if (in_softirq() && stdma_islocked()) {
+#endif
+                       // printk(KERN_INFO "Falcon SCSI does not hold ST-DMA 
lock in softirq!\n" );
+                       local_irq_restore(flags);
+                       return 1;
+               }
                if (!falcon_trying_lock) {
                        falcon_trying_lock = 1;
                        stdma_lock(scsi_falcon_intr, NULL);
@@ -569,6 +597,8 @@ static void falcon_get_lock(void)
        local_irq_restore(flags);
        if (!falcon_got_lock)
                panic("Falcon SCSI: someone stole the lock :-(\n");
+
+       return 0;
 }
 
 
@@ -589,7 +619,7 @@ int atari_queue_command(Scsi_Cmnd *cmd, void 
(*done)(Scsi_Cmnd *))
 #endif
 
 
-int __init atari_scsi_detect(struct scsi_host_template *host)
+int atari_scsi_detect(struct scsi_host_template *host)
 {
        static int called = 0;
        struct Scsi_Host *instance;
@@ -652,6 +682,8 @@ int __init atari_scsi_detect(struct scsi_host_template 
*host)
                }
                atari_dma_phys_buffer = virt_to_phys(atari_dma_buffer);
                atari_dma_orig_addr = 0;
+               printk(KERN_ERR "atari_scsi_detect: ST-RAM "
+                       "double buffer at %p\n", atari_dma_phys_buffer);
        }
 #endif
        instance = scsi_register(host, sizeof(struct NCR5380_hostdata));
@@ -745,6 +777,7 @@ int atari_scsi_release(struct Scsi_Host *sh)
 {
        if (IS_A_TT())
                free_irq(IRQ_TT_MFP_SCSI, sh);
+       scsi_unregister(atari_scsi_host);
        if (atari_dma_buffer)
                atari_stram_free(atari_dma_buffer);
        return 1;
@@ -828,7 +861,7 @@ int atari_scsi_bus_reset(Scsi_Cmnd *cmd)
        } else {
                atari_turnon_irq(IRQ_MFP_FSCSI);
        }
-       if ((rv & SCSI_RESET_ACTION) == SCSI_RESET_SUCCESS)
+       if (rv == SUCCESS)
                falcon_release_lock_if_possible(hostdata);
 
        return rv;
-- 
1.5.6.5


-- 
To UNSUBSCRIBE, email to debian-68k-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to