drivers/fc4/fc.c in git-scsi-misc

drivers/fc4/fc.c in git-scsi-misc

[Andrew Morton]

davem won't be happy

drivers/fc4/fc.c: In function `fcp_scsi_receive':
drivers/fc4/fc.c:397: error: structure has no member named `done'
drivers/fc4/fc.c:450: error: structure has no member named `done'
drivers/fc4/fc.c: In function `fcp_scsi_queuecommand':
drivers/fc4/fc.c:837: error: structure has no member named `done'
drivers/fc4/fc.c:838: error: structure has no member named `done'
drivers/fc4/fc.c:839: error: structure has no member named `done'

Re: drivers/fc4/fc.c in git-scsi-misc

[Boaz Harrosh]

On Thu, Oct 11 2007 at 11:52 +0200, Andrew Morton <akpm@linux-foundation.org> wrote:
> davem won't be happy
> 
> drivers/fc4/fc.c: In function `fcp_scsi_receive':
> drivers/fc4/fc.c:397: error: structure has no member named `done'
> drivers/fc4/fc.c:450: error: structure has no member named `done'
> drivers/fc4/fc.c: In function `fcp_scsi_queuecommand':
> drivers/fc4/fc.c:837: error: structure has no member named `done'
> drivers/fc4/fc.c:838: error: structure has no member named `done'
> drivers/fc4/fc.c:839: error: structure has no member named `done'
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

OK that was a nice lesson in Kconfig and Linux-Makefile for me :)

So I can see that drivers/fc4/Kconfig is only "source"d from 
arch/sparc64/Kconfig. Hence no one experienced the breakage.

[
If I add to, say, drivrs/Kconfig
+source drivers/fc4/Kconfig
Than everything compiles and I can see the breakage now
]

Do you have a cross-compiling environment to compile all these
ARCHs, or you actually have these machines laying around for
compilation. (It's the cross-compiling I'm interested in)

Thanks
Boaz

Re: drivers/fc4/fc.c in git-scsi-misc

[Matthew Wilcox]

On Thu, Oct 11, 2007 at 02:52:03AM -0700, Andrew Morton wrote:
> davem won't be happy

I'm not happy either.  I think this should fix it, but obviously I have
no sparc machines of my own to test it, let alone a pluto.

----

Remove uses of the scsi_cmnd ->done method from the fc4 driver.  It was
being abused to flag commands that had already been through queuecommand;
use the fcmd->proto value for that instead.  The fcmd->done pointer now
becomes irrelevant.  Reuse the fcp_scsi_done name for an entirely different
function which provides a handy single place to call ->scsi_done (removing
some broken places that used to leak scsi_cmnds on error by calling ->done).

I had to add an include of <linux/pci.h> to get this to compile on non-sparc,
and pluto_detect_done() was unused, so remove it too.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

diff --git a/drivers/fc4/fc.c b/drivers/fc4/fc.c
index 82de9e1..4d71e64 100644
--- a/drivers/fc4/fc.c
+++ b/drivers/fc4/fc.c
@@ -33,6 +33,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/init.h>
+#include <linux/pci.h>
 
 #include <asm/pgtable.h>
 #include <asm/irq.h>
@@ -378,7 +379,12 @@ void fcp_register(fc_channel *fc, u8 type, int unregister)
 		printk ("FC: %segistering unknown type %02x\n", unregister ? "Unr" : "R", type);
 }
 
