Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1673059 > unrolled thread

Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup

Started byOndrej Zary <linux@rainbow-software.org>
First post2017-06-22 23:30 +0200
Last post2017-06-24 04:50 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup Ondrej Zary <linux@rainbow-software.org> - 2017-06-22 23:30 +0200
    Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup Finn Thain <fthain@telegraphics.com.au> - 2017-06-23 03:10 +0200
      Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup Finn Thain <fthain@telegraphics.com.au> - 2017-06-23 13:10 +0200
        Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup Ondrej Zary <linux@rainbow-software.org> - 2017-06-23 22:20 +0200
          Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup Finn Thain <fthain@telegraphics.com.au> - 2017-06-24 04:50 +0200

#1673059 — Re: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup

FromOndrej Zary <linux@rainbow-software.org>
Date2017-06-22 23:30 +0200
SubjectRe: [PATCH 0/4] g_NCR5380: PDMA fixes and cleanup
Message-ID<tVhPP-1Gi-5@gated-at.bofh.it>
On Thursday 15 June 2017 14:17:56 Finn Thain wrote:
> Ondrej, would you please test this patch series? One of your patches
> has been modified slightly and the two I wrote are untested.

Works only with HDD on non-DTC chips. CD-ROM hangs. DTC hangs even with HDD.
The PDMA code really needs to be fixed.

-- 
Ondrej Zary

[toc] | [next] | [standalone]

#1673157

FromFinn Thain <fthain@telegraphics.com.au>
Date2017-06-23 03:10 +0200
Message-ID<tVlgK-3S1-9@gated-at.bofh.it>
In reply to#1673059
On Thu, 22 Jun 2017, Ondrej Zary wrote:

> Works only with HDD on non-DTC chips. CD-ROM hangs. DTC hangs even with 
> HDD. The PDMA code really needs to be fixed.
> 

Does this patch help? It should be applied on top of this series of 4.

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index 4c31cb316a38..95ae8edbecbc 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -481,6 +481,30 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance)
 		release_mem_region(base, region_size);
 }
 
+/* wait_for_53c80_access - wait for 53C80 registers to become accessible
+ * @hostdata: scsi host private data
+ *
+ * The registers within the 53C80 logic block are inaccessible until
+ * bit 7 in the 53C400 control status register gets asserted.
+ */
+
+static int wait_for_53c80_access(struct NCR5380_hostdata *hostdata)
+{
+	int count = 10000;
+
+	do {
+		udelay(4); /* DTC436 chip hangs without this */
+		if (NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)
+			return 0;
+	} while (--count > 0);
+
+	scmd_printk(KERN_ERR, hostdata->connected,
+	            "53c80 registers not accessible, device will be reset\n");
+	NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
+	NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
+	return -1;
+}
+
 /**
  * generic_NCR5380_pread - pseudo DMA receive
  * @hostdata: scsi host private data
@@ -493,33 +517,19 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance)
 static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata,
                                         unsigned char *dst, int len)
 {
-	int start, retries;
-	u8 csr, basr;
+	int result;
+	int start;
 
 	NCR5380_write(hostdata->c400_ctl_status, CSR_BASE | CSR_TRANS_DIR);
 	NCR5380_write(hostdata->c400_blk_cnt, len / 128);
 
 	for (start = 0; start < len; start += 128) {
-		retries = 10000;
-		while (1) {	/* monitor IRQ while waiting for host buffer */
-			csr = NCR5380_read(hostdata->c400_ctl_status);
-			if (!(csr & CSR_HOST_BUF_NOT_RDY))
-				break;
-			if (csr & CSR_GATED_53C80_IRQ) {
-				basr = NCR5380_read(BUS_AND_STATUS_REG);
-				if (!(basr & BASR_PHASE_MATCH) ||
-				    (basr & BASR_BUSY_ERROR)) {
-					printk("basr=0x%02x csr=0x%02x at start=%d\n", basr, csr, start);
-					goto out_wait;
-				}
-			}
-			if (retries-- < 1) {
-				shost_printk(KERN_ERR, hostdata->host, "53C400r: host buffer not ready in time\n");
-				NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
-				NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
-				goto out_wait;
-			}
-		}
+		if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status,
+		                           CSR_HOST_BUF_NOT_RDY, 0,
+		                           hostdata->c400_ctl_status,
+		                           CSR_GATED_53C80_IRQ,
+		                           CSR_GATED_53C80_IRQ, HZ / 64) < 0)
+			break;
 
 		if (hostdata->io_port && hostdata->io_width == 2)
 			insw(hostdata->io_port + hostdata->c400_host_buf,
@@ -532,24 +542,14 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata,
 				hostdata->io + NCR53C400_host_buffer, 128);
 	}
 
