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 <schmitz@xxxxxxxxxx> 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 from this list: send the line "unsubscribe linux-m68k" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html