[PATCH] Re: AMD 53c974 SCSI driver in 2.6

[PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Sun, 26 Oct 2003, Guennadi Liakhovetski wrote:

> > The above (AM53C974) driver depends on BROKEN in 2.6 and doesn't compile.
> > Is anybody working on / planning to fix it?

Ok, I fixed it, well, at least, it works for me. What now? The fix is,
probably, not perfect. E.g. it doesn't support multiple cards now, but it
looks like the driver didn't support them even when it worked in its
latest version (sorry, if I am wrong).

Patch attached. Comments / improvement suggestions mostly welcome.

Guennadi
---
Guennadi Liakhovetski

diff -ur linux-2.6.0-test7.arm/drivers/scsi/AM53C974.c linux-2.6.0-test7/drivers/scsi/AM53C974.c
--- linux-2.6.0-test7.arm/drivers/scsi/AM53C974.c	Wed Oct  8 23:26:43 2003
+++ linux-2.6.0-test7/drivers/scsi/AM53C974.c	Thu Oct 30 23:00:40 2003
@@ -1,6 +1,5 @@
-#error Please convert me to Documentation/DMA-mapping.txt
-
 #include <linux/module.h>
+#include <linux/version.h>
 #include <linux/delay.h>
 #include <linux/signal.h>
 #include <linux/sched.h>
@@ -10,6 +9,7 @@
 #include <linux/blkdev.h>
 #include <linux/init.h>
 #include <linux/spinlock.h>
+#include <linux/interrupt.h>

 #include <asm/io.h>
 #include <asm/system.h>
@@ -361,8 +361,8 @@
 static __inline__ void initialize_SCp(Scsi_Cmnd * cmd);
 static __inline__ void run_main(void);
 static void AM53C974_main(void);
-static void AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs);
-static void do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs);
+static irqreturn_t AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs);
+static irqreturn_t do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs);
 static void AM53C974_intr_disconnect(struct Scsi_Host *instance);
 static int AM53C974_sync_neg(struct Scsi_Host *instance, int target, unsigned char *msg);
 static __inline__ void AM53C974_set_async(struct Scsi_Host *instance, int target);
@@ -382,7 +382,11 @@

 static struct Scsi_Host *first_instance;
 static Scsi_Host_Template *the_template;
+#undef AM53C974_MULTIPLE_CARD
+#ifdef AM53C974_MULTIPLE_CARD
+#error "FIXME! Multiple card support is broken. Looks like it never really worked. Might have to be fixed."
 static struct Scsi_Host *first_host;	/* Head of list of AMD boards */
+#endif
 static volatile int main_running;
 static int commandline_current;
 override_t overrides[7] =
@@ -457,8 +461,7 @@

 	printk("AM53C974: coroutine is%s running.\n", main_running ? "" : "n't");

-	save_flags(flags);
-	cli();
+	local_irq_save(flags);

 	if (!hostdata->connected) {
 		printk("scsi%d: no currently connected command\n", instance->host_no);
@@ -489,7 +492,7 @@
 			print_Scsi_Cmnd(ptr);
 	}

-	restore_flags(flags);
+	local_irq_restore(flags);
 }

 #endif				/* AM53C974_DEBUG */
@@ -504,14 +507,17 @@
 static void AM53C974_print(struct Scsi_Host *instance)
 {
 	AM53C974_local_declare();
+#if 0 /* Called only from error-handling paths with sufficient protection? */
 	unsigned long flags;
+#endif
 	unsigned long ctcreg, dmastc, dmaspa, dmawbc, dmawac;
 	unsigned char cmdreg, statreg, isreg, cfireg, cntlreg[4], dmacmd,
 	 dmastatus;
 	AM53C974_setio(instance);

-	save_flags(flags);
-	cli();
+#if 0 /* Called only from error-handling paths with sufficient protection? */
+	local_irq_save(flags);
+#endif
 	ctcreg = AM53C974_read_8(CTCHREG) << 16;
 	ctcreg |= AM53C974_read_8(CTCMREG) << 8;
 	ctcreg |= AM53C974_read_8(CTCLREG);
@@ -529,7 +535,9 @@
 	dmawbc = AM53C974_read_32(DMAWBC);
 	dmawac = AM53C974_read_32(DMAWAC);
 	dmastatus = AM53C974_read_8(DMASTATUS);
-	restore_flags(flags);
+#if 0 /* Called only from error-handling paths with sufficient protection? */
+	local_irq_restore(flags);
+#endif

 	printk("AM53C974 register dump:\n");
 	printk("IO base: 0x%04lx; CTCREG: 0x%04lx; CMDREG: 0x%02x; STATREG: 0x%02x; ISREG: 0x%02x\n",
@@ -559,15 +567,14 @@
 		return;
 #endif

-	save_flags(flags);
-	cli();
+	local_irq_save(flags);
 	while ((inb_p(0x64) & 0x01) != 0x01);
 #ifdef AM53C974_DEBUG
 	key = inb(0x60);
 	if (key == 0x93)
 		deb_stop = 0;	/* don't stop if 'r' was pressed */
 #endif
-	restore_flags(flags);
+	local_irq_restore(flags);
 }

 #ifndef MODULE
@@ -667,7 +674,10 @@
 {
 	AM53C974_local_declare();
 	int i, j;
-	struct Scsi_Host *instance, *search;
+	struct Scsi_Host *instance;
+#ifdef AM53C974_MULTIPLE_CARD
+	struct Scsi_Host *search;
+#endif
 	struct AM53C974_hostdata *hostdata;

 #ifdef AM53C974_OPTION_DEBUG_PROBE_ONLY
@@ -739,21 +749,24 @@
 		return 0;
 	}
 /* Set up an interrupt handler if we aren't already sharing an IRQ with another board */
+#ifdef AM53C974_MULTIPLE_CARD
 	for (search = first_host;
 	     search && (((the_template != NULL) && (search->hostt != the_template)) ||
 		 (search->irq != instance->irq) || (search == instance));
 	     search = search->next);
 	if (!search) {
+#endif
 		if (request_irq(instance->irq, do_AM53C974_intr, SA_SHIRQ, "AM53C974", instance)) {
 			printk("scsi%d: IRQ%d not free, detaching\n", instance->host_no, instance->irq);
 			scsi_unregister(instance);
 			return 0;
 		}
+#ifdef AM53C974_MULTIPLE_CARD
 	} else {
 		printk("scsi%d: using interrupt handler previously installed for scsi%d\n",
 		       instance->host_no, search->host_no);
 	}
-
+#endif
 	if (!the_template) {
 		the_template = instance->hostt;
 		first_instance = instance;
@@ -832,7 +845,7 @@
 	if (cmd->use_sg) {
 		cmd->SCp.buffer = (struct scatterlist *) cmd->buffer;
 		cmd->SCp.buffers_residual = cmd->use_sg - 1;
-		cmd->SCp.ptr = (char *) cmd->SCp.buffer->address;
+		cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
 		cmd->SCp.this_residual = cmd->SCp.buffer->length;
 	} else {
 		cmd->SCp.buffer = NULL;
@@ -858,15 +871,14 @@
 static __inline__ void run_main(void)
 {
 	unsigned long flags;
-	save_flags(flags);
-	cli();
+ 	local_irq_save(flags);
 	if (!main_running) {
 		/* main_running is cleared in AM53C974_main once it can't do
 		   more work, and AM53C974_main exits with interrupts disabled. */
 		main_running = 1;
 		AM53C974_main();
 	}
-	restore_flags(flags);
+	local_irq_restore(flags);
 }

 /**************************************************************************
@@ -891,8 +903,7 @@
 	struct AM53C974_hostdata *hostdata = (struct AM53C974_hostdata *) instance->hostdata;
 	Scsi_Cmnd *tmp;

-	save_flags(flags);
-	cli();
+	local_irq_save(flags);
 	DEB_QUEUE(printk(SEPARATOR_LINE));
 	DEB_QUEUE(printk("scsi%d: AM53C974_queue_command called\n", instance->host_no));
 	DEB_QUEUE(printk("cmd=%02x target=%02x lun=%02x bufflen=%d use_sg = %02x\n",
@@ -924,7 +935,7 @@

 /* Run the coroutine if it isn't already running. */
 	run_main();
-	restore_flags(flags);
+	local_irq_restore(flags);
 	return 0;
 }

@@ -952,12 +963,14 @@
  * the host adapters have anything that can be done, at which point
  * we set main_running to 0 and exit. */

-	save_flags(flags);
-	cli();		/* Freeze request queues */
+	local_irq_save(flags); /* Freeze request queues */
 	do {
 		done = 1;
+#ifdef AM53C974_MULTIPLE_CARD
 		for (instance = first_instance; instance && instance->hostt == the_template;
 		     instance = instance->next) {
+#endif
+			instance = first_instance;
 			hostdata = (struct AM53C974_hostdata *) instance->hostdata;
 			AM53C974_setio(instance);
 			/* start to select target if we are not connected and not in the
@@ -993,10 +1006,12 @@
 				DEB(printk("main: connected; cmd = 0x%lx, sel_cmd = 0x%lx\n",
 					   (long) hostdata->connected, (long) hostdata->sel_cmd));
 			}
+#ifdef AM53C974_MULTIPLE_CARD
 		}		/* for instance */
+#endif
 	} while (!done);
 	main_running = 0;
-	restore_flags(flags);
+	local_irq_restore(flags);
 }

 /************************************************************************
@@ -1008,14 +1023,16 @@
 *                                                                       *
 * Returns : nothing                                                     *
 ************************************************************************/
-static void do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs)
+static irqreturn_t do_AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs)
 {
 	unsigned long flags;
 	struct Scsi_Host *dev = dev_id;
-
+	irqreturn_t ret;
+
 	spin_lock_irqsave(dev->host_lock, flags);
-	AM53C974_intr(irq, dev_id, regs);
+	ret = AM53C974_intr(irq, dev_id, regs);
 	spin_unlock_irqrestore(dev->host_lock, flags);
+	return ret;
 }

 /************************************************************************
@@ -1027,7 +1044,7 @@
 *                                                                       *
 * Returns : nothing                                                     *
 ************************************************************************/
-static void AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs)
+static irqreturn_t AM53C974_intr(int irq, void *dev_id, struct pt_regs *regs)
 {
 	AM53C974_local_declare();
 	struct Scsi_Host *instance;
@@ -1035,10 +1052,13 @@
 	unsigned char cmdreg, dmastatus, statreg, isreg, instreg, cfifo;

 /* find AM53C974 hostadapter responsible for this interrupt */
+#ifdef AM53C974_MULTIPLE_CARD
 	for (instance = first_instance; instance; instance = instance->next)
+#endif
+	instance = first_instance;
 		if ((instance->irq == irq) && (instance->hostt == the_template))
 			goto FOUND;
-	return;
+	return IRQ_NONE;

 /* found; now decode and process */
 	FOUND:
@@ -1065,8 +1085,7 @@
 		/* DMA transfer done */
 		unsigned long residual;
 		unsigned long flags;
-		save_flags(flags);
-		cli();
+		local_irq_save(flags);
 		if (!(AM53C974_read_8(DMACMD) & DMACMD_DIR)) {
 			do {
 				dmastatus = AM53C974_read_8(DMASTATUS);
@@ -1095,10 +1114,10 @@
 			AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo,
 						      dmastatus);
 		}
-		restore_flags(flags);
+		local_irq_restore(flags);
 	}
 	if (!(dmastatus & DMASTATUS_SCSIINT)) {
-		return;
+		return IRQ_HANDLED;
 	}
 /*** SCSI related interrupts ***/
 	cmdreg = AM53C974_read_8(CMDREG);
@@ -1138,8 +1157,7 @@
 #endif
 		DEB(printk("Bus reset interrupt received\n"));
 		AM53C974_intr_bus_reset(instance);
-		save_flags(flags);
-		cli();
+		local_irq_save(flags);
 		if (hostdata->connected) {
 			hostdata->connected->result = DID_RESET << 16;
 			hostdata->connected->scsi_done((Scsi_Cmnd *) hostdata->connected);
@@ -1151,11 +1169,11 @@
 				hostdata->sel_cmd = NULL;
 			}
 		}