-out_wait:
-	/* wait for 53C80 registers to be available */
-	retries = 10000;
-	while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) {
-		if (retries-- < 1) {
-			shost_printk(KERN_ERR, hostdata->host, "53C400r: 53C80 registers not ready in time\n");
-			NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
-			NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
-			break;
-		}
-	}
+	result = wait_for_53c80_access(hostdata);
 
 	if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_END_DMA_TRANSFER))
 		pr_err("%s: No end dma signal (%d/%d)\n", __func__, start, len);
 
-	hostdata->pdma_residual = len - start;
+	hostdata->pdma_residual = NCR5380_read(hostdata->c400_blk_cnt) * 128;
 
-	return 0;
+	return result;
 }
 
 /**
@@ -564,41 +564,19 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata,
 static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata,
                                          unsigned char *src, int len)
 {
-	int start, retries;
-	u8 csr, basr;
+	int result;
+	int start;
 
 	NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
 	NCR5380_write(hostdata->c400_blk_cnt, len / 128);
 
 	for (start = 0; start < len; start += 128) {
-		retries = 10000;
-		while (1) {	/* monitor IRQ while waiting for host buffer */
-			csr = NCR5380_read(hostdata->c400_ctl_status);
-			if (!(csr & CSR_HOST_BUF_NOT_RDY))
-				break;
-			if (csr & CSR_GATED_53C80_IRQ) {
-				basr = NCR5380_read(BUS_AND_STATUS_REG);
-				if (!(basr & BASR_PHASE_MATCH) ||
-				    (basr & BASR_BUSY_ERROR)) {
-					printk("w basr=0x%02x csr=0x%02x at start=%d\n", basr, csr, start);
-					/* the previous block was not written properly */
-					start -= 2 * 128;
-					if (start < 0)
-						start = 0;
-					goto out_wait;
-				}
-			}
-			if (retries-- < 1) {
-				shost_printk(KERN_ERR, hostdata->host, "53C400w: host buffer not ready in time\n");
-				NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
-				NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
-				/* the previous block was not written properly */
-				start -= 2 * 128;
-				if (start < 0)
-					start = 0;
-				goto out_wait;
-			}
-		}
+		if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status,
+		                           CSR_HOST_BUF_NOT_RDY, 0,
+		                           hostdata->c400_ctl_status,
+		                           CSR_GATED_53C80_IRQ,
+		                           CSR_GATED_53C80_IRQ, HZ / 64) < 0)
+			break;
 
 		if (hostdata->io_port && hostdata->io_width == 2)
 			outsw(hostdata->io_port + hostdata->c400_host_buf,
@@ -611,24 +589,12 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata,
 			            src + start, 128);
 	}
 
-out_wait:
-	/* wait for 53C80 registers to be available */
-	udelay(4); /* DTC436 chip hangs without this */
-	retries = 10000;
-	while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) {
-		udelay(4); /* DTC436 chip hangs without this */
-		if (retries-- < 1) {
-			shost_printk(KERN_ERR, hostdata->host, "53C400w: 53C80 registers not ready in time, start=%d, len=%d\n", start, len);
-			NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
-			NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
-			break;
-		}
-	}
+	result = wait_for_53c80_access(hostdata);
 
 	if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_END_DMA_TRANSFER))
 		pr_err("%s: No end dma signal (%d/%d)\n", __func__, start, len);
 
-	hostdata->pdma_residual = len - start;
+	hostdata->pdma_residual = NCR5380_read(hostdata->c400_blk_cnt) * 128;
 
 	if (hostdata->pdma_residual == 0 &&
 	    NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG,
@@ -637,7 +603,7 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata,
 		scmd_printk(KERN_ERR, hostdata->connected,
 		            "%s: Last Byte Sent timeout\n", __func__);
 
-	return 0;
+	return result;
 }
 
 static int generic_NCR5380_dma_xfer_len(struct NCR5380_hostdata *hostdata,

[toc] | [prev] | [next] | [standalone]

#1673475

FromFinn Thain <fthain@telegraphics.com.au>
Date2017-06-23 13:10 +0200
Message-ID<tVuDo-1pp-5@gated-at.bofh.it>
In reply to#1673157
On Fri, 23 Jun 2017, I wrote:

> 
> Does this patch help? It should be applied on top of this series of 4.
> 

Sorry, I sent the wrong diff. Please try this patch instead.

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index e9a942d86865..95ae8edbecbc 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -481,6 +481,30 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance)
 		release_mem_region(base, region_size);
 }
 