-static void fcp_scsi_done(struct scsi_cmnd *SCpnt);
+static void fcp_scsi_done(struct scsi_cmnd *SCpnt, unsigned int result)
+{
+	SCpnt->result = result;
+	FCD(("Calling scsi_done with %08x\n", SCpnt->result))
+	SCpnt->scsi_done(SCpnt);
+}
 
 static inline void fcp_scsi_receive(fc_channel *fc, int token, int status, fc_hdr *fch)
 {
@@ -394,9 +400,6 @@ static inline void fcp_scsi_receive(fc_channel *fc, int token, int status, fc_hd
 	rsp = (fcp_rsp *) (fc->scsi_rsp_pool + fc->rsp_size * token);
 	SCpnt = SC_FCMND(fcmd);
 
-	if (SCpnt->done != fcp_scsi_done)
-		return;
-
 	rsp_status = rsp->fcp_status;
 	FCD(("rsp_status %08x status %08x\n", rsp_status, status))
 	switch (status) {
@@ -442,17 +445,9 @@ static inline void fcp_scsi_receive(fc_channel *fc, int token, int status, fc_hd
 		printk ("%s: (%d,%d) Received rsp_status 0x%x\n", fc->name, SCpnt->device->channel, SCpnt->device->id, rsp_status);
 	}	
 	
-	SCpnt->result = (host_status << 16) | (rsp_status & 0xff);
-#ifdef FCDEBUG	
-	if (host_status || SCpnt->result || rsp_status) printk("FC: host_status %d, packet status %d\n",
-			host_status, SCpnt->result);
-#endif
-	SCpnt->done = fcmd->done;
-	fcmd->done=NULL;
 	clear_bit(token, fc->scsi_bitmap);
 	fc->scsi_free++;
-	FCD(("Calling scsi_done with %08x\n", SCpnt->result))
-	SCpnt->scsi_done(SCpnt);
+	fcp_scsi_done(SCpnt, (host_status << 16) | (rsp_status & 0xff));
 }
 
 void fcp_receive_solicited(fc_channel *fc, int proto, int token, int status, fc_hdr *fch)
@@ -751,13 +746,6 @@ void fcp_release(fc_channel *fcchain, int count)  /* count must > 0 */
 	 */
 }
 
-
-static void fcp_scsi_done(struct scsi_cmnd *SCpnt)
-{
-	if (FCP_CMND(SCpnt)->done)
-		FCP_CMND(SCpnt)->done(SCpnt);
-}
-
 static int fcp_scsi_queue_it(fc_channel *fc, struct scsi_cmnd *SCpnt,
 			     fcp_cmnd *fcmd, int prepare)
 {
@@ -773,8 +761,7 @@ static int fcp_scsi_queue_it(fc_channel *fc, struct scsi_cmnd *SCpnt,
 		if (fc->encode_addr (SCpnt, cmd->fcp_addr, fc, fcmd)) {
 			/* Invalid channel/id/lun and couldn't map it into fcp_addr */
 			clear_bit (i, fc->scsi_bitmap);
-			SCpnt->result = (DID_BAD_TARGET << 16);
-			SCpnt->scsi_done(SCpnt);
+			fcp_scsi_done(SCpnt, (DID_BAD_TARGET << 16));
 			return 0;
 		}
 		fc->scsi_free--;
@@ -834,21 +821,20 @@ int fcp_scsi_queuecommand(struct scsi_cmnd *SCpnt,
 	fc_channel *fc = FC_SCMND(SCpnt);
 	
 	FCD(("Entering SCSI queuecommand %p\n", fcmd))
-	if (SCpnt->done != fcp_scsi_done) {
-		fcmd->done = SCpnt->done;
-		SCpnt->done = fcp_scsi_done;
-		SCpnt->scsi_done = done;
-		fcmd->proto = TYPE_SCSI_FCP;
-		if (!fc->scsi_free) {
-			FCD(("FC: !scsi_free, putting cmd on ML queue\n"))
+	SCpnt->scsi_done = done;
+	if (!fc->scsi_free) {
+		FCD(("FC: !scsi_free, putting cmd on ML queue\n"))
 #if (FCP_SCSI_USE_NEW_EH_CODE == 0)
-			printk("fcp_scsi_queue_command: queue full, losing cmd, bad\n");
+		printk("fcp_scsi_queue_command: queue full, losing cmd, bad\n");
 #endif
-			return 1;
-		}
+		return 1;
+	}
+	if (fcmd->proto == TYPE_SCSI_FCP) {
+		return fcp_scsi_queue_it(fc, SCpnt, fcmd, 0);
+	} else {
+		fcmd->proto = TYPE_SCSI_FCP;
 		return fcp_scsi_queue_it(fc, SCpnt, fcmd, 1);
 	}
-	return fcp_scsi_queue_it(fc, SCpnt, fcmd, 0);
 }
 
 void fcp_queue_empty(fc_channel *fc)
@@ -890,8 +876,7 @@ int fcp_scsi_abort(struct scsi_cmnd *SCpnt)
 	 */
 
 	if (++fc->abort_count < (fc->can_queue >> 1)) {
-		SCpnt->result = DID_ABORT;
-		fcmd->done(SCpnt);
+		fcp_scsi_done(SCpnt, DID_ABORT);
 		printk("FC: soft abort\n");
 		return SUCCESS;
 	} else {
@@ -949,8 +934,6 @@ int fcp_scsi_dev_reset(struct scsi_cmnd *SCpnt)
 		if (fc->rst_pkt->eh_state == SCSI_STATE_QUEUED)
 			return FAILED; /* or SUCCESS. Only these */
 	}
-	fc->rst_pkt->done = NULL;
-
 
         fc->rst_pkt->eh_state = SCSI_STATE_QUEUED;
 	init_timer(&fc->rst_pkt->eh_timeout);
@@ -966,7 +949,7 @@ int fcp_scsi_dev_reset(struct scsi_cmnd *SCpnt)
 
 	fc->rst_pkt->device->host->eh_action = &sem;
 
-	fc->rst_pkt->done = fcp_scsi_reset_done;
+	fc->rst_pkt->scsi_done = fcp_scsi_reset_done;
 
 	spin_lock_irqsave(SCpnt->device->host->host_lock, flags);
 	fcp_scsi_queue_it(fc, fc->rst_pkt, fcmd, 0);
@@ -1000,8 +983,7 @@ static int __fcp_scsi_host_reset(struct scsi_cmnd *SCpnt)
 
 	for (i=0; i < fc->can_queue; i++) {
 		if (fc->cmd_slots[i] && SCpnt->result != DID_ABORT) {
-			SCpnt->result = DID_RESET;
-			fcmd->done(SCpnt);
+			fcp_scsi_done(SCpnt, DID_RESET);
 			fc->cmd_slots[i] = NULL;
 		}
 	}
diff --git a/drivers/fc4/fcp_impl.h b/drivers/fc4/fcp_impl.h
index 1ac6133..41fa149 100644
--- a/drivers/fc4/fcp_impl.h
+++ b/drivers/fc4/fcp_impl.h
@@ -39,7 +39,6 @@ struct _fc_channel;
 typedef struct fcp_cmnd {
 	struct fcp_cmnd		*next;
 	struct fcp_cmnd		*prev;
-	void			(*done)(struct scsi_cmnd *);
 	unsigned short		proto;
 	unsigned short		token;
 	unsigned int		did;
diff --git a/drivers/scsi/pluto.c b/drivers/scsi/pluto.c
index 0363c1c..c9279e7 100644
--- a/drivers/scsi/pluto.c
+++ b/drivers/scsi/pluto.c
@@ -55,11 +55,6 @@ static DECLARE_COMPLETION(fc_detect_complete);
 
 static int pluto_encode_addr(Scsi_Cmnd *SCpnt, u16 *addr, fc_channel *fc, fcp_cmnd *fcmd);
 
-static void __init pluto_detect_done(Scsi_Cmnd *SCpnt)
-{
-	/* Do nothing */
-}
-
 static void __init pluto_detect_scsi_done(Scsi_Cmnd *SCpnt)
 {
 	PLND(("Detect done %08lx\n", (long)SCpnt))

-- 
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."

Re: drivers/fc4/fc.c in git-scsi-misc

[Andrew Morton]

On Thu, 11 Oct 2007 13:05:32 +0200 Boaz Harrosh <bharrosh@panasas.com> wrote:

> On Thu, Oct 11 2007 at 11:52 +0200, Andrew Morton <akpm@linux-foundation.org> wrote:
> > davem won't be happy
> > 
> > drivers/fc4/fc.c: In function `fcp_scsi_receive':
> > drivers/fc4/fc.c:397: error: structure has no member named `done'
> > drivers/fc4/fc.c:450: error: structure has no member named `done'
> > drivers/fc4/fc.c: In function `fcp_scsi_queuecommand':
> > drivers/fc4/fc.c:837: error: structure has no member named `done'
> > drivers/fc4/fc.c:838: error: structure has no member named `done'
> > drivers/fc4/fc.c:839: error: structure has no member named `done'
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> OK that was a nice lesson in Kconfig and Linux-Makefile for me :)
> 
> So I can see that drivers/fc4/Kconfig is only "source"d from 
> arch/sparc64/Kconfig. Hence no one experienced the breakage.
> 
> [
> If I add to, say, drivrs/Kconfig
> +source drivers/fc4/Kconfig
> Than everything compiles and I can see the breakage now
> ]

Well if fc4.c compiles OK on non-sparc64 then perhaps we should enable
compilation on non-sparc64.  It will increase maintainability and code
quality and stuff.

otoh people might end up shipping useless drivers in x86 distros.

> Do you have a cross-compiling environment to compile all these
> ARCHs, or you actually have these machines laying around for
> compilation. (It's the cross-compiling I'm interested in)

There are a number of precompiled (by me) cross-compilers at
http://userweb.kernel.org/~akpm/cross-compilers/

Re: drivers/fc4/fc.c in git-scsi-misc

[David Miller]

From: Andrew Morton <akpm@linux-foundation.org>
Date: Thu, 11 Oct 2007 11:20:58 -0700

> Well if fc4.c compiles OK on non-sparc64 then perhaps we should enable
> compilation on non-sparc64.  It will increase maintainability and code
> quality and stuff.

No objections to including drivers/fc4/Kconfig directly from
drivers/Kconfig.

It's only included directly from arch/sparc64/Kconfig because
long long ago drivers/Kconfig could not be safely included
there.

Re: drivers/fc4/fc.c in git-scsi-misc

[James Bottomley]

On Thu, 2007-10-11 at 17:38 -0700, David Miller wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Thu, 11 Oct 2007 11:20:58 -0700
> 
> > Well if fc4.c compiles OK on non-sparc64 then perhaps we should enable
> > compilation on non-sparc64.  It will increase maintainability and code
> > quality and stuff.
> 
> No objections to including drivers/fc4/Kconfig directly from
> drivers/Kconfig.
> 
> It's only included directly from arch/sparc64/Kconfig because
> long long ago drivers/Kconfig could not be safely included
> there.

Would you be able to test out Matthew's patch and ack it?  Not having
fc4 compile is a blocker for the necessary SCSI merge.

James

Re: drivers/fc4/fc.c in git-scsi-misc

[David Miller]

From: James Bottomley <James.Bottomley@SteelEye.com>
Date: Fri, 12 Oct 2007 13:04:33 -0500

> On Thu, 2007-10-11 at 17:38 -0700, David Miller wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> > Date: Thu, 11 Oct 2007 11:20:58 -0700
> > 
> > > Well if fc4.c compiles OK on non-sparc64 then perhaps we should enable
> > > compilation on non-sparc64.  It will increase maintainability and code
> > > quality and stuff.
> > 
> > No objections to including drivers/fc4/Kconfig directly from
> > drivers/Kconfig.
> > 
> > It's only included directly from arch/sparc64/Kconfig because
> > long long ago drivers/Kconfig could not be safely included
> > there.
> 
> Would you be able to test out Matthew's patch and ack it?  Not having
> fc4 compile is a blocker for the necessary SCSI merge.

Unfortunately it doesn't build.  The drivers/scsi/pluto.c part
fails with:

drivers/scsi/pluto.c: In function ^[$,1rx^[(Bpluto_detect^[$,1ry^[(B:
drivers/scsi/pluto.c:159: error: ^[$,1rx^[(Bpluto_detect_done^[$,1ry^[(B undeclared (first use in this function)
drivers/scsi/pluto.c:159: error: (Each undeclared identifier is reported only once
drivers/scsi/pluto.c:159: error: for each function it appears in.)
make[1]: *** [drivers/scsi/pluto.o] Error 1
make: *** [drivers/scsi/pluto.o] Error 2

I think it can be fixed by merely removing the assignment on that
line, but I'l like Matthew to look at that and generate a new
patch if he agrees that is sufficient.

Also, drivers/fc4/fc.c gives a warning:

drivers/fc4/fc.c: In function ^[$,1rx^[(Bfcp_scsi_abort^[$,1ry^[(B:
drivers/fc4/fc.c:867: warning: unused variable ^[$,1rx^[(Bfcmd^[$,1ry^[(B
drivers/fc4/fc.c: In function ^[$,1rx^[(B__fcp_scsi_host_reset^[$,1ry^[(B:
drivers/fc4/fc.c:990: warning: unused variable ^[$,1rx^[(Bfcmd^[$,1ry^[(B

which is fixed thusly:

--- ./drivers/fc4/fc.c~	2007-10-12 15:21:20.000000000 -0700
+++ ./drivers/fc4/fc.c	2007-10-12 15:22:03.000000000 -0700
@@ -864,7 +864,6 @@
 int fcp_scsi_abort(struct scsi_cmnd *SCpnt)
 {
 	/* Internal bookkeeping only. Lose 1 cmd_slots slot. */
-	fcp_cmnd *fcmd = FCP_CMND(SCpnt);
 	fc_channel *fc = FC_SCMND(SCpnt);
 	
 	/*
@@ -987,7 +986,6 @@
 static int __fcp_scsi_host_reset(struct scsi_cmnd *SCpnt)
 {
 	fc_channel *fc = FC_SCMND(SCpnt);
-	fcp_cmnd *fcmd = FCP_CMND(SCpnt);
 	int i;
 
 	printk ("FC: host reset\n");

Re: drivers/fc4/fc.c in git-scsi-misc

[Matthew Wilcox]

On Fri, Oct 12, 2007 at 03:25:08PM -0700, David Miller wrote:
> Unfortunately it doesn't build.  The drivers/scsi/pluto.c part
> fails with:
> 
> drivers/scsi/pluto.c: In function ^[$,1rxpluto_detect^[$,1ry:
> drivers/scsi/pluto.c:159: error: ^[$,1rxpluto_detect_done^[$,1ry undeclared (first use in this function)

I bet you didn't try this patch on top of scsi-misc.  The assignment of
pluto_detect_done was removed in an earlier patch, but I neglected to
remove the definition of pluto_detect_done.

Looking at that patch again, I may have made the cardinal mistake of
believing the comment instead of the code:

http://git.kernel.org/?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commitdiff;h=4ad9c591633c117d013b40a146825666f9accf30;hp=2e4eab24e9597920e761a1fd4b1c4f278ae3b01e

I replaced an indirect call to pluto_detect_done with a call to
pluto_detect_scsi_done.  Whoops.  I think the call should simply be
deleted (since pluto_detect_done is a no-op).

> Also, drivers/fc4/fc.c gives a warning:

Fix incorporated ... patch against scsi-misc.

diff --git a/drivers/fc4/fc.c b/drivers/fc4/fc.c
index 82de9e1..48c3b62 100644
--- a/drivers/fc4/fc.c
+++ b/drivers/fc4/fc.c
@@ -33,6 +33,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/init.h>
+#include <linux/pci.h>
 
 #include <asm/pgtable.h>
 #include <asm/irq.h>
@@ -378,7 +379,12 @@ void fcp_register(fc_channel *fc, u8 type, int unregister)
 		printk ("FC: %segistering unknown type %02x\n", unregister ? "Unr" : "R", type);
 }
 
-static void fcp_scsi_done(struct scsi_cmnd *SCpnt);
+static void fcp_scsi_done(struct scsi_cmnd *SCpnt, unsigned int result)
+{
+	SCpnt->result = result;
+	FCD(("Calling scsi_done with %08x\n", SCpnt->result))
+	SCpnt->scsi_done(SCpnt);
+}
 
 static inline void fcp_scsi_receive(fc_channel *fc, int token, int status, fc_hdr *fch)
 {
@@ -394,9 +400,6 @@ static inline void fcp_scsi_receive(fc_channel *fc, int token, int status, fc_hd
 	rsp = (fcp_rsp *) (fc->scsi_rsp_pool + fc->rsp_size * token);
 	SCpnt = SC_FCMND(fcmd);
 
-	if (SCpnt->done != fcp_scsi_done)
-		return;
-
 	rsp_status = rsp->fcp_status;
 	FCD(("rsp_status %08x status %08x\n", rsp_status, status))
 	switch (status) {
@@ -442,17 +445,9 @@ static inline void fcp_scsi_receive(fc_channel *fc, int token, int status, fc_hd
 		printk ("%s: (%d,%d) Received rsp_status 0x%x\n", fc->name, SCpnt->device->channel, SCpnt->device->id, rsp_status);
 	}	
 	
-	SCpnt->result = (host_status << 16) | (rsp_status & 0xff);
-#ifdef FCDEBUG	
-	if (host_status || SCpnt->result || rsp_status) printk("FC: host_status %d, packet status %d\n",
-			host_status, SCpnt->result);
-#endif
-	SCpnt->done = fcmd->done;
-	fcmd->done=NULL;
 	clear_bit(token, fc->scsi_bitmap);
 	fc->scsi_free++;
-	FCD(("Calling scsi_done with %08x\n", SCpnt->result))
-	SCpnt->scsi_done(SCpnt);
+	fcp_scsi_done(SCpnt, (host_status << 16) | (rsp_status & 0xff));
 }
 
 void fcp_receive_solicited(fc_channel *fc, int proto, int token, int status, fc_hdr *fch)
@@ -751,13 +746,6 @@ void fcp_release(fc_channel *fcchain, int count)  /* count must > 0 */
 	 */
 }
 
-
-static void fcp_scsi_done(struct scsi_cmnd *SCpnt)
-{
-	if (FCP_CMND(SCpnt)->done)
-		FCP_CMND(SCpnt)->done(SCpnt);
-}
-
 static int fcp_scsi_queue_it(fc_channel *fc, struct scsi_cmnd *SCpnt,
 			     fcp_cmnd *fcmd, int prepare)
 {
@@ -773,8 +761,7 @@ static int fcp_scsi_queue_it(fc_channel *fc, struct scsi_cmnd *SCpnt,
 		if (fc->encode_addr (SCpnt, cmd->fcp_addr, fc, fcmd)) {
 			/* Invalid channel/id/lun and couldn't map it into fcp_addr */
 			clear_bit (i, fc->scsi_bitmap);
-			SCpnt->result = (DID_BAD_TARGET << 16);
-			SCpnt->scsi_done(SCpnt);
+			fcp_scsi_done(SCpnt, (DID_BAD_TARGET << 16));
 			return 0;
 		}
 		fc->scsi_free--;
@@ -834,21 +821,20 @@ int fcp_scsi_queuecommand(struct scsi_cmnd *SCpnt,
 	fc_channel *fc = FC_SCMND(SCpnt);
 	
 	FCD(("Entering SCSI queuecommand %p\n", fcmd))
-	if (SCpnt->done != fcp_scsi_done) {
-		fcmd->done = SCpnt->done;
-		SCpnt->done = fcp_scsi_done;
-		SCpnt->scsi_done = done;
-		fcmd->proto = TYPE_SCSI_FCP;
-		if (!fc->scsi_free) {
-			FCD(("FC: !scsi_free, putting cmd on ML queue\n"))
+	SCpnt->scsi_done = done;
+	if (!fc->scsi_free) {
+		FCD(("FC: !scsi_free, putting cmd on ML queue\n"))
 #if (FCP_SCSI_USE_NEW_EH_CODE == 0)
-			printk("fcp_scsi_queue_command: queue full, losing cmd, bad\n");
+		printk("fcp_scsi_queue_command: queue full, losing cmd, bad\n");
 #endif
-			return 1;
-		}
+		return 1;
+	}
+	if (fcmd->proto == TYPE_SCSI_FCP) {
+		return fcp_scsi_queue_it(fc, SCpnt, fcmd, 0);
+	} else {
+		fcmd->proto = TYPE_SCSI_FCP;
 		return fcp_scsi_queue_it(fc, SCpnt, fcmd, 1);
 	}
-	return fcp_scsi_queue_it(fc, SCpnt, fcmd, 0);
 }
 
 void fcp_queue_empty(fc_channel *fc)
@@ -867,7 +853,6 @@ void fcp_queue_empty(fc_channel *fc)
 int fcp_scsi_abort(struct scsi_cmnd *SCpnt)
 {
 	/* Internal bookkeeping only. Lose 1 cmd_slots slot. */
-	fcp_cmnd *fcmd = FCP_CMND(SCpnt);
 	fc_channel *fc = FC_SCMND(SCpnt);
 	
 	/*
@@ -890,8 +875,7 @@ int fcp_scsi_abort(struct scsi_cmnd *SCpnt)
 	 */
 
 	if (++fc->abort_count < (fc->can_queue >> 1)) {
-		SCpnt->result = DID_ABORT;
-		fcmd->done(SCpnt);
+		fcp_scsi_done(SCpnt, DID_ABORT);
 		printk("FC: soft abort\n");
 		return SUCCESS;
 	} else {
@@ -949,8 +933,6 @@ int fcp_scsi_dev_reset(struct scsi_cmnd *SCpnt)
 		if (fc->rst_pkt->eh_state == SCSI_STATE_QUEUED)
 			return FAILED; /* or SUCCESS. Only these */
 	}
-	fc->rst_pkt->done = NULL;
-
 
         fc->rst_pkt->eh_state = SCSI_STATE_QUEUED;
 	init_timer(&fc->rst_pkt->eh_timeout);
@@ -966,7 +948,7 @@ int fcp_scsi_dev_reset(struct scsi_cmnd *SCpnt)
 
 	fc->rst_pkt->device->host->eh_action = &sem;
 
-	fc->rst_pkt->done = fcp_scsi_reset_done;
+	fc->rst_pkt->scsi_done = fcp_scsi_reset_done;
 
 	spin_lock_irqsave(SCpnt->device->host->host_lock, flags);
 	fcp_scsi_queue_it(fc, fc->rst_pkt, fcmd, 0);
@@ -993,15 +975,13 @@ int fcp_scsi_dev_reset(struct scsi_cmnd *SCpnt)
 static int __fcp_scsi_host_reset(struct scsi_cmnd *SCpnt)
 {
 	fc_channel *fc = FC_SCMND(SCpnt);
-	fcp_cmnd *fcmd = FCP_CMND(SCpnt);
 	int i;
 
 	printk ("FC: host reset\n");
 
 	for (i=0; i < fc->can_queue; i++) {
 		if (fc->cmd_slots[i] && SCpnt->result != DID_ABORT) {
-			SCpnt->result = DID_RESET;
-			fcmd->done(SCpnt);
+			fcp_scsi_done(SCpnt, DID_RESET);
 			fc->cmd_slots[i] = NULL;
 		}
 	}
diff --git a/drivers/fc4/fcp_impl.h b/drivers/fc4/fcp_impl.h
index 1ac6133..41fa149 100644
--- a/drivers/fc4/fcp_impl.h
+++ b/drivers/fc4/fcp_impl.h
@@ -39,7 +39,6 @@ struct _fc_channel;
 typedef struct fcp_cmnd {
 	struct fcp_cmnd		*next;
 	struct fcp_cmnd		*prev;
-	void			(*done)(struct scsi_cmnd *);
 	unsigned short		proto;
 	unsigned short		token;
 	unsigned int		did;
diff --git a/drivers/scsi/pluto.c b/drivers/scsi/pluto.c
index 0363c1c..e598a90 100644
--- a/drivers/scsi/pluto.c
+++ b/drivers/scsi/pluto.c
@@ -55,14 +55,9 @@ static DECLARE_COMPLETION(fc_detect_complete);
 
 static int pluto_encode_addr(Scsi_Cmnd *SCpnt, u16 *addr, fc_channel *fc, fcp_cmnd *fcmd);
 
-static void __init pluto_detect_done(Scsi_Cmnd *SCpnt)
-{
-	/* Do nothing */
-}
-
 static void __init pluto_detect_scsi_done(Scsi_Cmnd *SCpnt)
 {
-	PLND(("Detect done %08lx\n", (long)SCpnt))
+	PLND(("Detect done %p\n", SCpnt))
 	if (atomic_dec_and_test (&fcss))
 		complete(&fc_detect_complete);
 }
@@ -193,9 +188,6 @@ int __init pluto_detect(struct scsi_host_template *tpnt)
 	
 		SCpnt = &(fcs[i].cmd);
 		
-		/* Let FC mid-level free allocated resources */
-		pluto_detect_scsi_done(SCpnt);
-		
 		if (!SCpnt->result) {
 			struct pluto_inquiry *inq;
 			struct pluto *pluto;

-- 
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."