-		restore_flags(flags);
+		local_irq_restore(flags);
 		if (hostdata->in_reset == 1)
 			goto EXIT;
 		else
-			return;
+			return IRQ_HANDLED;
 	}
 	if (instreg & INSTREG_ICMD) {
 		/* INVALID COMMAND INTERRUPT */
@@ -1172,20 +1190,18 @@
 		unsigned long flags;
 		/* DISCONNECT INTERRUPT */
 		DEB_INTR(printk("Disconnect interrupt received; "));
-		save_flags(flags);
-		cli();
+		local_irq_save(flags);
 		AM53C974_intr_disconnect(instance);
-		restore_flags(flags);
+		local_irq_restore(flags);
 		goto EXIT;
 	}
 	if (instreg & INSTREG_RESEL) {
 		unsigned long flags;
 		/* RESELECTION INTERRUPT */
 		DEB_INTR(printk("Reselection interrupt received\n"));
-		save_flags(flags);
-		cli();
+		local_irq_save(flags);
 		AM53C974_intr_reselect(instance, statreg);
-		restore_flags(flags);
+		local_irq_restore(flags);
 		goto EXIT;
 	}
 	if (instreg & INSTREG_SO) {
@@ -1193,15 +1209,14 @@
 		if (hostdata->selecting) {
 			unsigned long flags;
 			DEB_INTR(printk("DSR completed, starting select\n"));
-			save_flags(flags);
-			cli();
+			local_irq_save(flags);
 			AM53C974_select(instance, (Scsi_Cmnd *) hostdata->sel_cmd,
 			  (hostdata->sel_cmd->cmnd[0] == REQUEST_SENSE) ?
 					TAG_NONE : TAG_NEXT);
 			hostdata->selecting = 0;
 			AM53C974_set_sync(instance, hostdata->sel_cmd->device->id);
-			restore_flags(flags);
-			return;
+			local_irq_restore(flags);
+			return IRQ_HANDLED;
 		}
 		if (hostdata->sel_cmd != NULL) {
 			if (((isreg & ISREG_IS) != ISREG_OK_NO_STOP) &&
@@ -1209,22 +1224,20 @@
 				unsigned long flags;
 				/* UNSUCCESSFUL SELECTION */
 				DEB_INTR(printk("unsuccessful selection\n"));
-				save_flags(flags);
-				cli();
+				local_irq_save(flags);
 				hostdata->dma_busy = 0;
 				LIST(hostdata->sel_cmd, hostdata->issue_queue);
 				hostdata->sel_cmd->host_scribble = (unsigned char *) hostdata->issue_queue;
 				hostdata->issue_queue = hostdata->sel_cmd;
 				hostdata->sel_cmd = NULL;
 				hostdata->selecting = 0;
-				restore_flags(flags);
+				local_irq_restore(flags);
 				goto EXIT;
 			} else {
 				unsigned long flags;
 				/* SUCCESSFUL SELECTION */
 				DEB(printk("successful selection; cmd=0x%02lx\n", (long) hostdata->sel_cmd));
-				save_flags(flags);
-				cli();
+				local_irq_save(flags);
 				hostdata->dma_busy = 0;
 				hostdata->disconnecting = 0;
 				hostdata->connected = hostdata->sel_cmd;
@@ -1232,7 +1245,7 @@
 				hostdata->selecting = 0;
 #ifdef SCSI2
 				if (!hostdata->conneted->device->simple_tags)
-#else
+#endif
 					hostdata->busy[hostdata->connected->device->id] |= (1 << hostdata->connected->device->lun);
 				/* very strange -- use_sg is sometimes nonzero for request sense commands !! */
 				if ((hostdata->connected->cmnd[0] == REQUEST_SENSE) && hostdata->connected->use_sg) {
@@ -1243,16 +1256,15 @@
 				initialize_SCp((Scsi_Cmnd *) hostdata->connected);
 				hostdata->connected->SCp.phase = PHASE_CMDOUT;
 				AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo, dmastatus);
-				restore_flags(flags);
-				return;
+				local_irq_restore(flags);
+				return IRQ_HANDLED;
 			}
 		} else {
 			unsigned long flags;
-			save_flags(flags);
-			cli();
+			local_irq_save(flags);
 			AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo, dmastatus);
-			restore_flags(flags);
-			return;
+			local_irq_restore(flags);
+			return IRQ_HANDLED;
 		}
 	}
 	if (instreg & INSTREG_SR) {
@@ -1260,20 +1272,20 @@
 		if (hostdata->connected) {
 			unsigned long flags;
 			DEB_INTR(printk("calling information_transfer\n"));
-			save_flags(flags);
-			cli();
+			local_irq_save(flags);
 			AM53C974_information_transfer(instance, statreg, isreg, instreg, cfifo, dmastatus);
-			restore_flags(flags);
+			local_irq_restore(flags);
 		} else {
 			printk("scsi%d: weird: service request when no command connected\n", instance->host_no);
 			AM53C974_write_8(CMDREG, CMDREG_CFIFO);
 		}		/* clear FIFO */
-		return;
+		return IRQ_HANDLED;
 	}
 	EXIT:
 	    DEB_INTR(printk("intr: starting main\n"));
 	run_main();
 	DEB_INTR(printk("end of intr\n"));
+	return IRQ_HANDLED;
 }

 /**************************************************************************
@@ -1546,7 +1558,7 @@
 		if ((!cmd->SCp.this_residual) && cmd->SCp.buffers_residual) {
 			cmd->SCp.buffer++;
 			cmd->SCp.buffers_residual--;
-			cmd->SCp.ptr = (unsigned char *) cmd->SCp.buffer->address;
+			cmd->SCp.ptr = (unsigned char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
 			cmd->SCp.this_residual = cmd->SCp.buffer->length;
 		}
 		if (cmd->SCp.this_residual) {
@@ -2268,7 +2280,6 @@
 static int AM53C974_abort(Scsi_Cmnd * cmd)
 {
 	AM53C974_local_declare();
-	unsigned long flags;
 	struct Scsi_Host *instance = cmd->device->host;
 	struct AM53C974_hostdata *hostdata = (struct AM53C974_hostdata *) instance->hostdata;
 	Scsi_Cmnd *tmp, **prev;
@@ -2276,8 +2287,6 @@
 #ifdef AM53C974_DEBUG
 	deb_stop = 1;
 #endif
-	save_flags(flags);
-	cli();
 	AM53C974_setio(instance);

 	DEB_ABORT(printk(SEPARATOR_LINE));
@@ -2292,7 +2301,6 @@
 		DEB_ABORT(printk("scsi%d: aborting connected command\n", instance->host_no));
 		hostdata->aborted = 1;
 		hostdata->msgout[0] = ABORT;
-		restore_flags(flags);
 		return (SCSI_ABORT_PENDING);
 	}
 /* Case 2 : If the command hasn't been issued yet,
@@ -2307,7 +2315,6 @@
 			(*prev) = (Scsi_Cmnd *) tmp->host_scribble;
 			tmp->host_scribble = NULL;
 			tmp->result = DID_ABORT << 16;
-			restore_flags(flags);
 			tmp->done(tmp);
 			return (SCSI_ABORT_SUCCESS);
 		}
@@ -2329,7 +2336,6 @@
  *          we fail. */
 	if (hostdata->connected || hostdata->sel_cmd) {
 		DEB_ABORT(printk("scsi%d : abort failed, other command connected.\n", instance->host_no));
-		restore_flags(flags);
 		return (SCSI_ABORT_NOT_RUNNING);
 	}
 /* Case 4: If the command is currently disconnected from the bus, and
@@ -2345,7 +2351,6 @@
 			hostdata->selecting = 1;
 			hostdata->sel_cmd = tmp;
 			AM53C974_write_8(CMDREG, CMDREG_DSR);
-			restore_flags(flags);
 			return (SCSI_ABORT_PENDING);
 		}
 	}
@@ -2358,7 +2363,6 @@
  * so we won't panic, but we will notify the user in case something really
  * broke. */
 	DEB_ABORT(printk("scsi%d : abort failed, command not found.\n", instance->host_no));
-	restore_flags(flags);
 	return (SCSI_ABORT_NOT_RUNNING);
 }

@@ -2370,20 +2374,15 @@
 * Inputs : cmd -- which command within the command block was responsible for the reset
 *
 * Returns : status (SCSI_ABORT_SUCCESS)
-*
-* FIXME(eric) the reset_flags are ignored.
 **************************************************************************/
-static int AM53C974_reset(Scsi_Cmnd * cmd, unsigned int reset_flags)
+static int AM53C974_reset(Scsi_Cmnd * cmd)
 {
 	AM53C974_local_declare();
-	unsigned long flags;
 	int i;
 	struct Scsi_Host *instance = cmd->device->host;
 	struct AM53C974_hostdata *hostdata = (struct AM53C974_hostdata *) instance->hostdata;
 	AM53C974_setio(instance);

-	save_flags(flags);
-	cli();
 	DEB(printk("AM53C974_reset called; "));

 	printk("AM53C974_reset called\n");
@@ -2417,10 +2416,9 @@
 	udelay(40);
 	AM53C974_config_after_reset(instance);

-	restore_flags(flags);
 	cmd->result = DID_RESET << 16;
 	cmd->scsi_done(cmd);
-	return SCSI_ABORT_SUCCESS;
+	return SCSI_RESET_SUCCESS;
 }


@@ -2451,8 +2449,8 @@
 	.release        	= AM53C974_release,
 	.info			= AM53C974_info,
 	.queuecommand		= AM53C974_queue_command,
-	.abort			= AM53C974_abort,
-	.reset			= AM53C974_reset,
+	.eh_abort_handler	= AM53C974_abort,
+	.eh_bus_reset_handler	= AM53C974_reset,
 	.can_queue		= 12,
 	.this_id		= -1,
 	.sg_tablesize		= SG_ALL,
diff -ur linux-2.6.0-test7.arm/drivers/scsi/AM53C974.h linux-2.6.0-test7/drivers/scsi/AM53C974.h
--- linux-2.6.0-test7.arm/drivers/scsi/AM53C974.h	Sat Aug  9 06:40:41 2003
+++ linux-2.6.0-test7/drivers/scsi/AM53C974.h	Thu Oct 30 23:05:41 2003
@@ -53,9 +53,7 @@
 static int AM53C974_pci_detect(Scsi_Host_Template * tpnt);
 static int AM53C974_release(struct Scsi_Host *shp);
 static const char *AM53C974_info(struct Scsi_Host *);
-static int AM53C974_command(Scsi_Cmnd * SCpnt);
 static int AM53C974_queue_command(Scsi_Cmnd * cmd, void (*done) (Scsi_Cmnd *));
 static int AM53C974_abort(Scsi_Cmnd * cmd);
-static int AM53C974_reset(Scsi_Cmnd * cmd, unsigned int);
-
+static int AM53C974_reset(Scsi_Cmnd * cmd);
 #endif				/* AM53C974_H */

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Kurt Garloff]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Guennadi,

Great!

On Thu, Oct 30, 2003 at 11:12:57PM +0100, Guennadi Liakhovetski wrote:
> Ok, I fixed it, well, at least, it works for me. What now? The fix is,
> probably, not perfect. E.g. it doesn't support multiple cards now, but it
> looks like the driver didn't support them even when it worked in its
> latest version (sorry, if I am wrong).
> 
> Patch attached. Comments / improvement suggestions mostly welcome.

I think it should just be merged; Christoph will certainly find places
to criticize ... but that's the way the stuff gets better.

Interested in taking tmscsim as well?
I promised to port it, and I will, but it won't happen during the next 
ten days :-(

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Christoph Hellwig]

Any reason you fix this driver?  The tmcsim one for the same hardware
looks like much better structured (though a bit obsufacted :))?

> +#include <linux/version.h>

What do you need version.h for?

> +#undef AM53C974_MULTIPLE_CARD
> +#ifdef AM53C974_MULTIPLE_CARD
> +#error "FIXME! Multiple card support is broken. Looks like it never really worked. Might have to be fixed."
>  static struct Scsi_Host *first_host;	/* Head of list of AMD boards */
> +#endif

Why do you need the undef?  It looks like you need to kill a #define for
this symbol somewhere else :)

> -	save_flags(flags);
> -	cli();
> +	local_irq_save(flags);

