Re: [PATCH 76/71] ncr5380: Enable PDMA for DTC chips

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Fri, 4 Dec 2015, Ondrej Zary wrote:

Add I/O register mapping for DTC chips and enable PDMA mode.

These chips have 16-bit wide HOST BUFFER register (counter register at
offset 0x0d increments by 2 on each HOST BUFFER read). Detect it
automatically.

Large PIO transfers crash at least the DTCT-436P chip (all reads result
in 0xFF) so this patch actually makes it work.

The chip also crashes when we bang the C400 host status register too
heavily after PDMA write - a small udelay is needed.

Tested on DTCT-436P and verified that it does not break 53C400A.

Signed-off-by: Ondrej Zary <linux@xxxxxxxxxxxxxxxxxxxx>
---
 drivers/scsi/g_NCR5380.c |   59 ++++++++++++++++++++++++++++------------------
 drivers/scsi/g_NCR5380.h |    4 +++-
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 85da3c2..9816b81 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -328,7 +328,7 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
 			ports = ncr_53c400a_ports;
 			break;
 		case BOARD_DTC3181E:
-			flags = FLAG_NO_PSEUDO_DMA;
+			flags = FLAG_NO_DMA_FIXUP;

Nice!

 			ports = dtc_3181e_ports;
 			break;
 		}
@@ -412,10 +412,12 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
 			hostdata->c400_blk_cnt = 1;
 			hostdata->c400_host_buf = 4;
 		}
-		if (overrides[current_override].board == BOARD_NCR53C400A) {
+		if (overrides[current_override].board == BOARD_NCR53C400A ||
+		    overrides[current_override].board == BOARD_DTC3181E) {
 			hostdata->c400_ctl_status = 9;
 			hostdata->c400_blk_cnt = 10;
 			hostdata->c400_host_buf = 8;
+			hostdata->c400_host_idx = 13;
 		}
 #else
 		instance->base = overrides[current_override].NCR5380_map_name;
@@ -430,8 +432,20 @@ static int __init generic_NCR5380_detect(struct scsi_host_template *tpnt)
 		if (NCR5380_init(instance, flags))
 			goto out_unregister;
 
+#ifndef SCSI_G_NCR5380_MEM
+		/* read initial value of index register */
+		i = NCR5380_read(hostdata->c400_host_idx);
+		/* read something from host buffer */
+		NCR5380_read(hostdata->c400_host_buf);
+		/* I/O width = index register increment */
+		hostdata->io_width = NCR5380_read(hostdata->c400_host_idx) - i;
+		if (hostdata->io_width < 0)
+			hostdata->io_width += 128;
+#endif

Will the result depend on the initial state of the chip, such as
CSR_TRANS_DIR or CSR_HOST_BUF_NOT_RDY?

It is possible to generate an interrupt with the buffer read, which should 
be masked at the chip.

This buffer read may cause the 53C400 control logic to signal the 53C80 
core. I suppose it is unlikely to cause any signalling on the SCSI bus 
unless the 53C80 happened to be in DMA mode.

The possibility that io_width == 0 is not handled; wouldn't this result 
indicate that PDMA shouldn't be used?

io_width can be calculated without a conditional statement, which may be 
easier to read.

Can we be confident that detection will fail for all devices that don't 
support word-sized IO, to avoid a regression?

The patch seems to assume that no memory-mapped card needs word-sized IO 
for PDMA. Can you confirm?

The previous version of this patch was simpler and more predictable. You 
enabled word-size IO for DTC3181E which is testable. Does this version 
benefit any other cards?

+
 		if (overrides[current_override].board == BOARD_NCR53C400 ||
-		    overrides[current_override].board == BOARD_NCR53C400A)
+		    overrides[current_override].board == BOARD_NCR53C400A ||
+		    overrides[current_override].board == BOARD_DTC3181E)
 			NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
 
 		NCR5380_maybe_reset_bus(instance);
@@ -558,11 +572,10 @@ static inline int NCR5380_pread(struct Scsi_Host *instance, unsigned char *dst,
 		while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY);
 
 #ifndef SCSI_G_NCR5380_MEM