+/* wait_for_53c80_access - wait for 53C80 registers to become accessible
+ * @hostdata: scsi host private data
+ *
+ * The registers within the 53C80 logic block are inaccessible until
+ * bit 7 in the 53C400 control status register gets asserted.
+ */
+
+static int wait_for_53c80_access(struct NCR5380_hostdata *hostdata)
+{
+	int count = 10000;
+
+	do {
+		udelay(4); /* DTC436 chip hangs without this */
+		if (NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)
+			return 0;
+	} while (--count > 0);
+
+	scmd_printk(KERN_ERR, hostdata->connected,
+	            "53c80 registers not accessible, device will be reset\n");
+	NCR5380_write(hostdata->c400_ctl_status, CSR_RESET);
+	NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
+	return -1;
+}
+
 /**
  * generic_NCR5380_pread - pseudo DMA receive
  * @hostdata: scsi host private data
@@ -493,18 +517,19 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance)
 static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata,
                                         unsigned char *dst, int len)
 {
-	int blocks = len / 128;
-	int start = 0;
+	int result;
+	int start;
 
 	NCR5380_write(hostdata->c400_ctl_status, CSR_BASE | CSR_TRANS_DIR);
-	NCR5380_write(hostdata->c400_blk_cnt, blocks);
-	while (1) {
-		if (NCR5380_read(hostdata->c400_blk_cnt) == 0)
+	NCR5380_write(hostdata->c400_blk_cnt, len / 128);
+
+	for (start = 0; start < len; start += 128) {
+		if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status,
+		                           CSR_HOST_BUF_NOT_RDY, 0,
+		                           hostdata->c400_ctl_status,
+		                           CSR_GATED_53C80_IRQ,
+		                           CSR_GATED_53C80_IRQ, HZ / 64) < 0)
 			break;
-		if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ)
-			goto out_wait;
-		while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY)
-			; /* FIXME - no timeout */
 
 		if (hostdata->io_port && hostdata->io_width == 2)
 			insw(hostdata->io_port + hostdata->c400_host_buf,
@@ -515,43 +540,16 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata,
 		else
 			memcpy_fromio(dst + start,
 				hostdata->io + NCR53C400_host_buffer, 128);
-
-		start += 128;
-		blocks--;
-	}
-
-	if (blocks) {
-		while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY)
-			; /* FIXME - no timeout */
-
-		if (hostdata->io_port && hostdata->io_width == 2)
-			insw(hostdata->io_port + hostdata->c400_host_buf,
-							dst + start, 64);
-		else if (hostdata->io_port)
-			insb(hostdata->io_port + hostdata->c400_host_buf,
-							dst + start, 128);
-		else
-			memcpy_fromio(dst + start,
-				hostdata->io + NCR53C400_host_buffer, 128);
-
-		start += 128;
-		blocks--;
 	}
 
-	if (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ))
-		printk("53C400r: no 53C80 gated irq after transfer");
-
-out_wait:
-	/* wait for 53C80 registers to be available */
-	while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG))
-		;
+	result = wait_for_53c80_access(hostdata);
 
 	if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_END_DMA_TRANSFER))
 		pr_err("%s: No end dma signal (%d/%d)\n", __func__, start, len);
 
-	hostdata->pdma_residual = len - start;
+	hostdata->pdma_residual = NCR5380_read(hostdata->c400_blk_cnt) * 128;
 
