* [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
@ 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
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ 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] 6+ 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
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 02/11] scsi: mac_scsi: Refactor polling loop Finn Thain
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ 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] 6+ 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
2024-08-07 3:36 ` [PATCH 01/11] scsi: mac_scsi: Revise printk(KERN_DEBUG ...) messages Finn Thain
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-13 2:07 ` [PATCH 00/11] NCR5380: Bug fixes and other improvements Martin K. Petersen
2024-08-17 1:34 ` Martin K. Petersen
4 siblings, 0 replies; 6+ 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] 6+ 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
` (2 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
4 siblings, 0 replies; 6+ 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] 6+ 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
` (3 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
4 siblings, 0 replies; 6+ 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] 6+ messages in thread