-		{
-			int i;
-			for (i = 0; i < 128; i++)
-				dst[start + i] = NCR5380_read(hostdata->c400_host_buf);
-		}
+		if (hostdata->io_width == 2)
+			insw(instance->io_port + hostdata->c400_host_buf, dst + start, 64);
+		else
+			insb(instance->io_port + hostdata->c400_host_buf, dst + start, 128);
 #else
 		/* implies SCSI_G_NCR5380_MEM */
 		memcpy_fromio(dst + start,
@@ -579,11 +592,10 @@ static inline int NCR5380_pread(struct Scsi_Host *instance, unsigned char *dst,
 		}
 
 #ifndef SCSI_G_NCR5380_MEM
-		{
-			int i;	
-			for (i = 0; i < 128; i++)
-				dst[start + i] = NCR5380_read(hostdata->c400_host_buf);
-		}
+		if (hostdata->io_width == 2)
+			insw(instance->io_port + hostdata->c400_host_buf, dst + start, 64);
+		else
+			insb(instance->io_port + hostdata->c400_host_buf, dst + start, 128);
 #else
 		/* implies SCSI_G_NCR5380_MEM */
 		memcpy_fromio(dst + start,
@@ -642,10 +654,10 @@ static inline int NCR5380_pwrite(struct Scsi_Host *instance, unsigned char *src,
 		while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY)
 			; // FIXME - timeout
 #ifndef SCSI_G_NCR5380_MEM
-		{
-			for (i = 0; i < 128; i++)
-				NCR5380_write(hostdata->c400_host_buf, src[start + i]);
-		}
+		if (hostdata->io_width == 2)
+			outsw(instance->io_port + hostdata->c400_host_buf, src + start, 64);
+		else
+			outsb(instance->io_port + hostdata->c400_host_buf, src + start, 128);
 #else
 		/* implies SCSI_G_NCR5380_MEM */
 		memcpy_toio(hostdata->iomem + NCR53C400_host_buffer,
@@ -657,12 +669,11 @@ static inline int NCR5380_pwrite(struct Scsi_Host *instance, unsigned char *src,
 	if (blocks) {
 		while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY)
 			; // FIXME - no timeout
-
 #ifndef SCSI_G_NCR5380_MEM
-		{
-			for (i = 0; i < 128; i++)
-				NCR5380_write(hostdata->c400_host_buf, src[start + i]);
-		}
+		if (hostdata->io_width == 2)
+			outsw(instance->io_port + hostdata->c400_host_buf, src + start, 64);
+		else
+			outsb(instance->io_port + hostdata->c400_host_buf, src + start, 128);
 #else
 		/* implies SCSI_G_NCR5380_MEM */
 		memcpy_toio(hostdata->iomem + NCR53C400_host_buffer,
@@ -682,8 +693,10 @@ static inline int NCR5380_pwrite(struct Scsi_Host *instance, unsigned char *src,
 	/* All documentation says to check for this. Maybe my hardware is too
 	 * fast. Waiting for it seems to work fine! KLL
 	 */
-	while (!(i = NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ))
+	while (!(i = NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ)) {
+		udelay(4); /* DTC436 chip hangs without this */
 		;	// FIXME - no timeout
+	}

When you added the braces, the lone semicolon became redundant. But I 
think the entire loop is bogus.

Why do we wait for CSR_GATED_53C80_IRQ? I can understand testing it during 
a transfer but why afterwards? (The core driver could test for the IRQ 
flag in the Bus and Status Register, at the end of a DMA, if this scsi 
host has no IRQ line.)

The comments and the 53C400 datasheet say we should instead wait for 
CSR_53C80_REG. I agree. The g_NCR5380 wrapper driver can't safely return 
control to the core driver unless the 53C80 registers are available.

The algorithm in the datasheet waits for CSR_53C80_REG before checking 
BASR_END_DMA_TRANSFER, checking interrupt flags, disabling DMA mode etc. 

g_NCR5380.c has an '#if 0' around the BASR_END_DMA_TRANSFER check, because 
it doesn't wait for CSR_53C80_REG. Does NCR's algorithm work with your 
cards?

 
 	/*
 	 * I know. i is certainly != 0 here but the loop is new. See previous
diff --git a/drivers/scsi/g_NCR5380.h b/drivers/scsi/g_NCR5380.h
index c5e57b7..b3936aa 100644
--- a/drivers/scsi/g_NCR5380.h
+++ b/drivers/scsi/g_NCR5380.h
@@ -44,7 +44,9 @@
 #define NCR5380_implementation_fields \
 	int c400_ctl_status; \
 	int c400_blk_cnt; \
-	int c400_host_buf;
+	int c400_host_buf; \
+	int c400_host_idx; \
+	int io_width;
 
 #else 
 /* therefore SCSI_G_NCR5380_MEM */


-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux