* [PATCH 06/11] scsi: NCR5380: Initialize buffer for MSG IN and STATUS transfers
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 10/11] scsi: NCR5380: Remove obsolete comment Finn Thain
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
Following an incomplete transfer in MSG IN phase, the driver would not
notice the problem and would make use of invalid data. Initialize 'tmp'
appropriately and bail out if no message was received. For STATUS phase,
preserve the existing status code unless a new value was transferred.
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/NCR5380.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 00e245173320..4fcb73b727aa 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -1807,8 +1807,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);
+ if (tmp == 0xff)
+ break;
ncmd->message = tmp;
switch (tmp) {
@@ -1996,6 +1999,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;
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 10/11] scsi: NCR5380: Remove obsolete comment
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
2024-08-07 3:36 ` [PATCH 06/11] scsi: NCR5380: Initialize buffer for MSG IN and STATUS transfers Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 11/11] scsi: NCR5380: Clean up indentation Finn Thain
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
This comment should have been removed in commit e7734ef14ead ("scsi:
NCR5380: Remove context check") when the irqs_disabled() conditional
was removed.
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/NCR5380.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 931b2581a33d..94501773506b 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -198,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.
*/
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 11/11] scsi: NCR5380: Clean up indentation
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
2024-08-07 3:36 ` [PATCH 06/11] scsi: NCR5380: Initialize buffer for MSG IN and STATUS transfers Finn Thain
2024-08-07 3:36 ` [PATCH 10/11] scsi: NCR5380: Remove obsolete comment Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 08/11] scsi: NCR5380: Drop redundant member from struct NCR5380_cmd Finn Thain
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
Tidy up a few indentation annoyances. No functional change.
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/NCR5380.c | 92 +++++++++++++++++++++-------------------
drivers/scsi/NCR5380.h | 14 +++---
drivers/scsi/sun3_scsi.c | 2 +-
3 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 94501773506b..0e10502660de 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -1318,17 +1318,19 @@ static void 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);
@@ -1559,39 +1561,41 @@ 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.
- */
+ /*
+ * 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) &&
(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH)) {
diff --git a/drivers/scsi/NCR5380.h b/drivers/scsi/NCR5380.h
index 64a1c6ce5e1b..d402d4bffcb2 100644
--- a/drivers/scsi/NCR5380.h
+++ b/drivers/scsi/NCR5380.h
@@ -3,10 +3,10 @@
* NCR 5380 defines
*
* Copyright 1993, Drew Eckhardt
- * Visionary Computing
- * (Unix consulting and custom programming)
- * drew@colorado.edu
- * +1 (303) 666-5836
+ * Visionary Computing
+ * (Unix consulting and custom programming)
+ * drew@colorado.edu
+ * +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
diff --git a/drivers/scsi/sun3_scsi.c b/drivers/scsi/sun3_scsi.c
index 4a8cc2e8238e..3d7075714e34 100644
--- a/drivers/scsi/sun3_scsi.c
+++ b/drivers/scsi/sun3_scsi.c
@@ -304,7 +304,7 @@ static int sun3scsi_dma_setup(struct NCR5380_hostdata *hostdata,
sun3_udc_write(UDC_INT_ENABLE, UDC_CSR);
#endif
- return count;
+ return count;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 08/11] scsi: NCR5380: Drop redundant member from struct NCR5380_cmd
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (2 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 11/11] scsi: NCR5380: Clean up indentation Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 07/11] scsi: NCR5380: Handle BSY signal loss during information transfer phases Finn Thain
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
The 'message' member is stored but never loaded so just remove it.
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/NCR5380.c | 2 --
drivers/scsi/NCR5380.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 8a9df2ab9569..a47a825e7220 100644
--- a/drivers/scsi/NCR5380.c
+++ b/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)
@@ -1807,7 +1806,6 @@ static void NCR5380_information_transfer(struct Scsi_Host *instance)
NCR5380_transfer_pio(instance, &phase, &len, &data, 0);
if (tmp == 0xff)
break;
- ncmd->message = tmp;
switch (tmp) {
case ABORT:
diff --git a/drivers/scsi/NCR5380.h b/drivers/scsi/NCR5380.h
index 8dc2be4212dc..84db14b036e4 100644
--- a/drivers/scsi/NCR5380.h
+++ b/drivers/scsi/NCR5380.h
@@ -231,7 +231,6 @@ struct NCR5380_cmd {
int this_residual;
struct scatterlist *buffer;
int status;
- int message;
int phase;
struct list_head list;
};
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 07/11] scsi: NCR5380: Handle BSY signal loss during information transfer phases
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (3 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 08/11] scsi: NCR5380: Drop redundant member from struct NCR5380_cmd Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 04/11] scsi: NCR5380: Check for phase match during PDMA fixup Finn Thain
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
Improve robustness by checking for a lost BSY signal during the
information transfer loop. The status register is being polled anyway,
so a BSY check costs nothing. BSY signal loss could be caused by a
target error or a kicked plug etc. A bus reset is another possibility
but that is already handled and hostdata->connected would be NULL.
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/NCR5380.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 4fcb73b727aa..8a9df2ab9569 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -1244,8 +1244,6 @@ static bool NCR5380_select(struct Scsi_Host *instance, struct scsi_cmnd *cmd)
* is in same phase.
*
* Also, *phase, *count, *data are modified in place.
- *
- * XXX Note : handling for bus free may be useful.
*/
/*
@@ -1277,8 +1275,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");
@@ -1666,9 +1664,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)
@@ -2009,9 +2004,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);
+ }
}
}
}
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 04/11] scsi: NCR5380: Check for phase match during PDMA fixup
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (4 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 07/11] scsi: NCR5380: Handle BSY signal loss during information transfer phases Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 01/11] scsi: mac_scsi: Revise printk(KERN_DEBUG ...) messages Finn Thain
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
It's not an error for a target to change the bus phase during a transfer.
Unfortunately, the FLAG_DMA_FIXUP workaround does not allow for that --
a phase change produces a DRQ timeout error and the device borken flag
will be set.
Check the phase match bit during FLAG_DMA_FIXUP processing. Don't forget
to decrement the command residual. While we are here, change
shost_printk() into scmd_printk() for better consistency with other DMA
error messages.
Tested-by: Stan Johnson <userm57@yahoo.com>
Fixes: 55181be8ced1 ("ncr5380: Replace redundant flags with FLAG_NO_DMA_FIXUP")
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/NCR5380.c | 78 +++++++++++++++++++++---------------------
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index cea3a79d538e..00e245173320 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -1485,6 +1485,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 +1497,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)
@@ -1608,45 +1609,44 @@ static int NCR5380_transfer_dma(struct Scsi_Host *instance,
* 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.
- */
-
- 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");
}
}
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 01/11] scsi: mac_scsi: Revise printk(KERN_DEBUG ...) messages
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (5 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 04/11] scsi: NCR5380: Check for phase match during PDMA fixup Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 09/11] scsi: NCR5380: Remove redundant result calculation from NCR5380_transfer_pio() Finn Thain
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
stable, linux-scsi, linux-kernel
After a bus fault, capture and log the chip registers immediately,
if the NDEBUG_PSEUDO_DMA macro is defined. Remove some
printk(KERN_DEBUG ...) messages that aren't needed any more.
Don't skip the debug message when bytes == 0. Show all of the byte
counters in the debug messages.
Cc: stable@vger.kernel.org # 5.15+
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
This is not really a bug fix, but has been sent to @stable because it is a
prerequisite for the bug fixes which follow.
---
drivers/scsi/mac_scsi.c | 42 +++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c
index a402c4dc4645..5ae8f4a1e9ca 100644
--- a/drivers/scsi/mac_scsi.c
+++ b/drivers/scsi/mac_scsi.c
@@ -286,13 +286,14 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG,
BASR_DRQ | BASR_PHASE_MATCH,
BASR_DRQ | BASR_PHASE_MATCH, 0)) {
- int bytes;
+ int bytes, chunk_bytes;
if (macintosh_config->ident == MAC_MODEL_IIFX)
write_ctrl_reg(hostdata, CTRL_HANDSHAKE_MODE |
CTRL_INTERRUPTS_ENABLE);
- bytes = mac_pdma_recv(s, d, min(hostdata->pdma_residual, 512));
+ chunk_bytes = min(hostdata->pdma_residual, 512);
+ bytes = mac_pdma_recv(s, d, chunk_bytes);
if (bytes > 0) {
d += bytes;
@@ -302,23 +303,23 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
if (hostdata->pdma_residual == 0)
goto out;
- if (NCR5380_poll_politely2(hostdata, STATUS_REG, SR_REQ, SR_REQ,
- BUS_AND_STATUS_REG, BASR_ACK,
- BASR_ACK, 0) < 0)
- scmd_printk(KERN_DEBUG, hostdata->connected,
- "%s: !REQ and !ACK\n", __func__);
if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH))
goto out;
if (bytes == 0)
udelay(MAC_PDMA_DELAY);
- if (bytes >= 0)
+ if (bytes > 0)
continue;
- dsprintk(NDEBUG_PSEUDO_DMA, hostdata->host,
- "%s: bus error (%d/%d)\n", __func__, d - dst, len);
NCR5380_dprint(NDEBUG_PSEUDO_DMA, hostdata->host);
+ dsprintk(NDEBUG_PSEUDO_DMA, hostdata->host,
+ "%s: bus error [%d/%d] (%d/%d)\n",
+ __func__, d - dst, len, bytes, chunk_bytes);
+
+ if (bytes == 0)
+ continue;
+
result = -1;
goto out;
}
@@ -345,13 +346,14 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG,
BASR_DRQ | BASR_PHASE_MATCH,
BASR_DRQ | BASR_PHASE_MATCH, 0)) {
- int bytes;
+ int bytes, chunk_bytes;
if (macintosh_config->ident == MAC_MODEL_IIFX)
write_ctrl_reg(hostdata, CTRL_HANDSHAKE_MODE |
CTRL_INTERRUPTS_ENABLE);
- bytes = mac_pdma_send(s, d, min(hostdata->pdma_residual, 512));
+ chunk_bytes = min(hostdata->pdma_residual, 512);
+ bytes = mac_pdma_send(s, d, chunk_bytes);
if (bytes > 0) {
s += bytes;
@@ -370,23 +372,23 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
goto out;
}
- if (NCR5380_poll_politely2(hostdata, STATUS_REG, SR_REQ, SR_REQ,
- BUS_AND_STATUS_REG, BASR_ACK,
- BASR_ACK, 0) < 0)
- scmd_printk(KERN_DEBUG, hostdata->connected,
- "%s: !REQ and !ACK\n", __func__);
if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH))
goto out;
if (bytes == 0)
udelay(MAC_PDMA_DELAY);
- if (bytes >= 0)
+ if (bytes > 0)
continue;
- dsprintk(NDEBUG_PSEUDO_DMA, hostdata->host,
- "%s: bus error (%d/%d)\n", __func__, s - src, len);
NCR5380_dprint(NDEBUG_PSEUDO_DMA, hostdata->host);
+ dsprintk(NDEBUG_PSEUDO_DMA, hostdata->host,
+ "%s: bus error [%d/%d] (%d/%d)\n",
+ __func__, s - src, len, bytes, chunk_bytes);
+
+ if (bytes == 0)
+ continue;
+
result = -1;
goto out;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 09/11] scsi: NCR5380: Remove redundant result calculation from NCR5380_transfer_pio()
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (6 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 01/11] scsi: mac_scsi: Revise printk(KERN_DEBUG ...) messages Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 03/11] scsi: mac_scsi: Disallow bus errors during PDMA send Finn Thain
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
NCR5380_transfer_pio() returns an ambiguous value which is ignored by
callers. Make it void and remove the redundant calculation. Adopt
kernel-doc format for the updated description.
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/NCR5380.c | 34 +++++++++++-----------------------
drivers/scsi/NCR5380.h | 5 +++--
2 files changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index a47a825e7220..931b2581a33d 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -1227,22 +1227,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.
+/**
+ * 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
*
- * Also, *phase, *count, *data are modified in place.
+ * Returns: void. *phase, *count, *data are modified in place.
*/
/*
@@ -1251,9 +1244,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;
@@ -1358,11 +1351,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;
}
/**
diff --git a/drivers/scsi/NCR5380.h b/drivers/scsi/NCR5380.h
index 84db14b036e4..64a1c6ce5e1b 100644
--- a/drivers/scsi/NCR5380.h
+++ b/drivers/scsi/NCR5380.h
@@ -285,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);
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 03/11] scsi: mac_scsi: Disallow bus errors during PDMA send
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (7 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 09/11] scsi: NCR5380: Remove redundant result calculation from NCR5380_transfer_pio() Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 05/11] scsi: mac_scsi: Enable scatter/gather by default Finn Thain
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
stable, linux-scsi, linux-kernel
SD cards can produce write latency spikes on the order of a hundred
milliseconds. If the target firmware does not hide that latency during
DATA IN and OUT phases it can cause the PDMA circuitry to raise a
processor bus fault which in turn leads to an unreliable byte count
and a DMA overrun.
The Last Byte Sent flag is used to detect the overrun but this mechanism
is unreliable on some systems. Instead, set a DID_ERROR result whenever
there is a bus fault during a PDMA send, unless the cause was a phase
mismatch.
Cc: stable@vger.kernel.org # 5.15+
Reported-and-tested-by: Stan Johnson <userm57@yahoo.com>
Fixes: 7c1f3e3447a1 ("scsi: mac_scsi: Treat Last Byte Sent time-out as failure")
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/mac_scsi.c | 44 ++++++++++++++++++-----------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c
index 39814c841113..2e9fad1e3069 100644
--- a/drivers/scsi/mac_scsi.c
+++ b/drivers/scsi/mac_scsi.c
@@ -102,11 +102,15 @@ __setup("mac5380=", mac_scsi_setup);
* Linux SCSI drivers lack knowledge of the timing behaviour of SCSI targets
* so bus errors are unavoidable.
*
- * If a MOVE.B instruction faults, we assume that zero bytes were transferred
- * and simply retry. That assumption probably depends on target behaviour but
- * seems to hold up okay. The NOP provides synchronization: without it the
- * fault can sometimes occur after the program counter has moved past the
- * offending instruction. Post-increment addressing can't be used.
+ * If a MOVE.B instruction faults during a receive operation, we assume the
+ * target sent nothing and try again. That assumption probably depends on
+ * target firmware but it seems to hold up okay. If a fault happens during a
+ * send operation, the target may or may not have seen /ACK and got the byte.
+ * It's uncertain so the whole SCSI command gets retried.
+ *
+ * The NOP is needed for synchronization because the fault address in the
+ * exception stack frame may or may not be the instruction that actually
+ * caused the bus error. Post-increment addressing can't be used.
*/
#define MOVE_BYTE(operands) \
@@ -243,22 +247,21 @@ static inline int mac_pdma_send(unsigned char *start, void __iomem *io, int n)
if (n >= 1) {
MOVE_BYTE("%0@,%3@");
if (result)
- goto out;
+ return -1;
}
if (n >= 1 && ((unsigned long)addr & 1)) {
MOVE_BYTE("%0@,%3@");
if (result)
- goto out;
+ return -2;
}
while (n >= 32)
MOVE_16_WORDS("%0@+,%3@");
while (n >= 2)
MOVE_WORD("%0@+,%3@");
if (result)
- return start - addr; /* Negated to indicate uncertain length */
+ return start - addr - 1; /* Negated to indicate uncertain length */
if (n == 1)
MOVE_BYTE("%0@,%3@");
-out:
return addr - start;
}
@@ -307,7 +310,6 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
{
u8 __iomem *s = hostdata->pdma_io + (INPUT_DATA_REG << 4);
unsigned char *d = dst;
- int result = 0;
hostdata->pdma_residual = len;
@@ -343,11 +345,12 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
if (bytes == 0)
continue;
- result = -1;
+ if (macscsi_wait_for_drq(hostdata) <= 0)
+ set_host_byte(hostdata->connected, DID_ERROR);
break;
}
- return result;
+ return 0;
}
static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
@@ -355,7 +358,6 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
{
unsigned char *s = src;
u8 __iomem *d = hostdata->pdma_io + (OUTPUT_DATA_REG << 4);
- int result = 0;
hostdata->pdma_residual = len;
@@ -377,17 +379,8 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
hostdata->pdma_residual -= bytes;
}
- if (hostdata->pdma_residual == 0) {
- if (NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG,
- TCR_LAST_BYTE_SENT,
- TCR_LAST_BYTE_SENT,
- 0) < 0) {
- scmd_printk(KERN_ERR, hostdata->connected,
- "%s: Last Byte Sent timeout\n", __func__);
- result = -1;
- }
+ if (hostdata->pdma_residual == 0)
break;
- }
if (bytes > 0)
continue;
@@ -400,11 +393,12 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
if (bytes == 0)
continue;
- result = -1;
+ if (macscsi_wait_for_drq(hostdata) <= 0)
+ set_host_byte(hostdata->connected, DID_ERROR);
break;
}
- return result;
+ return 0;
}
static int macscsi_dma_xfer_len(struct NCR5380_hostdata *hostdata,
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 05/11] scsi: mac_scsi: Enable scatter/gather by default
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (8 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 03/11] scsi: mac_scsi: Disallow bus errors during PDMA send Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-07 3:36 ` [PATCH 02/11] scsi: mac_scsi: Refactor polling loop Finn Thain
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
linux-scsi, linux-kernel
Now that FLAG_DMA_FIXUP has itself been fixed up, it can be used to enable
scatter/gather. Increase the default value for sg_tablesize to SG_ALL
for those systems which are compatible with FLAG_DMA_FIXUP.
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
drivers/scsi/mac_scsi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c
index 2e9fad1e3069..6ab7d82c9a99 100644
--- a/drivers/scsi/mac_scsi.c
+++ b/drivers/scsi/mac_scsi.c
@@ -432,7 +432,7 @@ static struct scsi_host_template mac_scsi_template = {
.eh_host_reset_handler = macscsi_host_reset,
.can_queue = 16,
.this_id = 7,
- .sg_tablesize = 1,
+ .sg_tablesize = SG_ALL,
.cmd_per_lun = 2,
.dma_boundary = PAGE_SIZE - 1,
.cmd_size = sizeof(struct NCR5380_cmd),
@@ -470,6 +470,9 @@ static int __init mac_scsi_probe(struct platform_device *pdev)
if (setup_hostid >= 0)
mac_scsi_template.this_id = setup_hostid & 7;
+ if (macintosh_config->ident == MAC_MODEL_IIFX)
+ mac_scsi_template.sg_tablesize = 1;
+
instance = scsi_host_alloc(&mac_scsi_template,
sizeof(struct NCR5380_hostdata));
if (!instance)
@@ -491,6 +494,9 @@ static int __init mac_scsi_probe(struct platform_device *pdev)
host_flags |= setup_toshiba_delay > 0 ? FLAG_TOSHIBA_DELAY : 0;
+ if (instance->sg_tablesize > 1)
+ host_flags |= FLAG_DMA_FIXUP;
+
error = NCR5380_init(instance, host_flags | FLAG_LATE_DMA_SETUP);
if (error)
goto fail_init;
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 02/11] scsi: mac_scsi: Refactor polling loop
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (9 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 05/11] scsi: mac_scsi: Enable scatter/gather by default Finn Thain
@ 2024-08-07 3:36 ` Finn Thain
2024-08-13 2:07 ` [PATCH 00/11] NCR5380: Bug fixes and other improvements Martin K. Petersen
2024-08-17 1:34 ` Martin K. Petersen
12 siblings, 0 replies; 14+ messages in thread
From: Finn Thain @ 2024-08-07 3:36 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Hannes Reinecke, Michael Schmitz, Ondrej Zary, Stan Johnson,
stable, linux-scsi, linux-kernel
Before the error handling can be revised, some preparation is needed.
Refactor the polling loop with a new function, macscsi_wait_for_drq().
This function will gain more call sites in the next patch.
Cc: stable@vger.kernel.org # 5.15+
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
---
This is not really a bug fix, but has been sent to @stable because it is a
prerequisite for the bug fixes which follow.
---
drivers/scsi/mac_scsi.c | 80 +++++++++++++++++++++--------------------
1 file changed, 42 insertions(+), 38 deletions(-)
diff --git a/drivers/scsi/mac_scsi.c b/drivers/scsi/mac_scsi.c
index 5ae8f4a1e9ca..39814c841113 100644
--- a/drivers/scsi/mac_scsi.c
+++ b/drivers/scsi/mac_scsi.c
@@ -208,8 +208,6 @@ __setup("mac5380=", mac_scsi_setup);
".previous \n" \
: "+a" (addr), "+r" (n), "+r" (result) : "a" (io))
-#define MAC_PDMA_DELAY 32
-
static inline int mac_pdma_recv(void __iomem *io, unsigned char *start, int n)
{
unsigned char *addr = start;
@@ -274,6 +272,36 @@ static inline void write_ctrl_reg(struct NCR5380_hostdata *hostdata, u32 value)
out_be32(hostdata->io + (CTRL_REG << 4), value);
}
+static inline int macscsi_wait_for_drq(struct NCR5380_hostdata *hostdata)
+{
+ unsigned int n = 1; /* effectively multiplies NCR5380_REG_POLL_TIME */
+ unsigned char basr;
+
+again:
+ basr = NCR5380_read(BUS_AND_STATUS_REG);
+
+ if (!(basr & BASR_PHASE_MATCH))
+ return 1;
+
+ if (basr & BASR_IRQ)
+ return -1;
+
+ if (basr & BASR_DRQ)
+ return 0;
+
+ if (n-- == 0) {
+ NCR5380_dprint(NDEBUG_PSEUDO_DMA, hostdata->host);
+ dsprintk(NDEBUG_PSEUDO_DMA, hostdata->host,
+ "%s: DRQ timeout\n", __func__);
+ return -1;
+ }
+
+ NCR5380_poll_politely2(hostdata,
+ BUS_AND_STATUS_REG, BASR_DRQ, BASR_DRQ,
+ BUS_AND_STATUS_REG, BASR_PHASE_MATCH, 0, 0);
+ goto again;
+}
+
static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
unsigned char *dst, int len)
{
@@ -283,9 +311,7 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
hostdata->pdma_residual = len;
- while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG,
- BASR_DRQ | BASR_PHASE_MATCH,
- BASR_DRQ | BASR_PHASE_MATCH, 0)) {
+ while (macscsi_wait_for_drq(hostdata) == 0) {
int bytes, chunk_bytes;
if (macintosh_config->ident == MAC_MODEL_IIFX)
@@ -295,19 +321,16 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
chunk_bytes = min(hostdata->pdma_residual, 512);
bytes = mac_pdma_recv(s, d, chunk_bytes);
+ if (macintosh_config->ident == MAC_MODEL_IIFX)
+ write_ctrl_reg(hostdata, CTRL_INTERRUPTS_ENABLE);
+
if (bytes > 0) {
d += bytes;
hostdata->pdma_residual -= bytes;
}
if (hostdata->pdma_residual == 0)
- goto out;
-
- if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH))
- goto out;
-
- if (bytes == 0)
- udelay(MAC_PDMA_DELAY);
+ break;
if (bytes > 0)
continue;
@@ -321,16 +344,9 @@ static inline int macscsi_pread(struct NCR5380_hostdata *hostdata,
continue;
result = -1;
- goto out;
+ break;
}
- scmd_printk(KERN_ERR, hostdata->connected,
- "%s: phase mismatch or !DRQ\n", __func__);
- NCR5380_dprint(NDEBUG_PSEUDO_DMA, hostdata->host);
- result = -1;
-out:
- if (macintosh_config->ident == MAC_MODEL_IIFX)
- write_ctrl_reg(hostdata, CTRL_INTERRUPTS_ENABLE);
return result;
}
@@ -343,9 +359,7 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
hostdata->pdma_residual = len;
- while (!NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG,
- BASR_DRQ | BASR_PHASE_MATCH,
- BASR_DRQ | BASR_PHASE_MATCH, 0)) {
+ while (macscsi_wait_for_drq(hostdata) == 0) {
int bytes, chunk_bytes;
if (macintosh_config->ident == MAC_MODEL_IIFX)
@@ -355,6 +369,9 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
chunk_bytes = min(hostdata->pdma_residual, 512);
bytes = mac_pdma_send(s, d, chunk_bytes);
+ if (macintosh_config->ident == MAC_MODEL_IIFX)
+ write_ctrl_reg(hostdata, CTRL_INTERRUPTS_ENABLE);
+
if (bytes > 0) {
s += bytes;
hostdata->pdma_residual -= bytes;
@@ -369,15 +386,9 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
"%s: Last Byte Sent timeout\n", __func__);
result = -1;
}
- goto out;
+ break;
}
- if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_PHASE_MATCH))
- goto out;
-
- if (bytes == 0)
- udelay(MAC_PDMA_DELAY);
-
if (bytes > 0)
continue;
@@ -390,16 +401,9 @@ static inline int macscsi_pwrite(struct NCR5380_hostdata *hostdata,
continue;
result = -1;
- goto out;
+ break;
}
- scmd_printk(KERN_ERR, hostdata->connected,
- "%s: phase mismatch or !DRQ\n", __func__);
- NCR5380_dprint(NDEBUG_PSEUDO_DMA, hostdata->host);
- result = -1;
-out:
- if (macintosh_config->ident == MAC_MODEL_IIFX)
- write_ctrl_reg(hostdata, CTRL_INTERRUPTS_ENABLE);
return result;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 00/11] NCR5380: Bug fixes and other improvements
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (10 preceding siblings ...)
2024-08-07 3:36 ` [PATCH 02/11] scsi: mac_scsi: Refactor polling loop Finn Thain
@ 2024-08-13 2:07 ` Martin K. Petersen
2024-08-17 1:34 ` Martin K. Petersen
12 siblings, 0 replies; 14+ messages in thread
From: Martin K. Petersen @ 2024-08-13 2:07 UTC (permalink / raw)
To: Finn Thain
Cc: James E.J. Bottomley, Martin K. Petersen, Hannes Reinecke,
linux-kernel, linux-scsi, Ondrej Zary, Michael Schmitz, stable,
Stan Johnson
Finn,
> 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.
Applied to 6.12/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 00/11] NCR5380: Bug fixes and other improvements
2024-08-07 3:36 [PATCH 00/11] NCR5380: Bug fixes and other improvements Finn Thain
` (11 preceding siblings ...)
2024-08-13 2:07 ` [PATCH 00/11] NCR5380: Bug fixes and other improvements Martin K. Petersen
@ 2024-08-17 1:34 ` Martin K. Petersen
12 siblings, 0 replies; 14+ messages in thread
From: Martin K. Petersen @ 2024-08-17 1:34 UTC (permalink / raw)
To: James E.J. Bottomley, Finn Thain
Cc: Martin K . Petersen, Hannes Reinecke, linux-kernel, linux-scsi,
Ondrej Zary, Michael Schmitz, stable, Stan Johnson
On Wed, 07 Aug 2024 13:36:28 +1000, Finn Thain wrote:
> 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.
>
> [...]
Applied to 6.12/scsi-queue, thanks!
[01/11] scsi: mac_scsi: Revise printk(KERN_DEBUG ...) messages
https://git.kernel.org/mkp/scsi/c/5ec4f820cb97
[02/11] scsi: mac_scsi: Refactor polling loop
https://git.kernel.org/mkp/scsi/c/5545c3165cbc
[03/11] scsi: mac_scsi: Disallow bus errors during PDMA send
https://git.kernel.org/mkp/scsi/c/5551bc30e4a6
[04/11] scsi: NCR5380: Check for phase match during PDMA fixup
https://git.kernel.org/mkp/scsi/c/5768718da941
[05/11] scsi: mac_scsi: Enable scatter/gather by default
https://git.kernel.org/mkp/scsi/c/2ac6d29716cd
[06/11] scsi: NCR5380: Initialize buffer for MSG IN and STATUS transfers
https://git.kernel.org/mkp/scsi/c/1c71065df2df
[07/11] scsi: NCR5380: Handle BSY signal loss during information transfer phases
https://git.kernel.org/mkp/scsi/c/086c4802cf99
[08/11] scsi: NCR5380: Drop redundant member from struct NCR5380_cmd
https://git.kernel.org/mkp/scsi/c/476f8c82e218
[09/11] scsi: NCR5380: Remove redundant result calculation from NCR5380_transfer_pio()
https://git.kernel.org/mkp/scsi/c/8663cadefd15
[10/11] scsi: NCR5380: Remove obsolete comment
https://git.kernel.org/mkp/scsi/c/c331df3d4a8d
[11/11] scsi: NCR5380: Clean up indentation
https://git.kernel.org/mkp/scsi/c/a8ebca904f8e
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 14+ messages in thread