Skip to contentSkip to conversation

slave_configure for qlogicpti

PATCH Matthew Wilcox 3 participants 5 messages Sep 19, 2007 – Oct 1, 2007 SCSI

lore
Share & tools
lore
#0 lore raw

I haven't even compile-tested this. But look how much ugly code it deletes if it works!

drivers/scsi/qlogicpti.c +27 -116
index 5948872..346bed6 100644
@@ -310,8 +310,6 @@ static inline void qlogicpti_set_hostdev_defaults(struct qlogicpti *qpti)
 		}
 		qpti->dev_param[i].device_enable = 1;
 	}
-	/* this is very important to set! */
-	qpti->sbits = 1 << qpti->scsi_id;
 }
 
 static int qlogicpti_reset_hardware(struct Scsi_Host *host)
@@ -981,123 +979,35 @@ static void scsi_rbuf_put(struct scsi_cmnd *cmd, unsigned char *buf)
 	}
 }
 
-/*
- * Until we scan the entire bus with inquiries, go throught this fella...
- */
-static void ourdone(struct scsi_cmnd *Cmnd)
+static int qlogicpti_slave_configure(struct scsi_device *sdev)
 {
-	struct qlogicpti *qpti = (struct qlogicpti *) Cmnd->device->host->hostdata;
-	int tgt = Cmnd->device->id;
-	void (*done) (struct scsi_cmnd *);
-
-	/* This grot added by DaveM, blame him for ugliness.
-	 * The issue is that in the 2.3.x driver we use the
-	 * host_scribble portion of the scsi command as a
-	 * completion linked list at interrupt service time,
-	 * so we have to store the done function pointer elsewhere.
-	 */
-	done = (void (*)(struct scsi_cmnd *))
-		(((unsigned long) Cmnd->SCp.Message)
-#ifdef __sparc_v9__
-		 | ((unsigned long) Cmnd->SCp.Status << 32UL)
-#endif
-		 );
-
-	if ((qpti->sbits & (1 << tgt)) = 0) {
-		int ok = host_byte(Cmnd->result) = DID_OK;
-		if (Cmnd->cmnd[0] = 0x12 && ok) {
-			unsigned char *iqd;
-			unsigned int iqd_len;
-
-			iqd_len = scsi_rbuf_get(Cmnd, &iqd);
-
-			/* tags handled in midlayer */
-			/* enable sync mode? */
-			if (iqd[7] & 0x10) {
-				qpti->dev_param[tgt].device_flags |= 0x10;
-			} else {
-				qpti->dev_param[tgt].synchronous_offset = 0;
-				qpti->dev_param[tgt].synchronous_period = 0;
-			}
-			/* are we wide capable? */
-			if (iqd[7] & 0x20) {
-				qpti->dev_param[tgt].device_flags |= 0x20;
-			}
-
-			scsi_rbuf_put(Cmnd, iqd);
-
-			qpti->sbits |= (1 << tgt);
-		} else if (!ok) {
-			qpti->sbits |= (1 << tgt);
-		}
-	}
-	done(Cmnd);
-}
-
-static int qlogicpti_queuecommand(struct scsi_cmnd *Cmnd, void (*done)(struct scsi_cmnd *));
-
-static int qlogicpti_queuecommand_slow(struct scsi_cmnd *Cmnd,
-				       void (*done)(struct scsi_cmnd *))
-{
-	struct qlogicpti *qpti = (struct qlogicpti *) Cmnd->device->host->hostdata;
-
-	/*
-	 * done checking this host adapter?
-	 * If not, then rewrite the command
-	 * to finish through ourdone so we
-	 * can peek at Inquiry data results.
-	 */
-	if (qpti->sbits && qpti->sbits != 0xffff) {
-		/* See above about in ourdone this ugliness... */
-		Cmnd->SCp.Message = ((unsigned long)done) & 0xffffffff;
-#ifdef CONFIG_SPARC64
-		Cmnd->SCp.Status = ((unsigned long)done >> 32UL) & 0xffffffff;
-#endif
-		return qlogicpti_queuecommand(Cmnd, ourdone);
-	}
+	struct qlogicpti *qpti = shost_priv(sdev->host);
+	int tgt = sdev->id;
+	u_short param[6];
 
-	/*
-	 * We've peeked at all targets for this bus- time
-	 * to set parameters for devices for real now.
-	 */
-	if (qpti->sbits = 0xffff) {
-		int i;
-		for(i = 0; i < MAX_TARGETS; i++) {
-			u_short param[6];
-			param[0] = MBOX_SET_TARGET_PARAMS;
-			param[1] = (i << 8);
-			param[2] = (qpti->dev_param[i].device_flags << 8);
-			if (qpti->dev_param[i].device_flags & 0x10) {
-				param[3] = (qpti->dev_param[i].synchronous_offset << 8) |
-					qpti->dev_param[i].synchronous_period;
-			} else {
-				param[3] = 0;
-			}
-			(void) qlogicpti_mbox_command(qpti, param, 0);
-		}
-		/*
-		 * set to zero so any traverse through ourdone
-		 * doesn't start the whole process again,
-		 */
-		qpti->sbits = 0;
+	/* tags handled in midlayer */
+	/* enable sync mode? */
+	if (sdev->sdtr) {
+		qpti->dev_param[tgt].device_flags |= 0x10;
+	} else {
+		qpti->dev_param[tgt].synchronous_offset = 0;
+		qpti->dev_param[tgt].synchronous_period = 0;
 	}
-
-	/* check to see if we're done with all adapters... */
-	for (qpti = qptichain; qpti != NULL; qpti = qpti->next) {
-		if (qpti->sbits) {
-			break;
-		}
+	/* are we wide capable? */
+	if (sdev->wdtr)
+		qpti->dev_param[tgt].device_flags |= 0x20;
+
+	param[0] = MBOX_SET_TARGET_PARAMS;
+	param[1] = (tgt << 8);
+	param[2] = (qpti->dev_param[tgt].device_flags << 8);
+	if (qpti->dev_param[tgt].device_flags & 0x10) {
+		param[3] = (qpti->dev_param[tgt].synchronous_offset << 8) |
+			qpti->dev_param[tgt].synchronous_period;
+	} else {
+		param[3] = 0;
 	}
-
-	/*
-	 * if we hit the end of the chain w/o finding adapters still
-	 * capability-configuring, then we're done with all adapters
-	 * and can rock on..
-	 */
-	if (qpti = NULL)
-		Cmnd->device->host->hostt->queuecommand = qlogicpti_queuecommand;
-
-	return qlogicpti_queuecommand(Cmnd, done);
+	qlogicpti_mbox_command(qpti, param, 0);
+	return 0;
 }
 
 /*
@@ -1390,7 +1300,8 @@ static struct scsi_host_template qpti_template = {
 	.module			= THIS_MODULE,
 	.name			= "qlogicpti",
 	.info			= qlogicpti_info,
-	.queuecommand		= qlogicpti_queuecommand_slow,
+	.queuecommand		= qlogicpti_queuecommand,
+	.slave_configure	= qlogicpti_slave_configure,
 	.eh_abort_handler	= qlogicpti_abort,
 	.eh_bus_reset_handler	= qlogicpti_reset,
 	.can_queue		= QLOGICPTI_REQ_QUEUE_LEN,
drivers/scsi/qlogicpti.h +1 -2
index 6cd1c07..ef6da2d 100644
@@ -380,8 +380,7 @@ struct qlogicpti {
 	unsigned char             swsreg;
 	unsigned int	
 		gotirq	:	1,	/* this instance got an irq */
-		is_pti	: 	1,	/* Non-zero if this is a PTI board. */
-		sbits	:	16;	/* syncmode known bits */
+		is_pti	: 	1;	/* Non-zero if this is a PTI board. */
 };
 
 /* How to twiddle them bits... */
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
Acked-by
#1 lore raw

From: Matthew Wilcox <[email protected]>
Date: Wed, 19 Sep 2007 06:46:37 -0600

Quoted 3 lines

I haven't even compile-tested this. But look how much ugly code it deletes if it works!

It compiles :-) You deleted the only uses of scsi_rbuf_{get,put}() so you can kill those off too.

I don't have the time to setup things for testing this patch on the qlogicpti hardware I have, but with the scsi_rbuf_{get,put}() deletion added to the patch I would really like to see this go into 2.6.24 anyways.

If things explode I'll pick up the pieces.

#2 lore raw
Quoted 2 lines · David Miller

It compiles :-) You deleted the only uses of scsi_rbuf_{get,put}() so you can kill those off too.

Seeing as how they are exact duplicates of libata's ata_scsi_rbuf_{get,put}, I wonder how they got there in the first place (rather than becoming common code), and I wonder how many more copies are floating out there...

Jeff

#3 lore raw

From: Jeff Garzik <[email protected]>
Date: Sun, 30 Sep 2007 20:52:45 -0400

Quoted 9 lines
Quoted 2 lines · David Miller

It compiles :-) You deleted the only uses of scsi_rbuf_{get,put}() so you can kill those off too.

Seeing as how they are exact duplicates of libata's ata_scsi_rbuf_{get,put}, I wonder how they got there in the first place (rather than becoming common code), and I wonder how many more copies are floating out there...

I bet: 1) they were common code in scsi_lib.c 2) qlogicpti and libata became the only remaining users so 3) they got copied to those two users verbatim anticipating that the qlogicpti usage would eventually be deleted.

Interested parties can search the tree history to determine for suer :-)

#4 lore raw
Quoted 12 lines · David Miller

From: Jeff Garzik <[email protected]>
Date: Sun, 30 Sep 2007 20:52:45 -0400

Quoted 8 lines
Quoted 2 lines · David Miller

It compiles :-) You deleted the only uses of scsi_rbuf_{get,put}() so you can kill those off too.

Seeing as how they are exact duplicates of libata's ata_scsi_rbuf_{get,put}, I wonder how they got there in the first place (rather than becoming common code), and I wonder how many more copies are floating out there...

Quoted 4 lines

I bet: 1) they were common code in scsi_lib.c 2) qlogicpti and libata became the only remaining users so 3) they got copied to those two users verbatim anticipating that the qlogicpti usage would eventually be deleted.

I wrote the libata code from scratch, so I was just sorta curious. No biggie either (read: I'm too lazy to search the history).

After the qlogicpti use goes away, libata-scsi is the remaining user of that logic. The code is already local to libata, so... all good.

Jeff