That's not safe on SMP, you must mark the driver BROKEN_ON_SMP or better fix
this.

>  static void AM53C974_print(struct Scsi_Host *instance)
>  {
>  	AM53C974_local_declare();
> +#if 0 /* Called only from error-handling paths with sufficient protection? */
>  	unsigned long flags;
> +#endif

So don't if 0 it but completely kill it.

>  /* Set up an interrupt handler if we aren't already sharing an IRQ with another board */
> +#ifdef AM53C974_MULTIPLE_CARD
>  	for (search = first_host;
>  	     search && (((the_template != NULL) && (search->hostt != the_template)) ||
>  		 (search->irq != instance->irq) || (search == instance));
>  	     search = search->next);
>  	if (!search) {
> +#endif

Not sure whether you're interested in fixing this, but the proper way
to fix that would be to call request_irq for each card, mark the irq's
sharable.  The irq handler then can use the void * argument to find the
right host and you can kill all this ugly host list walking.  That shold
get multiple host support working again in theory  (ok, except that the
driver has a totally broken single state machine..)

>  	if (cmd->use_sg) {
>  		cmd->SCp.buffer = (struct scatterlist *) cmd->buffer;
>  		cmd->SCp.buffers_residual = cmd->use_sg - 1;
> -		cmd->SCp.ptr = (char *) cmd->SCp.buffer->address;
> +		cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;

This means you need a dma_mask < highmem to work.  I don't think we
want such crude hacks merged, could you please convert it to the proper
dma API?

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Christoph Hellwig]

On Fri, Oct 31, 2003 at 12:52:04AM +0100, Kurt Garloff wrote:
> Interested in taking tmscsim as well?
> I promised to port it, and I will, but it won't happen during the next 
> ten days :-(

Any reason why we'd keep both drivers?  Given that there's not much ressources
for fixing drivers for older hardware I'd rather see us not keeping multiple
drivers for the same hardware.

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Kurt Garloff]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

Christoph,

On Fri, Oct 31, 2003 at 09:47:42AM +0000, Christoph Hellwig wrote:
> Any reason why we'd keep both drivers?  Given that there's not much ressources
> for fixing drivers for older hardware I'd rather see us not keeping multiple
> drivers for the same hardware.

As long as there are people willing to do the work, there's no reason to
drop any of them.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Christoph Hellwig]

On Fri, Oct 31, 2003 at 10:59:10AM +0100, Kurt Garloff wrote:
> On Fri, Oct 31, 2003 at 09:47:42AM +0000, Christoph Hellwig wrote:
> > Any reason why we'd keep both drivers?  Given that there's not much ressources
> > for fixing drivers for older hardware I'd rather see us not keeping multiple
> > drivers for the same hardware.
> 
> As long as there are people willing to do the work, there's no reason to
> drop any of them.

Well, yes.  If we had two properly maintained drivers there would be
no reason to drop one.  But we currently have two broken drivers, one of
them just resurrected to compiling and mostly working on UP state..

--
Christoph Hellwig <hch@lst.de>		-	Freelance Hacker
Contact me for driver hacking and kernel development consulting

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

(Replying fromo the web-interface, hope, the email will not be scrued up 
completely).

> Any reason you fix this driver?  The tmcsim one for the same hardware
> looks like much better structured (though a bit obsufacted :))?

This is the easiest to asnwer: Kurt Garloff wrote to me, saying that he 
would fix the tmcsim driver, so, I didn't want to duplicate the work. Also, 
I was considering this mostly as a fun-excersice, not really hoping / aiming

at merging it inthe kernel (also given the current freeze). But now, if Kurt
doesn't mind, I can try fixing the other driver, and, hopefully, with your
help, I'll try to do it properly.

> > +#include <linux/version.h>
> 
> What do you need version.h for?

Yep, I had my changes encapsulated in #if KERNEL_VERSION... first, but then 
I removed them, but forgot to remove this include.

> > +#undef AM53C974_MULTIPLE_CARD
> > +#ifdef AM53C974_MULTIPLE_CARD
> > +#error "FIXME! Multiple card support is broken. Looks like it never
> really worked. Might have to be fixed."
> >  static struct Scsi_Host *first_host;	/* Head of list of AMD boards */
> > +#endif
> 
> Why do you need the undef?  It looks like you need to kill a #define for
> this symbol somewhere else :)

So that, when it is fixed, somebody can easily switch it on / off:-)

> > -	save_flags(flags);
> > -	cli();
> > +	local_irq_save(flags);
> 
> That's not safe on SMP, you must mark the driver BROKEN_ON_SMP or better
> fix this.

Yes. Again - the fix was pretty much mechanical. I didn't understand why it
was considered SMP-safe to just disable local interrupts, so, just preferred
to go the "minimal modifications" path.

> >  static void AM53C974_print(struct Scsi_Host *instance)
> >  {
> >  	AM53C974_local_declare();
> > +#if 0 /* Called only from error-handling paths with sufficient
> protection? */
> >  	unsigned long flags;
> > +#endif
> 
> So don't if 0 it but completely kill it.

There's a "?" there:-) Which means, I wasn't quite sure, so, is my
assumption correct? I just saw, that other drivers do not implement any 
locking in these functions.

> >  /* Set up an interrupt handler if we aren't already sharing an IRQ with
> another board */
> > +#ifdef AM53C974_MULTIPLE_CARD
> >  	for (search = first_host;
> >  	     search && (((the_template != NULL) && (search->hostt !=
> the_template)) ||
> >  		 (search->irq != instance->irq) || (search == instance));
> >  	     search = search->next);
> >  	if (!search) {
> > +#endif
> 
> Not sure whether you're interested in fixing this, but the proper way
> to fix that would be to call request_irq for each card, mark the irq's
> sharable.  The irq handler then can use the void * argument to find the
> right host and you can kill all this ugly host list walking.  That shold
> get multiple host support working again in theory  (ok, except that the
> driver has a totally broken single state machine..)

Sure - in irq-handler. But are these lists needed anywhere else, where a
function is called in an "abstract" context without a dev-pointer? Probably,
not.

> >  	if (cmd->use_sg) {
> >  		cmd->SCp.buffer = (struct scatterlist *) cmd->buffer;
> >  		cmd->SCp.buffers_residual = cmd->use_sg - 1;
> > -		cmd->SCp.ptr = (char *) cmd->SCp.buffer->address;
> > +		cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) +
> cmd->SCp.buffer->offset;
> 
> This means you need a dma_mask < highmem to work.  I don't think we
> want such crude hacks merged, could you please convert it to the proper
> dma API?

Found in a "working" driver (which has to be fixed too, then). Will try to 
find a proper fix (for the new driver).

So, thanks a lot for commenting on the patch, and, if nobody minds, I'll try
to fix the tmcsim driver, will try to do it better this time:-)

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Christoph Hellwig]

On Fri, Oct 31, 2003 at 12:25:08PM +0100, Guennadi Liakhovetski wrote:
> (Replying fromo the web-interface, hope, the email will not be scrued up 
> completely).

Seems fine.

> > Any reason you fix this driver?  The tmcsim one for the same hardware
> > looks like much better structured (though a bit obsufacted :))?
> 
> This is the easiest to asnwer: Kurt Garloff wrote to me, saying that he 
> would fix the tmcsim driver, so, I didn't want to duplicate the work. Also, 
> I was considering this mostly as a fun-excersice, not really hoping / aiming

Ah, okay.

> > > +#ifdef AM53C974_MULTIPLE_CARD
> > > +#error "FIXME! Multiple card support is broken. Looks like it never
> > really worked. Might have to be fixed."
> > >  static struct Scsi_Host *first_host;	/* Head of list of AMD boards */
> > > +#endif
> > 
> > Why do you need the undef?  It looks like you need to kill a #define for
> > this symbol somewhere else :)
> 
> So that, when it is fixed, somebody can easily switch it on / off:-)

Then just comment ou the #define.  Rule of the thumb:  #undef always means
you screwed up elsewhere :)

> 
> > > -	save_flags(flags);
> > > -	cli();
> > > +	local_irq_save(flags);
> > 
> > That's not safe on SMP, you must mark the driver BROKEN_ON_SMP or better
> > fix this.
> 
> Yes. Again - the fix was pretty much mechanical. I didn't understand why it
> was considered SMP-safe to just disable local interrupts, so, just preferred
> to go the "minimal modifications" path.

cli() disabled all intereupts in 2.4 which was safe, you only disable local
interrupts which is ok on UP but doesn't work on SMP.  The right fix would
be to introduce spinlocks.  But given the driver design this might be
everything but easy - that's why I think killing the driver in favour of tmcsim
might be the better idea.

> > Not sure whether you're interested in fixing this, but the proper way
> > to fix that would be to call request_irq for each card, mark the irq's
> > sharable.  The irq handler then can use the void * argument to find the
> > right host and you can kill all this ugly host list walking.  That shold
> > get multiple host support working again in theory  (ok, except that the
> > driver has a totally broken single state machine..)
> 
> Sure - in irq-handler. But are these lists needed anywhere else, where a
> function is called in an "abstract" context without a dev-pointer? Probably,
> not.

A poper driver shouldn't do that, but we already know this one isn't..

> > >  	if (cmd->use_sg) {
> > >  		cmd->SCp.buffer = (struct scatterlist *) cmd->buffer;
> > >  		cmd->SCp.buffers_residual = cmd->use_sg - 1;
> > > -		cmd->SCp.ptr = (char *) cmd->SCp.buffer->address;
> > > +		cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) +
> > cmd->SCp.buffer->offset;
> > 
> > This means you need a dma_mask < highmem to work.  I don't think we
> > want such crude hacks merged, could you please convert it to the proper
> > dma API?
> 
> Found in a "working" driver (which has to be fixed too, then). Will try to 
> find a proper fix (for the new driver).

Oh.  What driver did you find this construct in?

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Fri, 31 Oct 2003, Christoph Hellwig wrote:

> On Fri, Oct 31, 2003 at 12:25:08PM +0100, Guennadi Liakhovetski wrote:
> > > > +#ifdef AM53C974_MULTIPLE_CARD
> > > > +#error "FIXME! Multiple card support is broken. Looks like it never
> > > really worked. Might have to be fixed."
> > > >  static struct Scsi_Host *first_host;	/* Head of list of AMD boards */
> > > > +#endif
> > >
> > > Why do you need the undef?  It looks like you need to kill a #define for
> > > this symbol somewhere else :)
> >
> > So that, when it is fixed, somebody can easily switch it on / off:-)
>
> Then just comment ou the #define.  Rule of the thumb:  #undef always means
> you screwed up elsewhere :)

Grrrr... Commented out code...

> cli() disabled all intereupts in 2.4 which was safe, you only disable local

Auch, ok, forgot, that cli() was global in 2.4, thanks for reminding, I
don't work with SMPs due to the lack thereof:-(

> Oh.  What driver did you find this construct in?

grep... (maybe not all of them are real hits, but, most of them do
certainly look very much like what I've done). BTW, I also saw arrays of
cards in some drvers, which also doesn't correspond to your suggestions:

drivers/scsi/arm/scsi.h-41-		SCp->ptr = (char *)
drivers/scsi/arm/scsi.h:42:			 (page_address(SCp->buffer->page) +
drivers/scsi/arm/scsi.h-43-			  SCp->buffer->offset);
--
drivers/scsi/arm/scsi.h-79-		SCpnt->SCp.ptr = (char *)
drivers/scsi/arm/scsi.h:80:			 (page_address(SCpnt->SCp.buffer->page) +
drivers/scsi/arm/scsi.h-81-			  SCpnt->SCp.buffer->offset);
--
drivers/scsi/sg.c-1071-	return (sclp && sclp->page) ?
drivers/scsi/sg.c:1072:	    (unsigned char *) page_address(sclp->page) + sclp->offset : NULL;
--
drivers/scsi/st.c:3517:		res = copy_from_user(page_address(st_bp->frp[i].page) + offset, ubp, cnt);
--
drivers/scsi/st.c:3550:		res = copy_to_user(ubp, page_address(st_bp->frp[i].page) + offset, cnt);
--
drivers/scsi/st.c:3593:		memmove(page_address(st_bp->frp[dst_seg].page) + dst_offset,
drivers/scsi/st.c:3594:			page_address(st_bp->frp[src_seg].page) + src_offset, count);
--
drivers/scsi/scsi_debug.c:260:		return (unsigned char *)page_address(sclp->page) +
drivers/scsi/scsi_debug.c-261-		       sclp->offset;
--
drivers/scsi/in2000.c:377:		cmd->SCp.ptr = (char *) page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
--
drivers/scsi/in2000.c:769:		cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
--
drivers/scsi/ide-scsi.c:149:		buf = page_address(pc->sg->page) + pc->sg->offset;
--
drivers/scsi/ide-scsi.c:171:		buf = page_address(pc->sg->page) + pc->sg->offset;
--
drivers/scsi/NCR53C9x.c:924:				(char *) virt_to_phys((page_address(sp->SCp.buffer->page) + sp->SCp.buffer->offset));
--
drivers/scsi/NCR53C9x.c:1758:		sp->SCp.ptr = (char *) virt_to_phys((page_address(sp->SCp.buffer->page) + sp->SCp.buffer->offset));
--
drivers/scsi/imm.c:840:		cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
--
drivers/scsi/imm.c:981:	    cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
--
drivers/scsi/ips.c:215:#define IPS_SG_ADDRESS(sg)      (page_address((sg)->page) ? \
drivers/scsi/ips.c:216:                                     page_address((sg)->page)+(sg)->offset : 0)
--
drivers/scsi/ppa.c:752:		cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
--
drivers/scsi/ppa.c:903:	    cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
--
drivers/scsi/qlogicfas.c:426:				buf = page_address(sglist->page) + sglist->offset;
--
drivers/scsi/sun3x_esp.c:349:	    sg[sz].dvma_address = dvma_map((unsigned long)page_address(sg[sz].page) +
drivers/scsi/sun3x_esp.c-350-					   sg[sz].offset, sg[sz].length);
--
drivers/scsi/aha1542.c:72:	       page_address(sgpnt[badseg].page) + sgpnt[badseg].offset,
--
drivers/scsi/aha1542.c:719:					       (page_address(sgpnt[i].page) +
drivers/scsi/aha1542.c-720-						sgpnt[i].offset),
--
drivers/scsi/aha152x.c:584:#define SG_ADDRESS(buffer)	((char *) (page_address((buffer)->page)+(buffer)->offset))
--
drivers/scsi/oktagon_esp.c:559:        sp->SCp.ptr = page_address(sp->SCp.buffer->page)+
drivers/scsi/oktagon_esp.c-560-		      sp->SCp.buffer->offset;
--
drivers/scsi/oktagon_esp.c:573:	sp->SCp.ptr = page_address(sp->SCp.buffer->page)+
drivers/scsi/oktagon_esp.c-574-		      sp->SCp.buffer->offset;
--
drivers/scsi/megaraid.c:2040:					page_address((&sgl[0])->page) +
drivers/scsi/megaraid.c-2041-					(&sgl[0])->offset;
--
drivers/scsi/fd_mcs.c:1024:					current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset;
--
drivers/scsi/fd_mcs.c:1057:				current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset;
--
drivers/scsi/fd_mcs.c:1160:		current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset;
--
drivers/scsi/dc395x.c:238:#define CPU_ADDR(sg)		(page_address((sg).page)+(sg).offset)
--
drivers/scsi/sun3_NCR5380.c:273:#define SGADDR(buffer) (void *)(((unsigned long)page_address((buffer)->page)) + \
drivers/scsi/sun3_NCR5380.c-274-			(buffer)->offset)
--
drivers/scsi/seagate.c:989:					     page_address(buffer[i].page) + buffer[i].offset,
--
drivers/scsi/seagate.c:996:			data = page_address(buffer->page) + buffer->offset;
--
drivers/scsi/seagate.c:1243:					data = page_address(buffer->page) + buffer->offset;
--
drivers/scsi/seagate.c:1417:					data = page_address(buffer->page) + buffer->offset;
--
drivers/scsi/fdomain.c:1294:	       current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset;
--
drivers/scsi/fdomain.c:1327:	    current_SC->SCp.ptr = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset;
--
drivers/scsi/fdomain.c:1413:      current_SC->SCp.ptr              = page_address(current_SC->SCp.buffer->page) + current_SC->SCp.buffer->offset;
--
drivers/scsi/osst.c:4280:		STp->buffer->aux = (os_aux_t *) (page_address(STp->buffer->sg[i].page) + OS_DATA_SIZE - b_size);
--
drivers/scsi/osst.c:5146:		res = copy_from_user(page_address(st_bp->sg[i].page) + offset, ubp, cnt);
--
drivers/scsi/osst.c:5179:		res = copy_to_user(ubp, page_address(st_bp->sg[i].page) + offset, cnt);
--
drivers/scsi/osst.c:5212:		memset(page_address(st_bp->sg[i].page) + offset, 0, cnt);
--
drivers/scsi/NCR53c406a.c:884:					NCR53c406a_pio_write(page_address(sglist->page) + sglist->offset, sglist->length);
--
drivers/scsi/NCR53c406a.c:911:					NCR53c406a_pio_read(page_address(sglist->page) + sglist->offset, sglist->length);
--
drivers/scsi/atari_NCR5380.c:472:	 virt_to_phys(page_address(cmd->SCp.buffer[1].page)+
drivers/scsi/atari_NCR5380.c-473-		      cmd->SCp.buffer[1].offset) == endaddr; ) {
--
drivers/scsi/atari_NCR5380.c:510:	cmd->SCp.ptr = (char *)page_address(cmd->SCp.buffer->page)+
drivers/scsi/atari_NCR5380.c-511-		       cmd->SCp.buffer->offset;
--
drivers/scsi/atari_NCR5380.c:2044:		    cmd->SCp.ptr = page_address(cmd->SCp.buffer->page)+
drivers/scsi/atari_NCR5380.c-2045-				   cmd->SCp.buffer->offset;
--
drivers/scsi/sym53c416.c:200:#define SG_ADDRESS(buffer)     ((char *) (page_address((buffer)->page)+(buffer)->offset))
--
drivers/scsi/53c7xx.c:3453:	    ? (u32)page_address(((struct scatterlist *)cmd->buffer)[i].page)+
drivers/scsi/53c7xx.c-3454-	      ((struct scatterlist *)cmd->buffer)[i].offset
--
drivers/scsi/53c7xx.c:5420:    	    	     buffers && !((found = ((ptr >= (char *)page_address(segment->page)+segment->offset) &&
drivers/scsi/53c7xx.c:5421:    	    	    	    (ptr < ((char *)page_address(segment->page)+segment->offset+segment->length)))));
--
drivers/scsi/53c7xx.c:5425:			cmd->device->host->host_no, saved, page_address(segment->page+segment->offset);
--
drivers/scsi/53c7xx.c:5429:    	    	    offset += ptr - ((char *)page_address(segment->page)+segment->offset);
--
drivers/scsi/eata_pio.c:184:			SCp->ptr = page_address(SCp->buffer->page) + SCp->buffer->offset;
--
drivers/scsi/eata_pio.c:417:		cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) + cmd->SCp.buffer->offset;
--
drivers/scsi/NCR5380.c:337:		cmd->SCp.ptr = page_address(cmd->SCp.buffer->page)+
drivers/scsi/NCR5380.c-338-			       cmd->SCp.buffer->offset;
--
drivers/scsi/NCR5380.c:2338:					cmd->SCp.ptr = page_address(cmd->SCp.buffer->page)+
drivers/scsi/NCR5380.c-2339-						       cmd->SCp.buffer->offset;
--
drivers/scsi/wd33c93.c:388:		cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) +
drivers/scsi/wd33c93.c-389-		    cmd->SCp.buffer->offset;
--
drivers/scsi/wd33c93.c:721:		cmd->SCp.ptr = page_address(cmd->SCp.buffer->page) +
drivers/scsi/wd33c93.c-722-		    cmd->SCp.buffer->offset;
--
drivers/scsi/advansys.c:6728:		(unsigned char *)page_address(slp->page) + slp->offset));
--
drivers/scsi/advansys.c:6987:                   (unsigned char *)page_address(slp->page) + slp->offset));

Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Russell King]

On Fri, Oct 31, 2003 at 01:19:56PM +0100, Guennadi Liakhovetski wrote:
> > Oh.  What driver did you find this construct in?
> 
> grep... (maybe not all of them are real hits, but, most of them do
> certainly look very much like what I've done). BTW, I also saw arrays of
> cards in some drvers, which also doesn't correspond to your suggestions:
> 
> drivers/scsi/arm/scsi.h-41-		SCp->ptr = (char *)
> drivers/scsi/arm/scsi.h:42:			 (page_address(SCp->buffer->page) +
> drivers/scsi/arm/scsi.h-43-			  SCp->buffer->offset);
> --
> drivers/scsi/arm/scsi.h-79-		SCpnt->SCp.ptr = (char *)
> drivers/scsi/arm/scsi.h:80:			 (page_address(SCpnt->SCp.buffer->page) +
> drivers/scsi/arm/scsi.h-81-			  SCpnt->SCp.buffer->offset);

The drivers which use this will never be used on highmem-capable machines.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

> > > Any reason you fix this driver?  The tmcsim one for the same hardware
> > > looks like much better structured (though a bit obsufacted :))?

Ok, started looking at the tmscsim. A couple of questions:

1) After the "next" element has disappeared from the struct scsi_cmnd,
what is the "correct" / preferred way to queue scsi commands in drivers?
I saw aic7xxx (new) casting a part of struct scsi_pointer SCp in
scsi_cmnd, starting from Status to a list_head (or an anology thereof),
which doesn't seem very nice. Anyway, I didn't find any "standard" way for
doing this. Should host_scribble be used?

2) Actually, which scsi driver (or, better, several drivers) can be
considered well-written and can be taken as examples? I tried looking at
aic7xxx, as it is a pretty new one, but I am not sure if it is really a
good example to follow and it is pretty big too.

Thanks
Guennadi

P.S. Should this thread be taken to linux-scsi or is it better to continue
it on lkml?
---
Guennadi Liakhovetski

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

> > > Any reason you fix this driver?  The tmcsim one for the same hardware
> > > looks like much better structured (though a bit obsufacted :))?

Ok, started looking at the tmscsim. A couple of questions:

1) After the "next" element has disappeared from the struct scsi_cmnd,
what is the "correct" / preferred way to queue scsi commands in drivers?
I saw aic7xxx (new) casting a part of struct scsi_pointer SCp in
scsi_cmnd, starting from Status to a list_head (or an anology thereof),
which doesn't seem very nice. Anyway, I didn't find any "standard" way for
doing this. Should host_scribble be used?

2) Actually, which scsi driver (or, better, several drivers) can be
considered well-written and can be taken as examples? I tried looking at
aic7xxx, as it is a pretty new one, but I am not sure if it is really a
good example to follow and it is pretty big too.

Thanks
Guennadi

P.S. Should this thread be taken to linux-scsi or is it better to continue
it on lkml?
---
Guennadi Liakhovetski

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Christoph Hellwig]

On Sun, Nov 02, 2003 at 08:22:23PM +0100, Guennadi Liakhovetski wrote:
> Ok, started looking at the tmscsim. A couple of questions:
> 
> 1) After the "next" element has disappeared from the struct scsi_cmnd,
> what is the "correct" / preferred way to queue scsi commands in drivers?
> I saw aic7xxx (new) casting a part of struct scsi_pointer SCp in
> scsi_cmnd, starting from Status to a list_head (or an anology thereof),
> which doesn't seem very nice. Anyway, I didn't find any "standard" way for
> doing this. Should host_scribble be used?

Just overlay your own structure over the scsi_pointer - you can easily
embedd a list_head in there.  But in 2.6 a driver really schouldn't need
to keep it's own list anyway :)

> 2) Actually, which scsi driver (or, better, several drivers) can be
> considered well-written and can be taken as examples? I tried looking at
> aic7xxx, as it is a pretty new one, but I am not sure if it is really a
> good example to follow and it is pretty big too.

aic7xxx is a very bad example.  It's a BSDish driver glued into Linux.
I'd suggest looking into qla1280 or the 53c700 driver and it's glue
drivers instead. In qla1280 you can also see an example for the above
embedding, looks for struct srb.

> P.S. Should this thread be taken to linux-scsi or is it better to continue
> it on lkml?

I think linux-scsi is the right list, and dropped the lkml Cc.

-- 
Christoph Hellwig <hch@lst.de>		-	Freelance Hacker
Contact me for driver hacking and kernel development consulting

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

Hello

So, since that last email I was spending a couple of hours practically
every day, trying to fix tmscsim in 2.6. It turned to be A LOT more work,
than with AM53C974, a lot more difficult, and it still doesn't work. The
problem I am currently having, is the following: initialisation and
several first commands seem to complete fine, but then I get an Oops,
because dma_map_page is called with direction == DMA_NONE. Below is
attached a log with debugging turned on and an Oops. So, the question,
under which conditions can there be a command with direction NONE issued?
I see, it comes from the elevator, from the block layer, but what does
this mean? What is wrong?

Thanks
Guennadi
---
Guennadi Liakhovetski

DC390(0): IO_PORT=7100,IRQ=b
DC390_init: No EEPROM found! Trying default settings ...
DC390: Used defaults: AdaptID=7, SpeedIdx=0 (10.0 MHz), DevMode=0x1f, AdaptMode=0x0f, TaggedCmnds=3 (16), DelayReset=1s
DC390: pSH = c1772000, Index 00,<6>DC390: Adapter index 0, ID 7, IO 0x00007100, IRQ 0x0b

DC390: pACB = c17721b4, pDCBmap = c17721f0, pSRB_array = c1772288
DC390: ACB size= 1074, DCB size=   30, SRB size=   50
DC390: 1 adapters found
scsi0 : Tekram DC390/AM53C974 V2.0f 2000-12-20
DC390: Queue Cmd=12,Tgt=0,LUN=0 (pid=0)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
DC390: We were just reset and don't accept commands yet!
DC390: Insert pSRB c1772288 cmd 0 to Waiting
DC390: Debug: Waiting queue woken up by timer!
DC390: Remove SRB c1772288 from head of Waiting
DC390: Append SRB c1772288 to Going
DISC, SRBdone (00,00000000), SRB c1772288, pid 0
DC390: INQUIRY: result: 00000000
DC390: Remove SRB c1772288 from Going
DC390: Free SRB c1772288
  Vendor:           Model:                   Rev:
  Type:   Direct-Access                      ANSI SCSI revision: 00
DC390: Queue Cmd=12,Tgt=1,LUN=0 (pid=1)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
DC390: Append SRB c1772288 to Going
DISC, SRBdone (ff,00000000), SRB c1772288, pid 1
DC390: INQUIRY: result: 00010000
DC390: Free SRB c1772288
DC390: Queue Cmd=12,Tgt=2,LUN=0 (pid=2)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
DC390: Append SRB c1772288 to Going
DISC, SRBdone (ff,00000000), SRB c1772288, pid 2
DC390: INQUIRY: result: 00010000
DC390: Free SRB c1772288
DC390: Queue Cmd=12,Tgt=3,LUN=0 (pid=3)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
DC390: Append SRB c1772288 to Going
DISC, SRBdone (ff,00000000), SRB c1772288, pid 3
DC390: INQUIRY: result: 00010000
DC390: Free SRB c1772288
DC390: Queue Cmd=12,Tgt=4,LUN=0 (pid=4)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
DC390: Append SRB c1772288 to Going
DISC, SRBdone (ff,00000000), SRB c1772288, pid 4
DC390: INQUIRY: result: 00010000
DC390: Free SRB c1772288
DC390: Queue Cmd=12,Tgt=5,LUN=0 (pid=5)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
DC390: Append SRB c1772288 to Going
DISC, SRBdone (ff,00000000), SRB c1772288, pid 5
DC390: INQUIRY: result: 00010000
DC390: Free SRB c1772288
DC390: Queue Cmd=12,Tgt=6,LUN=0 (pid=6)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
DC390: Append SRB c1772288 to Going
DISC, SRBdone (ff,00000000), SRB c1772288, pid 6
DC390: INQUIRY: result: 00010000
DC390: Free SRB c1772288
DC390: Queue Cmd=00,Tgt=0,LUN=0 (pid=7)
DC390: Get Free SRB c1772288
DC390: Free SRB w/o Waiting
------------[ cut here ]------------
kernel BUG at include/asm/dma-mapping.h:53!
invalid operand: 0000 [#1]
CPU:    0
EIP:    0060:[<c01ecd2e>]    Not tainted
EFLAGS: 00010046
EIP is at dc390_BuildSRB+0xda/0x17c
eax: c1027fd8   ebx: c1776ce0   ecx: c1000000   edx: c0fffe00
esi: 00000e00   edi: c1772288   ebp: c17e5dac   esp: c17e5d98
ds: 007b   es: 007b   ss: 0068
Process swapper (pid: 1, threadinfo=c17e4000 task=c17d38c0)
Stack: c1772288 c01e4e34 c17721b4 c0117133 c1772288 c17e5dd0 c01ed203 c1776ce0
       c10407a0 c1772288 00000293 c1776ce0 c1772000 c10407a0 c17e5df0 c01e4c5f
       c1776ce0 c01e4e34 c1776ce0 c1078400 c1772000 00000000 c17e5e0c c01e9a4c
Call Trace:
 [<c01e4e34>] scsi_done+0x0/0x68
 [<c0117133>] printk+0x107/0x118
 [<c01ed203>] DC390_queue_command+0x2e7/0x2fc
 [<c01e4c5f>] scsi_dispatch_cmd+0x1d3/0x244
 [<c01e4e34>] scsi_done+0x0/0x68
 [<c01e9a4c>] scsi_request_fn+0x258/0x2c8
 [<c01cd099>] blk_insert_request+0x81/0x90
 [<c01e89d4>] scsi_insert_special_req+0x24/0x2c
 [<c01e8adf>] scsi_do_req+0x67/0x74
 [<c01e8ba3>] scsi_wait_req+0x67/0x98
 [<c01e8aec>] scsi_wait_done+0x0/0x50
 [<c01f26fe>] sd_spinup_disk+0xa2/0x26c
 [<c01f302f>] sd_revalidate_disk+0xc7/0x130
 [<c01f3257>] sd_probe+0x1bf/0x270
 [<c01c9992>] bus_match+0x32/0x58
 [<c01c9a88>] driver_attach+0x44/0x8c
 [<c01c9cee>] bus_add_driver+0x72/0x90
 [<c01ca07a>] driver_register+0x36/0x3c
 [<c01eb97b>] scsi_register_driver+0x13/0x18
 [<c02f57b6>] init_sd+0x56/0x70
 [<c02e468d>] do_initcalls+0x39/0x94
 [<c010505c>] init+0x0/0xe4
 [<c02e4704>] do_basic_setup+0x1c/0x28
 [<c0105079>] init+0x1d/0xe4
 [<c010505c>] init+0x0/0xe4
 [<c0107209>] kernel_thread_helper+0x5/0xc

Code: 0f 0b 35 00 0e 8f 28 c0 29 c8 8d 04 40 89 c2 c1 e2 04 01 d0
 <0>Kernel panic: Attempted to kill init!

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Sat, 15 Nov 2003, Guennadi Liakhovetski wrote:

> several first commands seem to complete fine, but then I get an Oops,
> because dma_map_page is called with direction == DMA_NONE. Below is

Ok, I found where the data_direction is set to DMA_NONE, but I still don't
understand why, and how this is supposed to work. So, in
sd_revalidate_disk() sr_data_direction is initialised by
scsi_allocate_request() to DMA_BIDIRECTIONAL:
                sreq->sr_data_direction = DMA_BIDIRECTIONAL;
, but already in sd_spinup_disk() it is reset to DMA_NONE:
                        SRpnt->sr_data_direction = DMA_NONE;
I haven't found yet how sr_data_direction gets copied to sc_data_direction
(so far it only looks like struct scsi_cmnd is just overlayd on struct
scsi_request, but I may be wrong there... Or is it really so?

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Randy.Dunlap]

On Mon, 17 Nov 2003 01:03:17 +0100 (CET) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

| On Sat, 15 Nov 2003, Guennadi Liakhovetski wrote:
| 
| > several first commands seem to complete fine, but then I get an Oops,
| > because dma_map_page is called with direction == DMA_NONE. Below is
| 
| Ok, I found where the data_direction is set to DMA_NONE, but I still don't
| understand why, and how this is supposed to work. So, in
| sd_revalidate_disk() sr_data_direction is initialised by
| scsi_allocate_request() to DMA_BIDIRECTIONAL:
|                 sreq->sr_data_direction = DMA_BIDIRECTIONAL;
| , but already in sd_spinup_disk() it is reset to DMA_NONE:
|                         SRpnt->sr_data_direction = DMA_NONE;

Doing a TEST_UNIT_READY doesn't require any DMA transfers, in the
sense of buffers (i.e., buffer length is 0), so DMA_NONE makes sense,
doesn't it?

| I haven't found yet how sr_data_direction gets copied to sc_data_direction
| (so far it only looks like struct scsi_cmnd is just overlayd on struct
| scsi_request, but I may be wrong there... Or is it really so?

scsi.c::scsi_init_cmd_from_req(), line 504 in 2.6.0-test9 (?),
although I haven't followed the code flow all the way from tmscsim
to that function.

--
~Randy
MOTD:  Always include version info.

PS:  I prefer this on linux-scsi instead of linux-kernel.

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Mon, 17 Nov 2003, Randy.Dunlap wrote:

> On Mon, 17 Nov 2003 01:03:17 +0100 (CET) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
>
> | sd_revalidate_disk() sr_data_direction is initialised by
> | scsi_allocate_request() to DMA_BIDIRECTIONAL:
> |                 sreq->sr_data_direction = DMA_BIDIRECTIONAL;
> | , but already in sd_spinup_disk() it is reset to DMA_NONE:
> |                         SRpnt->sr_data_direction = DMA_NONE;
>
> Doing a TEST_UNIT_READY doesn't require any DMA transfers, in the
> sense of buffers (i.e., buffer length is 0), so DMA_NONE makes sense,
> doesn't it?

Ok, thanks, so, that case (bufflen == 0) should be handled properly, will
double-check. But the problem seems to be occurring much earlier - see
below.

> | I haven't found yet how sr_data_direction gets copied to sc_data_direction
> | (so far it only looks like struct scsi_cmnd is just overlayd on struct
> | scsi_request, but I may be wrong there... Or is it really so?
>
> scsi.c::scsi_init_cmd_from_req(), line 504 in 2.6.0-test9 (?),
> although I haven't followed the code flow all the way from tmscsim
> to that function.

Yep, found that one already, thanks. Also found where scsi_request is
replaced with scsi_cmnd in request->special - in scsi_prep_fn() in
scsi_lib.c.

> PS:  I prefer this on linux-scsi instead of linux-kernel.

Well, if you're referring to my last post... - ok, will keep it at
linux-scsi.

So, already the very first command produces a wrong (or no) result:

DC390(0): IO_PORT=7100,IRQ=b
DC390_init: No EEPROM found! Trying default settings ...
DC390: Used defaults: AdaptID=7, SpeedIdx=0 (10.0 MHz), DevMode=0x1f,
AdaptMode=0x0f, TaggedCmnds=3 (16), DelayReset=1s
Wake up parent of scsi_eh_0
Error handler scsi_eh_0 sleeping
DC390: pSH = c14ec000, Index 00
DC390: Adapter index 0, ID 7, IO 0x00007100, IRQ 0x0b
DC390: pACB = c14ec204, pDCBmap = c14ec240, pSRB_array = c14ec2ec
DC390: ACB size= 1088, DCB size=   30, SRB size=   50
DC390: 1 adapters found
scsi0 : Tekram DC390/AM53C974 V2.0f 2000-12-20
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
DC390: Queue Cmd=12,Tgt=0,LUN=0 (pid=0), direction=2
DC390: Get Free SRB c14ec2ec
DC390: Free SRB w/o Waiting
DC390: We were just reset and don't accept commands yet!
DC390: Insert pSRB c14ec2ec cmd 0 to Waiting
DC390: Debug: Waiting queue woken up by timer!
DC390: Remove SRB c14ec2ec from head of Waiting
DC390: Append SRB c14ec2ec to Going
DISC, SRBdone (00,00000000), SRB c14ec2ec, pid 0
DC390: INQUIRY: result: 00000000
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
  Vendor:           Model:                   Rev:
  Type:   Direct-Access                      ANSI SCSI revision: 00

Whereas the 2.4 version fills in correct data for the drive. Any ideas
would be appreciated.

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Randy.Dunlap]

On Mon, 17 Nov 2003 23:42:59 +0100 (CET) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

| Ok, thanks, so, that case (bufflen == 0) should be handled properly, will
| double-check. But the problem seems to be occurring much earlier - see
| below.
| 
...
| 
| > PS:  I prefer this on linux-scsi instead of linux-kernel.
| 
| Well, if you're referring to my last post... - ok, will keep it at
| linux-scsi.
| 
| So, already the very first command produces a wrong (or no) result:
| 
| DC390(0): IO_PORT=7100,IRQ=b
| DC390_init: No EEPROM found! Trying default settings ...
| DC390: Used defaults: AdaptID=7, SpeedIdx=0 (10.0 MHz), DevMode=0x1f,
| AdaptMode=0x0f, TaggedCmnds=3 (16), DelayReset=1s
| Wake up parent of scsi_eh_0
| Error handler scsi_eh_0 sleeping
| DC390: pSH = c14ec000, Index 00
| DC390: Adapter index 0, ID 7, IO 0x00007100, IRQ 0x0b
| DC390: pACB = c14ec204, pDCBmap = c14ec240, pSRB_array = c14ec2ec
| DC390: ACB size= 1088, DCB size=   30, SRB size=   50
| DC390: 1 adapters found
| scsi0 : Tekram DC390/AM53C974 V2.0f 2000-12-20
| scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
| DC390: Queue Cmd=12,Tgt=0,LUN=0 (pid=0), direction=2
| DC390: Get Free SRB c14ec2ec
| DC390: Free SRB w/o Waiting
| DC390: We were just reset and don't accept commands yet!
| DC390: Insert pSRB c14ec2ec cmd 0 to Waiting
| DC390: Debug: Waiting queue woken up by timer!

Does this say that the IO timed out (timer expired)?
I.e., the IO isn't happening?

Have you posted your current patches?  Maybe someone could see
what needs to be changed...

| DC390: Remove SRB c14ec2ec from head of Waiting
| DC390: Append SRB c14ec2ec to Going
| DISC, SRBdone (00,00000000), SRB c14ec2ec, pid 0
| DC390: INQUIRY: result: 00000000
| DC390: Remove SRB c14ec2ec from Going
| DC390: Free SRB c14ec2ec
| scsi_delete_timer: scmd: c14c8ea8, rtn: 1
|   Vendor:           Model:                   Rev:
|   Type:   Direct-Access                      ANSI SCSI revision: 00
| 
| Whereas the 2.4 version fills in correct data for the drive. Any ideas
| would be appreciated.


--
~Randy
MOTD:  Always include version info.

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Kurt Garloff]

[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]

On Mon, Nov 17, 2003 at 03:14:31PM -0800, Randy.Dunlap wrote:
> | DC390: We were just reset and don't accept commands yet!
> | DC390: Insert pSRB c14ec2ec cmd 0 to Waiting
> | DC390: Debug: Waiting queue woken up by timer!
> 
> Does this say that the IO timed out (timer expired)?
> I.e., the IO isn't happening?

No. If you try to send down commands after a reset, the controller
will queue them for a second before actually trying to send them.
That's controlled by a timer.
(Well, with new queuing we could just refuse I guess.)

> | DC390: INQUIRY: result: 00000000
> | DC390: Remove SRB c14ec2ec from Going
> | DC390: Free SRB c14ec2ec
> | scsi_delete_timer: scmd: c14c8ea8, rtn: 1
> |   Vendor:           Model:                   Rev:
> |   Type:   Direct-Access                      ANSI SCSI revision: 00
> | 
> | Whereas the 2.4 version fills in correct data for the drive. Any ideas
> | would be appreciated.

Hmm, the INQUIRY did not yield any data :-(
pci_unmap() or at least pci_sync happened at that moment already? 

All the debug stuff looks completely sane ...

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Randy.Dunlap]

On Tue, 18 Nov 2003 00:28:29 +0100 Kurt Garloff <garloff@suse.de> wrote:

| On Mon, Nov 17, 2003 at 03:14:31PM -0800, Randy.Dunlap wrote:
| > | DC390: We were just reset and don't accept commands yet!
| > | DC390: Insert pSRB c14ec2ec cmd 0 to Waiting
| > | DC390: Debug: Waiting queue woken up by timer!
| > 
| > Does this say that the IO timed out (timer expired)?
| > I.e., the IO isn't happening?
| 
| No. If you try to send down commands after a reset, the controller
| will queue them for a second before actually trying to send them.
| That's controlled by a timer.
| (Well, with new queuing we could just refuse I guess.)

OK, I see.

| > | DC390: INQUIRY: result: 00000000
| > | DC390: Remove SRB c14ec2ec from Going
| > | DC390: Free SRB c14ec2ec
| > | scsi_delete_timer: scmd: c14c8ea8, rtn: 1
| > |   Vendor:           Model:                   Rev:
| > |   Type:   Direct-Access                      ANSI SCSI revision: 00
| > | 
| > | Whereas the 2.4 version fills in correct data for the drive. Any ideas
| > | would be appreciated.
| 
| Hmm, the INQUIRY did not yield any data :-(

Yes, that's what Guenaddi is trying to debug for now...unless
I misunderstood him.

| pci_unmap() or at least pci_sync happened at that moment already? 
| 
| All the debug stuff looks completely sane ...


--
~Randy
MOTD:  Always include version info.

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Tue, 18 Nov 2003, Kurt Garloff wrote:

> Hmm, the INQUIRY did not yield any data :-(
> pci_unmap() or at least pci_sync happened at that moment already?

Well, I just realised, that I don't, actually, unmap at the right place,
this has to be fixed, so, no, don't think, it can be a problem...:-( I
thought, it had to be done on DMA_XFER_DONE interrupt, but it is actually
commented out with

#define DMA_INT 0
#if DMA_INT
...
#endif

Thanks
Guennadi
---
Guennadi Liakhovetski

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 842 bytes --]

Guennadi,

I have no clue what's going wrong yet with INQUIRY. I believe it has
to do with some pci mapping we get wrong. 

I deferred the pci mapping to StartSCSI, so we don't create mappings
for queued SRBs. Some hardware may have IOMMUs and will have limited
sizes available.

It also allows to clean up some code as we can rely on a mapping being
created if needed upon StartSCSI. Likewise, I moved pci mapping cleanup
to SRBdone. Every command should go through here.
I still need to check the error handling paths, though.

Patch is on top of yours.

Likely, it won't help with the INQUIRY problem :(
I still have to get a machine with the DC390 up again.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #1.2: tmscsim-pcimap.diff --]
[-- Type: text/plain, Size: 9332 bytes --]

--- drivers/scsi/tmscsim.c.guennadi	2003-11-18 10:15:30.000000000 +0100
+++ drivers/scsi/tmscsim.c	2003-11-18 10:24:08.232277272 +0100
@@ -970,6 +970,72 @@
     }
 }
 
