Merge patch series "NCR5380: Bug fixes and other improvements"

Finn Thain <[email protected]> says:

This series begins with some work on the mac_scsi driver to improve
compatibility with SCSI2SD v5 devices. Better error handling is needed
there because the PDMA hardware does not tolerate the write latency
spikes which SD cards can produce.

A bug is fixed in the 5380 core driver so that scatter/gather can be
enabled in mac_scsi.

Several patches at the end of this series improve robustness and
correctness in the core driver.

This series has been tested on a variety of mac_scsi hosts. A variety
of SCSI targets was also tested, including Quantum HDD, Fujitsu HDD,
Iomega FDD, Ricoh CD-RW, Matsushita CD-ROM, SCSI2SD and BlueSCSI.

Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Martin K. Petersen <[email protected]>

Author: martinkpetersen

drivers/scsi/NCR5380.c

@@ -157,7 +157,6 @@ static inline void initialize_SCp(struct scsi_cmnd *cmd)
 	}
 
 	ncmd->status = 0;
-	ncmd->message = 0;
 }
 
 static inline void advance_sg_buffer(struct NCR5380_cmd *ncmd)
@@ -199,7 +198,6 @@ static inline void set_resid_from_SCp(struct scsi_cmnd *cmd)
  * Polls the chip in a reasonably efficient manner waiting for an
  * event to occur. After a short quick poll we begin to yield the CPU
  * (if possible). In irq contexts the time-out is arbitrarily limited.
- * Callers may hold locks as long as they are held in irq mode.
  *
  * Returns 0 if either or both event(s) occurred otherwise -ETIMEDOUT.
  */
@@ -1228,24 +1226,15 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
 	return ret;
 }
 
-/*
- * Function : int NCR5380_transfer_pio (struct Scsi_Host *instance,
- * unsigned char *phase, int *count, unsigned char **data)
- *
- * Purpose : transfers data in given phase using polled I/O
- *
- * Inputs : instance - instance of driver, *phase - pointer to
- * what phase is expected, *count - pointer to number of
- * bytes to transfer, **data - pointer to data pointer,
- * can_sleep - 1 or 0 when sleeping is permitted or not, respectively.
- *
- * Returns : -1 when different phase is entered without transferring
- * maximum number of bytes, 0 if all bytes are transferred or exit
- * is in same phase.
- *
- * Also, *phase, *count, *data are modified in place.
+/**
+ * NCR5380_transfer_pio() - transfers data in given phase using polled I/O
+ * @instance: instance of driver
+ * @phase: pointer to what phase is expected
+ * @count: pointer to number of bytes to transfer
+ * @data: pointer to data pointer
+ * @can_sleep: 1 or 0 when sleeping is permitted or not, respectively
  *
- * XXX Note : handling for bus free may be useful.
+ * Returns: void. *phase, *count, *data are modified in place.
  */
 
 /*
@@ -1254,9 +1243,9 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
  * counts, we will always do a pseudo DMA or DMA transfer.
  */
 
-static int NCR5380_transfer_pio(struct Scsi_Host *instance,
-				unsigned char *phase, int *count,
-				unsigned char **data, unsigned int can_sleep)
+static void NCR5380_transfer_pio(struct Scsi_Host *instance,
+				 unsigned char *phase, int *count,
+				 unsigned char **data, unsigned int can_sleep)
 {
 	struct NCR5380_hostdata *hostdata = shost_priv(instance);
 	unsigned char p = *phase, tmp;
@@ -1277,8 +1266,8 @@ static int NCR5380_transfer_pio(struct Scsi_Host *instance,
 		 * valid
 		 */
 
-		if (NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ,
-					  HZ * can_sleep) < 0)
+		if (NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ | SR_BSY,
+					  SR_REQ | SR_BSY, HZ * can_sleep) < 0)
 			break;
 
 		dsprintk(NDEBUG_HANDSHAKE, instance, "REQ asserted\n");
@@ -1329,17 +1318,19 @@ static int NCR5380_transfer_pio(struct Scsi_Host *instance,
 
 		dsprintk(NDEBUG_HANDSHAKE, instance, "REQ negated, handshake complete\n");
 
-/*
- * We have several special cases to consider during REQ/ACK handshaking :
- * 1.  We were in MSGOUT phase, and we are on the last byte of the
- * message.  ATN must be dropped as ACK is dropped.
- *
- * 2.  We are in a MSGIN phase, and we are on the last byte of the
- * message.  We must exit with ACK asserted, so that the calling
- * code may raise ATN before dropping ACK to reject the message.
- *
- * 3.  ACK and ATN are clear and the target may proceed as normal.
- */
+		/*
+		 * We have several special cases to consider during REQ/ACK
+		 * handshaking:
+		 *
+		 * 1.  We were in MSGOUT phase, and we are on the last byte of
+		 * the message. ATN must be dropped as ACK is dropped.
+		 *
+		 * 2.  We are in MSGIN phase, and we are on the last byte of the
+		 * message. We must exit with ACK asserted, so that the calling
+		 * code may raise ATN before dropping ACK to reject the message.
+		 *
+		 * 3.  ACK and ATN are clear & the target may proceed as normal.
+		 */
 		if (!(p == PHASE_MSGIN && c == 1)) {
 			if (p == PHASE_MSGOUT && c > 1)
 				NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE | ICR_ASSERT_ATN);
@@ -1361,11 +1352,6 @@ static int NCR5380_transfer_pio(struct Scsi_Host *instance,
 		*phase = tmp & PHASE_MASK;
 	else
 		*phase = PHASE_UNKNOWN;
-
-	if (!c || (*phase == p))
-		return 0;
-	else
-		return -1;
 }
 
 /**
@@ -1485,6 +1471,7 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance,
 				unsigned char **data)
 {
 	struct NCR5380_hostdata *hostdata = shost_priv(instance);
+	struct NCR5380_cmd *ncmd = NCR5380_to_ncmd(hostdata->connected);
 	int c = *count;
 	unsigned char p = *phase;
 	unsigned char *d = *data;
@@ -1496,7 +1483,7 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance,
 		return -1;
 	}
 
-	NCR5380_to_ncmd(hostdata->connected)->phase = p;
+	ncmd->phase = p;
 
 	if (p & SR_IO) {
 		if (hostdata->read_overruns)
@@ -1574,79 +1561,80 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance,
 	/* The result is zero iff pseudo DMA send/receive was completed. */
 	hostdata->dma_len = c;
 
-/*
- * A note regarding the DMA errata workarounds for early NMOS silicon.
- *
- * For DMA sends, we want to wait until the last byte has been
- * transferred out over the bus before we turn off DMA mode.  Alas, there
- * seems to be no terribly good way of doing this on a 5380 under all
- * conditions.  For non-scatter-gather operations, we can wait until REQ
- * and ACK both go false, or until a phase mismatch occurs.  Gather-sends
- * are nastier, since the device will be expecting more data than we
- * are prepared to send it, and REQ will remain asserted.  On a 53C8[01] we
- * could test Last Byte Sent to assure transfer (I imagine this is precisely
- * why this signal was added to the newer chips) but on the older 538[01]
- * this signal does not exist.  The workaround for this lack is a watchdog;
- * we bail out of the wait-loop after a modest amount of wait-time if
- * the usual exit conditions are not met.  Not a terribly clean or
- * correct solution :-%
- *
- * DMA receive is equally tricky due to a nasty characteristic of the NCR5380.
- * If the chip is in DMA receive mode, it will respond to a target's
- * REQ by latching the SCSI data into the INPUT DATA register and asserting
- * ACK, even if it has _already_ been notified by the DMA controller that
- * the current DMA transfer has completed!  If the NCR5380 is then taken
- * out of DMA mode, this already-acknowledged byte is lost. This is
- * not a problem for "one DMA transfer per READ command", because
- * the situation will never arise... either all of the data is DMA'ed
- * properly, or the target switches to MESSAGE IN phase to signal a
- * disconnection (either operation bringing the DMA to a clean halt).
- * However, in order to handle scatter-receive, we must work around the
- * problem.  The chosen fix is to DMA fewer bytes, then check for the
- * condition before taking the NCR5380 out of DMA mode.  One or two extra
- * bytes are transferred via PIO as necessary to fill out the original
- * request.
- */
-
-	if (hostdata->flags & FLAG_DMA_FIXUP) {
-		if (p & SR_IO) {
-			/*
-			 * The workaround was to transfer fewer bytes than we
-			 * intended to with the pseudo-DMA read function, wait for
-			 * the chip to latch the last byte, read it, and then disable
-			 * pseudo-DMA mode.
-			 *
-			 * After REQ is asserted, the NCR5380 asserts DRQ and ACK.
-			 * REQ is deasserted when ACK is asserted, and not reasserted
-			 * until ACK goes false.  Since the NCR5380 won't lower ACK
-			 * until DACK is asserted, which won't happen unless we twiddle
-			 * the DMA port or we take the NCR5380 out of DMA mode, we
-			 * can guarantee that we won't handshake another extra
-			 * byte.
-			 */
+	/*
+	 * A note regarding the DMA errata workarounds for early NMOS silicon.
+	 *
+	 * For DMA sends, we want to wait until the last byte has been
+	 * transferred out over the bus before we turn off DMA mode. Alas, there
+	 * seems to be no terribly good way of doing this on a 5380 under all
+	 * conditions. For non-scatter-gather operations, we can wait until REQ
+	 * and ACK both go false, or until a phase mismatch occurs. Gather-sends
+	 * are nastier, since the device will be expecting more data than we
+	 * are prepared to send it, and REQ will remain asserted. On a 53C8[01]
+	 * we could test Last Byte Sent to assure transfer (I imagine this is
+	 * precisely why this signal was added to the newer chips) but on the
+	 * older 538[01] this signal does not exist. The workaround for this
+	 * lack is a watchdog; we bail out of the wait-loop after a modest
+	 * amount of wait-time if the usual exit conditions are not met.
+	 * Not a terribly clean or correct solution :-%
+	 *
+	 * DMA receive is equally tricky due to a nasty characteristic of the
+	 * NCR5380. If the chip is in DMA receive mode, it will respond to a
+	 * target's REQ by latching the SCSI data into the INPUT DATA register
+	 * and asserting ACK, even if it has _already_ been notified by the
+	 * DMA controller that the current DMA transfer has completed! If the
+	 * NCR5380 is then taken out of DMA mode, this already-acknowledged
+	 * byte is lost.
+	 *
+	 * This is not a problem for "one DMA transfer per READ
+	 * command", because the situation will never arise... either all of
+	 * the data is DMA'ed properly, or the target switches to MESSAGE IN
+	 * phase to signal a disconnection (either operation bringing the DMA
+	 * to a clean halt). However, in order to handle scatter-receive, we
+	 * must work around the problem. The chosen fix is to DMA fewer bytes,
+	 * then check for the condition before taking the NCR5380 out of DMA
+	 * mode. One or two extra bytes are transferred via PIO as necessary
+	 * to fill out the original request.
+	 */
 
-			if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG,
-			                          BASR_DRQ, BASR_DRQ, 0) < 0) {
-				result = -1;
-				shost_printk(KERN_ERR, instance, "PDMA read: DRQ timeout\n");
-			}
-			if (NCR5380_poll_politely(hostdata, STATUS_REG,
-			                          SR_REQ, 0, 0) < 0) {
-				result = -1;
-				shost_printk(KERN_ERR, instance, "PDMA read: !REQ timeout\n");
-			}
-			d[*count - 1] = NCR5380_read(INPUT_DATA_REG);
-		} else {
-			/*
-			 * Wait for the last byte to be sent.  If REQ is being asserted for
-			 * the byte we're interested, we'll ACK it and it will go false.
-			 */
-			if (NCR5380_poll_politely2(hostdata,
-			     BUS_AND_STATUS_REG, BASR_DRQ, BASR_DRQ,
-			     BUS_AND_STATUS_REG, BASR_PHASE_MATCH, 0, 0) < 0) {
-				result = -1;
-				shost_printk(KERN_ERR, instance, "PDMA write: DRQ and phase timeout\n");
+	if ((hostdata->flags & FLAG_DMA_FIXUP) &&
+	    (NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH)) {
+		/*
+		 * The workaround was to transfer fewer bytes than we
+		 * intended to with the pseudo-DMA receive function, wait for
+		 * the chip to latch the last byte, read it, and then disable
+		 * DMA mode.
+		 *
+		 * After REQ is asserted, the NCR5380 asserts DRQ and ACK.
+		 * REQ is deasserted when ACK is asserted, and not reasserted
+		 * until ACK goes false. Since the NCR5380 won't lower ACK
+		 * until DACK is asserted, which won't happen unless we twiddle
+		 * the DMA port or we take the NCR5380 out of DMA mode, we
+		 * can guarantee that we won't handshake another extra
+		 * byte.
+		 *
+		 * If sending, wait for the last byte to be sent. If REQ is
+		 * being asserted for the byte we're interested, we'll ACK it
+		 * and it will go false.
+		 */
+		if (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG,
+					   BASR_DRQ, BASR_DRQ, 0)) {
+			if ((p & SR_IO) &&
+			    (NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH)) {
+				if (!NCR5380_poll_politely(hostdata, STATUS_REG,
+							   SR_REQ, 0, 0)) {
+					d[c] = NCR5380_read(INPUT_DATA_REG);
+					--ncmd->this_residual;
+				} else {
+					result = -1;
+					scmd_printk(KERN_ERR, hostdata->connected,
+						    "PDMA fixup: !REQ timeout\n");
+				}
 			}
+		} else if (NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH) {
+			result = -1;
+			scmd_printk(KERN_ERR, hostdata->connected,
+				    "PDMA fixup: DRQ timeout\n");
 		}
 	}
 
@@ -1666,9 +1654,6 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance,
  * Side effects : SCSI things happen, the disconnected queue will be
  * modified if a command disconnects, *instance->connected will
  * change.
- *
- * XXX Note : we need to watch for bus free or a reset condition here
- * to recover from an unexpected bus free condition.
  */
 
 static void NCR5380_information_transfer(struct Scsi_Host *instance)
@@ -1807,9 +1792,11 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 				return;
 			case PHASE_MSGIN:
 				len = 1;
+				tmp = 0xff;
 				data = &tmp;
 				NCR5380_transfer_pio(instance, &phase, &len, &data, 0);
-				ncmd->message = tmp;
+				if (tmp == 0xff)
+					break;
 
 				switch (tmp) {
 				case ABORT:
@@ -1996,6 +1983,7 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 				break;
 			case PHASE_STATIN:
 				len = 1;
+				tmp = ncmd->status;
 				data = &tmp;
 				NCR5380_transfer_pio(instance, &phase, &len, &data, 0);
 				ncmd->status = tmp;
@@ -2005,9 +1993,20 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
 				NCR5380_dprint(NDEBUG_ANY, instance);
 			} /* switch(phase) */
 		} else {
+			int err;
+
 			spin_unlock_irq(&hostdata->lock);
-			NCR5380_poll_politely(hostdata, STATUS_REG, SR_REQ, SR_REQ, HZ);
+			err = NCR5380_poll_politely(hostdata, STATUS_REG,
+						    SR_REQ, SR_REQ, HZ);
 			spin_lock_irq(&hostdata->lock);
+
+			if (err < 0 && hostdata->connected &&
+			    !(NCR5380_read(STATUS_REG) & SR_BSY)) {
+				scmd_printk(KERN_ERR, hostdata->connected,
+					    "BSY signal lost\n");
+				do_reset(instance);
+				bus_reset_cleanup(instance);
+			}
 		}
 	}
 }

