Re: [RFC PATCH 75/71] ncr5380: Remove FLAG_DTC3181E

[Finn Thain <fthain@telegraphics.com.au>]

On Sun, 29 Nov 2015, Ondrej Zary wrote:

> The FLAG_DTC3181E is used to activate a work-around for arbitration lost
> condition that these chips see when ICR is written during arbitration.
> 
> Move the ICR write (to set SEL and BSY) after the arbitration loss check
> and remove FLAG_DTC3181E.

The first test for ICR_ARBITRATION_LOST happens after the required 
arbitration delay, 2.4 us. The second test for ICR_ARBITRATION_LOST 
happens after ICR_ASSERT_SEL.

This second test seems to be pointless. It comes from the flow chart in 
the NCR datasheet (see download link in patch 17). The spec does not 
require this test but some 5380 devices may do. Who knows? It's almost 
impossible to be sure, because it would mean losing a race with another 
bus device right at the end of the arbitration delay (and we extend that 
delay to 3 us anyway).

Certainly one can find other datasheets with sample code and flow charts 
that don't do this second check. The reason is that ICR_ARBITRATION_LOST 
can be triggered when SEL is asserted by any device, so it may be 
triggered after we've won arbitration (because we then set ICR_ASSERT_SEL 
ourselves in order to enter selection phase).

> 
> ... Weird, we now have two consecutive checks for ICR_ARBITRATION_LOST 
> and do different things when they fail...

They do different things because the second exit has to cleanup after the 
ICR write.

I agree that it would be nice to remove the DTC3181E special case. It 
would mean replacing patch 49.

The patch below is another version of your patch 75. It really needs to be 
tested on all kinds of 5380 device, and if possible with a contested bus 
(which would imply diconnection privileges, for which the driver still 
requires that the chip has a working irq).