-	return 0;
+	return result;
 }
 
 /**
@@ -566,36 +564,19 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata,
 static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata,
                                          unsigned char *src, int len)
 {
-	int blocks = len / 128;
-	int start = 0;
+	int result;
+	int start;
 
 	NCR5380_write(hostdata->c400_ctl_status, CSR_BASE);
-	NCR5380_write(hostdata->c400_blk_cnt, blocks);
-	while (1) {
-		if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ)
-			goto out_wait;
-
-		if (NCR5380_read(hostdata->c400_blk_cnt) == 0)
+	NCR5380_write(hostdata->c400_blk_cnt, len / 128);
+
+	for (start = 0; start < len; start += 128) {
+		if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status,
+		                           CSR_HOST_BUF_NOT_RDY, 0,
+		                           hostdata->c400_ctl_status,
+		                           CSR_GATED_53C80_IRQ,
+		                           CSR_GATED_53C80_IRQ, HZ / 64) < 0)
 			break;
-		while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY)
-			; // FIXME - timeout
-
-		if (hostdata->io_port && hostdata->io_width == 2)
-			outsw(hostdata->io_port + hostdata->c400_host_buf,
-							src + start, 64);
-		else if (hostdata->io_port)
-			outsb(hostdata->io_port + hostdata->c400_host_buf,
-							src + start, 128);
-		else
-			memcpy_toio(hostdata->io + NCR53C400_host_buffer,
-			            src + start, 128);
-
-		start += 128;
-		blocks--;
-	}
-	if (blocks) {
-		while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY)
-			; // FIXME - no timeout
 
 		if (hostdata->io_port && hostdata->io_width == 2)
 			outsw(hostdata->io_port + hostdata->c400_host_buf,
@@ -606,26 +587,23 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata,
 		else
 			memcpy_toio(hostdata->io + NCR53C400_host_buffer,
 			            src + start, 128);
-
-		start += 128;
-		blocks--;
 	}
 
-out_wait:
-	/* wait for 53C80 registers to be available */
-	while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) {
-		udelay(4); /* DTC436 chip hangs without this */
-		/* FIXME - no timeout */
-	}
+	result = wait_for_53c80_access(hostdata);
 
 	if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_END_DMA_TRANSFER))
 		pr_err("%s: No end dma signal (%d/%d)\n", __func__, start, len);
 
-	hostdata->pdma_residual = len - start;
+	hostdata->pdma_residual = NCR5380_read(hostdata->c400_blk_cnt) * 128;
 
-	while (!(NCR5380_read(TARGET_COMMAND_REG) & TCR_LAST_BYTE_SENT))
-		; 	// TIMEOUT
-	return 0;
+	if (hostdata->pdma_residual == 0 &&
+	    NCR5380_poll_politely(hostdata, TARGET_COMMAND_REG,
+	                          TCR_LAST_BYTE_SENT, TCR_LAST_BYTE_SENT,
+	                          HZ / 64) < 0)
+		scmd_printk(KERN_ERR, hostdata->connected,
+		            "%s: Last Byte Sent timeout\n", __func__);
+
+	return result;
 }
 
 static int generic_NCR5380_dma_xfer_len(struct NCR5380_hostdata *hostdata,

[toc] | [prev] | [next] | [standalone]

#1673816

FromOndrej Zary <linux@rainbow-software.org>
Date2017-06-23 22:20 +0200
Message-ID<tVDdD-6Lb-7@gated-at.bofh.it>
In reply to#1673475
On Friday 23 June 2017 13:01:53 Finn Thain wrote:
> On Fri, 23 Jun 2017, I wrote:
> > Does this patch help? It should be applied on top of this series of 4.
>
> Sorry, I sent the wrong diff. Please try this patch instead.

Thanks, much better now: both HDD and CD-ROM seem to work on DTC and non-DTC 
chips. I get many of these messages with CD-ROM:
[  912.397076] generic_NCR5380_pread: No end dma signal (4096/4096)
[  913.141225] generic_NCR5380_pread: No end dma signal (4096/4096)

Maybe just remove this error message as in my original patch?

-- 
Ondrej Zary

[toc] | [prev] | [next] | [standalone]

#1673985

FromFinn Thain <fthain@telegraphics.com.au>
Date2017-06-24 04:50 +0200
Message-ID<tVJj3-22E-13@gated-at.bofh.it>
In reply to#1673816
On Fri, 23 Jun 2017, Ondrej Zary wrote:

> On Friday 23 June 2017 13:01:53 Finn Thain wrote:
> > On Fri, 23 Jun 2017, I wrote:
> > > Does this patch help? It should be applied on top of this series of 4.
> >
> > Sorry, I sent the wrong diff. Please try this patch instead.
> 
> Thanks, much better now: both HDD and CD-ROM seem to work on DTC and non-DTC 
> chips. I get many of these messages with CD-ROM:
> [  912.397076] generic_NCR5380_pread: No end dma signal (4096/4096)
> [  913.141225] generic_NCR5380_pread: No end dma signal (4096/4096)
> 
> Maybe just remove this error message as in my original patch?
> 

The "data loop" algorithm in the datasheet syas that if End of DMA doesn't 
become asserted after a transfer, this is an error condition.

Your log (above) shows 4096/4096 bytes so I think that End of DMA is 
tested too soon: the datasheet says that End of DMA takes 100 ns to become 
asserted even after all of the relevant signals reach their final state.

I'll re-do the series to account for this.

Thanks for testing.

--

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web