drivers/scsi/NCR5380.h

@@ -3,10 +3,10 @@
  * NCR 5380 defines
  *
  * Copyright 1993, Drew Eckhardt
- *	Visionary Computing
- *	(Unix consulting and custom programming)
- * 	[email protected]
- *      +1 (303) 666-5836
+ * Visionary Computing
+ * (Unix consulting and custom programming)
+ * [email protected]
+ * +1 (303) 666-5836
  *
  * For more information, please consult 
  *
@@ -78,7 +78,7 @@
 #define ICR_DIFF_ENABLE		0x20	/* wo Set to enable diff. drivers */
 #define ICR_ASSERT_ACK		0x10	/* rw ini Set to assert ACK */
 #define ICR_ASSERT_BSY		0x08	/* rw Set to assert BSY */
-#define ICR_ASSERT_SEL 		0x04	/* rw Set to assert SEL */
+#define ICR_ASSERT_SEL		0x04	/* rw Set to assert SEL */
 #define ICR_ASSERT_ATN		0x02	/* rw Set to assert ATN */
 #define ICR_ASSERT_DATA		0x01	/* rw SCSI_DATA_REG is asserted */
 
@@ -135,7 +135,7 @@
 #define BASR_IRQ		0x10	/* ro mirror of IRQ pin */
 #define BASR_PHASE_MATCH	0x08	/* ro Set when MSG CD IO match TCR */
 #define BASR_BUSY_ERROR		0x04	/* ro Unexpected change to inactive state */
-#define BASR_ATN 		0x02	/* ro BUS status */
+#define BASR_ATN		0x02	/* ro BUS status */
 #define BASR_ACK		0x01	/* ro BUS status */
 
 /* Write any value to this register to start a DMA send */
@@ -170,7 +170,7 @@
 #define CSR_BASE CSR_53C80_INTR
 
 /* Note : PHASE_* macros are based on the values of the STATUS register */
-#define PHASE_MASK 	(SR_MSG | SR_CD | SR_IO)
+#define PHASE_MASK		(SR_MSG | SR_CD | SR_IO)
 
 #define PHASE_DATAOUT		0
 #define PHASE_DATAIN		SR_IO
@@ -231,7 +231,6 @@ struct NCR5380_cmd {
 	int this_residual;
 	struct scatterlist *buffer;
 	int status;
-	int message;
 	int phase;
 	struct list_head list;
 };
@@ -286,8 +285,9 @@ static const char *NCR5380_info(struct Scsi_Host *instance);
 static void NCR5380_reselect(struct Scsi_Host *instance);
 static bool NCR5380_select(struct Scsi_Host *, struct scsi_cmnd *);
 static int NCR5380_transfer_dma(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data);
-static int NCR5380_transfer_pio(struct Scsi_Host *instance, unsigned char *phase, int *count, unsigned char **data,
-				unsigned int can_sleep);
+static void NCR5380_transfer_pio(struct Scsi_Host *instance,
+				 unsigned char *phase, int *count,
+				 unsigned char **data, unsigned int can_sleep);
 static int NCR5380_poll_politely2(struct NCR5380_hostdata *,
                                   unsigned int, u8, u8,
                                   unsigned int, u8, u8, unsigned long);