- 07 Jan, 2016 40 commits
-
-
Finn Thain authored
The workarounds for chip errata appear twice, in slightly different forms. One is used when defined(REAL_DMA) || defined(REAL_DMA_POLL), the other when defined(PSEUDO_DMA). In the PDMA case, the workarounds have been made conditional on FLAG_NO_DMA_FIXUPS. Do the same for the DMA case, to eliminate the READ_OVERRUNS macro. Signed-off-by:
Finn Thain <fthain@telegraphics.com.au> Reviewed-by:
Hannes Reinecke <hare@suse.com> Tested-by:
Ondrej Zary <linux@rainbow-software.org> Signed-off-by:
Martin K. Petersen <martin.petersen@oracle.com>
-
Finn Thain authored
The flags DMA_WORKS_RIGHT, FLAG_NCR53C400 and FLAG_HAS_LAST_BYTE_SENT all mean the same thing, i.e. the chip is not a 538[01]. (More recent devices such as the 53C80 have a 'Last Byte Sent' bit in the Target Command Register as well as other fixes for End-of-DMA errata.) These flags have no additional meanings since previous cleanup patches eliminated the NCR53C400 macro, moved g_NCR5380-specific code out of the core driver and standardized interrupt handling. Use the FLAG_NO_DMA_FIXUP flag to suppress End-of-DMA errata workarounds, for those cards and drivers that make use of the TCR_LAST_BYTE_SENT bit. Remove the old flags. Signed-off-by:
-
Finn Thain authored
SCSI bus protocol sometimes requires monitoring two related conditions simultaneously. Enhance NCR5380_poll_politely() for this purpose, and put it to use in the arbitration algorithm. It will also find use in pseudo DMA. Signed-off-by:
Michael Schmitz <schmitzmic@gmail.com> Signed-off-by:
-
Finn Thain authored
Because interrupt handling is crucial to the core driver(s), all wrapper drivers need to agree on this code. This patch removes discrepancies. NCR5380_intr() in NCR5380.c has the following pointless loop that differs from the code in atari_NCR5380.c. done = 1; do { /* ... */ } while (!done); The 'done' flag gets cleared when a reconnected command is to be processed from the work queue. But in NCR5380.c, the flag is also used to cause the interrupt conditions to be re-examined. Perhaps this was because NCR5380_reselect() was expected to cause another interrupt, or perhaps the remaining present interrupt conditions need to be handled after the NCR5380_reselect() call? Actually, both possibilities are bogus, as is the loop itself. It seems have been overlooked in the hit-and-miss removal of scsi host instance list iteration many years ago; see history/history.git commit 491447e1 ("[PATCH] next NCR5380 updates") and commit 69e1a948 ("[PATCH] fix up NCR5380 private data"). See also my earlier patch, "Always retry arbitration and selection". The datasheet says, "IRQ can be reset simply by reading the Reset Parity/Interrupt Register". So don't treat the chip IRQ like a level-triggered interrupt. Of the conditions that set the IRQ flag, some are level-triggered and some are edge-triggered, which means IRQ itself must be edge-triggered. Some interrupt conditions are latched and some are not. Before clearing the chip IRQ flag, clear all state that may cause it to be raised. That means clearing the DMA Mode and Busy Monitor bits in the Mode Register and clearing the host ID in the Select Enable register. Also clean up some printk's and some comments. Keep atari_NCR5380.c and NCR5380.c in agreement. Signed-off-by: -
Finn Thain authored
Configuring core drivers using macros like this one prevents re-unifying the core driver forks, and prevents implementing the core driver as a library or a platform driver. The UNSAFE macro in particular is a poor workaround for the problem of interrupt latency. Releasing the locks complicates things because then we would have to handle the possibility of EH handler invocation during a PDMA transfer. The comments say that instead of using this macro, "you're going to be better off twiddling with transfersize". I agree. Remove this stuff. Signed-off-by:
-
Finn Thain authored
The complex main_running/queue_main mechanism is peculiar to atari_NCR5380.c. It isn't SMP safe and offers little value given that the work queue already offers concurrency management. Remove this complexity to bring atari_NCR5380.c closer to NCR5380.c. It is not a good idea to call the information transfer state machine from queuecommand because, according to Documentation/scsi/scsi_mid_low_api.txt that could happen in soft irq context. Fix this. Signed-off-by:
-
Finn Thain authored
Each host instance now has it's own work queue so the main() work item can sleep when necessary. That means we can use a simple work item rather than a delayed work item. This brings NCR5380.c closer to atari_NCR5380.c. Signed-off-by:
-
Finn Thain authored
When there is a queued command and no connected command, NCR5380_select() is called and arbitration begins. The chip waits for BUS FREE once the MR_ARBITRATE bit in the mode register is enabled. That means there is no need to wait for BUS FREE after disconnecting. There is presently no polling for BUS FREE after sending an ABORT or other message that might lead to disconnection. It only happens after COMMAND COMPLETE or DISCONNECT messages, which seems inconsistent. Remove the polling for !BSY in the COMMAND COMPLETE and DISCONNECT cases. BTW, the comments say "avoid nasty timeouts" and perhaps BUS FREE polling was somehow helpful back in Linux v0.99.14u, when it was introduced. The relevant timeout is presently 1 second (for bus arbitration). Signed-off-by:
-
Finn Thain authored
Allow target selection to fail with a timeout instead of waiting in infinite loops. This gets rid of the unused NCR_TIMEOUT macro, it is more defensive and has proved helpful in debugging. Signed-off-by:
-
Finn Thain authored
Use timeouts in do_abort() in atari_NCR5380.c instead of infinite loops. Also fix the kernel-doc comment. Keep the two core driver forks in sync. Signed-off-by:
-
Finn Thain authored
NCR5380_poll_politely() returns either 0 (success) or -ETIMEDOUT. However, in do_abort(), the return value is incorrectly taken to be the status register value. This means that the bus is put into DATA OUT phase instead of MESSAGE OUT. Fix this. Signed-off-by:
-
Finn Thain authored
NCR5380_poll_politely() never returns -1. That means do_abort() can fail to handle a timeout after waiting for the target to negate REQ. Fix this and cleanup other NCR5380_poll_politely() call sites. Signed-off-by:
-
Finn Thain authored
MESSAGE REJECT does not imply DISCONNECT: the target is about to enter MESSAGE IN or MESSAGE OUT phase. This bug fix comes from atari_NCR5380.c. Unfortunately it never made it into the original NCR5380.c core driver. Signed-off-by:
-
Hannes Reinecke authored
Some old drivers partially implemented support for linked commands using a "proposed" next_link pointer in struct scsi_cmnd that never actually existed. Remove this code. Signed-off-by:
-
Finn Thain authored
Remove the DEF_SCSI_QCMD macro (already removed from atari_NCR5380.c). The lock provided by DEF_SCSI_QCMD is only needed for queue data structures. Signed-off-by:
-
Finn Thain authored
The host spin lock needs to be acquired by NCR5380_abort() before it calls NCR5380_select(). This patch doesn't actually fix the EH issues in this driver but it does avoid this: BUG: spinlock already unlocked on CPU#0, kworker/u4:1/14 lock: 0xc0c0f834, .magic: dead4ead, .owner: <none>/-1, .owner_cpu: -1 CPU: 0 PID: 14 Comm: kworker/u4:1 Not tainted 3.15.5 #5 Workqueue: scsi_tmf_4 scmd_eh_abort_handler Call Trace: [ef885d70] [c0008acc] show_stack+0x70/0x1bc (unreliable) [ef885db0] [c0492a00] dump_stack+0x84/0x684 [ef885dc0] [c006f314] spin_dump+0xd0/0xe8 [ef885dd0] [c006f460] do_raw_spin_unlock+0xd4/0xd8 [ef885df0] [c0491c8c] _raw_spin_unlock_irq+0x10/0x3c [ef885e00] [f381fe3c] NCR5380_select+0x3e4/0x6e8 [dmx3191d] [ef885e40] [f382026c] NCR5380_abort+0x12c/0x190 [dmx3191d] [ef885e60] [c02fec9c] scmd_eh_abort_handler+0x100/0x460 [ef885e80] [c0046470] process_one_work+0x16c/0x420 [ef885ea0] [c0046870] worker_thread+0x14c/0x430 [ef885ed0] [c004e4f4] kthread+0xd8/0xec [ef885f40] [c00124d4] ret_from_kernel_thread+0x5c/0x64 Signed-off-by:
-
Finn Thain authored
According to the SCSI-2 draft revision 10L, atari_NCR5380.c is correct when it says that the phase lines are valid up until ACK is negated following the transmission of the last byte in MESSAGE IN phase. This is true for all information transfer phases, from target to initiator. Sample the phase bits in STATUS_REG so that NCR5380_transfer_pio() can return the correct result. The return value is presently unused (perhaps because of bugs like this) but this change at least fixes the caller's phase variable, which is passed by reference. Signed-off-by:
-
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:
-
Finn Thain authored
Follow the example of the atari_NCR5380.c core driver and adopt the NCR5380_dma_xfer_len() hook. Implement NCR5380_dma_xfer_len() for dtc.c and g_NCR5380.c to take care of the limitations of these cards. Keep the default for drivers using PSEUDO_DMA. Eliminate the unused macro LIMIT_TRANSFERSIZE. Signed-off-by:
-
Finn Thain authored
If NCR5380_select() returns -1, it means arbitration was lost or selection failed and should be retried. If the main loop simply terminates when there are still commands on the issue queue, they will remain queued until they expire. Fix this by clearing the 'done' flag after selection failure or lost arbitration. The "else break" clause in NCR5380_main() that gets removed here appears to be a vestige of a long-gone loop that iterated over host instances. See commit 491447e1 ("[PATCH] next NCR5380 updates") in history/history.git. Signed-off-by:
-
Finn Thain authored
Linux v2.1.105 changed the algorithm for polling for the BSY signal in NCR5380_select() and NCR5380_main(). Presently, this code has a bug. Back then, NCR5380_set_timer(hostdata, 1) meant reschedule main() after sleeping for 10 ms. Repeated 25 times this provided the recommended 250 ms selection time-out delay. This got broken when HZ became configurable. We could fix this but there's no need to reschedule the main loop. This BSY polling presently happens when the NCR5380_main() work queue item calls NCR5380_select(), which in turn schedules NCR5380_main(), which calls NCR5380_select() again, and so on. This algorithm is a deviation from the simpler one in atari_NCR5380.c. The extra complexity and state is pointless. There's no reason to stop selection half-way and return to to the main loop when the main loop can do nothing useful until selection completes. So just poll for BSY. We can sleep while polling now that we have a suitable workqueue. Signed-off-by:
-
Finn Thain authored
When in process context, sleep during polling if doing so won't add significant latency. In interrupt context or if the lock is held, poll briefly then give up. Keep both core drivers in sync. Calibrate busy-wait iterations to allow for variation in chip register access times between different 5380 hardware implementations. Signed-off-by:
-
Finn Thain authored
Allocate a work queue that will permit busy waiting and sleeping. This means NCR5380_init() can potentially fail, so add this error path. Signed-off-by:
-
Finn Thain authored
Commit 8b801ead ("[ARM] rpc: update Acorn SCSI drivers to modern ecard interfaces") neglected to remove a request_region() call in cumana_1.c. Commit eda32612 ("[PATCH] give all LLDD driver a ->release method") in history/history.git added some pointless release_region() calls in dtc.c, pas16.c and t128.c. Fix these issues. Signed-off-by:
-
Finn Thain authored
Linux 2.1.105 introduced the USLEEP_WAITLONG delay, apparently "needed for Mustek scanners". It is intended to stall the issue queue for 5 seconds. There are a number of problems with this. 1. Only g_NCR5380 enables the delay, which implies that the other five drivers using the NCR5380.c core driver remain incompatible with Mustek scanners. 2. The delay is not implemented by atari_NCR5380.c, which is problematic for re-unifying the two core driver forks. 3. The delay is implemented using NCR5380_set_timer() which makes it unreliable. A new command queued by the mid-layer cancels the delay. 4. The delay is applied indiscriminately in several situations in which NCR5380_select() returns -1. These are-- reselection by the target, failure of the target to assert BSY, and failure of the target to assert REQ. It's clear from the comments that USLEEP_WAITLONG is not relevant to the reselection case. And reportedly, these scanners do not disconnect. 5. atari_NCR5380.c was forked before Linux 2.1.105, so it was spared some of the damage done to NCR5380.c. In this case, the atari_NCR5380.c core driver was more standard-compliant and may not have needed any workaround like the USLEEP_WAITLONG kludge. The compliance issue was addressed in the previous patch. If these scanners still don't work, we need a better solution. Retrying selection until EH aborts a command offers equivalent robustness. Bugs in the existing driver prevent EH working correctly but this is addressed in a subsequent patch. Remove USLEEP_WAITLONG. Signed-off-by:
-
Finn Thain authored
NCR5380.c is not compliant with the SCSI-2 standard (at least, not with the draft revision 10L that I have to refer to). The selection algorithm in atari_NCR5380.c is correct, so use that. Signed-off-by:
-
Finn Thain authored
If a target disappears from the SCSI bus, NCR5380_select() may subsequently fail with a time-out. In this situation, scsi_done is called and NCR5380_select() returns 0. Both hostdata->connected and hostdata->selecting are NULL and the main loop should proceed with the next command in the issue queue. Clarify this logic. Signed-off-by:
-
Finn Thain authored
Remove the restart_select and targets_present variables introduced in Linux v1.1.38. The former was used only for a questionable debug printk and the latter "so we can call a select failure a retryable condition". Well, retrying select failure in general is a different problem to a target that doesn't assert BSY. We need to handle these two cases differently; the latter case can be left to the SCSI ML. Signed-off-by:
-
Finn Thain authored
The "failed" label in NCR5380_select() is not helpful. Some failures return 0, others -1. Use return instead of goto to improve clarity and brevity, like atari_NCR5380.c does. Fix the relevant comments. Signed-off-by:
-
Finn Thain authored
Remove the duplicate write to the Select Enable Register that appeared in v1.1.38. Also remove the redundant write to Initiator Command Register prior to calling do_abort(). Signed-off-by:
-
Finn Thain authored
The aborted flag was introduced in v1.1.38 but never used. Remove it. Signed-off-by:
-
Finn Thain authored
Make use of do_reset() in the bus reset handler in atari_NCR5380.c. The version in NCR5380.c already does so. Keep them in sync. Signed-off-by:
-
Finn Thain authored
The atari_NCR5380.c core driver now takes care of bus reset upon driver initialization if required (same as NCR5380.c). Move the Toshiba CD-ROM support into the core driver, enabled with a host flag, so that all NCR5380 drivers can make use of it. Drop the RESET_BOOT macros and the ATARI_SCSI_RESET_BOOT and ATARI_SCSI_TOSHIBA_DELAY Kconfig symbols, which are now redundant. Remove the atari_scsi_reset_boot(), mac_scsi_reset_boot() and sun3_scsi_reset_boot() routines. None of this duplicated code is needed now that all drivers can use NCR5380_maybe_reset_bus(). This brings atari_scsi, mac_scsi and sun3_scsi into line with all of the other NCR5380 drivers. The bus reset may raise an interrupt. That would be new behaviour for atari_scsi only when CONFIG_ATARI_SCSI_RESET_BOOT=n. The ST DMA interrupt is not assigned to atari_scsi at this stage, so CONFIG_ATARI_SCSI_RESET_BOOT=y may well be problematic already. Regardless, do_reset() now raises and clears the interrupt within local_irq_save/restore which should avoid problems. Signed-off-by:
-
Finn Thain authored
Merge the bus reset code from NCR5380.c into atari_NCR5380.c. This allows for removal of a lot of duplicated code conditional on the RESET_BOOT macro (in the next patch). The atari_NCR5380.c fork lacks the do_reset() and NCR5380_poll_politely() routines from NCR5380.c, so introduce them. They are indispensible. Keep the two implementations in sync. Signed-off-by:
-
Finn Thain authored
Move board-specific code like this, NCR5380_write(C400_CONTROL_STATUS_REG, CSR_BASE); from the core driver to the board driver. Eliminate the NCR53C400 macro from the core driver. Removal of all macros like this one will be necessary in order to have one core driver that can support all kinds of boards. Signed-off-by:
-
Finn Thain authored
This patch splits the NCR5380_init() function into two parts, similar to the scheme used with atari_NCR5380.c. This avoids two problems. Firstly, NCR5380_init() may perform a bus reset, which would cause the chip to assert IRQ. The chip is unable to mask its bus reset interrupt. Drivers can't call request_irq() before calling NCR5380_init(), because initialization must happen before the interrupt handler executes. If driver initialization causes an interrupt it may be problematic on some platforms. To avoid that, first move the bus reset code into NCR5380_maybe_reset_bus(). Secondly, NCR5380_init() contains some board-specific interrupt setup code for the NCR53C400 that does not belong in the core driver. In moving this code, better not re-order interrupt initialization and bus reset. Again, the solution is to move the bus reset code into NCR5380_maybe_reset_bus(). Signed-off-by:
-
Finn Thain authored
This macro makes the code cryptic. Remove it. Signed-off-by:
-
Finn Thain authored
The NCR5380_local_declare and NCR5380_setup macros exist to define and initialize a particular local variable, to provide the address of the chip registers needed for the driver's implementation of its NCR5380_read/write register access macros. In cumana_1 and macscsi, these macros generate pointless code like this, struct Scsi_Host *_instance; _instance = instance; In pas16, the use of NCR5380_read/write in pas16_hw_detect() requires that the io_port local variable has been defined and initialized, but the NCR5380_local_declare and NCR5380_setup macros can't be used for that purpose because the Scsi_Host struct has not yet been instantiated. Moreover, these macros were removed from atari_NCR5380.c long ago and now they constitute yet another discrepancy between the two core driver forks. Remove these "optimizations". Signed-off-by:
-
Finn Thain authored
ASM macro is never defined. rtrc in pas16.c is not used. NCR5380_map_config, do_NCR5380_intr, do_t128_intr and do_pas16_intr are unused. NCR_NOT_SET harms readability. Remove them. Signed-off-by:
-
Finn Thain authored
Replace {P,T,DTC}DEBUG_INIT with NDEBUG_INIT. Remove dead debugging code, including code that's conditional upon *DEBUG_TRANSFER. Signed-off-by:
-