Index: linux/drivers/scsi/NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/NCR5380.c	2015-11-30 15:34:39.000000000 +1100
+++ linux/drivers/scsi/NCR5380.c	2015-11-30 15:34:39.000000000 +1100
@@ -482,14 +482,13 @@ static void prepare_info(struct Scsi_Hos
 	         "base 0x%lx, irq %d, "
 	         "can_queue %d, cmd_per_lun %d, "
 	         "sg_tablesize %d, this_id %d, "
-	         "flags { %s%s%s%s}, "
+	         "flags { %s%s%s}, "
 	         "options { %s} ",
 	         instance->hostt->name, instance->io_port, instance->n_io_port,
 	         instance->base, instance->irq,
 	         instance->can_queue, instance->cmd_per_lun,
 	         instance->sg_tablesize, instance->this_id,
 	         hostdata->flags & FLAG_NO_DMA_FIXUP  ? "NO_DMA_FIXUP "  : "",
-	         hostdata->flags & FLAG_DTC3181E      ? "DTC3181E "      : "",
 	         hostdata->flags & FLAG_NO_PSEUDO_DMA ? "NO_PSEUDO_DMA " : "",
 	         hostdata->flags & FLAG_TOSHIBA_DELAY ? "TOSHIBA_DELAY "  : "",
 #ifdef AUTOPROBE_IRQ
@@ -1085,18 +1084,6 @@ static struct scsi_cmnd *NCR5380_select(
 	NCR5380_write(INITIATOR_COMMAND_REG,
 		      ICR_BASE | ICR_ASSERT_SEL | ICR_ASSERT_BSY);
 
-	/* RvC: DTC3181E has some trouble with this so we simply removed it.
-	 * Seems to work with only Mustek scanner attached.
-	 */
-	if (!(hostdata->flags & FLAG_DTC3181E) &&
-	    (NCR5380_read(INITIATOR_COMMAND_REG) & ICR_ARBITRATION_LOST)) {
-		NCR5380_write(MODE_REG, MR_BASE);
-		NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE);
-		dsprintk(NDEBUG_ARBITRATION, instance, "arbitration lost, negating SEL\n");
-		spin_lock_irq(&hostdata->lock);
-		goto out;
-	}
-
 	/*
 	 * Again, bus clear + bus settle time is 1.2us, however, this is
 	 * a minimum so we'll udelay ceil(1.2)
Index: linux/drivers/scsi/NCR5380.h
===================================================================
--- linux.orig/drivers/scsi/NCR5380.h	2015-11-30 15:34:36.000000000 +1100
+++ linux/drivers/scsi/NCR5380.h	2015-11-30 15:34:39.000000000 +1100
@@ -233,7 +233,6 @@
 
 #define FLAG_NO_DMA_FIXUP		1	/* No DMA errata workarounds */
 #define FLAG_NO_PSEUDO_DMA		8	/* Inhibit DMA */
-#define FLAG_DTC3181E			16	/* DTC3181E */
 #define FLAG_LATE_DMA_SETUP		32	/* Setup NCR before DMA H/W */
 #define FLAG_TAGGED_QUEUING		64	/* as X3T9.2 spelled it */
 #define FLAG_TOSHIBA_DELAY		128	/* Allow for borken CD-ROMs */
Index: linux/drivers/scsi/atari_NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/atari_NCR5380.c	2015-11-30 15:34:39.000000000 +1100
+++ linux/drivers/scsi/atari_NCR5380.c	2015-11-30 15:34:39.000000000 +1100
@@ -586,13 +586,12 @@ static void prepare_info(struct Scsi_Hos
 	         "base 0x%lx, irq %d, "
 	         "can_queue %d, cmd_per_lun %d, "
 	         "sg_tablesize %d, this_id %d, "
-	         "flags { %s%s%s}, "
+	         "flags { %s%s}, "
 	         "options { %s} ",
 	         instance->hostt->name, instance->io_port, instance->n_io_port,
 	         instance->base, instance->irq,
 	         instance->can_queue, instance->cmd_per_lun,
 	         instance->sg_tablesize, instance->this_id,
-	         hostdata->flags & FLAG_DTC3181E       ? "DTC3181E "       : "",
 	         hostdata->flags & FLAG_TAGGED_QUEUING ? "TAGGED_QUEUING " : "",
 	         hostdata->flags & FLAG_TOSHIBA_DELAY  ? "TOSHIBA_DELAY "  : "",
 #ifdef DIFFERENTIAL
@@ -1286,18 +1285,6 @@ static struct scsi_cmnd *NCR5380_select(
 	NCR5380_write(INITIATOR_COMMAND_REG,
 		      ICR_BASE | ICR_ASSERT_SEL | ICR_ASSERT_BSY);
 
-	/* RvC: DTC3181E has some trouble with this so we simply removed it.
-	 * Seems to work with only Mustek scanner attached.
-	 */
-	if (!(hostdata->flags & FLAG_DTC3181E) &&
-	    (NCR5380_read(INITIATOR_COMMAND_REG) & ICR_ARBITRATION_LOST)) {
-		NCR5380_write(MODE_REG, MR_BASE);
-		NCR5380_write(INITIATOR_COMMAND_REG, ICR_BASE);
-		dsprintk(NDEBUG_ARBITRATION, instance, "arbitration lost, negating SEL\n");
-		spin_lock_irq(&hostdata->lock);
-		goto out;
-	}
-
 	/*
 	 * Again, bus clear + bus settle time is 1.2us, however, this is
 	 * a minimum so we'll udelay ceil(1.2)
Index: linux/drivers/scsi/g_NCR5380.c
===================================================================
--- linux.orig/drivers/scsi/g_NCR5380.c	2015-11-30 15:34:39.000000000 +1100
+++ linux/drivers/scsi/g_NCR5380.c	2015-11-30 15:34:39.000000000 +1100
@@ -326,7 +326,7 @@ static int __init generic_NCR5380_detect
 			ports = ncr_53c400a_ports;
 			break;
 		case BOARD_DTC3181E:
-			flags = FLAG_NO_PSEUDO_DMA | FLAG_DTC3181E;
+			flags = FLAG_NO_PSEUDO_DMA;
 			ports = dtc_3181e_ports;
 			break;
 		}
Index: linux/drivers/scsi/dmx3191d.c
===================================================================
--- linux.orig/drivers/scsi/dmx3191d.c	2015-11-30 15:34:35.000000000 +1100
+++ linux/drivers/scsi/dmx3191d.c	2015-11-30 15:34:39.000000000 +1100
@@ -92,7 +92,7 @@ static int dmx3191d_probe_one(struct pci
 	 */
 	shost->irq = NO_IRQ;
 
-	error = NCR5380_init(shost, FLAG_NO_PSEUDO_DMA | FLAG_DTC3181E);
+	error = NCR5380_init(shost, FLAG_NO_PSEUDO_DMA);
 	if (error)
 		goto out_host_put;