+
+/* Create pci mapping */
+static int dc390_pci_map (PSRB pSRB)
+{
+	int error = 0;
+	Scsi_Cmnd *pcmd = pSRB->pcmd;
+	struct pci_dev *pdev = pSRB->pSRBDCB->pDCBACB->pdev;
+	dc390_cmd_scp_t* cmdp = ((dc390_cmd_scp_t*)(&pcmd->SCp));
+	/* Map sense buffer */
+	if (pSRB->SRBFlag | AUTO_REQSENSE) {
+		cmdp->saved_dma_handle = pci_map_page(pdev,
+				virt_to_page(pcmd->sense_buffer),
+				(unsigned long)pcmd->sense_buffer & ~PAGE_MASK,
+				sizeof(pcmd->sense_buffer),
+				DMA_FROM_DEVICE);
+		pSRB->Segmentx.length = sizeof(pcmd->sense_buffer);
+		pSRB->SGcount = 1;
+		pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
+	/* Make SG list */	
+	} else if (pcmd->use_sg) {
+		pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
+		pSRB->SGcount = pci_map_sg(pdev, pSRB->pSegmentList,
+				pcmd->use_sg, 
+				scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+		/* TODO: error handling */
+		if (!pSRB->SGcount)
+			error = 1;
+	/* Map single segment */	
+	} else if (pcmd->request_buffer) {
+		struct page *page = virt_to_page(pcmd->request_buffer);
+		unsigned long off = (unsigned long)pcmd->request_buffer & ~PAGE_MASK;
+		cmdp->saved_dma_handle = pci_map_page(pdev, 
+				page, off, pcmd->request_bufflen,
+				scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+		/* TODO: error handling */
+		pSRB->Segmentx.length = pcmd->request_bufflen;
+		pSRB->SGcount = 1;
+		pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
+	/* No mapping !? */	
+    	} else
+		pSRB->SGcount = 0;
+	return error;
+}
+
+/* Remove pci mapping */
+static void dc390_pci_unmap (PSRB pSRB)
+{
+	Scsi_Cmnd* pcmd = pSRB->pcmd;
+	struct pci_dev *pdev = pSRB->pSRBDCB->pDCBACB->pdev;
+	dc390_cmd_scp_t* cmdp = ((dc390_cmd_scp_t*)(&pcmd->SCp));
+
+	if (pSRB->SRBFlag) {
+		pci_unmap_page(pdev, cmdp->saved_dma_handle,
+			       sizeof(pcmd->sense_buffer), DMA_FROM_DEVICE);
+	} else if (pcmd->use_sg) {
+		pci_unmap_sg(pdev, pcmd->request_buffer, pcmd->use_sg,
+			     scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	} else if (pcmd->request_buffer) {
+		pci_unmap_page(pdev,
+			       cmdp->saved_dma_handle,
+			       pcmd->request_bufflen,
+			       scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	}
+}
+
+
 /***********************************************************************
  * Function: static void dc390_BuildSRB (Scsi_Cmd *pcmd, PDCB pDCB, 
  * 					 PSRB pSRB)
@@ -986,26 +1052,7 @@
     //pSRB->ScsiCmdLen = pcmd->cmd_len;
     //memcpy (pSRB->CmdBlock, pcmd->cmnd, pcmd->cmd_len);
     
-		printk("%s(): direction %d\n", __FUNCTION__, pcmd->sc_data_direction);
-    if( pcmd->use_sg )
-    {
-	pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
-	pSRB->SGcount = pci_map_sg(pdev, pSRB->pSegmentList,
-				   pcmd->use_sg, scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-    }
-    else if( pcmd->request_buffer )
-    {
-	struct page *page = virt_to_page(pcmd->request_buffer);
-	unsigned long off = (unsigned long)pcmd->request_buffer & ~PAGE_MASK;
-	((dc390_cmd_scp_t*)(&pcmd->SCp))->saved_dma_handle = pci_map_page(pdev, page, off,
-									  pcmd->request_bufflen,
-									  scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-	pSRB->Segmentx.length = pcmd->request_bufflen;
-	pSRB->SGcount = 1;
-	pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
-    }
-    else
-	pSRB->SGcount = 0;
+	printk("%s(): direction %d\n", __FUNCTION__, pcmd->sc_data_direction);
 
     pSRB->SGIndex = 0;
     pSRB->AdaptStatus = 0;
@@ -1024,6 +1071,7 @@
     pSRB->ScsiPhase = 0;
     pSRB->EndMessage = 0;
     pSRB->TagNumber = 255;
+    /* KG: deferred PCI mapping to dc390_StartSCSI */
 }
 
 /* Put cmnd from Query to Waiting list and send next Waiting cmnd */
--- drivers/scsi/scsiiom.c.guennadi	2003-11-18 10:15:30.000000000 +0100
+++ drivers/scsi/scsiiom.c	2003-11-18 10:20:55.350599744 +0100
@@ -36,6 +36,9 @@
 	DEBUG0(printk ("DC390: We were just reset and don't accept commands yet!\n"));
 	return 1;
     }
+    /* KG: Moved pci mapping here */
+    dc390_pci_map(pSRB);
+    /* TODO: error handling */
     DC390_write8 (Scsi_Dest_ID, pDCB->TargetID);
     DC390_write8 (Sync_Period, pDCB->SyncPeriod);
     DC390_write8 (Sync_Offset, pDCB->SyncOffset);
@@ -189,19 +192,7 @@
 	    if (!ctr) printk (KERN_CRIT "DC390: dma_intr: DMA aborted unfinished: %06x bytes remain!!\n", DC390_read32 (DMA_Wk_ByteCntr));
 	    else /*if (!residual)*/
 	    {
-		Scsi_Cmnd* pcmd = pSRB->pcmd;
-		if (pcmd->use_sg)
-		{
-			pci_unmap_sg(pACB->pdev, pcmd->request_buffer, pcmd->use_sg,
-				     scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-		}
-		else if (pcmd->request_buffer)
-		{
-			pci_unmap_page(pACB->pdev,
-				       ((dc390_cmd_scp_t*)(&pcmd->SCp))->saved_dma_handle,
-				       pcmd->request_bufflen,
-				       scsi_to_pci_dma_dir(pcmd->sc_data_direction));
-		}
+		    /* Done: Moved pci unmap to dc390_SRBdone */
 	    }
 	    /* residual =  ... */
 	  }
@@ -747,14 +738,17 @@
     struct pci_dev *pdev = pACB->pdev;
     pSRB->TotalXferredLen = 0;
     pSRB->SGIndex = 0;
-    if( pcmd->use_sg )
-    {
+    if (pcmd->use_sg) {
 	pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
 	psgl = pSRB->pSegmentList;
+	/*
 	pci_unmap_sg(pdev, psgl, pcmd->use_sg,
 		     scsi_to_pci_dma_dir(pcmd->sc_data_direction));
 	pSRB->SGcount = pci_map_sg(pdev, psgl,
 				   pcmd->use_sg, scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	 */
+	//dc390_pci_sync(pSRB);
+
 	while (pSRB->TotalXferredLen + (ULONG) psgl->length < pSRB->Saved_Ptr)
 	{
 	    pSRB->TotalXferredLen += (ULONG) psgl->length;
@@ -773,11 +767,12 @@
 	pSRB->SGBusAddr += (pSRB->Saved_Ptr - pSRB->TotalXferredLen);
 	printk (KERN_INFO "DC390: Pointer restored. Segment %i, Total %li, Bus %08lx\n",
 		pSRB->SGIndex, pSRB->Saved_Ptr, pSRB->SGBusAddr);
-    }
-    else if( pcmd->request_buffer )
-    {
+
+    } else if( pcmd->request_buffer ) {
+   
 	struct page *page = virt_to_page(pcmd->request_buffer + pSRB->Saved_Ptr);
 	unsigned long off = (unsigned long)(pcmd->request_buffer + pSRB->Saved_Ptr) & ~PAGE_MASK;
+	/*
 	dc390_cmd_scp_t* cmdp = ((dc390_cmd_scp_t*)(&pcmd->SCp));
 
 	if (cmdp->saved_dma_handle)
@@ -787,23 +782,23 @@
 	cmdp->saved_dma_handle = pci_map_page(pdev, page, off,
 					      pcmd->request_bufflen - pSRB->Saved_Ptr,
 					      scsi_to_pci_dma_dir(pcmd->sc_data_direction));
+	 */
+	//dc390_pci_sync(pSRB);
 
 	pSRB->Segmentx.length = pcmd->request_bufflen - pSRB->Saved_Ptr;
 	pSRB->SGcount = 1;
 	pSRB->pSegmentList = (PSGL) &pSRB->Segmentx;
-    }
-     else
-       {
+    } else {
 	 pSRB->SGcount = 0;
 	 printk (KERN_INFO "DC390: RESTORE_PTR message for Transfer without Scatter-Gather ??\n");
-       };
+    };
 
   pSRB->TotalXferredLen = pSRB->Saved_Ptr;
 };
 
 
 /* According to the docs, the AM53C974 reads the message and 
- * generates a Succesful Operation IRQ before asserting ACK for
+ * generates a Successful Operation IRQ before asserting ACK for
  * the last byte (how does it know whether it's the last ?) */
 /* The old code handled it in another way, indicating, that on
  * every message byte an IRQ is generated and every byte has to
@@ -1395,6 +1390,9 @@
     PSGL   ptr2;
     ULONG  swlval;
 
+    /* KG: Moved pci_unmap here */
+    dc390_pci_unmap(pSRB);
+
     pcmd = pSRB->pcmd; DCB_removed = 0;
     status = pSRB->TargetStatus;
     ptr = (PSCSI_INQDATA) (pcmd->request_buffer);
@@ -1468,6 +1466,7 @@
 	    pSRB->SGIndex = 0;
 	    pSRB->TotalXferredLen = 0;
 	    pSRB->SGToBeXferLen = 0;
+	    /*
 	    if( pcmd->use_sg ) {
 		pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
 		pSRB->SGcount = pci_map_sg(pACB->pdev, pSRB->pSegmentList,
@@ -1484,6 +1483,8 @@
 										  scsi_to_pci_dma_dir(pcmd->sc_data_direction));
 		pSRB->Segmentx.length = pcmd->request_bufflen;
 	    }
+	    */
+
 	    if( dc390_StartSCSI( pACB, pDCB, pSRB ) ) {
 		dc390_Going_to_Waiting ( pDCB, pSRB );
 		dc390_waiting_timer (pACB, HZ/5);
@@ -1551,6 +1552,7 @@
 		pSRB->SGIndex = 0;
 		pSRB->TotalXferredLen = 0;
 		pSRB->SGToBeXferLen = 0;
+		/*
 		if( pcmd->use_sg )
 		{
 		    pSRB->pSegmentList = (PSGL) pcmd->request_buffer;
@@ -1567,6 +1569,7 @@
 										      scsi_to_pci_dma_dir(pcmd->sc_data_direction));
 		    pSRB->Segmentx.length = pcmd->request_bufflen;
 		}
+		*/
 		if( dc390_StartSCSI( pACB, pDCB, pSRB ) ) {
 		    dc390_Going_to_Waiting ( pDCB, pSRB );
 		    dc390_waiting_timer (pACB, HZ/5);
@@ -1805,6 +1808,9 @@
 
     pcmd = pSRB->pcmd;
 
+    /* We are called from SRBdone, original PCI mapping has been removed
+     * already, new one is set up from StartSCSI */
+    /*
     ((dc390_cmd_scp_t*)(&pcmd->SCp))->saved_dma_handle = pci_map_page(pACB->pdev,
 								      virt_to_page(pcmd->sense_buffer),
 								      (unsigned long)pcmd->sense_buffer & ~PAGE_MASK,
@@ -1813,6 +1819,7 @@
     pSRB->Segmentx.length = sizeof(pcmd->sense_buffer);
     pSRB->pSegmentList = &pSRB->Segmentx;
     pSRB->SGcount = 1;
+    */
     pSRB->SGIndex = 0;
 
     //pSRB->CmdBlock[0] = REQUEST_SENSE;

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Tue, 18 Nov 2003, Kurt Garloff wrote:

> Guennadi,
>
> I have no clue what's going wrong yet with INQUIRY. I believe it has
> to do with some pci mapping we get wrong.
>
> I deferred the pci mapping to StartSCSI, so we don't create mappings
> for queued SRBs. Some hardware may have IOMMUs and will have limited
> sizes available.
>
> It also allows to clean up some code as we can rely on a mapping being
> created if needed upon StartSCSI. Likewise, I moved pci mapping cleanup
> to SRBdone. Every command should go through here.
> I still need to check the error handling paths, though.
>
> Patch is on top of yours.

Great! Thanks. I, probably, won't have (much) time today, but I'll
continue tomorrow evening.

> Likely, it won't help with the INQUIRY problem :(

Well, probably will have to put lots of dirty debugging printk()'s of
various addresses, etc.

> I still have to get a machine with the DC390 up again.

Yep, testing one / some of those would be needed too - I only have an
onboard 53C974 in a Compaq PC.

Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Tue, 18 Nov 2003, Kurt Garloff wrote:

> I have no clue what's going wrong yet with INQUIRY. I believe it has
> to do with some pci mapping we get wrong.
>
> I deferred the pci mapping to StartSCSI, so we don't create mappings
> for queued SRBs. Some hardware may have IOMMUs and will have limited
> sizes available.
>
> It also allows to clean up some code as we can rely on a mapping being
> created if needed upon StartSCSI. Likewise, I moved pci mapping cleanup
> to SRBdone. Every command should go through here.
> I still need to check the error handling paths, though.

Ok, did a short test without much thinking. Applied your patch, the first
test returned the same result. Then I put printk()'s in pci_map / unmap
functions, and, also, added a check for bufflen == 0 for single buffer
mappings. The latter removed the Oops I was getting up to now, but it
doesn't look like a correct fix - a non-NULL buffer with a 0-bufflen
doesn't make much sense to me, or is it fine?

Yes, I presume, in your patch, Kurt, you meant
-       if (pSRB->SRBFlag | AUTO_REQSENSE) {
+       if (pSRB->SRBFlag & AUTO_REQSENSE) {
right?

> Likely, it won't help with the INQUIRY problem :(

No, it is still there.

Thanks
Guennadi
---
Guennadi Liakhovetski

DC390(0): IO_PORT=7100,IRQ=b
DC390_init: No EEPROM found! Trying default settings ...
DC390: Used defaults: AdaptID=7, SpeedIdx=0 (10.0 MHz), DevMode=0x1f, AdaptMode=0x0f, TaggedCmnds=3 (16), DelayReset=1s
Wake up parent of scsi_eh_0
Error handler scsi_eh_0 sleeping
DC390: pSH = c14ec000, Index 00
DC390: Adapter index 0, ID 7, IO 0x00007100, IRQ 0x0b
DC390: pACB = c14ec204, pDCBmap = c14ec240, pSRB_array = c14ec2ec
DC390: ACB size= 1088, DCB size=   30, SRB size=   50
DC390: 1 adapters found
scsi0 : Tekram DC390/AM53C974 V2.0f 2000-12-20
scsi_scan_host_selected: <0:4294967295:4294967295:4294967295>
scsi scan: INQUIRY to host 0 channel 0 id 0 lun 0
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 0, command = c14c8efc, buffer = c03f5ef8,
bufflen = 36, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=12,Tgt=0,LUN=0 (pid=0), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
DC390: We were just reset and don't accept commands yet!
DC390: Insert pSRB c14ec2ec cmd 0 to Waiting
leaving scsi_dispatch_cmnd()
DC390: Debug: Waiting queue woken up by timer!
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Remove SRB c14ec2ec from head of Waiting
DC390: Append SRB c14ec2ec to Going
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (00,00000000), SRB c14ec2ec, pid 0
DC390: INQUIRY: result: 00000000
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command finished 1 0 0x0
Notifying upper driver of completion for device 0 0
scsi scan: 1st INQUIRY successful with code 0x0
  Vendor:           Model:                   Rev:
  Type:   Direct-Access                      ANSI SCSI revision: 00
scsi scan: Sequential scan of host 0 channel 0 id 0
scsi scan: INQUIRY to host 0 channel 0 id 1 lun 0
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 1, command = c14c8efc, buffer = c03f5ef8,
bufflen = 36, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=12,Tgt=1,LUN=0 (pid=1), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (ff,00000000), SRB c14ec2ec, pid 1
DC390: INQUIRY: result: 00010000
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command finished 1 0 0x10000
Notifying upper driver of completion for device 1 10000
scsi scan: 1st INQUIRY failed with code 0x10000
scsi scan: INQUIRY to host 0 channel 0 id 2 lun 0
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 2, command = c14c8efc, buffer = c03f5ef8,
bufflen = 36, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=12,Tgt=2,LUN=0 (pid=2), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (ff,00000000), SRB c14ec2ec, pid 2
DC390: INQUIRY: result: 00010000
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command finished 1 0 0x10000
Notifying upper driver of completion for device 2 10000
scsi scan: 1st INQUIRY failed with code 0x10000
scsi scan: INQUIRY to host 0 channel 0 id 3 lun 0
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 3, command = c14c8efc, buffer = c03f5ef8,
bufflen = 36, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=12,Tgt=3,LUN=0 (pid=3), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (ff,00000000), SRB c14ec2ec, pid 3
DC390: INQUIRY: result: 00010000
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command finished 1 0 0x10000
Notifying upper driver of completion for device 3 10000
scsi scan: 1st INQUIRY failed with code 0x10000
scsi scan: INQUIRY to host 0 channel 0 id 4 lun 0
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 4, command = c14c8efc, buffer = c03f5ef8,
bufflen = 36, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=12,Tgt=4,LUN=0 (pid=4), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (ff,00000000), SRB c14ec2ec, pid 4
DC390: INQUIRY: result: 00010000
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command finished 1 0 0x10000
Notifying upper driver of completion for device 4 10000
scsi scan: 1st INQUIRY failed with code 0x10000
scsi scan: INQUIRY to host 0 channel 0 id 5 lun 0
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 5, command = c14c8efc, buffer = c03f5ef8,
bufflen = 36, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=12,Tgt=5,LUN=0 (pid=5), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (ff,00000000), SRB c14ec2ec, pid 5
DC390: INQUIRY: result: 00010000
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command finished 1 0 0x10000
Notifying upper driver of completion for device 5 10000
scsi scan: 1st INQUIRY failed with code 0x10000
scsi scan: INQUIRY to host 0 channel 0 id 6 lun 0
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 6000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 6, command = c14c8efc, buffer = c03f5ef8,
bufflen = 36, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=12,Tgt=6,LUN=0 (pid=6), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (ff,00000000), SRB c14ec2ec, pid 6
DC390: INQUIRY: result: 00010000
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command finished 1 0 0x10000
Notifying upper driver of completion for device 6 10000
scsi scan: 1st INQUIRY failed with code 0x10000
sd_revalidate_disk(): direction 0
sd_spinup_disk(): direction 3
scsi_wait_req(): direction 3
scsi_do_req(): direction 3
scsi_insert_special_req(): direction 3
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 3
scsi_add_timer: scmd: c14c8ea8, time: 30000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 0, command = c14c8efc, buffer = c0fffdf8,
bufflen = 0, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 3
DC390: Queue Cmd=00,Tgt=0,LUN=0 (pid=7), direction=3
DC390_queue_command(): direction 3
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 3
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 3
DC390: Append SRB c14ec2ec to Going
DISC, SRBdone (02,00000000), SRB c14ec2ec, pid 7
dc390_pci_map(): Mapped sense buffer c14c8f7c at 14c8f7c
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped sense buffer at 14c8f7c
 SRBdone (00,00000000), SRB c14ec2ec, pid 7
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
Command failed c14c8ea8 8000002 busy=1 failed=0
bh?: old sense key No Sense
Non-extended sense class 0 code 0x0
Waking error handler thread
Error handler scsi_eh_0 waking up
scsi_eh_prt_fail_stats: 0:0:0:0 cmds failed: 1, cancel: 0
Total of 1 commands on 1 devices require eh work
scsi_eh_0: requesting sense for id: 0
scsi_add_timer: scmd: c14c8ea8, time: 10000, (c02423cc)
DC390: Queue Cmd=03,Tgt=0,LUN=0 (pid=7), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c03f5ef8 at 003f5ef8
DC390: Append SRB c14ec2ec to Going
DISC,dc390_pci_unmap(): Unmapped request buffer at 3f5ef8
 SRBdone (00,00000000), SRB c14ec2ec, pid 7
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_eh_done scmd: c14c8ea8 result: 0
scsi_send_eh_cmnd: scmd: c14c8ea8, rtn:2002
scsi_send_eh_cmnd: scsi_eh_completed_normally 2002
sense requested for c14c8ea8 result 8000002
bh?: old sense key No Sense
Non-extended sense class 0 code 0x0
scsi_eh_0: Sending BDR sdev: 0xc14d0bf8
scsi_eh_0: BDR failed sdev:0xc14d0bf8
scsi_eh_0: Sending BRST chan: 0
scsi_try_bus_reset: Snd Bus RST
DC390: RESET ... done
scsi_eh_0: BRST failed chan: 0
scsi_eh_0: Sending HRST
scsi_try_host_reset: Snd Host RST
scsi_eh_0: HRST failed
scsi: Device offlined - not ready after error recovery: host 0 channel 0 id 0 lun 0
scsi_eh_0: flush finish cmd: c14c8ea8
Notifying upper driver of completion for device 0 e000002
sd_spinup_disk(): direction 3
scsi_wait_req(): direction 3
scsi_do_req(): direction 3
scsi_insert_special_req(): direction 3
Leaving scsi_init_cmd_from_req()
scsi_restart_operations: waking up host to restart
scsi_request_fn(): direction 3
scsi_add_timer: scmd: c14c8ea8, time: 30000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 0, command = c14c8efc, buffer = c0fffdf8,
bufflen = 0, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 3
DC390: Queue Cmd=00,Tgt=0,LUN=0 (pid=8), direction=3
DC390_queue_command(): direction 3
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 3
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 3
DC390: We were just reset and don't accept commands yet!
DC390: Insert pSRB c14ec2ec cmd 8 to Waiting
leaving scsi_dispatch_cmnd()
Error handler scsi_eh_0 sleeping
DC390: Debug: Waiting queue woken up by timer!
DC390: Remove SRB c14ec2ec from head of Waiting
DC390: Append SRB c14ec2ec to Going
DISC, SRBdone (02,00000000), SRB c14ec2ec, pid 8
dc390_pci_map(): Mapped sense buffer c14c8f7c at 14c8f7c
DISC,dc390_pci_unmap(): Unmapped sense buffer at 14c8f7c
 SRBdone (00,00000000), SRB c14ec2ec, pid 8
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
scsi_decide_disposition: device offline - report as SUCCESS
Command finished 1 0 0x8000002
Notifying upper driver of completion for device 0 8000002
sd_spinup_disk(): direction 3
scsi_wait_req(): direction 3
scsi_do_req(): direction 3
scsi_insert_special_req(): direction 3
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 3
scsi_add_timer: scmd: c14c8ea8, time: 30000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 0, command = c14c8efc, buffer = c0fffdf8,
bufflen = 0, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 3
DC390: Queue Cmd=00,Tgt=0,LUN=0 (pid=9), direction=3
DC390_queue_command(): direction 3
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 3
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 3
DC390: Append SRB c14ec2ec to Going
DISC, SRBdone (00,00000000), SRB c14ec2ec, pid 9
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
scsi_decide_disposition: device offline - report as SUCCESS
Command finished 1 0 0x0
Notifying upper driver of completion for device 0 0
leaving scsi_dispatch_cmnd()
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 30000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 0, command = c14c8efc, buffer = c0fffdf8,
bufflen = 8, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=25,Tgt=0,LUN=0 (pid=10), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c0fffdf8 at 00fffdf8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at fffdf8
 SRBdone (00,00000000), SRB c14ec2ec, pid 10
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
scsi_decide_disposition: device offline - report as SUCCESS
Command finished 1 0 0x0
Notifying upper driver of completion for device 0 0
sda : sector size 0 reported, assuming 512.
SCSI device sda: 1 512-byte hdwr sectors (0 MB)
scsi_wait_req(): direction 2
scsi_do_req(): direction 2
scsi_insert_special_req(): direction 2
Leaving scsi_init_cmd_from_req()
scsi_request_fn(): direction 2
scsi_add_timer: scmd: c14c8ea8, time: 30000, (c02421c8)
scsi_dispatch_cmnd (host = 0, channel = 0, target = 0, command = c14c8efc, buffer = c0fffdf8,
bufflen = 4, done = c0244720)
queuecommand : routine at c024a11c
scsi_dispatch_cmd(): direction 2
DC390: Queue Cmd=1a,Tgt=0,LUN=0 (pid=11), direction=2
DC390_queue_command(): direction 2
DC390: Get Free SRB c14ec2ec
DC390_queue_command(): direction 2
DC390: Free SRB w/o Waiting
dc390_BuildSRB(): direction 2
dc390_pci_map(): Mapped request buffer c0fffdf8 at 00fffdf8
DC390: Append SRB c14ec2ec to Going
leaving scsi_dispatch_cmnd()
DISC,dc390_pci_unmap(): Unmapped request buffer at fffdf8
 SRBdone (00,00000000), SRB c14ec2ec, pid 11
DC390: Remove SRB c14ec2ec from Going
DC390: Free SRB c14ec2ec
scsi_delete_timer: scmd: c14c8ea8, rtn: 1
scsi_decide_disposition: device offline - report as SUCCESS
Command finished 1 0 0x0
Notifying upper driver of completion for device 0 0
sda: asking for cache data failed
sda: assuming drive cache: write through
scsi_block_when_processing_errors: rtn: 0
Attached scsi disk sda at scsi0, channel 0, id 0, lun 0
sg_attach: dev=0
Attached scsi generic sg0 at scsi0, channel 0, id 0, lun 0,  type 0

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Kurt Garloff]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Wed, Nov 19, 2003 at 12:07:06AM +0100, Guennadi Liakhovetski wrote:
> Yes, I presume, in your patch, Kurt, you meant
> -       if (pSRB->SRBFlag | AUTO_REQSENSE) {
> +       if (pSRB->SRBFlag & AUTO_REQSENSE) {
> right?

Yes, of course.
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

Hi

Tests yesterday showed, DMA-buffers are always empty (on receiving),
I printed out the first 32 bytes of the buffer on each interrupt, and it
is either all 0, or all 0x5a (or was it 0xa5?), or first 8 bytes 0, then
0x5a... So, either DMA isn't set up properly, or we are handling the wrong
buffer somewhere... Because, AFAIU, the chip receives commands from the
host correctly, according to the status-register readings...

Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany

[PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Tomas Martisius]

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

On Sun, 26 Oct 2003, Guennadi Liakhovetski posted patch for
AM53c974 driver. May be it is not perfect, but it is still not applied
to kernel source. I think it is better to have at least not broken
driver compared with existing.
I have Compaq deskpro XL 590 PC with integrated such controler, and this
driver affter applinig Guennadi Liakhovetski pach to 2.6.0-test11 source
works for me.

Best regards

Tomas Martisius
VMU network administrator
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/2DJcAMslIG6CXk4RAtb9AJ98s5l+T7IqACGrp1l/Mv7PBxuFtQCg5g+0
rwpS2VetZGl8rS49hXE2aPM=
=EYze
-----END PGP SIGNATURE-----

[-- Attachment #2: AM53C974.diff.gz --]
[-- Type: application/x-tar, Size: 4888 bytes --]

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Christoph Hellwig]

On Thu, Dec 11, 2003 at 11:01:16AM +0200, Tomas Martisius wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hello,
> 
> On Sun, 26 Oct 2003, Guennadi Liakhovetski posted patch for
> AM53c974 driver. May be it is not perfect, but it is still not applied
> to kernel source. I think it is better to have at least not broken
> driver compared with existing.
> I have Compaq deskpro XL 590 PC with integrated such controler, and this
> driver affter applinig Guennadi Liakhovetski pach to 2.6.0-test11 source
> works for me.

The patch is broken but he has a better patch for the other driver for
that hardware, that will hopefully go into 2.6.1.

Re: [PATCH] Re: AMD 53c974 SCSI driver in 2.6

[Guennadi Liakhovetski]

On Thu, 11 Dec 2003, Christoph Hellwig wrote:

> On Thu, Dec 11, 2003 at 11:01:16AM +0200, Tomas Martisius wrote:
> > On Sun, 26 Oct 2003, Guennadi Liakhovetski posted patch for
> > AM53c974 driver. May be it is not perfect, but it is still not applied
> > to kernel source. I think it is better to have at least not broken
> > driver compared with existing.
> > I have Compaq deskpro XL 590 PC with integrated such controler, and this
> > driver affter applinig Guennadi Liakhovetski pach to 2.6.0-test11 source
> > works for me.
>
> The patch is broken but he has a better patch for the other driver for
> that hardware, that will hopefully go into 2.6.1.

Right, I think, you can safely use the patch for AM53C974 for now,
although it is broken, it should work for you (I have a slightly newer
Deskpro XL 5133), wait for 2.6.1 and just enable tmscsim:-)

Guennadi
---
Guennadi Liakhovetski