[PATCH 1/17] sym53c8xx: Work around 53c896 erratum

[PATCH 1/17] sym53c8xx: Work around 53c896 erratum

[Matthew Wilcox]

Prevent DMA transfers from crossing the 16MB limit for early 53c896 chips.

From: Kai Makisara <Kai.Makisara@kolumbus.fi>
Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 3db2232..d5455e3 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1531,6 +1531,10 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	BUG_ON(sym2_transport_template == NULL);
 	instance->transportt	= sym2_transport_template;
 
+	/* 53c896 rev 1 errata: DMA may not cross 16MB boundary */
+	if (pdev->device == PCI_DEVICE_ID_NCR_53C896 && np->revision_id < 2)
+		instance->dma_boundary = 0xFFFFFF;
+
 	spin_unlock_irqrestore(instance->host_lock, flags);
 
 	return instance;
-- 
1.4.4.2

[PATCH 2/17] sym53c8xx: Use pci_dev irq number

[Matthew Wilcox]

Don't cache a private copy of the interrupt number

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |    7 +++----
 drivers/scsi/sym53c8xx_2/sym_glue.h |    1 -
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index d5455e3..11e0a28 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1288,7 +1288,7 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
 			 "revision id 0x%x\n",
 			 np->s.chip_name, np->device_id, np->revision_id);
 	copy_info(&info, "At PCI address %s, IRQ " IRQ_FMT "\n",
-		pci_name(np->s.device), IRQ_PRM(np->s.irq));
+		pci_name(np->s.device), IRQ_PRM(np->s.device->irq));
 	copy_info(&info, "Min. period factor %d, %s SCSI BUS%s\n",
 			 (int) (np->minsync_dt ? np->minsync_dt : np->minsync),
 			 np->maxwide ? "Wide" : "Narrow",
@@ -1341,8 +1341,8 @@ static void sym_free_resources(struct sym_hcb *np, struct pci_dev *pdev)
 	/*
 	 *  Free O/S specific resources.
 	 */
-	if (np->s.irq)
-		free_irq(np->s.irq, np);
+	if (pdev->irq)
+		free_irq(pdev->irq, np);
 	if (np->s.ioaddr)
 		pci_iounmap(pdev, np->s.ioaddr);
 	if (np->s.ramaddr)
@@ -1491,7 +1491,6 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 			sym_name(np), pdev->irq);
 		goto attach_failed;
 	}
-	np->s.irq = pdev->irq;
 
 	/*
 	 *  After SCSI devices have been opened, we cannot
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index 0f097ba..bea7bcc 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -184,7 +184,6 @@ struct sym_shcb {
 	void __iomem *	ioaddr;		/* MMIO kernel io address	*/
 	void __iomem *	ramaddr;	/* RAM  kernel io address	*/
 	u_short		io_ws;		/* IO window size		*/
-	int		irq;		/* IRQ number			*/
 
 	struct timer_list timer;	/* Timer handler link header	*/
 	u_long		lasttime;
-- 
1.4.4.2

Re: [PATCH 2/17] sym53c8xx: Use pci_dev irq number

[Jeff Garzik]

Matthew Wilcox wrote:
> Don't cache a private copy of the interrupt number
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  drivers/scsi/sym53c8xx_2/sym_glue.c |    7 +++----
>  drivers/scsi/sym53c8xx_2/sym_glue.h |    1 -
>  2 files changed, 3 insertions(+), 5 deletions(-)

ACK patches 1-2

[PATCH 3/17] sym53c8xx: Remove data_mapping and data_mapped

[Matthew Wilcox]

Before all commands used sg, data_mapping and data_mapped were used to
distinguish whether the command had used map_single or map_sg.  Now all
commands are sg, so we can delete data_mapping, data_mapped and the
wrapper functions __unmap_scsi_data, __map_scsi_sg_data, unmap_scsi_data
and map_scsi_sg_data.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |   33 +++------------------------------
 1 files changed, 3 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 11e0a28..924b5dd 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -134,8 +134,6 @@ static struct scsi_transport_template *sym2_transport_template = NULL;
  *  Driver private area in the SCSI command structure.
  */
 struct sym_ucmd {		/* Override the SCSI pointer structure */
-	dma_addr_t	data_mapping;
-	unsigned char	data_mapped;
 	unsigned char	to_do;			/* For error handling */
 	void (*old_done)(struct scsi_cmnd *);	/* For error handling */
 	struct completion *eh_done;		/* For error handling */
@@ -144,37 +142,12 @@ struct sym_ucmd {		/* Override the SCSI pointer structure */
 #define SYM_UCMD_PTR(cmd)  ((struct sym_ucmd *)(&(cmd)->SCp))
 #define SYM_SOFTC_PTR(cmd) sym_get_hcb(cmd->device->host)
 
-static void __unmap_scsi_data(struct pci_dev *pdev, struct scsi_cmnd *cmd)
-{
-	if (SYM_UCMD_PTR(cmd)->data_mapped)
-		scsi_dma_unmap(cmd);
-
-	SYM_UCMD_PTR(cmd)->data_mapped = 0;
-}
-
-static int __map_scsi_sg_data(struct pci_dev *pdev, struct scsi_cmnd *cmd)
-{
-	int use_sg;
-
-	use_sg = scsi_dma_map(cmd);
-	if (use_sg > 0) {
-		SYM_UCMD_PTR(cmd)->data_mapped  = 2;
-		SYM_UCMD_PTR(cmd)->data_mapping = use_sg;
-	}
-
-	return use_sg;
-}
-
-#define unmap_scsi_data(np, cmd)	\
-		__unmap_scsi_data(np->s.device, cmd)
-#define map_scsi_sg_data(np, cmd)	\
-		__map_scsi_sg_data(np->s.device, cmd)
 /*
  *  Complete a pending CAM CCB.
  */
 void sym_xpt_done(struct sym_hcb *np, struct scsi_cmnd *cmd)
 {
-	unmap_scsi_data(np, cmd);
+	scsi_dma_unmap(cmd);
 	cmd->scsi_done(cmd);
 }
 
@@ -307,14 +280,14 @@ static int sym_scatter(struct sym_hcb *np, struct sym_ccb *cp, struct scsi_cmnd
 
 	cp->data_len = 0;
 
-	use_sg = map_scsi_sg_data(np, cmd);
+	use_sg = scsi_dma_map(cmd);
 	if (use_sg > 0) {
 		struct scatterlist *sg;
 		struct sym_tcb *tp = &np->target[cp->target];
 		struct sym_tblmove *data;
 
 		if (use_sg > SYM_CONF_MAX_SG) {
-			unmap_scsi_data(np, cmd);
+			scsi_dma_unmap(cmd);
 			return -1;
 		}
 
-- 
1.4.4.2

Re: [PATCH 3/17] sym53c8xx: Remove data_mapping and data_mapped

[Jeff Garzik]

Matthew Wilcox wrote:
> Before all commands used sg, data_mapping and data_mapped were used to
> distinguish whether the command had used map_single or map_sg.  Now all
> commands are sg, so we can delete data_mapping, data_mapped and the
> wrapper functions __unmap_scsi_data, __map_scsi_sg_data, unmap_scsi_data
> and map_scsi_sg_data.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  drivers/scsi/sym53c8xx_2/sym_glue.c |   33 +++------------------------------
>  1 files changed, 3 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
> index 11e0a28..924b5dd 100644
> --- a/drivers/scsi/sym53c8xx_2/sym_glue.c
> +++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
> @@ -134,8 +134,6 @@ static struct scsi_transport_template *sym2_transport_template = NULL;
>   *  Driver private area in the SCSI command structure.
>   */
>  struct sym_ucmd {		/* Override the SCSI pointer structure */
> -	dma_addr_t	data_mapping;
> -	unsigned char	data_mapped;
>  	unsigned char	to_do;			/* For error handling */
>  	void (*old_done)(struct scsi_cmnd *);	/* For error handling */
>  	struct completion *eh_done;		/* For error handling */
> @@ -144,37 +142,12 @@ struct sym_ucmd {		/* Override the SCSI pointer structure */
>  #define SYM_UCMD_PTR(cmd)  ((struct sym_ucmd *)(&(cmd)->SCp))
>  #define SYM_SOFTC_PTR(cmd) sym_get_hcb(cmd->device->host)
>  
> -static void __unmap_scsi_data(struct pci_dev *pdev, struct scsi_cmnd *cmd)
> -{
> -	if (SYM_UCMD_PTR(cmd)->data_mapped)
> -		scsi_dma_unmap(cmd);
> -
> -	SYM_UCMD_PTR(cmd)->data_mapped = 0;
> -}
> -
> -static int __map_scsi_sg_data(struct pci_dev *pdev, struct scsi_cmnd *cmd)
> -{
> -	int use_sg;
> -
> -	use_sg = scsi_dma_map(cmd);
> -	if (use_sg > 0) {
> -		SYM_UCMD_PTR(cmd)->data_mapped  = 2;
> -		SYM_UCMD_PTR(cmd)->data_mapping = use_sg;
> -	}
> -
> -	return use_sg;
> -}
> -
> -#define unmap_scsi_data(np, cmd)	\
> -		__unmap_scsi_data(np->s.device, cmd)
> -#define map_scsi_sg_data(np, cmd)	\
> -		__map_scsi_sg_data(np->s.device, cmd)
>  /*
>   *  Complete a pending CAM CCB.
>   */
>  void sym_xpt_done(struct sym_hcb *np, struct scsi_cmnd *cmd)
>  {
> -	unmap_scsi_data(np, cmd);
> +	scsi_dma_unmap(cmd);
>  	cmd->scsi_done(cmd);
>  }
>  
> @@ -307,14 +280,14 @@ static int sym_scatter(struct sym_hcb *np, struct sym_ccb *cp, struct scsi_cmnd
>  
>  	cp->data_len = 0;
>  
> -	use_sg = map_scsi_sg_data(np, cmd);
> +	use_sg = scsi_dma_map(cmd);
>  	if (use_sg > 0) {
>  		struct scatterlist *sg;
>  		struct sym_tcb *tp = &np->target[cp->target];
>  		struct sym_tblmove *data;
>  
>  		if (use_sg > SYM_CONF_MAX_SG) {
> -			unmap_scsi_data(np, cmd);
> +			scsi_dma_unmap(cmd);
>  			return -1;

check scsi_dma_map() for negative return value (failure)

ACK once that is done

[PATCH 4/17] sym53c8xx: Remove unnecessary check in queuecommand

[Matthew Wilcox]

The midlayer won't scan the host ID, so we don't need to check.
This is the only caller of sym_xpt_done2, so remove that too.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |   18 +-----------------
 1 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 924b5dd..d614a9c 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -151,13 +151,6 @@ void sym_xpt_done(struct sym_hcb *np, struct scsi_cmnd *cmd)
 	cmd->scsi_done(cmd);
 }
 
-static void sym_xpt_done2(struct sym_hcb *np, struct scsi_cmnd *cmd, int cam_status)
-{
-	sym_set_cam_status(cmd, cam_status);
-	sym_xpt_done(np, cmd);
-}
-
-
 /*
  *  Tell the SCSI layer about a BUS RESET.
  */
@@ -324,15 +317,6 @@ static int sym_queue_command(struct sym_hcb *np, struct scsi_cmnd *cmd)
 	int	order;
 
 	/*
-	 *  Minimal checkings, so that we will not 
-	 *  go outside our tables.
-	 */
-	if (sdev->id == np->myaddr) {
-		sym_xpt_done2(np, cmd, DID_NO_CONNECT);
-		return 0;
-	}
-
-	/*
 	 *  Retrieve the target descriptor.
 	 */
 	tp = &np->target[sdev->id];
@@ -537,7 +521,7 @@ static int sym53c8xx_queue_command(struct scsi_cmnd *cmd,
 	struct sym_ucmd *ucp = SYM_UCMD_PTR(cmd);
 	int sts = 0;
 
-	cmd->scsi_done     = done;
+	cmd->scsi_done = done;
 	memset(ucp, 0, sizeof(*ucp));
 
 	/*
-- 
1.4.4.2

[PATCH 5/17] sym53c8xx: Don't disable interrupts in the interrupt handler

[Matthew Wilcox]

Interrupts can't be re-entered, so it's sufficient to call spin_lock, not
spin_lock_irqsave().

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index d614a9c..4d49a9b 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -550,14 +550,13 @@ static int sym53c8xx_queue_command(struct scsi_cmnd *cmd,
  */
 static irqreturn_t sym53c8xx_intr(int irq, void *dev_id)
 {
-	unsigned long flags;
-	struct sym_hcb *np = (struct sym_hcb *)dev_id;
+	struct sym_hcb *np = dev_id;
 
 	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("[");
 
-	spin_lock_irqsave(np->s.host->host_lock, flags);
+	spin_lock(np->s.host->host_lock);
 	sym_interrupt(np);
-	spin_unlock_irqrestore(np->s.host->host_lock, flags);
+	spin_unlock(np->s.host->host_lock);
 
 	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("]\n");
 
-- 
1.4.4.2

[PATCH 6/17] sym53c8xx: Stop overriding scsi_done

[Matthew Wilcox]

Instead of telling the reset routine that the command completed from
sym_eh_done, do it from sym_xpt_done.  The 'to_do' element of the ucmd
is redundant -- it serves only to tell whether eh_done is valid or not,
and we can tell this by checking to see if it's NULL.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |   60 ++++++++++------------------------
 1 files changed, 18 insertions(+), 42 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 4d49a9b..6bc8789 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -134,8 +134,6 @@ static struct scsi_transport_template *sym2_transport_template = NULL;
  *  Driver private area in the SCSI command structure.
  */
 struct sym_ucmd {		/* Override the SCSI pointer structure */
-	unsigned char	to_do;			/* For error handling */
-	void (*old_done)(struct scsi_cmnd *);	/* For error handling */
 	struct completion *eh_done;		/* For error handling */
 };
 
@@ -147,6 +145,12 @@ struct sym_ucmd {		/* Override the SCSI pointer structure */
  */
 void sym_xpt_done(struct sym_hcb *np, struct scsi_cmnd *cmd)
 {
+	struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
+	BUILD_BUG_ON(sizeof(struct scsi_pointer) < sizeof(struct sym_ucmd));
+
+	if (ucmd->eh_done)
+		complete(ucmd->eh_done);
+
 	scsi_dma_unmap(cmd);
 	cmd->scsi_done(cmd);
 }
@@ -586,26 +590,6 @@ static void sym53c8xx_timer(unsigned long npref)
 #define SYM_EH_HOST_RESET	3
 
 /*
- *  What we will do regarding the involved SCSI command.
- */
-#define SYM_EH_DO_IGNORE	0
-#define SYM_EH_DO_WAIT		2
-
-/*
- *  scsi_done() alias when error recovery is in progress.
- */
-static void sym_eh_done(struct scsi_cmnd *cmd)
-{
-	struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
-	BUILD_BUG_ON(sizeof(struct scsi_pointer) < sizeof(struct sym_ucmd));
-
-	cmd->scsi_done = ucmd->old_done;
-
-	if (ucmd->to_do == SYM_EH_DO_WAIT)
-		complete(ucmd->eh_done);
-}
-
-/*
  *  Generic method for our eh processing.
  *  The 'op' argument tells what we have to do.
  */
@@ -615,7 +599,7 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
 	struct Scsi_Host *host = cmd->device->host;
 	SYM_QUEHEAD *qp;
-	int to_do = SYM_EH_DO_IGNORE;
+	int cmd_queued = 0;
 	int sts = -1;
 	struct completion eh_done;
 
@@ -626,19 +610,11 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	FOR_EACH_QUEUED_ELEMENT(&np->busy_ccbq, qp) {
 		struct sym_ccb *cp = sym_que_entry(qp, struct sym_ccb, link_ccbq);
 		if (cp->cmd == cmd) {
-			to_do = SYM_EH_DO_WAIT;
+			cmd_queued = 1;
 			break;
 		}
 	}
 
-	if (to_do == SYM_EH_DO_WAIT) {
-		init_completion(&eh_done);
-		ucmd->old_done = cmd->scsi_done;
-		ucmd->eh_done = &eh_done;
-		wmb();
-		cmd->scsi_done = sym_eh_done;
-	}
-
 	/* Try to proceed the operation we have been asked for */
 	sts = -1;
 	switch(op) {
@@ -662,21 +638,21 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	}
 
 	/* On error, restore everything and cross fingers :) */
-	if (sts) {
-		cmd->scsi_done = ucmd->old_done;
-		to_do = SYM_EH_DO_IGNORE;
-	}
-
-	ucmd->to_do = to_do;
-	spin_unlock_irq(host->host_lock);
+	if (sts)
+		cmd_queued = 0;
 
-	if (to_do == SYM_EH_DO_WAIT) {
+	if (cmd_queued) {
+		init_completion(&eh_done);
+		ucmd->eh_done = &eh_done;
+		spin_unlock_irq(host->host_lock);
 		if (!wait_for_completion_timeout(&eh_done, 5*HZ)) {
-			ucmd->to_do = SYM_EH_DO_IGNORE;
-			wmb();
+			ucmd->eh_done = NULL;
 			sts = -2;
 		}
+	} else {
+		spin_unlock_irq(host->host_lock);
 	}
+
 	dev_warn(&cmd->device->sdev_gendev, "%s operation %s.\n", opname,
 			sts==0 ? "complete" :sts==-2 ? "timed-out" : "failed");
 	return sts ? SCSI_FAILED : SCSI_SUCCESS;
-- 
1.4.4.2

[PATCH 7/17] sym53c8xx: PCI Error Recovery support

[Matthew Wilcox]

From: Linas Vepstas <linas@austin.ibm.com>

This patch adds the PCI error recovery callbacks to the Symbios SCSI device
driver.  It includes support for First Failure Data Capture.

Signed-off-by: Linas Vepstas <linas@austin.ibm.com>

Assorted changes to initial patches, including returning IRQ_NONE from the
interrupt handler if the device is offline and re-using the eh_done completion
in the scsi error handler.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |  179 ++++++++++++++++++++++++++++++++++-
 drivers/scsi/sym53c8xx_2/sym_glue.h |    3 +
 drivers/scsi/sym53c8xx_2/sym_hipd.c |   25 ++++-
 3 files changed, 200 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 6bc8789..fec9c9c 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -134,7 +134,7 @@ static struct scsi_transport_template *sym2_transport_template = NULL;
  *  Driver private area in the SCSI command structure.
  */
 struct sym_ucmd {		/* Override the SCSI pointer structure */
-	struct completion *eh_done;		/* For error handling */
+	struct completion *eh_done;		/* SCSI error handling */
 };
 
 #define SYM_UCMD_PTR(cmd)  ((struct sym_ucmd *)(&(cmd)->SCp))
@@ -556,6 +556,10 @@ static irqreturn_t sym53c8xx_intr(int irq, void *dev_id)
 {
 	struct sym_hcb *np = dev_id;
 
+	/* Avoid spinloop trying to handle interrupts on frozen device */
+	if (pci_channel_offline(np->s.device))
+		return IRQ_NONE;
+
 	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("[");
 
 	spin_lock(np->s.host->host_lock);
@@ -598,6 +602,7 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	struct sym_hcb *np = SYM_SOFTC_PTR(cmd);
 	struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
 	struct Scsi_Host *host = cmd->device->host;
+	struct pci_dev *pdev = np->s.device;
 	SYM_QUEHEAD *qp;
 	int cmd_queued = 0;
 	int sts = -1;
@@ -605,6 +610,38 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 
 	dev_warn(&cmd->device->sdev_gendev, "%s operation started.\n", opname);
 
+	/* We may be in an error condition because the PCI bus
+	 * went down. In this case, we need to wait until the
+	 * PCI bus is reset, the card is reset, and only then
+	 * proceed with the scsi error recovery.  There's no
+	 * point in hurrying; take a leisurely wait.
+	 */
+#define WAIT_FOR_PCI_RECOVERY	35
+	if (pci_channel_offline(pdev)) {
+		struct host_data *hostdata = shost_priv(host);
+		struct completion *io_reset;
+		int finished_reset = 0;
+		init_completion(&eh_done);
+		spin_lock_irq(host->host_lock);
+		/* Make sure we didn't race */
+		if (pci_channel_offline(pdev)) {
+			if (!hostdata->io_reset)
+				hostdata->io_reset = &eh_done;
+			io_reset = hostdata->io_reset;
+		} else {
+			io_reset = NULL;
+		}
+
+		if (!pci_channel_offline(pdev))
+			finished_reset = 1;
+		spin_unlock_irq(host->host_lock);
+		if (!finished_reset)
+			finished_reset = wait_for_completion_timeout(io_reset,
+						WAIT_FOR_PCI_RECOVERY*HZ);
+		if (!finished_reset)
+			return SCSI_FAILED;
+	}
+
 	spin_lock_irq(host->host_lock);
 	/* This one is queued in some place -> to wait for completion */
 	FOR_EACH_QUEUED_ELEMENT(&np->busy_ccbq, qp) {
@@ -630,7 +667,7 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 		break;
 	case SYM_EH_HOST_RESET:
 		sym_reset_scsi_bus(np, 0);
-		sym_start_up (np, 1);
+		sym_start_up(np, 1);
 		sts = 0;
 		break;
 	default:
@@ -1435,7 +1472,7 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	/*
 	 *  Start the SCRIPTS.
 	 */
-	sym_start_up (np, 1);
+	sym_start_up(np, 1);
 
 	/*
 	 *  Start the timer daemon
@@ -1822,6 +1859,134 @@ static void __devexit sym2_remove(struct pci_dev *pdev)
 	attach_count--;
 }
 
+/**
+ * sym2_io_error_detected() - called when PCI error is detected
+ * @pdev: pointer to PCI device
+ * @state: current state of the PCI slot
+ */
+static pci_ers_result_t sym2_io_error_detected(struct pci_dev *pdev,
+                                         enum pci_channel_state state)
+{
+	/* If slot is permanently frozen, turn everything off */
+	if (state == pci_channel_io_perm_failure) {
+		sym2_remove(pdev);
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
+	disable_irq(pdev->irq);
+	pci_disable_device(pdev);
+
+	/* Request that MMIO be enabled, so register dump can be taken. */
+	return PCI_ERS_RESULT_CAN_RECOVER;
+}
+
+/**
+ * sym2_io_slot_dump - Enable MMIO and dump debug registers
+ * @pdev: pointer to PCI device
+ */
+static pci_ers_result_t sym2_io_slot_dump(struct pci_dev *pdev)
+{
+	struct sym_hcb *np = pci_get_drvdata(pdev);
+
+	sym_dump_registers(np);
+
+	/* Request a slot reset. */
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+/**
+ * sym2_reset_workarounds - hardware-specific work-arounds
+ *
+ * This routine is similar to sym_set_workarounds(), except
+ * that, at this point, we already know that the device was
+ * succesfully intialized at least once before, and so most
+ * of the steps taken there are un-needed here.
+ */
+static void sym2_reset_workarounds(struct pci_dev *pdev)
+{
+	u_char revision;
+	u_short status_reg;
+	struct sym_chip *chip;
+
+	pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
+	chip = sym_lookup_chip_table(pdev->device, revision);
+
+	/* Work around for errant bit in 895A, in a fashion
+	 * similar to what is done in sym_set_workarounds().
+	 */
+	pci_read_config_word(pdev, PCI_STATUS, &status_reg);
+	if (!(chip->features & FE_66MHZ) && (status_reg & PCI_STATUS_66MHZ)) {
+		status_reg = PCI_STATUS_66MHZ;
+		pci_write_config_word(pdev, PCI_STATUS, status_reg);
+		pci_read_config_word(pdev, PCI_STATUS, &status_reg);
+	}
+}
+
+/**
+ * sym2_io_slot_reset() - called when the pci bus has been reset.
+ * @pdev: pointer to PCI device
+ *
+ * Restart the card from scratch.
+ */
+static pci_ers_result_t sym2_io_slot_reset(struct pci_dev *pdev)
+{
+	struct sym_hcb *np = pci_get_drvdata(pdev);
+
+	printk(KERN_INFO "%s: recovering from a PCI slot reset\n",
+	          sym_name(np));
+
+	if (pci_enable_device(pdev)) {
+		printk(KERN_ERR "%s: Unable to enable after PCI reset\n",
+		        sym_name(np));
+		return PCI_ERS_RESULT_DISCONNECT;
+	}
+
+	pci_set_master(pdev);
+	enable_irq(pdev->irq);
+
+	/* If the chip can do Memory Write Invalidate, enable it */
+	if (np->features & FE_WRIE) {
+		if (pci_set_mwi(pdev))
+			return PCI_ERS_RESULT_DISCONNECT;
+	}
+
+	/* Perform work-arounds, analogous to sym_set_workarounds() */
+	sym2_reset_workarounds(pdev);
+
+	/* Perform host reset only on one instance of the card */
+	if (PCI_FUNC(pdev->devfn) == 0) {
+		if (sym_reset_scsi_bus(np, 0)) {
+			printk(KERN_ERR "%s: Unable to reset scsi host\n",
+			        sym_name(np));
+			return PCI_ERS_RESULT_DISCONNECT;
+		}
+		sym_start_up(np, 1);
+	}
+
+	return PCI_ERS_RESULT_RECOVERED;
+}
+
+/**
+ * sym2_io_resume() - resume normal ops after PCI reset
+ * @pdev: pointer to PCI device
+ *
+ * Called when the error recovery driver tells us that its
+ * OK to resume normal operation. Use completion to allow
+ * halted scsi ops to resume.
+ */
+static void sym2_io_resume(struct pci_dev *pdev)
+{
+	struct sym_hcb *np = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = np->s.host;
+	struct host_data *hostdata = shost_priv(shost);
+
+	spin_lock_irq(shost->host_lock);
+	if (hostdata->io_reset)
+		complete_all(hostdata->io_reset);
+	hostdata->io_reset = NULL;
+	spin_unlock_irq(shost->host_lock);
+}
+
 static void sym2_get_signalling(struct Scsi_Host *shost)
 {
 	struct sym_hcb *np = sym_get_hcb(shost);
@@ -1984,11 +2149,19 @@ static struct pci_device_id sym2_id_table[] __devinitdata = {
 
 MODULE_DEVICE_TABLE(pci, sym2_id_table);
 
+static struct pci_error_handlers sym2_err_handler = {
+	.error_detected	= sym2_io_error_detected,
+	.mmio_enabled	= sym2_io_slot_dump,
+	.slot_reset	= sym2_io_slot_reset,
+	.resume		= sym2_io_resume,
+};
+
 static struct pci_driver sym2_driver = {
 	.name		= NAME53C8XX,
 	.id_table	= sym2_id_table,
 	.probe		= sym2_probe,
 	.remove		= __devexit_p(sym2_remove),
+	.err_handler 	= &sym2_err_handler,
 };
 
 static int __init sym2_init(void)
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index bea7bcc..d5ba5aa 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -40,6 +40,7 @@
 #ifndef SYM_GLUE_H
 #define SYM_GLUE_H
 
+#include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/ioport.h>
 #include <linux/pci.h>
@@ -220,6 +221,7 @@ struct sym_device {
  */
 struct host_data {
 	struct sym_hcb *ncb;
+	struct completion *io_reset;		/* PCI error handling */
 };
 
 static inline struct sym_hcb * sym_get_hcb(struct Scsi_Host *host)
@@ -265,5 +267,6 @@ void sym_xpt_async_bus_reset(struct sym_hcb *np);
 void sym_xpt_async_sent_bdr(struct sym_hcb *np, int target);
 int  sym_setup_data_and_start (struct sym_hcb *np, struct scsi_cmnd *csio, struct sym_ccb *cp);
 void sym_log_bus_error(struct sym_hcb *np);
+void sym_dump_registers(struct sym_hcb *np);
 
 #endif /* SYM_GLUE_H */
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 21cd4c7..af24c44 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -1180,10 +1180,10 @@ static void sym_log_hard_error(struct sym_hcb *np, u_short sist, u_char dstat)
 			scr_to_cpu((int) *(u32 *)(script_base + script_ofs)));
 	}
 
-        printf ("%s: regdump:", sym_name(np));
-        for (i=0; i<24;i++)
-            printf (" %02x", (unsigned)INB_OFF(np, i));
-        printf (".\n");
+	printf("%s: regdump:", sym_name(np));
+	for (i = 0; i < 24; i++)
+		printf(" %02x", (unsigned)INB_OFF(np, i));
+	printf(".\n");
 
 	/*
 	 *  PCI BUS error.
@@ -1192,6 +1192,16 @@ static void sym_log_hard_error(struct sym_hcb *np, u_short sist, u_char dstat)
 		sym_log_bus_error(np);
 }
 
+void sym_dump_registers(struct sym_hcb *np)
+{
+	u_short sist;
+	u_char dstat;
+
+	sist = INW(np, nc_sist);
+	dstat = INB(np, nc_dstat);
+	sym_log_hard_error(np, sist, dstat);
+}
+
 static struct sym_chip sym_dev_table[] = {
  {PCI_DEVICE_ID_NCR_53C810, 0x0f, "810", 4, 8, 4, 64,
  FE_ERL}
@@ -2809,6 +2819,13 @@ void sym_interrupt (struct sym_hcb *np)
 			dstat |= INB(np, nc_dstat);
 		istatc = INB(np, nc_istat);
 		istat |= istatc;
+
+		/* Prevent deadlock waiting on a condition that may
+		 * never clear. */
+		if (unlikely(sist == 0xffff && dstat == 0xff)) {
+			if (pci_channel_offline(np->s.device))
+				return;
+		}
 	} while (istatc & (SIP|DIP));
 
 	if (DEBUG_FLAGS & DEBUG_TINY)
-- 
1.4.4.2

Re: [PATCH 7/17] sym53c8xx: PCI Error Recovery support

[Jeff Garzik]

Matthew Wilcox wrote:
> From: Linas Vepstas <linas@austin.ibm.com>
> 
> This patch adds the PCI error recovery callbacks to the Symbios SCSI device
> driver.  It includes support for First Failure Data Capture.
> 
> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
> 
> Assorted changes to initial patches, including returning IRQ_NONE from the
> interrupt handler if the device is offline and re-using the eh_done completion
> in the scsi error handler.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  drivers/scsi/sym53c8xx_2/sym_glue.c |  179 ++++++++++++++++++++++++++++++++++-
>  drivers/scsi/sym53c8xx_2/sym_glue.h |    3 +
>  drivers/scsi/sym53c8xx_2/sym_hipd.c |   25 ++++-
>  3 files changed, 200 insertions(+), 7 deletions(-)

ACK patches 4-7

[PATCH 8/17] sym53c8xx: Use pdev->revision

[Matthew Wilcox]

Auke missed the sym2 driver in his initial sweep.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_fw.c   |    6 +++---
 drivers/scsi/sym53c8xx_2/sym_glue.c |   25 +++++++++----------------
 drivers/scsi/sym53c8xx_2/sym_hipd.c |    8 ++++----
 drivers/scsi/sym53c8xx_2/sym_hipd.h |    1 -
 4 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_fw.c b/drivers/scsi/sym53c8xx_2/sym_fw.c
index 9916a2a..2be0ae1 100644
--- a/drivers/scsi/sym53c8xx_2/sym_fw.c
+++ b/drivers/scsi/sym53c8xx_2/sym_fw.c
@@ -206,13 +206,13 @@ sym_fw2_patch(struct sym_hcb *np)
 	 *  they are not desirable. See `sym_fw2.h' for more details.
 	 */
 	if (!(np->device_id == PCI_DEVICE_ID_LSI_53C1010_66 &&
-	      np->revision_id < 0x1 &&
+	      np->s.device->revision < 0x1 &&
 	      np->pciclk_khz < 60000)) {
 		scripta0->datao_phase[0] = cpu_to_scr(SCR_NO_OP);
 		scripta0->datao_phase[1] = cpu_to_scr(0);
 	}
-	if (!(np->device_id == PCI_DEVICE_ID_LSI_53C1010_33 &&
-	      /* np->revision_id < 0xff */ 1)) {
+	if (!(np->device_id == PCI_DEVICE_ID_LSI_53C1010_33 /* &&
+	      np->s.device->revision < 0xff */)) {
 		scripta0->sel_done[0] = cpu_to_scr(SCR_NO_OP);
 		scripta0->sel_done[1] = cpu_to_scr(0);
 	}
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index fec9c9c..ef707a4 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1254,8 +1254,8 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
 	info.pos	= 0;
 
 	copy_info(&info, "Chip " NAME53C "%s, device id 0x%x, "
-			 "revision id 0x%x\n",
-			 np->s.chip_name, np->device_id, np->revision_id);
+			 "revision id 0x%x\n", np->s.chip_name,
+			 np->device_id, np->s.device->revision);
 	copy_info(&info, "At PCI address %s, IRQ " IRQ_FMT "\n",
 		pci_name(np->s.device), IRQ_PRM(np->s.device->irq));
 	copy_info(&info, "Min. period factor %d, %s SCSI BUS%s\n",
@@ -1368,10 +1368,9 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	unsigned long flags;
 	struct sym_fw *fw;
 
-	printk(KERN_INFO
-		"sym%d: <%s> rev 0x%x at pci %s irq " IRQ_FMT "\n",
-		unit, dev->chip.name, dev->chip.revision_id,
-		pci_name(pdev), IRQ_PRM(pdev->irq));
+	printk(KERN_INFO "sym%d: <%s> rev 0x%x at pci %s irq " IRQ_FMT "\n",
+		unit, dev->chip.name, pdev->revision, pci_name(pdev),
+		IRQ_PRM(pdev->irq));
 
 	/*
 	 *  Get the firmware for this chip.
@@ -1412,7 +1411,6 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	np->s.device	= pdev;
 	np->s.unit	= unit;
 	np->device_id	= dev->chip.device_id;
-	np->revision_id	= dev->chip.revision_id;
 	np->features	= dev->chip.features;
 	np->clock_divn	= dev->chip.nr_divisor;
 	np->maxoffs	= dev->chip.offset_max;
@@ -1500,7 +1498,7 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	instance->transportt	= sym2_transport_template;
 
 	/* 53c896 rev 1 errata: DMA may not cross 16MB boundary */
-	if (pdev->device == PCI_DEVICE_ID_NCR_53C896 && np->revision_id < 2)
+	if (pdev->device == PCI_DEVICE_ID_NCR_53C896 && pdev->revision < 2)
 		instance->dma_boundary = 0xFFFFFF;
 
 	spin_unlock_irqrestore(instance->host_lock, flags);
@@ -1545,7 +1543,6 @@ static int __devinit sym_check_supported(struct sym_device *device)
 {
 	struct sym_chip *chip;
 	struct pci_dev *pdev = device->pdev;
-	u_char revision;
 	unsigned long io_port = pci_resource_start(pdev, 0);
 	int i;
 
@@ -1565,14 +1562,12 @@ static int __devinit sym_check_supported(struct sym_device *device)
 	 * to our device structure so we can make it match the actual device
 	 * and options.
 	 */
-	pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
-	chip = sym_lookup_chip_table(pdev->device, revision);
+	chip = sym_lookup_chip_table(pdev->device, pdev->revision);
 	if (!chip) {
 		dev_info(&pdev->dev, "device not supported\n");
 		return -ENODEV;
 	}
 	memcpy(&device->chip, chip, sizeof(device->chip));
-	device->chip.revision_id = revision;
 
 	return 0;
 }
@@ -1613,7 +1608,7 @@ static int __devinit sym_set_workarounds(struct sym_device *device)
 	 *  We must ensure the chip will use WRITE AND INVALIDATE.
 	 *  The revision number limit is for now arbitrary.
 	 */
-	if (pdev->device == PCI_DEVICE_ID_NCR_53C896 && chip->revision_id < 0x4) {
+	if (pdev->device == PCI_DEVICE_ID_NCR_53C896 && pdev->revision < 0x4) {
 		chip->features	|= (FE_WRIE | FE_CLSE);
 	}
 
@@ -1904,12 +1899,10 @@ static pci_ers_result_t sym2_io_slot_dump(struct pci_dev *pdev)
  */
 static void sym2_reset_workarounds(struct pci_dev *pdev)
 {
-	u_char revision;
 	u_short status_reg;
 	struct sym_chip *chip;
 
-	pci_read_config_byte(pdev, PCI_CLASS_REVISION, &revision);
-	chip = sym_lookup_chip_table(pdev->device, revision);
+	chip = sym_lookup_chip_table(pdev->device, pdev->revision);
 
 	/* Work around for errant bit in 895A, in a fashion
 	 * similar to what is done in sym_set_workarounds().
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index af24c44..9f4198c 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -805,7 +805,7 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	 *  are used. Disable internal cycles.
 	 */
 	if (np->device_id == PCI_DEVICE_ID_LSI_53C1010_33 &&
-	    np->revision_id < 0x1)
+	    np->s.device->revision < 0x1)
 		np->rv_ccntl0	|=  DILS;
 
 	/*
@@ -829,9 +829,9 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	 *  LOAD/STORE instructions does not need this work-around.
 	 */
 	if ((np->device_id == PCI_DEVICE_ID_NCR_53C810 &&
-	     np->revision_id >= 0x10 && np->revision_id <= 0x11) ||
+	     np->s.device->revision >= 0x10 && np->s.device->revision <= 0x11) ||
 	    (np->device_id == PCI_DEVICE_ID_NCR_53C860 &&
-	     np->revision_id <= 0x1))
+	     np->s.device->revision <= 0x1))
 		np->features &= ~(FE_WRIE|FE_ERL|FE_ERMP);
 
 	/*
@@ -1809,7 +1809,7 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	 *  I just don't want. :)
 	 */
 	if (np->device_id == PCI_DEVICE_ID_LSI_53C1010_33 &&
-	    np->revision_id < 1)
+	    np->s.device->revision < 1)
 		OUTB(np, nc_stest1, INB(np, nc_stest1) | 0x30);
 
 	/*
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h
index 79ab6a1..3aa8060 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
@@ -919,7 +919,6 @@ struct sym_hcb {
 	 *  General controller parameters and configuration.
 	 */
 	u_short	device_id;	/* PCI device id		*/
-	u_char	revision_id;	/* PCI device revision id	*/
 	u_int	features;	/* Chip features map		*/
 	u_char	myaddr;		/* SCSI id of the adapter	*/
 	u_char	maxburst;	/* log base 2 of dwords burst	*/
-- 
1.4.4.2

[PATCH 9/17] sym53c8xx: Remove ->device_id

[Matthew Wilcox]

Following the same path as ->revision_id, remove ->device_id

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_fw.c    |    4 ++--
 drivers/scsi/sym53c8xx_2/sym_glue.c  |    4 +---
 drivers/scsi/sym53c8xx_2/sym_glue.h  |    1 -
 drivers/scsi/sym53c8xx_2/sym_hipd.c  |   16 ++++++++--------
 drivers/scsi/sym53c8xx_2/sym_hipd.h  |    1 -
 drivers/scsi/sym53c8xx_2/sym_nvram.c |    2 +-
 6 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_fw.c b/drivers/scsi/sym53c8xx_2/sym_fw.c
index 2be0ae1..3d4db80 100644
--- a/drivers/scsi/sym53c8xx_2/sym_fw.c
+++ b/drivers/scsi/sym53c8xx_2/sym_fw.c
@@ -205,13 +205,13 @@ sym_fw2_patch(struct sym_hcb *np)
 	 *  Remove a couple of work-arounds specific to C1010 if 
 	 *  they are not desirable. See `sym_fw2.h' for more details.
 	 */
-	if (!(np->device_id == PCI_DEVICE_ID_LSI_53C1010_66 &&
+	if (!(np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_66 &&
 	      np->s.device->revision < 0x1 &&
 	      np->pciclk_khz < 60000)) {
 		scripta0->datao_phase[0] = cpu_to_scr(SCR_NO_OP);
 		scripta0->datao_phase[1] = cpu_to_scr(0);
 	}
-	if (!(np->device_id == PCI_DEVICE_ID_LSI_53C1010_33 /* &&
+	if (!(np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_33 /* &&
 	      np->s.device->revision < 0xff */)) {
 		scripta0->sel_done[0] = cpu_to_scr(SCR_NO_OP);
 		scripta0->sel_done[1] = cpu_to_scr(0);
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index ef707a4..357903f 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1255,7 +1255,7 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
 
 	copy_info(&info, "Chip " NAME53C "%s, device id 0x%x, "
 			 "revision id 0x%x\n", np->s.chip_name,
-			 np->device_id, np->s.device->revision);
+			 np->s.device->device, np->s.device->revision);
 	copy_info(&info, "At PCI address %s, IRQ " IRQ_FMT "\n",
 		pci_name(np->s.device), IRQ_PRM(np->s.device->irq));
 	copy_info(&info, "Min. period factor %d, %s SCSI BUS%s\n",
@@ -1410,7 +1410,6 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	np->verbose	= sym_driver_setup.verbose;
 	np->s.device	= pdev;
 	np->s.unit	= unit;
-	np->device_id	= dev->chip.device_id;
 	np->features	= dev->chip.features;
 	np->clock_divn	= dev->chip.nr_divisor;
 	np->maxoffs	= dev->chip.offset_max;
@@ -1528,7 +1527,6 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 static void __devinit sym_get_nvram(struct sym_device *devp, struct sym_nvram *nvp)
 {
 	devp->nvram = nvp;
-	devp->device_id = devp->chip.device_id;
 	nvp->type = 0;
 
 	sym_read_nvram(devp, nvp);
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index d5ba5aa..b961f70 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -212,7 +212,6 @@ struct sym_device {
 	} s;
 	struct sym_chip chip;
 	struct sym_nvram *nvram;
-	u_short device_id;
 	u_char host_id;
 };
 
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 9f4198c..5443394 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -804,7 +804,7 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	 *  In dual channel mode, contention occurs if internal cycles
 	 *  are used. Disable internal cycles.
 	 */
-	if (np->device_id == PCI_DEVICE_ID_LSI_53C1010_33 &&
+	if (np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_33 &&
 	    np->s.device->revision < 0x1)
 		np->rv_ccntl0	|=  DILS;
 
@@ -828,9 +828,9 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	 *  this driver. The generic ncr driver that does not use 
 	 *  LOAD/STORE instructions does not need this work-around.
 	 */
-	if ((np->device_id == PCI_DEVICE_ID_NCR_53C810 &&
+	if ((np->s.device->device == PCI_DEVICE_ID_NCR_53C810 &&
 	     np->s.device->revision >= 0x10 && np->s.device->revision <= 0x11) ||
-	    (np->device_id == PCI_DEVICE_ID_NCR_53C860 &&
+	    (np->s.device->device == PCI_DEVICE_ID_NCR_53C860 &&
 	     np->s.device->revision <= 0x1))
 		np->features &= ~(FE_WRIE|FE_ERL|FE_ERMP);
 
@@ -897,7 +897,7 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	if ((SYM_SETUP_SCSI_LED || 
 	     (nvram->type == SYM_SYMBIOS_NVRAM ||
 	      (nvram->type == SYM_TEKRAM_NVRAM &&
-	       np->device_id == PCI_DEVICE_ID_NCR_53C895))) &&
+	       np->s.device->device == PCI_DEVICE_ID_NCR_53C895))) &&
 	    !(np->features & FE_LEDC) && !(np->sv_gpcntl & 0x01))
 		np->features |= FE_LED0;
 
@@ -1798,7 +1798,7 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	/*
 	 *  For now, disable AIP generation on C1010-66.
 	 */
-	if (np->device_id == PCI_DEVICE_ID_LSI_53C1010_66)
+	if (np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_66)
 		OUTB(np, nc_aipcntl1, DISAIP);
 
 	/*
@@ -1808,7 +1808,7 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	 *  that from SCRIPTS for each selection/reselection, but 
 	 *  I just don't want. :)
 	 */
-	if (np->device_id == PCI_DEVICE_ID_LSI_53C1010_33 &&
+	if (np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_33 &&
 	    np->s.device->revision < 1)
 		OUTB(np, nc_stest1, INB(np, nc_stest1) | 0x30);
 
@@ -1817,9 +1817,9 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	 *  Disable overlapped arbitration for some dual function devices, 
 	 *  regardless revision id (kind of post-chip-design feature. ;-))
 	 */
-	if (np->device_id == PCI_DEVICE_ID_NCR_53C875)
+	if (np->s.device->device == PCI_DEVICE_ID_NCR_53C875)
 		OUTB(np, nc_ctest0, (1<<5));
-	else if (np->device_id == PCI_DEVICE_ID_NCR_53C896)
+	else if (np->s.device->device == PCI_DEVICE_ID_NCR_53C896)
 		np->rv_ccntl0 |= DPR;
 
 	/*
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h
index 3aa8060..b8908e1 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
@@ -918,7 +918,6 @@ struct sym_hcb {
 	/*
 	 *  General controller parameters and configuration.
 	 */
-	u_short	device_id;	/* PCI device id		*/
 	u_int	features;	/* Chip features map		*/
 	u_char	myaddr;		/* SCSI id of the adapter	*/
 	u_char	maxburst;	/* log base 2 of dwords burst	*/
diff --git a/drivers/scsi/sym53c8xx_2/sym_nvram.c b/drivers/scsi/sym53c8xx_2/sym_nvram.c
index 15d6929..5662fbb 100644
--- a/drivers/scsi/sym53c8xx_2/sym_nvram.c
+++ b/drivers/scsi/sym53c8xx_2/sym_nvram.c
@@ -696,7 +696,7 @@ static int sym_read_Tekram_nvram (struct sym_device *np, Tekram_nvram *nvram)
 	u_short	csum;
 	int x;
 
-	switch (np->device_id) {
+	switch (np->pdev->device) {
 	case PCI_DEVICE_ID_NCR_53C885:
 	case PCI_DEVICE_ID_NCR_53C895:
 	case PCI_DEVICE_ID_NCR_53C896:
-- 
1.4.4.2

[PATCH 10/17] sym53c8xx: Remove io_ws, mmio_ws and ram_ws elements

[Matthew Wilcox]

These struct elements record info that is never needed

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |    5 +----
 drivers/scsi/sym53c8xx_2/sym_glue.h |    1 -
 drivers/scsi/sym53c8xx_2/sym_hipd.c |    7 ++-----
 drivers/scsi/sym53c8xx_2/sym_hipd.h |    3 ---
 4 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 357903f..6865d30 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1432,17 +1432,14 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	np->mmio_ba = (u32)dev->mmio_base;
 	np->s.ioaddr	= dev->s.ioaddr;
 	np->s.ramaddr	= dev->s.ramaddr;
-	np->s.io_ws = (np->features & FE_IO256) ? 256 : 128;
 
 	/*
 	 *  Map on-chip RAM if present and supported.
 	 */
 	if (!(np->features & FE_RAM))
 		dev->ram_base = 0;
-	if (dev->ram_base) {
+	if (dev->ram_base)
 		np->ram_ba = (u32)dev->ram_base;
-		np->ram_ws = (np->features & FE_RAM8K) ? 8192 : 4096;
-	}
 
 	if (sym_hcb_attach(instance, fw, dev->nvram))
 		goto attach_failed;
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index b961f70..ab2de1c 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -184,7 +184,6 @@ struct sym_shcb {
 
 	void __iomem *	ioaddr;		/* MMIO kernel io address	*/
 	void __iomem *	ramaddr;	/* RAM  kernel io address	*/
-	u_short		io_ws;		/* IO window size		*/
 
 	struct timer_list timer;	/* Timer handler link header	*/
 	u_long		lasttime;
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 5443394..39f84bb 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -1910,7 +1910,7 @@ void sym_start_up (struct sym_hcb *np, int reason)
 		if (sym_verbose >= 2)
 			printf("%s: Downloading SCSI SCRIPTS.\n", sym_name(np));
 		memcpy_toio(np->s.ramaddr, np->scripta0, np->scripta_sz);
-		if (np->ram_ws == 8192) {
+		if (np->features & FE_RAM8K) {
 			memcpy_toio(np->s.ramaddr + 4096, np->scriptb0, np->scriptb_sz);
 			phys = scr_to_cpu(np->scr_ram_seg);
 			OUTL(np, nc_mmws, phys);
@@ -5595,16 +5595,13 @@ int sym_hcb_attach(struct Scsi_Host *shost, struct sym_fw *fw, struct sym_nvram
 	np->scriptz_ba	= vtobus(np->scriptz0);
 
 	if (np->ram_ba) {
-		np->scripta_ba	= np->ram_ba;
+		np->scripta_ba = np->ram_ba;
 		if (np->features & FE_RAM8K) {
-			np->ram_ws = 8192;
 			np->scriptb_ba = np->scripta_ba + 4096;
 #if 0	/* May get useful for 64 BIT PCI addressing */
 			np->scr_ram_seg = cpu_to_scr(np->scripta_ba >> 32);
 #endif
 		}
-		else
-			np->ram_ws = 4096;
 	}
 
 	/*
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h
index b8908e1..f76b27f 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
@@ -883,10 +883,7 @@ struct sym_hcb {
 	 *  Physical bus addresses of the chip.
 	 */
 	u32		mmio_ba;	/* MMIO 32 bit BUS address	*/
-	int		mmio_ws;	/* MMIO Window size		*/
-
 	u32		ram_ba;		/* RAM 32 bit BUS address	*/
-	int		ram_ws;		/* RAM window size		*/
 
 	/*
 	 *  SCRIPTS virtual and physical bus addresses.
-- 
1.4.4.2

Re: [PATCH 10/17] sym53c8xx: Remove io_ws, mmio_ws and ram_ws elements

[Jeff Garzik]

Matthew Wilcox wrote:
> These struct elements record info that is never needed
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  drivers/scsi/sym53c8xx_2/sym_glue.c |    5 +----
>  drivers/scsi/sym53c8xx_2/sym_glue.h |    1 -
>  drivers/scsi/sym53c8xx_2/sym_hipd.c |    7 ++-----
>  drivers/scsi/sym53c8xx_2/sym_hipd.h |    3 ---
>  4 files changed, 3 insertions(+), 13 deletions(-)

ACK patches 8-10

[PATCH 11/17] sym53c8xx: Remove tag_ctrl module parameter

[Matthew Wilcox]

With sysfs making these options tunable at runtime, there's no
justification for keeping this horrendously complex specification
string around.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 Documentation/scsi/sym53c8xx_2.txt   |   21 +++--------
 drivers/scsi/sym53c8xx_2/sym53c8xx.h |    1 -
 drivers/scsi/sym53c8xx_2/sym_glue.c  |   66 ++--------------------------------
 3 files changed, 8 insertions(+), 80 deletions(-)

diff --git a/Documentation/scsi/sym53c8xx_2.txt b/Documentation/scsi/sym53c8xx_2.txt
index 3d9f06b..49ea5c5 100644
--- a/Documentation/scsi/sym53c8xx_2.txt
+++ b/Documentation/scsi/sym53c8xx_2.txt
@@ -449,25 +449,14 @@ options as above.
         cmd_per_lun=#tags (#tags > 1) tagged command queuing enabled
   #tags will be truncated to the max queued commands configuration parameter.
 
-10.2.2  Detailed control of tagged commands
-  This option allows you to specify a command queue depth for each device 
-  that supports tagged command queueing.
-  Example:
-      tag_ctrl=10/t2t3q16-t5q24/t1u2q32
-  will set devices queue depth as follow:
-      - controller #0 target #2 and target #3                  -> 16 commands,
-      - controller #0 target #5                                -> 24 commands,
-      - controller #1 target #1 logical unit #2                -> 32 commands,
-      - all other logical units (all targets, all controllers) -> 10 commands.
-
-10.2.3 Burst max
+10.2.2 Burst max
         burst=0    burst disabled
         burst=255  get burst length from initial IO register settings.
         burst=#x   burst enabled (1<<#x burst transfers max)
   #x is an integer value which is log base 2 of the burst transfers max.
   By default the driver uses the maximum value supported by the chip.
 
-10.2.4 LED support
+10.2.3 LED support
         led=1      enable  LED support
         led=0      disable LED support
   Do not enable LED support if your scsi board does not use SDMS BIOS.
@@ -560,9 +549,9 @@ Previously, the sym2 driver accepted arguments of the form
 	sym53c8xx=tags:4,sync:10,debug:0x200
 
 As a result of the new module parameters, this is no longer available.
-Most of the options have remained the same, but tags has split into
-cmd_per_lun and tag_ctrl for its two different purposes.  The sample above
-would be specified as:
+Most of the options have remained the same, but tags has become
+cmd_per_lun to reflect its different purposes.  The sample above would
+be specified as:
 	modprobe sym53c8xx cmd_per_lun=4 sync=10 debug=0x200
 
 or on the kernel boot line as:
diff --git a/drivers/scsi/sym53c8xx_2/sym53c8xx.h b/drivers/scsi/sym53c8xx_2/sym53c8xx.h
index 7519728..62d29cf 100644
--- a/drivers/scsi/sym53c8xx_2/sym53c8xx.h
+++ b/drivers/scsi/sym53c8xx_2/sym53c8xx.h
@@ -127,7 +127,6 @@ struct sym_driver_setup {
 	u_char	settle_delay;
 	u_char	use_nvram;
 	u_long	excludes[8];
-	char	tag_ctrl[100];
 };
 
 #define SYM_SETUP_MAX_TAG		sym_driver_setup.max_tag
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 6865d30..7717ab8 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -63,7 +63,6 @@ unsigned int sym_debug_flags = 0;
 static char *excl_string;
 static char *safe_string;
 module_param_named(cmd_per_lun, sym_driver_setup.max_tag, ushort, 0);
-module_param_string(tag_ctrl, sym_driver_setup.tag_ctrl, 100, 0);
 module_param_named(burst, sym_driver_setup.burst_order, byte, 0);
 module_param_named(led, sym_driver_setup.scsi_led, byte, 0);
 module_param_named(diff, sym_driver_setup.scsi_diff, byte, 0);
@@ -78,7 +77,6 @@ module_param_named(excl, excl_string, charp, 0);
 module_param_named(safe, safe_string, charp, 0);
 
 MODULE_PARM_DESC(cmd_per_lun, "The maximum number of tags to use by default");
-MODULE_PARM_DESC(tag_ctrl, "More detailed control over tags per LUN");
 MODULE_PARM_DESC(burst, "Maximum burst.  0 to disable, 255 to read from registers");
 MODULE_PARM_DESC(led, "Set to 1 to enable LED support");
 MODULE_PARM_DESC(diff, "0 for no differential mode, 1 for BIOS, 2 for always, 3 for not GPIO3");
@@ -744,59 +742,6 @@ static void sym_tune_dev_queuing(struct sym_tcb *tp, int lun, u_short reqtags)
 	}
 }
 
-/*
- *  Linux select queue depths function
- */
-#define DEF_DEPTH	(sym_driver_setup.max_tag)
-#define ALL_TARGETS	-2
-#define NO_TARGET	-1
-#define ALL_LUNS	-2
-#define NO_LUN		-1
-
-static int device_queue_depth(struct sym_hcb *np, int target, int lun)
-{
-	int c, h, t, u, v;
-	char *p = sym_driver_setup.tag_ctrl;
-	char *ep;
-
-	h = -1;
-	t = NO_TARGET;
-	u = NO_LUN;
-	while ((c = *p++) != 0) {
-		v = simple_strtoul(p, &ep, 0);
-		switch(c) {
-		case '/':
-			++h;
-			t = ALL_TARGETS;
-			u = ALL_LUNS;
-			break;
-		case 't':
-			if (t != target)
-				t = (target == v) ? v : NO_TARGET;
-			u = ALL_LUNS;
-			break;
-		case 'u':
-			if (u != lun)
-				u = (lun == v) ? v : NO_LUN;
-			break;
-		case 'q':
-			if (h == np->s.unit &&
-				(t == ALL_TARGETS || t == target) &&
-				(u == ALL_LUNS    || u == lun))
-				return v;
-			break;
-		case '-':
-			t = ALL_TARGETS;
-			u = ALL_LUNS;
-			break;
-		default:
-			break;
-		}
-		p = ep;
-	}
-	return DEF_DEPTH;
-}
-
 static int sym53c8xx_slave_alloc(struct scsi_device *sdev)
 {
 	struct sym_hcb *np = sym_get_hcb(sdev->host);
@@ -861,21 +806,16 @@ static int sym53c8xx_slave_configure(struct scsi_device *sdev)
 	 *  Use at least 2.
 	 *  Donnot use more than our maximum.
 	 */
-	reqtags = device_queue_depth(np, sdev->id, sdev->lun);
+	reqtags = sym_driver_setup.max_tag;
 	if (reqtags > tp->usrtags)
 		reqtags = tp->usrtags;
 	if (!sdev->tagged_supported)
 		reqtags = 0;
-#if 1 /* Avoid to locally queue commands for no good reasons */
 	if (reqtags > SYM_CONF_MAX_TAG)
 		reqtags = SYM_CONF_MAX_TAG;
-	depth_to_use = (reqtags ? reqtags : 2);
-#else
-	depth_to_use = (reqtags ? SYM_CONF_MAX_TAG : 2);
-#endif
+	depth_to_use = reqtags ? reqtags : 2;
 	scsi_adjust_queue_depth(sdev,
-				(sdev->tagged_supported ?
-				 MSG_SIMPLE_TAG : 0),
+				sdev->tagged_supported ? MSG_SIMPLE_TAG : 0,
 				depth_to_use);
 	lp->s.scdev_depth = depth_to_use;
 	sym_tune_dev_queuing(tp, sdev->lun, reqtags);
-- 
1.4.4.2

Re: [PATCH 11/17] sym53c8xx: Remove tag_ctrl module parameter

[Jeff Garzik]

Matthew Wilcox wrote:
> With sysfs making these options tunable at runtime, there's no
> justification for keeping this horrendously complex specification
> string around.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  Documentation/scsi/sym53c8xx_2.txt   |   21 +++--------
>  drivers/scsi/sym53c8xx_2/sym53c8xx.h |    1 -
>  drivers/scsi/sym53c8xx_2/sym_glue.c  |   66 ++--------------------------------
>  3 files changed, 8 insertions(+), 80 deletions(-)

ACK presuming you are convinced this will not break anybody in the field

Re: [PATCH 11/17] sym53c8xx: Remove tag_ctrl module parameter

[Matthew Wilcox]

On Fri, Oct 05, 2007 at 04:56:41PM -0400, Jeff Garzik wrote:
> Matthew Wilcox wrote:
> >With sysfs making these options tunable at runtime, there's no
> >justification for keeping this horrendously complex specification
> >string around.
> >
> >Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> >---
> > Documentation/scsi/sym53c8xx_2.txt   |   21 +++--------
> > drivers/scsi/sym53c8xx_2/sym53c8xx.h |    1 -
> > drivers/scsi/sym53c8xx_2/sym_glue.c  |   66 
> > ++--------------------------------
> > 3 files changed, 8 insertions(+), 80 deletions(-)
> 
> ACK presuming you are convinced this will not break anybody in the field

Yeah, I am.  When Rusty did the conversion of sym53c8xx="..." to
individual module params, I didn't hear a single complaint.  I feel sure
nobody's using these options any more.

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

[PATCH 12/17] sym53c8xx: Simplify DAC DMA handling

[Matthew Wilcox]

By introducing the use_dac(), set_dac() and DMA_DAC_MASK macros, we can
eliminate a lot of ifdefs from the code.  We now rely on the compiler to
optimise away a few things that we'd formerly relied on the preprocessor
to do.  This makes sym_setup_bus_dma_mask() small enough to inline into
its only caller.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_fw.c   |    2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c |   32 ++++++--------------------------
 drivers/scsi/sym53c8xx_2/sym_hipd.c |   23 ++++++++---------------
 drivers/scsi/sym53c8xx_2/sym_hipd.h |   11 +++++++++++
 4 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_fw.c b/drivers/scsi/sym53c8xx_2/sym_fw.c
index 3d4db80..aa230d2 100644
--- a/drivers/scsi/sym53c8xx_2/sym_fw.c
+++ b/drivers/scsi/sym53c8xx_2/sym_fw.c
@@ -167,7 +167,7 @@ sym_fw2_patch(struct sym_hcb *np)
 	 *  Remove useless 64 bit DMA specific SCRIPTS, 
 	 *  when this feature is not available.
 	 */
-	if (!np->use_dac) {
+	if (!use_dac(np)) {
 		scripta0->is_dmap_dirty[0] = cpu_to_scr(SCR_NO_OP);
 		scripta0->is_dmap_dirty[1] = 0;
 		scripta0->is_dmap_dirty[2] = cpu_to_scr(SCR_NO_OP);
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 7717ab8..e5b2213 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1265,31 +1265,6 @@ static void sym_free_resources(struct sym_hcb *np, struct pci_dev *pdev)
 }
 
 /*
- *  Ask/tell the system about DMA addressing.
- */
-static int sym_setup_bus_dma_mask(struct sym_hcb *np)
-{
-#if SYM_CONF_DMA_ADDRESSING_MODE > 0
-#if   SYM_CONF_DMA_ADDRESSING_MODE == 1
-#define	DMA_DAC_MASK	DMA_40BIT_MASK
-#elif SYM_CONF_DMA_ADDRESSING_MODE == 2
-#define	DMA_DAC_MASK	DMA_64BIT_MASK
-#endif
-	if ((np->features & FE_DAC) &&
-			!pci_set_dma_mask(np->s.device, DMA_DAC_MASK)) {
-		np->use_dac = 1;
-		return 0;
-	}
-#endif
-
-	if (!pci_set_dma_mask(np->s.device, DMA_32BIT_MASK))
-		return 0;
-
-	printf_warning("%s: No suitable DMA available\n", sym_name(np));
-	return -1;
-}
-
-/*
  *  Host attach and initialisations.
  *
  *  Allocate host data and ncb structure.
@@ -1362,8 +1337,13 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	strlcpy(np->s.chip_name, dev->chip.name, sizeof(np->s.chip_name));
 	sprintf(np->s.inst_name, "sym%d", np->s.unit);
 
-	if (sym_setup_bus_dma_mask(np))
+	if ((SYM_CONF_DMA_ADDRESSING_MODE > 0) && (np->features & FE_DAC) &&
+			!pci_set_dma_mask(np->s.device, DMA_DAC_MASK)) {
+		set_dac(np);
+	} else if (pci_set_dma_mask(np->s.device, DMA_32BIT_MASK)) {
+		printf_warning("%s: No suitable DMA available\n", sym_name(np));
 		goto attach_failed;
+	}
 
 	/*
 	 *  Try to map the controller chip to
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 39f84bb..5d0d356 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -778,19 +778,12 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	 *  64 bit addressing  (895A/896/1010) ?
 	 */
 	if (np->features & FE_DAC) {
-#if   SYM_CONF_DMA_ADDRESSING_MODE == 0
-		np->rv_ccntl1	|= (DDAC);
-#elif SYM_CONF_DMA_ADDRESSING_MODE == 1
-		if (!np->use_dac)
-			np->rv_ccntl1	|= (DDAC);
-		else
-			np->rv_ccntl1	|= (XTIMOD | EXTIBMV);
-#elif SYM_CONF_DMA_ADDRESSING_MODE == 2
-		if (!np->use_dac)
-			np->rv_ccntl1	|= (DDAC);
-		else
-			np->rv_ccntl1	|= (0 | EXTIBMV);
-#endif
+		if (!use_dac(np))
+			np->rv_ccntl1 |= (DDAC);
+		else if (SYM_CONF_DMA_ADDRESSING_MODE == 1)
+			np->rv_ccntl1 |= (XTIMOD | EXTIBMV);
+		else if (SYM_CONF_DMA_ADDRESSING_MODE == 2)
+			np->rv_ccntl1 |= (0 | EXTIBMV);
 	}
 
 	/*
@@ -1322,7 +1315,7 @@ int sym_lookup_dmap(struct sym_hcb *np, u32 h, int s)
 {
 	int i;
 
-	if (!np->use_dac)
+	if (!use_dac(np))
 		goto weird;
 
 	/* Look up existing mappings */
@@ -1837,7 +1830,7 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	 *  Set up scratch C and DRS IO registers to map the 32 bit 
 	 *  DMA address range our data structures are located in.
 	 */
-	if (np->use_dac) {
+	if (use_dac(np)) {
 		np->dmap_bah[0] = 0;	/* ??? */
 		OUTL(np, nc_scrx[0], np->dmap_bah[0]);
 		OUTL(np, nc_drs, np->dmap_bah[0]);
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h
index f76b27f..4354571 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
@@ -1026,6 +1026,14 @@ struct sym_hcb {
 #endif
 };
 
+#if SYM_CONF_DMA_ADDRESSING_MODE == 0
+#define use_dac(np)	0
+#define set_dac(np)	do { } while (0)
+#else
+#define use_dac(np)	(np)->use_dac
+#define set_dac(np)	(np)->use_dac = 1
+#endif
+
 #define HCB_BA(np, lbl)	(np->hcb_ba + offsetof(struct sym_hcb, lbl))
 
 
@@ -1068,18 +1076,21 @@ int sym_hcb_attach(struct Scsi_Host *shost, struct sym_fw *fw, struct sym_nvram
  */
 
 #if   SYM_CONF_DMA_ADDRESSING_MODE == 0
+#define DMA_DAC_MASK	DMA_32BIT_MASK
 #define sym_build_sge(np, data, badd, len)	\
 do {						\
 	(data)->addr = cpu_to_scr(badd);	\
 	(data)->size = cpu_to_scr(len);		\
 } while (0)
 #elif SYM_CONF_DMA_ADDRESSING_MODE == 1
+#define DMA_DAC_MASK	DMA_40BIT_MASK
 #define sym_build_sge(np, data, badd, len)				\
 do {									\
 	(data)->addr = cpu_to_scr(badd);				\
 	(data)->size = cpu_to_scr((((badd) >> 8) & 0xff000000) + len);	\
 } while (0)
 #elif SYM_CONF_DMA_ADDRESSING_MODE == 2
+#define DMA_DAC_MASK	DMA_64BIT_MASK
 int sym_lookup_dmap(struct sym_hcb *np, u32 h, int s);
 static __inline void 
 sym_build_sge(struct sym_hcb *np, struct sym_tblmove *data, u64 badd, int len)
-- 
1.4.4.2

[PATCH 13/17] sym53c8xx: Use scmd_printk where appropriate

[Matthew Wilcox]

If we have a scsi_cmnd, it gives the user more information than the
sym_name, and maybe the target.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |    4 ++--
 drivers/scsi/sym53c8xx_2/sym_hipd.c |   35 +++++++++++++++++------------------
 2 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index e5b2213..e731618 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -392,7 +392,7 @@ int sym_setup_data_and_start(struct sym_hcb *np, struct scsi_cmnd *cmd, struct s
 	 */
 	switch (dir) {
 	case DMA_BIDIRECTIONAL:
-		printk("%s: got DMA_BIDIRECTIONAL command", sym_name(np));
+		scmd_printk(KERN_INFO, cmd, "got DMA_BIDIRECTIONAL command");
 		sym_set_cam_status(cmd, DID_ERROR);
 		goto out_abort;
 	case DMA_TO_DEVICE:
@@ -606,7 +606,7 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	int sts = -1;
 	struct completion eh_done;
 
-	dev_warn(&cmd->device->sdev_gendev, "%s operation started.\n", opname);
+	scmd_printk(KERN_WARNING, cmd, "%s operation started.\n", opname);
 
 	/* We may be in an error condition because the PCI bus
 	 * went down. In this case, we need to wait until the
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 5d0d356..13fd5b2 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -52,7 +52,7 @@
  *  Needed function prototypes.
  */
 static void sym_int_ma (struct sym_hcb *np);
-static void sym_int_sir (struct sym_hcb *np);
+static void sym_int_sir(struct sym_hcb *);
 static struct sym_ccb *sym_alloc_ccb(struct sym_hcb *np);
 static struct sym_ccb *sym_ccb_from_dsa(struct sym_hcb *np, u32 dsa);
 static void sym_alloc_lcb_tags (struct sym_hcb *np, u_char tn, u_char ln);
@@ -1522,7 +1522,8 @@ void sym_put_start_queue(struct sym_hcb *np, struct sym_ccb *cp)
 	np->squeueput = qidx;
 
 	if (DEBUG_FLAGS & DEBUG_QUEUE)
-		printf ("%s: queuepos=%d.\n", sym_name (np), np->squeueput);
+		scmd_printk(KERN_DEBUG, cp->cmd, "queuepos=%d\n",
+							np->squeueput);
 
 	/*
 	 *  Script processor may be waiting for reselect.
@@ -2852,7 +2853,7 @@ void sym_interrupt (struct sym_hcb *np)
 	    !(dstat & (MDPE|BF|ABRT|IID))) {
 		if	(sist & PAR)	sym_int_par (np, sist);
 		else if (sist & MA)	sym_int_ma (np);
-		else if (dstat & SIR)	sym_int_sir (np);
+		else if (dstat & SIR)	sym_int_sir(np);
 		else if (dstat & SSI)	OUTONB_STD();
 		else			goto unknown_int;
 		return;
@@ -4314,7 +4315,7 @@ static void sym_nego_rejected(struct sym_hcb *np, struct sym_tcb *tp, struct sym
 /*
  *  chip exception handler for programmed interrupts.
  */
-static void sym_int_sir (struct sym_hcb *np)
+static void sym_int_sir(struct sym_hcb *np)
 {
 	u_char	num	= INB(np, nc_dsps);
 	u32	dsa	= INL(np, nc_dsa);
@@ -4353,31 +4354,30 @@ static void sym_int_sir (struct sym_hcb *np)
 		return;
 	/*
 	 *  The device didn't go to MSG OUT phase after having 
-	 *  been selected with ATN. We donnot want to handle 
-	 *  that.
+	 *  been selected with ATN.  We do not want to handle that.
 	 */
 	case SIR_SEL_ATN_NO_MSG_OUT:
-		printf ("%s:%d: No MSG OUT phase after selection with ATN.\n",
-			sym_name (np), target);
+		scmd_printk(KERN_WARNING, cp->cmd,
+				"No MSG OUT phase after selection with ATN\n");
 		goto out_stuck;
 	/*
 	 *  The device didn't switch to MSG IN phase after 
-	 *  having reseleted the initiator.
+	 *  having reselected the initiator.
 	 */
 	case SIR_RESEL_NO_MSG_IN:
-		printf ("%s:%d: No MSG IN phase after reselection.\n",
-			sym_name (np), target);
+		scmd_printk(KERN_WARNING, cp->cmd,
+				"No MSG IN phase after reselection\n");
 		goto out_stuck;
 	/*
 	 *  After reselection, the device sent a message that wasn't 
 	 *  an IDENTIFY.
 	 */
 	case SIR_RESEL_NO_IDENTIFY:
-		printf ("%s:%d: No IDENTIFY after reselection.\n",
-			sym_name (np), target);
+		scmd_printk(KERN_WARNING, cp->cmd,
+				"No IDENTIFY after reselection\n");
 		goto out_stuck;
 	/*
-	 *  The device reselected a LUN we donnot know about.
+	 *  The device reselected a LUN we do not know about.
 	 */
 	case SIR_RESEL_BAD_LUN:
 		np->msgout[0] = M_RESET;
@@ -4390,8 +4390,7 @@ static void sym_int_sir (struct sym_hcb *np)
 		np->msgout[0] = M_ABORT;
 		goto out;
 	/*
-	 *  The device reselected for a tagged nexus that we donnot 
-	 *  have.
+	 * The device reselected for a tagged nexus that we do not have.
 	 */
 	case SIR_RESEL_BAD_I_T_L_Q:
 		np->msgout[0] = M_ABORT_TAG;
@@ -4403,8 +4402,8 @@ static void sym_int_sir (struct sym_hcb *np)
 	case SIR_RESEL_ABORTED:
 		np->lastmsg = np->msgout[0];
 		np->msgout[0] = M_NOOP;
-		printf ("%s:%d: message %x sent on bad reselection.\n",
-			sym_name (np), target, np->lastmsg);
+		scmd_printk(KERN_WARNING, cp->cmd,
+			"message %x sent on bad reselection\n", np->lastmsg);
 		goto out;
 	/*
 	 *  The SCRIPTS let us know that a message has been 
-- 
1.4.4.2

Re: [PATCH 13/17] sym53c8xx: Use scmd_printk where appropriate

[Jeff Garzik]

Matthew Wilcox wrote:
> If we have a scsi_cmnd, it gives the user more information than the
> sym_name, and maybe the target.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  drivers/scsi/sym53c8xx_2/sym_glue.c |    4 ++--
>  drivers/scsi/sym53c8xx_2/sym_hipd.c |   35 +++++++++++++++++------------------
>  2 files changed, 19 insertions(+), 20 deletions(-)

ACK patches 12-13

[PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

[Matthew Wilcox]

These macros aren't needed any more.  They used to be used for SPARC.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index e731618..a82ed13 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -54,9 +54,6 @@
 #define NAME53C		"sym53c"
 #define NAME53C8XX	"sym53c8xx"
 
-#define IRQ_FMT "%d"
-#define IRQ_PRM(x) (x)
-
 struct sym_driver_setup sym_driver_setup = SYM_LINUX_DRIVER_SETUP;
 unsigned int sym_debug_flags = 0;
 
@@ -1196,8 +1193,8 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
 	copy_info(&info, "Chip " NAME53C "%s, device id 0x%x, "
 			 "revision id 0x%x\n", np->s.chip_name,
 			 np->s.device->device, np->s.device->revision);
-	copy_info(&info, "At PCI address %s, IRQ " IRQ_FMT "\n",
-		pci_name(np->s.device), IRQ_PRM(np->s.device->irq));
+	copy_info(&info, "At PCI address %s, IRQ %d\n",
+			 pci_name(np->s.device), np->s.device->irq);
 	copy_info(&info, "Min. period factor %d, %s SCSI BUS%s\n",
 			 (int) (np->minsync_dt ? np->minsync_dt : np->minsync),
 			 np->maxwide ? "Wide" : "Narrow",
@@ -1283,9 +1280,9 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	unsigned long flags;
 	struct sym_fw *fw;
 
-	printk(KERN_INFO "sym%d: <%s> rev 0x%x at pci %s irq " IRQ_FMT "\n",
+	printk(KERN_INFO "sym%d: <%s> rev 0x%x at pci %s irq %d\n",
 		unit, dev->chip.name, pdev->revision, pci_name(pdev),
-		IRQ_PRM(pdev->irq));
+		pdev->irq);
 
 	/*
 	 *  Get the firmware for this chip.
-- 
1.4.4.2

Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

[Jeff Garzik]

Matthew Wilcox wrote:
> @@ -1196,8 +1193,8 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
>  	copy_info(&info, "Chip " NAME53C "%s, device id 0x%x, "
>  			 "revision id 0x%x\n", np->s.chip_name,
>  			 np->s.device->device, np->s.device->revision);
> -	copy_info(&info, "At PCI address %s, IRQ " IRQ_FMT "\n",
> -		pci_name(np->s.device), IRQ_PRM(np->s.device->irq));
> +	copy_info(&info, "At PCI address %s, IRQ %d\n",
> +			 pci_name(np->s.device), np->s.device->irq);
>  	copy_info(&info, "Min. period factor %d, %s SCSI BUS%s\n",
>  			 (int) (np->minsync_dt ? np->minsync_dt : np->minsync),
>  			 np->maxwide ? "Wide" : "Narrow",
> @@ -1283,9 +1280,9 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
>  	unsigned long flags;
>  	struct sym_fw *fw;
>  
> -	printk(KERN_INFO "sym%d: <%s> rev 0x%x at pci %s irq " IRQ_FMT "\n",
> +	printk(KERN_INFO "sym%d: <%s> rev 0x%x at pci %s irq %d\n",
>  		unit, dev->chip.name, pdev->revision, pci_name(pdev),
> -		IRQ_PRM(pdev->irq));
> +		pdev->irq);
>  
>  	/*
>  	 *  Get the firmware for this chip.

NAK

The proper format string is '%u' because irq is unsigned

Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

[Matthew Wilcox]

On Fri, Oct 05, 2007 at 04:59:47PM -0400, Jeff Garzik wrote:
> NAK
> 
> The proper format string is '%u' because irq is unsigned

I'll change it to %u in the next revision.  However, I respectfully
decline your NAK as this was not a regression.

-- 
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: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

[Jeff Garzik]

Matthew Wilcox wrote:
> On Fri, Oct 05, 2007 at 04:59:47PM -0400, Jeff Garzik wrote:
>> NAK
>>
>> The proper format string is '%u' because irq is unsigned
> 
> I'll change it to %u in the next revision.  However, I respectfully
> decline your NAK as this was not a regression.

Not true.  The original code used 'int irq', and the printf format was 
correct as a result.

You changed the code to use 'unsigned int irq' as found in struct 
pci_dev, without changing IRQ_FMT as needed.

If you wanted to avoid the regression another way, I suppose patch #1 
should update IRQ_FMT to use "%u", and patch #14 should move the "%u" 
use to its caller.

	Jeff

Re: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

[Matthew Wilcox]

On Tue, Oct 09, 2007 at 11:35:14AM -0400, Jeff Garzik wrote:
> Not true.  The original code used 'int irq', and the printf format was 
> correct as a result.
> 
> You changed the code to use 'unsigned int irq' as found in struct 
> pci_dev, without changing IRQ_FMT as needed.
> 
> If you wanted to avoid the regression another way, I suppose patch #1 
> should update IRQ_FMT to use "%u", and patch #14 should move the "%u" 
> use to its caller.

What do you think the regression is, exactly?

The *only* difference in how vsnprintf handles %d vs %u is the sign bit.

-- 
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: [PATCH 14/17] sym53c8xx: Get rid of IRQ_FMT and IRQ_PRM

[Jeff Garzik]

Matthew Wilcox wrote:
> On Tue, Oct 09, 2007 at 11:35:14AM -0400, Jeff Garzik wrote:
>> Not true.  The original code used 'int irq', and the printf format was 
>> correct as a result.
>>
>> You changed the code to use 'unsigned int irq' as found in struct 
>> pci_dev, without changing IRQ_FMT as needed.
>>
>> If you wanted to avoid the regression another way, I suppose patch #1 
>> should update IRQ_FMT to use "%u", and patch #14 should move the "%u" 
>> use to its caller.
> 
> What do you think the regression is, exactly?
> 
> The *only* difference in how vsnprintf handles %d vs %u is the sign bit.

Because when you change the type, you should change the printf format to 
match?

It used to match, before your patchset.  Now it doesn't.

	Jeff

[PATCH 15/17] sym53c8xx: Make interrupt handler capable of returning IRQ_NONE

[Matthew Wilcox]

Make sym_interrupt return an irqreturn_t instead of void, and take a
Scsi_Host instead of a sym_hcb.  Pass the Scsi_Host to the interrupt
handler instead of the sym_hcb.  Rename the host_data to sym_data.
Keep a pci_dev pointer in the sym_data.  Rename the Scsi_Host from
instance to shost.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |  107 +++++++++++++++++------------------
 drivers/scsi/sym53c8xx_2/sym_glue.h |    6 +-
 drivers/scsi/sym53c8xx_2/sym_hipd.c |   16 +++--
 drivers/scsi/sym53c8xx_2/sym_hipd.h |    2 +-
 4 files changed, 66 insertions(+), 65 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index a82ed13..e515485 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -39,7 +39,6 @@
  */
 #include <linux/ctype.h>
 #include <linux/init.h>
-#include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/spinlock.h>
@@ -549,21 +548,23 @@ static int sym53c8xx_queue_command(struct scsi_cmnd *cmd,
  */
 static irqreturn_t sym53c8xx_intr(int irq, void *dev_id)
 {
-	struct sym_hcb *np = dev_id;
+	struct Scsi_Host *shost = dev_id;
+	struct sym_data *sym_data = shost_priv(shost);
+	irqreturn_t result;
 
 	/* Avoid spinloop trying to handle interrupts on frozen device */
-	if (pci_channel_offline(np->s.device))
+	if (pci_channel_offline(sym_data->pdev))
 		return IRQ_NONE;
 
 	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("[");
 
-	spin_lock(np->s.host->host_lock);
-	sym_interrupt(np);
-	spin_unlock(np->s.host->host_lock);
+	spin_lock(shost->host_lock);
+	result = sym_interrupt(shost);
+	spin_unlock(shost->host_lock);
 
 	if (DEBUG_FLAGS & DEBUG_TINY) printf_debug ("]\n");
 
-	return IRQ_HANDLED;
+	return result;
 }
 
 /*
@@ -613,22 +614,19 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	 */
 #define WAIT_FOR_PCI_RECOVERY	35
 	if (pci_channel_offline(pdev)) {
-		struct host_data *hostdata = shost_priv(host);
+		struct sym_data *sym_data = shost_priv(host);
 		struct completion *io_reset;
 		int finished_reset = 0;
 		init_completion(&eh_done);
 		spin_lock_irq(host->host_lock);
 		/* Make sure we didn't race */
 		if (pci_channel_offline(pdev)) {
-			if (!hostdata->io_reset)
-				hostdata->io_reset = &eh_done;
-			io_reset = hostdata->io_reset;
+			if (!sym_data->io_reset)
+				sym_data->io_reset = &eh_done;
+			io_reset = sym_data->io_reset;
 		} else {
-			io_reset = NULL;
-		}
-
-		if (!pci_channel_offline(pdev))
 			finished_reset = 1;
+		}
 		spin_unlock_irq(host->host_lock);
 		if (!finished_reset)
 			finished_reset = wait_for_completion_timeout(io_reset,
@@ -1273,9 +1271,9 @@ static void sym_free_resources(struct sym_hcb *np, struct pci_dev *pdev)
 static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 		int unit, struct sym_device *dev)
 {
-	struct host_data *host_data;
+	struct sym_data *sym_data;
 	struct sym_hcb *np = NULL;
-	struct Scsi_Host *instance = NULL;
+	struct Scsi_Host *shost;
 	struct pci_dev *pdev = dev->pdev;
 	unsigned long flags;
 	struct sym_fw *fw;
@@ -1289,15 +1287,12 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	 */
 	fw = sym_find_firmware(&dev->chip);
 	if (!fw)
-		goto attach_failed;
+		return NULL;
 
-	/*
-	 *	Allocate host_data structure
-	 */
-	instance = scsi_host_alloc(tpnt, sizeof(*host_data));
-	if (!instance)
-		goto attach_failed;
-	host_data = (struct host_data *) instance->hostdata;
+	shost = scsi_host_alloc(tpnt, sizeof(*sym_data));
+	if (!shost)
+		return NULL;
+	sym_data = shost_priv(shost);
 
 	/*
 	 *  Allocate immediately the host control block, 
@@ -1310,8 +1305,9 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 		goto attach_failed;
 	np->s.device = pdev;
 	np->bus_dmat = &pdev->dev; /* Result in 1 DMA pool per HBA */
-	host_data->ncb = np;
-	np->s.host = instance;
+	sym_data->ncb = np;
+	sym_data->pdev = pdev;
+	np->s.host = shost;
 
 	pci_set_drvdata(pdev, np);
 
@@ -1358,7 +1354,7 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	if (dev->ram_base)
 		np->ram_ba = (u32)dev->ram_base;
 
-	if (sym_hcb_attach(instance, fw, dev->nvram))
+	if (sym_hcb_attach(shost, fw, dev->nvram))
 		goto attach_failed;
 
 	/*
@@ -1366,7 +1362,8 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	 *  If we synchonize the C code with SCRIPTS on interrupt, 
 	 *  we do not want to share the INTR line at all.
 	 */
-	if (request_irq(pdev->irq, sym53c8xx_intr, IRQF_SHARED, NAME53C8XX, np)) {
+	if (request_irq(pdev->irq, sym53c8xx_intr, IRQF_SHARED, NAME53C8XX,
+								shost)) {
 		printf_err("%s: request irq %d failure\n",
 			sym_name(np), pdev->irq);
 		goto attach_failed;
@@ -1376,7 +1373,7 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	 *  After SCSI devices have been opened, we cannot
 	 *  reset the bus safely, so we do it here.
 	 */
-	spin_lock_irqsave(instance->host_lock, flags);
+	spin_lock_irqsave(shost->host_lock, flags);
 	if (sym_reset_scsi_bus(np, 0))
 		goto reset_failed;
 
@@ -1398,37 +1395,37 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	 *  Fill Linux host instance structure
 	 *  and return success.
 	 */
-	instance->max_channel	= 0;
-	instance->this_id	= np->myaddr;
-	instance->max_id	= np->maxwide ? 16 : 8;
-	instance->max_lun	= SYM_CONF_MAX_LUN;
-	instance->unique_id	= pci_resource_start(pdev, 0);
-	instance->cmd_per_lun	= SYM_CONF_MAX_TAG;
-	instance->can_queue	= (SYM_CONF_MAX_START-2);
-	instance->sg_tablesize	= SYM_CONF_MAX_SG;
-	instance->max_cmd_len	= 16;
+	shost->max_channel	= 0;
+	shost->this_id		= np->myaddr;
+	shost->max_id		= np->maxwide ? 16 : 8;
+	shost->max_lun		= SYM_CONF_MAX_LUN;
+	shost->unique_id	= pci_resource_start(pdev, 0);
+	shost->cmd_per_lun	= SYM_CONF_MAX_TAG;
+	shost->can_queue	= (SYM_CONF_MAX_START-2);
+	shost->sg_tablesize	= SYM_CONF_MAX_SG;
+	shost->max_cmd_len	= 16;
 	BUG_ON(sym2_transport_template == NULL);
-	instance->transportt	= sym2_transport_template;
+	shost->transportt	= sym2_transport_template;
 
 	/* 53c896 rev 1 errata: DMA may not cross 16MB boundary */
 	if (pdev->device == PCI_DEVICE_ID_NCR_53C896 && pdev->revision < 2)
-		instance->dma_boundary = 0xFFFFFF;
+		shost->dma_boundary = 0xFFFFFF;
 
-	spin_unlock_irqrestore(instance->host_lock, flags);
+	spin_unlock_irqrestore(shost->host_lock, flags);
 
-	return instance;
+	return shost;
 
  reset_failed:
 	printf_err("%s: FATAL ERROR: CHECK SCSI BUS - CABLES, "
 		   "TERMINATION, DEVICE POWER etc.!\n", sym_name(np));
-	spin_unlock_irqrestore(instance->host_lock, flags);
+	spin_unlock_irqrestore(shost->host_lock, flags);
  attach_failed:
-	if (!instance)
+	if (!shost)
 		return NULL;
 	printf_info("%s: giving up ...\n", sym_name(np));
 	if (np)
 		sym_free_resources(np, pdev);
-	scsi_host_put(instance);
+	scsi_host_put(shost);
 
 	return NULL;
  }
@@ -1701,7 +1698,7 @@ static int __devinit sym2_probe(struct pci_dev *pdev,
 {
 	struct sym_device sym_dev;
 	struct sym_nvram nvram;
-	struct Scsi_Host *instance;
+	struct Scsi_Host *shost;
 
 	memset(&sym_dev, 0, sizeof(sym_dev));
 	memset(&nvram, 0, sizeof(nvram));
@@ -1728,13 +1725,13 @@ static int __devinit sym2_probe(struct pci_dev *pdev,
 
 	sym_get_nvram(&sym_dev, &nvram);
 
-	instance = sym_attach(&sym2_template, attach_count, &sym_dev);
-	if (!instance)
+	shost = sym_attach(&sym2_template, attach_count, &sym_dev);
+	if (!shost)
 		goto free;
 
-	if (scsi_add_host(instance, &pdev->dev))
+	if (scsi_add_host(shost, &pdev->dev))
 		goto detach;
-	scsi_scan_host(instance);
+	scsi_scan_host(shost);
 
 	attach_count++;
 
@@ -1883,12 +1880,12 @@ static void sym2_io_resume(struct pci_dev *pdev)
 {
 	struct sym_hcb *np = pci_get_drvdata(pdev);
 	struct Scsi_Host *shost = np->s.host;
-	struct host_data *hostdata = shost_priv(shost);
+	struct sym_data *sym_data = shost_priv(shost);
 
 	spin_lock_irq(shost->host_lock);
-	if (hostdata->io_reset)
-		complete_all(hostdata->io_reset);
-	hostdata->io_reset = NULL;
+	if (sym_data->io_reset)
+		complete_all(sym_data->io_reset);
+	sym_data->io_reset = NULL;
 	spin_unlock_irq(shost->host_lock);
 }
 
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index ab2de1c..98261a5 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -42,6 +42,7 @@
 
 #include <linux/completion.h>
 #include <linux/delay.h>
+#include <linux/interrupt.h>
 #include <linux/ioport.h>
 #include <linux/pci.h>
 #include <linux/string.h>
@@ -217,14 +218,15 @@ struct sym_device {
 /*
  *  Driver host data structure.
  */
-struct host_data {
+struct sym_data {
 	struct sym_hcb *ncb;
 	struct completion *io_reset;		/* PCI error handling */
+	struct pci_dev *pdev;
 };
 
 static inline struct sym_hcb * sym_get_hcb(struct Scsi_Host *host)
 {
-	return ((struct host_data *)host->hostdata)->ncb;
+	return ((struct sym_data *)host->hostdata)->ncb;
 }
 
 #include "sym_fw.h"
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 13fd5b2..5d2079f 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -2760,8 +2760,9 @@ reset_all:
  *  Use at your own decision and risk.
  */
 
-void sym_interrupt (struct sym_hcb *np)
+irqreturn_t sym_interrupt(struct Scsi_Host *shost)
 {
+	struct sym_hcb *np = sym_get_hcb(shost);
 	u_char	istat, istatc;
 	u_char	dstat;
 	u_short	sist;
@@ -2786,7 +2787,7 @@ void sym_interrupt (struct sym_hcb *np)
 	}
 
 	if (!(istat & (SIP|DIP)))
-		return;
+		return (istat & INTF) ? IRQ_HANDLED : IRQ_NONE;
 
 #if 0	/* We should never get this one */
 	if (istat & CABRT)
@@ -2818,7 +2819,7 @@ void sym_interrupt (struct sym_hcb *np)
 		 * never clear. */
 		if (unlikely(sist == 0xffff && dstat == 0xff)) {
 			if (pci_channel_offline(np->s.device))
-				return;
+				return IRQ_NONE;
 		}
 	} while (istatc & (SIP|DIP));
 
@@ -2856,7 +2857,7 @@ void sym_interrupt (struct sym_hcb *np)
 		else if (dstat & SIR)	sym_int_sir(np);
 		else if (dstat & SSI)	OUTONB_STD();
 		else			goto unknown_int;
-		return;
+		return IRQ_HANDLED;
 	}
 
 	/*
@@ -2873,7 +2874,7 @@ void sym_interrupt (struct sym_hcb *np)
 	if (sist & RST) {
 		printf("%s: SCSI BUS reset detected.\n", sym_name(np));
 		sym_start_up (np, 1);
-		return;
+		return IRQ_HANDLED;
 	}
 
 	OUTB(np, nc_ctest3, np->rv_ctest3 | CLF);	/* clear dma fifo  */
@@ -2885,7 +2886,7 @@ void sym_interrupt (struct sym_hcb *np)
 		else if (sist & STO)	sym_int_sto (np);
 		else if (sist & UDC)	sym_int_udc (np);
 		else			goto unknown_int;
-		return;
+		return IRQ_HANDLED;
 	}
 
 	/*
@@ -2900,7 +2901,7 @@ void sym_interrupt (struct sym_hcb *np)
 	if ((sist & (GEN|HTH|SGE)) ||
 		(dstat & (MDPE|BF|ABRT|IID))) {
 		sym_start_reset(np);
-		return;
+		return IRQ_HANDLED;
 	}
 
 unknown_int:
@@ -2911,6 +2912,7 @@ unknown_int:
 	printf(	"%s: unknown interrupt(s) ignored, "
 		"ISTAT=0x%x DSTAT=0x%x SIST=0x%x\n",
 		sym_name(np), istat, dstat, sist);
+	return IRQ_NONE;
 }
 
 /*
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h
index 4354571..a9c0866 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
@@ -1056,7 +1056,7 @@ void sym_start_next_ccbs(struct sym_hcb *np, struct sym_lcb *lp, int maxn);
 void sym_put_start_queue(struct sym_hcb *np, struct sym_ccb *cp);
 #endif
 void sym_start_up(struct sym_hcb *np, int reason);
-void sym_interrupt(struct sym_hcb *np);
+irqreturn_t sym_interrupt(struct Scsi_Host *);
 int sym_clear_tasks(struct sym_hcb *np, int cam_status, int target, int lun, int task);
 struct sym_ccb *sym_get_ccb(struct sym_hcb *np, struct scsi_cmnd *cmd, u_char tag_order);
 void sym_free_ccb(struct sym_hcb *np, struct sym_ccb *cp);
-- 
1.4.4.2

Re: [PATCH 15/17] sym53c8xx: Make interrupt handler capable of returning IRQ_NONE

[Jeff Garzik]

Matthew Wilcox wrote:
> Make sym_interrupt return an irqreturn_t instead of void, and take a
> Scsi_Host instead of a sym_hcb.  Pass the Scsi_Host to the interrupt
> handler instead of the sym_hcb.  Rename the host_data to sym_data.
> Keep a pci_dev pointer in the sym_data.  Rename the Scsi_Host from
> instance to shost.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

ACK

Additionally, consider checking for istat==0xffffffff and returning 
IRQ_NONE, like most other drivers.  This handles the window during 
hot-unplug where you wind up reading a hardware register after the 
hardware is gone, but before the driver has been notified of unplug. 
The normal behavior of hardware for this case is to return all 1's upon 
PCI device fault/unplug.

Presuming you don't have fancy-shmancy PCI error reporting hw :)

[PATCH 16/17] sym53c8xx: Remove pci_dev pointer from sym_shcb

[Matthew Wilcox]

This structure is accessed by the device; the fewer Linux things in it,
the better.  Using the pci_dev pointer from the hostdata requires a lot
of changes:

 - Pass Scsi_Host to a lot of routines which currently take a sym_hcb.
 - Set the Scsi_Host as the pci drvdata (instead of the sym_hcb)

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_fw.c   |   16 ++++--
 drivers/scsi/sym53c8xx_2/sym_fw.h   |    2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c |   97 ++++++++++++++++++-----------------
 drivers/scsi/sym53c8xx_2/sym_glue.h |    5 +-
 drivers/scsi/sym53c8xx_2/sym_hipd.c |   60 +++++++++++++---------
 drivers/scsi/sym53c8xx_2/sym_hipd.h |    4 +-
 6 files changed, 99 insertions(+), 85 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_fw.c b/drivers/scsi/sym53c8xx_2/sym_fw.c
index aa230d2..190770b 100644
--- a/drivers/scsi/sym53c8xx_2/sym_fw.c
+++ b/drivers/scsi/sym53c8xx_2/sym_fw.c
@@ -104,8 +104,9 @@ static struct sym_fwz_ofs sym_fw2z_ofs = {
  *  Patch routine for firmware #1.
  */
 static void
-sym_fw1_patch(struct sym_hcb *np)
+sym_fw1_patch(struct Scsi_Host *shost)
 {
+	struct sym_hcb *np = sym_get_hcb(shost);
 	struct sym_fw1a_scr *scripta0;
 	struct sym_fw1b_scr *scriptb0;
 
@@ -145,8 +146,11 @@ sym_fw1_patch(struct sym_hcb *np)
  *  Patch routine for firmware #2.
  */
 static void
-sym_fw2_patch(struct sym_hcb *np)
+sym_fw2_patch(struct Scsi_Host *shost)
 {
+	struct sym_data *sym_data = shost_priv(shost);
+	struct pci_dev *pdev = sym_data->pdev;
+	struct sym_hcb *np = sym_data->ncb;
 	struct sym_fw2a_scr *scripta0;
 	struct sym_fw2b_scr *scriptb0;
 
@@ -205,14 +209,14 @@ sym_fw2_patch(struct sym_hcb *np)
 	 *  Remove a couple of work-arounds specific to C1010 if 
 	 *  they are not desirable. See `sym_fw2.h' for more details.
 	 */
-	if (!(np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_66 &&
-	      np->s.device->revision < 0x1 &&
+	if (!(pdev->device == PCI_DEVICE_ID_LSI_53C1010_66 &&
+	      pdev->revision < 0x1 &&
 	      np->pciclk_khz < 60000)) {
 		scripta0->datao_phase[0] = cpu_to_scr(SCR_NO_OP);
 		scripta0->datao_phase[1] = cpu_to_scr(0);
 	}
-	if (!(np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_33 /* &&
-	      np->s.device->revision < 0xff */)) {
+	if (!(pdev->device == PCI_DEVICE_ID_LSI_53C1010_33 /* &&
+	      pdev->revision < 0xff */)) {
 		scripta0->sel_done[0] = cpu_to_scr(SCR_NO_OP);
 		scripta0->sel_done[1] = cpu_to_scr(0);
 	}
diff --git a/drivers/scsi/sym53c8xx_2/sym_fw.h b/drivers/scsi/sym53c8xx_2/sym_fw.h
index 66ec35b..ae7e0f9 100644
--- a/drivers/scsi/sym53c8xx_2/sym_fw.h
+++ b/drivers/scsi/sym53c8xx_2/sym_fw.h
@@ -143,7 +143,7 @@ struct sym_fw {
 		*z_ofs;		/* Useful offsets in script Z	*/
 	/* Setup and patch methods for this firmware */
 	void	(*setup)(struct sym_hcb *, struct sym_fw *);
-	void	(*patch)(struct sym_hcb *);
+	void	(*patch)(struct Scsi_Host *);
 };
 
 /*
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index e515485..4c7b8a6 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -497,14 +497,16 @@ static void sym_timer(struct sym_hcb *np)
 /*
  *  PCI BUS error handler.
  */
-void sym_log_bus_error(struct sym_hcb *np)
+void sym_log_bus_error(struct Scsi_Host *shost)
 {
-	u_short pci_sts;
-	pci_read_config_word(np->s.device, PCI_STATUS, &pci_sts);
+	struct sym_data *sym_data = shost_priv(shost);
+	struct pci_dev *pdev = sym_data->pdev;
+	unsigned short pci_sts;
+	pci_read_config_word(pdev, PCI_STATUS, &pci_sts);
 	if (pci_sts & 0xf900) {
-		pci_write_config_word(np->s.device, PCI_STATUS, pci_sts);
-		printf("%s: PCI STATUS = 0x%04x\n",
-			sym_name(np), pci_sts & 0xf900);
+		pci_write_config_word(pdev, PCI_STATUS, pci_sts);
+		shost_printk(KERN_WARNING, shost,
+			"PCI bus error: status = 0x%04x\n", pci_sts & 0xf900);
 	}
 }
 
@@ -595,16 +597,17 @@ static void sym53c8xx_timer(unsigned long npref)
  */
 static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 {
-	struct sym_hcb *np = SYM_SOFTC_PTR(cmd);
 	struct sym_ucmd *ucmd = SYM_UCMD_PTR(cmd);
-	struct Scsi_Host *host = cmd->device->host;
-	struct pci_dev *pdev = np->s.device;
+	struct Scsi_Host *shost = cmd->device->host;
+	struct sym_data *sym_data = shost_priv(shost);
+	struct pci_dev *pdev = sym_data->pdev;
+	struct sym_hcb *np = sym_data->ncb;
 	SYM_QUEHEAD *qp;
 	int cmd_queued = 0;
 	int sts = -1;
 	struct completion eh_done;
 
-	scmd_printk(KERN_WARNING, cmd, "%s operation started.\n", opname);
+	scmd_printk(KERN_WARNING, cmd, "%s operation started\n", opname);
 
 	/* We may be in an error condition because the PCI bus
 	 * went down. In this case, we need to wait until the
@@ -614,11 +617,10 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	 */
 #define WAIT_FOR_PCI_RECOVERY	35
 	if (pci_channel_offline(pdev)) {
-		struct sym_data *sym_data = shost_priv(host);
 		struct completion *io_reset;
 		int finished_reset = 0;
 		init_completion(&eh_done);
-		spin_lock_irq(host->host_lock);
+		spin_lock_irq(shost->host_lock);
 		/* Make sure we didn't race */
 		if (pci_channel_offline(pdev)) {
 			if (!sym_data->io_reset)
@@ -627,7 +629,7 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 		} else {
 			finished_reset = 1;
 		}
-		spin_unlock_irq(host->host_lock);
+		spin_unlock_irq(shost->host_lock);
 		if (!finished_reset)
 			finished_reset = wait_for_completion_timeout(io_reset,
 						WAIT_FOR_PCI_RECOVERY*HZ);
@@ -635,7 +637,7 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 			return SCSI_FAILED;
 	}
 
-	spin_lock_irq(host->host_lock);
+	spin_lock_irq(shost->host_lock);
 	/* This one is queued in some place -> to wait for completion */
 	FOR_EACH_QUEUED_ELEMENT(&np->busy_ccbq, qp) {
 		struct sym_ccb *cp = sym_que_entry(qp, struct sym_ccb, link_ccbq);
@@ -660,7 +662,7 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 		break;
 	case SYM_EH_HOST_RESET:
 		sym_reset_scsi_bus(np, 0);
-		sym_start_up(np, 1);
+		sym_start_up(shost, 1);
 		sts = 0;
 		break;
 	default:
@@ -674,13 +676,13 @@ static int sym_eh_handler(int op, char *opname, struct scsi_cmnd *cmd)
 	if (cmd_queued) {
 		init_completion(&eh_done);
 		ucmd->eh_done = &eh_done;
-		spin_unlock_irq(host->host_lock);
+		spin_unlock_irq(shost->host_lock);
 		if (!wait_for_completion_timeout(&eh_done, 5*HZ)) {
 			ucmd->eh_done = NULL;
 			sts = -2;
 		}
 	} else {
-		spin_unlock_irq(host->host_lock);
+		spin_unlock_irq(shost->host_lock);
 	}
 
 	dev_warn(&cmd->device->sdev_gendev, "%s operation %s.\n", opname,
@@ -993,8 +995,9 @@ static int is_keyword(char *ptr, int len, char *verb)
  * Parse a control command
  */
 
-static int sym_user_command(struct sym_hcb *np, char *buffer, int length)
+static int sym_user_command(struct Scsi_Host *shost, char *buffer, int length)
 {
+	struct sym_hcb *np = sym_get_hcb(shost);
 	char *ptr	= buffer;
 	int len		= length;
 	struct sym_usrcmd cmd, *uc = &cmd;
@@ -1121,9 +1124,9 @@ printk("sym_user_command: data=%ld\n", uc->data);
 	else {
 		unsigned long flags;
 
-		spin_lock_irqsave(np->s.host->host_lock, flags);
-		sym_exec_user_command (np, uc);
-		spin_unlock_irqrestore(np->s.host->host_lock, flags);
+		spin_lock_irqsave(shost->host_lock, flags);
+		sym_exec_user_command(np, uc);
+		spin_unlock_irqrestore(shost->host_lock, flags);
 	}
 	return length;
 }
@@ -1179,8 +1182,11 @@ static int copy_info(struct info_str *info, char *fmt, ...)
 /*
  *  Copy formatted information into the input buffer.
  */
-static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
+static int sym_host_info(struct Scsi_Host *shost, char *ptr, off_t offset, int len)
 {
+	struct sym_data *sym_data = shost_priv(shost);
+	struct pci_dev *pdev = sym_data->pdev;
+	struct sym_hcb *np = sym_data->ncb;
 	struct info_str info;
 
 	info.buffer	= ptr;
@@ -1190,9 +1196,9 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
 
 	copy_info(&info, "Chip " NAME53C "%s, device id 0x%x, "
 			 "revision id 0x%x\n", np->s.chip_name,
-			 np->s.device->device, np->s.device->revision);
+			 pdev->device, pdev->revision);
 	copy_info(&info, "At PCI address %s, IRQ %d\n",
-			 pci_name(np->s.device), np->s.device->irq);
+			 pci_name(pdev), pdev->irq);
 	copy_info(&info, "Min. period factor %d, %s SCSI BUS%s\n",
 			 (int) (np->minsync_dt ? np->minsync_dt : np->minsync),
 			 np->maxwide ? "Wide" : "Narrow",
@@ -1211,15 +1217,14 @@ static int sym_host_info(struct sym_hcb *np, char *ptr, off_t offset, int len)
  *  - func = 0 means read  (returns adapter infos)
  *  - func = 1 means write (not yet merget from sym53c8xx)
  */
-static int sym53c8xx_proc_info(struct Scsi_Host *host, char *buffer,
+static int sym53c8xx_proc_info(struct Scsi_Host *shost, char *buffer,
 			char **start, off_t offset, int length, int func)
 {
-	struct sym_hcb *np = sym_get_hcb(host);
 	int retv;
 
 	if (func) {
 #ifdef	SYM_LINUX_USER_COMMAND_SUPPORT
-		retv = sym_user_command(np, buffer, length);
+		retv = sym_user_command(shost, buffer, length);
 #else
 		retv = -EINVAL;
 #endif
@@ -1227,7 +1232,7 @@ static int sym53c8xx_proc_info(struct Scsi_Host *host, char *buffer,
 		if (start)
 			*start = buffer;
 #ifdef SYM_LINUX_USER_INFO_SUPPORT
-		retv = sym_host_info(np, buffer, offset, length);
+		retv = sym_host_info(shost, buffer, offset, length);
 #else
 		retv = -EINVAL;
 #endif
@@ -1303,20 +1308,18 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	np = __sym_calloc_dma(&pdev->dev, sizeof(*np), "HCB");
 	if (!np)
 		goto attach_failed;
-	np->s.device = pdev;
 	np->bus_dmat = &pdev->dev; /* Result in 1 DMA pool per HBA */
 	sym_data->ncb = np;
 	sym_data->pdev = pdev;
 	np->s.host = shost;
 
-	pci_set_drvdata(pdev, np);
+	pci_set_drvdata(pdev, shost);
 
 	/*
 	 *  Copy some useful infos to the HCB.
 	 */
 	np->hcb_ba	= vtobus(np);
 	np->verbose	= sym_driver_setup.verbose;
-	np->s.device	= pdev;
 	np->s.unit	= unit;
 	np->features	= dev->chip.features;
 	np->clock_divn	= dev->chip.nr_divisor;
@@ -1331,9 +1334,9 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	sprintf(np->s.inst_name, "sym%d", np->s.unit);
 
 	if ((SYM_CONF_DMA_ADDRESSING_MODE > 0) && (np->features & FE_DAC) &&
-			!pci_set_dma_mask(np->s.device, DMA_DAC_MASK)) {
+			!pci_set_dma_mask(pdev, DMA_DAC_MASK)) {
 		set_dac(np);
-	} else if (pci_set_dma_mask(np->s.device, DMA_32BIT_MASK)) {
+	} else if (pci_set_dma_mask(pdev, DMA_32BIT_MASK)) {
 		printf_warning("%s: No suitable DMA available\n", sym_name(np));
 		goto attach_failed;
 	}
@@ -1380,7 +1383,7 @@ static struct Scsi_Host * __devinit sym_attach(struct scsi_host_template *tpnt,
 	/*
 	 *  Start the SCRIPTS.
 	 */
-	sym_start_up(np, 1);
+	sym_start_up(shost, 1);
 
 	/*
 	 *  Start the timer daemon
@@ -1645,8 +1648,9 @@ static void sym_config_pqs(struct pci_dev *pdev, struct sym_device *sym_dev)
  *  Detach the host.
  *  We have to free resources and halt the NCR chip.
  */
-static int sym_detach(struct sym_hcb *np, struct pci_dev *pdev)
+static int sym_detach(struct Scsi_Host *shost, struct pci_dev *pdev)
 {
+	struct sym_hcb *np = sym_get_hcb(shost);
 	printk("%s: detaching ...\n", sym_name(np));
 
 	del_timer_sync(&np->s.timer);
@@ -1749,14 +1753,11 @@ static int __devinit sym2_probe(struct pci_dev *pdev,
 
 static void __devexit sym2_remove(struct pci_dev *pdev)
 {
-	struct sym_hcb *np = pci_get_drvdata(pdev);
-	struct Scsi_Host *host = np->s.host;
-
-	scsi_remove_host(host);
-	scsi_host_put(host);
-
-	sym_detach(np, pdev);
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
 
+	scsi_remove_host(shost);
+	scsi_host_put(shost);
+	sym_detach(shost, pdev);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 
@@ -1790,9 +1791,9 @@ static pci_ers_result_t sym2_io_error_detected(struct pci_dev *pdev,
  */
 static pci_ers_result_t sym2_io_slot_dump(struct pci_dev *pdev)
 {
-	struct sym_hcb *np = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
 
-	sym_dump_registers(np);
+	sym_dump_registers(shost);
 
 	/* Request a slot reset. */
 	return PCI_ERS_RESULT_NEED_RESET;
@@ -1832,7 +1833,8 @@ static void sym2_reset_workarounds(struct pci_dev *pdev)
  */
 static pci_ers_result_t sym2_io_slot_reset(struct pci_dev *pdev)
 {
-	struct sym_hcb *np = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct sym_hcb *np = sym_get_hcb(shost);
 
 	printk(KERN_INFO "%s: recovering from a PCI slot reset\n",
 	          sym_name(np));
@@ -1862,7 +1864,7 @@ static pci_ers_result_t sym2_io_slot_reset(struct pci_dev *pdev)
 			        sym_name(np));
 			return PCI_ERS_RESULT_DISCONNECT;
 		}
-		sym_start_up(np, 1);
+		sym_start_up(shost, 1);
 	}
 
 	return PCI_ERS_RESULT_RECOVERED;
@@ -1878,8 +1880,7 @@ static pci_ers_result_t sym2_io_slot_reset(struct pci_dev *pdev)
  */
 static void sym2_io_resume(struct pci_dev *pdev)
 {
-	struct sym_hcb *np = pci_get_drvdata(pdev);
-	struct Scsi_Host *shost = np->s.host;
+	struct Scsi_Host *shost = pci_get_drvdata(pdev);
 	struct sym_data *sym_data = shost_priv(shost);
 
 	spin_lock_irq(shost->host_lock);
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index 98261a5..e5bd0af 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -179,7 +179,6 @@ struct sym_shcb {
 	int		unit;
 	char		inst_name[16];
 	char		chip_name[8];
-	struct pci_dev	*device;
 
 	struct Scsi_Host *host;
 
@@ -266,7 +265,7 @@ void sym_xpt_done(struct sym_hcb *np, struct scsi_cmnd *ccb);
 void sym_xpt_async_bus_reset(struct sym_hcb *np);
 void sym_xpt_async_sent_bdr(struct sym_hcb *np, int target);
 int  sym_setup_data_and_start (struct sym_hcb *np, struct scsi_cmnd *csio, struct sym_ccb *cp);
-void sym_log_bus_error(struct sym_hcb *np);
-void sym_dump_registers(struct sym_hcb *np);
+void sym_log_bus_error(struct Scsi_Host *);
+void sym_dump_registers(struct Scsi_Host *);
 
 #endif /* SYM_GLUE_H */
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 5d2079f..3cf1209 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -684,6 +684,8 @@ static void sym_set_bus_mode(struct sym_hcb *np, struct sym_nvram *nvram)
  */
 static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, struct sym_nvram *nvram)
 {
+	struct sym_data *sym_data = shost_priv(shost);
+	struct pci_dev *pdev = sym_data->pdev;
 	u_char	burst_max;
 	u32	period;
 	int i;
@@ -797,8 +799,8 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	 *  In dual channel mode, contention occurs if internal cycles
 	 *  are used. Disable internal cycles.
 	 */
-	if (np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_33 &&
-	    np->s.device->revision < 0x1)
+	if (pdev->device == PCI_DEVICE_ID_LSI_53C1010_33 &&
+	    pdev->revision < 0x1)
 		np->rv_ccntl0	|=  DILS;
 
 	/*
@@ -821,10 +823,10 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	 *  this driver. The generic ncr driver that does not use 
 	 *  LOAD/STORE instructions does not need this work-around.
 	 */
-	if ((np->s.device->device == PCI_DEVICE_ID_NCR_53C810 &&
-	     np->s.device->revision >= 0x10 && np->s.device->revision <= 0x11) ||
-	    (np->s.device->device == PCI_DEVICE_ID_NCR_53C860 &&
-	     np->s.device->revision <= 0x1))
+	if ((pdev->device == PCI_DEVICE_ID_NCR_53C810 &&
+	     pdev->revision >= 0x10 && pdev->revision <= 0x11) ||
+	    (pdev->device == PCI_DEVICE_ID_NCR_53C860 &&
+	     pdev->revision <= 0x1))
 		np->features &= ~(FE_WRIE|FE_ERL|FE_ERMP);
 
 	/*
@@ -890,7 +892,7 @@ static int sym_prepare_setting(struct Scsi_Host *shost, struct sym_hcb *np, stru
 	if ((SYM_SETUP_SCSI_LED || 
 	     (nvram->type == SYM_SYMBIOS_NVRAM ||
 	      (nvram->type == SYM_TEKRAM_NVRAM &&
-	       np->s.device->device == PCI_DEVICE_ID_NCR_53C895))) &&
+	       pdev->device == PCI_DEVICE_ID_NCR_53C895))) &&
 	    !(np->features & FE_LEDC) && !(np->sv_gpcntl & 0x01))
 		np->features |= FE_LED0;
 
@@ -1128,8 +1130,9 @@ restart_test:
  *  First 24 register of the chip:
  *  	r0..rf
  */
-static void sym_log_hard_error(struct sym_hcb *np, u_short sist, u_char dstat)
+static void sym_log_hard_error(struct Scsi_Host *shost, u_short sist, u_char dstat)
 {
+	struct sym_hcb *np = sym_get_hcb(shost);
 	u32	dsp;
 	int	script_ofs;
 	int	script_size;
@@ -1182,17 +1185,18 @@ static void sym_log_hard_error(struct sym_hcb *np, u_short sist, u_char dstat)
 	 *  PCI BUS error.
 	 */
 	if (dstat & (MDPE|BF))
-		sym_log_bus_error(np);
+		sym_log_bus_error(shost);
 }
 
-void sym_dump_registers(struct sym_hcb *np)
+void sym_dump_registers(struct Scsi_Host *shost)
 {
+	struct sym_hcb *np = sym_get_hcb(shost);
 	u_short sist;
 	u_char dstat;
 
 	sist = INW(np, nc_sist);
 	dstat = INB(np, nc_dstat);
-	sym_log_hard_error(np, sist, dstat);
+	sym_log_hard_error(shost, sist, dstat);
 }
 
 static struct sym_chip sym_dev_table[] = {
@@ -1700,8 +1704,11 @@ static void sym_flush_busy_queue (struct sym_hcb *np, int cam_status)
  *     1: SCSI BUS RESET delivered or received.
  *     2: SCSI BUS MODE changed.
  */
-void sym_start_up (struct sym_hcb *np, int reason)
+void sym_start_up(struct Scsi_Host *shost, int reason)
 {
+	struct sym_data *sym_data = shost_priv(shost);
+	struct pci_dev *pdev = sym_data->pdev;
+	struct sym_hcb *np = sym_data->ncb;
  	int	i;
 	u32	phys;
 
@@ -1750,7 +1757,7 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	 *  This also let point to first position the start 
 	 *  and done queue pointers used from SCRIPTS.
 	 */
-	np->fw_patch(np);
+	np->fw_patch(shost);
 
 	/*
 	 *  Wakeup all pending jobs.
@@ -1792,7 +1799,7 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	/*
 	 *  For now, disable AIP generation on C1010-66.
 	 */
-	if (np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_66)
+	if (pdev->device == PCI_DEVICE_ID_LSI_53C1010_66)
 		OUTB(np, nc_aipcntl1, DISAIP);
 
 	/*
@@ -1802,8 +1809,8 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	 *  that from SCRIPTS for each selection/reselection, but 
 	 *  I just don't want. :)
 	 */
-	if (np->s.device->device == PCI_DEVICE_ID_LSI_53C1010_33 &&
-	    np->s.device->revision < 1)
+	if (pdev->device == PCI_DEVICE_ID_LSI_53C1010_33 &&
+	    pdev->revision < 1)
 		OUTB(np, nc_stest1, INB(np, nc_stest1) | 0x30);
 
 	/*
@@ -1811,9 +1818,9 @@ void sym_start_up (struct sym_hcb *np, int reason)
 	 *  Disable overlapped arbitration for some dual function devices, 
 	 *  regardless revision id (kind of post-chip-design feature. ;-))
 	 */
-	if (np->s.device->device == PCI_DEVICE_ID_NCR_53C875)
+	if (pdev->device == PCI_DEVICE_ID_NCR_53C875)
 		OUTB(np, nc_ctest0, (1<<5));
-	else if (np->s.device->device == PCI_DEVICE_ID_NCR_53C896)
+	else if (pdev->device == PCI_DEVICE_ID_NCR_53C896)
 		np->rv_ccntl0 |= DPR;
 
 	/*
@@ -2218,8 +2225,9 @@ static void sym_int_udc (struct sym_hcb *np)
  *  mode to eight bit asynchronous, etc...
  *  So, just reinitializing all except chip should be enough.
  */
-static void sym_int_sbmc (struct sym_hcb *np)
+static void sym_int_sbmc(struct Scsi_Host *shost)
 {
+	struct sym_hcb *np = sym_get_hcb(shost);
 	u_char scsi_mode = INB(np, nc_stest4) & SMODE;
 
 	/*
@@ -2232,7 +2240,7 @@ static void sym_int_sbmc (struct sym_hcb *np)
 	 *  Should suspend command processing for a few seconds and 
 	 *  reinitialize all except the chip.
 	 */
-	sym_start_up (np, 2);
+	sym_start_up(shost, 2);
 }
 
 /*
@@ -2762,7 +2770,9 @@ reset_all:
 
 irqreturn_t sym_interrupt(struct Scsi_Host *shost)
 {
-	struct sym_hcb *np = sym_get_hcb(shost);
+	struct sym_data *sym_data = shost_priv(shost);
+	struct sym_hcb *np = sym_data->ncb;
+	struct pci_dev *pdev = sym_data->pdev;
 	u_char	istat, istatc;
 	u_char	dstat;
 	u_short	sist;
@@ -2818,7 +2828,7 @@ irqreturn_t sym_interrupt(struct Scsi_Host *shost)
 		/* Prevent deadlock waiting on a condition that may
 		 * never clear. */
 		if (unlikely(sist == 0xffff && dstat == 0xff)) {
-			if (pci_channel_offline(np->s.device))
+			if (pci_channel_offline(pdev))
 				return IRQ_NONE;
 		}
 	} while (istatc & (SIP|DIP));
@@ -2873,7 +2883,7 @@ irqreturn_t sym_interrupt(struct Scsi_Host *shost)
 	 */
 	if (sist & RST) {
 		printf("%s: SCSI BUS reset detected.\n", sym_name(np));
-		sym_start_up (np, 1);
+		sym_start_up(shost, 1);
 		return IRQ_HANDLED;
 	}
 
@@ -2882,7 +2892,7 @@ irqreturn_t sym_interrupt(struct Scsi_Host *shost)
 
 	if (!(sist  & (GEN|HTH|SGE)) &&
 	    !(dstat & (MDPE|BF|ABRT|IID))) {
-		if	(sist & SBMC)	sym_int_sbmc (np);
+		if	(sist & SBMC)	sym_int_sbmc(shost);
 		else if (sist & STO)	sym_int_sto (np);
 		else if (sist & UDC)	sym_int_udc (np);
 		else			goto unknown_int;
@@ -2896,7 +2906,7 @@ irqreturn_t sym_interrupt(struct Scsi_Host *shost)
 	 *  Reset everything.
 	 */
 
-	sym_log_hard_error(np, sist, dstat);
+	sym_log_hard_error(shost, sist, dstat);
 
 	if ((sist & (GEN|HTH|SGE)) ||
 		(dstat & (MDPE|BF|ABRT|IID))) {
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h
index a9c0866..ad07880 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
@@ -909,7 +909,7 @@ struct sym_hcb {
 	struct sym_fwb_ba fwb_bas;	/* Useful SCRIPTB bus addresses	*/
 	struct sym_fwz_ba fwz_bas;	/* Useful SCRIPTZ bus addresses	*/
 	void		(*fw_setup)(struct sym_hcb *np, struct sym_fw *fw);
-	void		(*fw_patch)(struct sym_hcb *np);
+	void		(*fw_patch)(struct Scsi_Host *);
 	char		*fw_name;
 
 	/*
@@ -1055,7 +1055,7 @@ void sym_start_next_ccbs(struct sym_hcb *np, struct sym_lcb *lp, int maxn);
 #else
 void sym_put_start_queue(struct sym_hcb *np, struct sym_ccb *cp);
 #endif
-void sym_start_up(struct sym_hcb *np, int reason);
+void sym_start_up(struct Scsi_Host *, int reason);
 irqreturn_t sym_interrupt(struct Scsi_Host *);
 int sym_clear_tasks(struct sym_hcb *np, int cam_status, int target, int lun, int task);
 struct sym_ccb *sym_get_ccb(struct sym_hcb *np, struct scsi_cmnd *cmd, u_char tag_order);
-- 
1.4.4.2

Re: [PATCH 16/17] sym53c8xx: Remove pci_dev pointer from sym_shcb

[Jeff Garzik]

Matthew Wilcox wrote:
> This structure is accessed by the device; the fewer Linux things in it,
> the better.  Using the pci_dev pointer from the hostdata requires a lot
> of changes:
> 
>  - Pass Scsi_Host to a lot of routines which currently take a sym_hcb.
>  - Set the Scsi_Host as the pci drvdata (instead of the sym_hcb)
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

Obviously this is a judgement call on your part... but did you at least 
consider passing sym_hcb for all cases instead?

This is a common driver decisions, and many authors choose to store a 
pointer to the kernel struct (Scsi_Host in this case) in their adapter 
private structure, since it is often _far_ more common to pass the 
driver-private struct in arguments.

It doesn't make much sense to do the additional indirection IFF the 
majority of the routine actually want the driver-private structure.

I ACK the patch either way...

Re: [PATCH 16/17] sym53c8xx: Remove pci_dev pointer from sym_shcb

[Matthew Wilcox]

On Fri, Oct 05, 2007 at 05:07:39PM -0400, Jeff Garzik wrote:
> Obviously this is a judgement call on your part... but did you at least 
> consider passing sym_hcb for all cases instead?

I did.  There's a lot of crap stored in the sym_hcb that needs to move
to the sym_data, so Scsi_host is going to become more prevalent and
shm_hcb less used in future.  If sym2 were going to stay structured as
it is, this would be a bad move, but with the future changes I have
planned, we reach the tipping point fairly soon.

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

[PATCH 17/17] sym53c8xx: Remove sym_xpt_async_sent_bdr

[Matthew Wilcox]

This function just printed a message to the user; move the print to its
only caller, and turn it into an starget_printk.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/scsi/sym53c8xx_2/sym_glue.c |    8 --------
 drivers/scsi/sym53c8xx_2/sym_glue.h |    1 -
 drivers/scsi/sym53c8xx_2/sym_hipd.c |    3 ++-
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c b/drivers/scsi/sym53c8xx_2/sym_glue.c
index 4c7b8a6..74300dd 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -163,14 +163,6 @@ void sym_xpt_async_bus_reset(struct sym_hcb *np)
 }
 
 /*
- *  Tell the SCSI layer about a BUS DEVICE RESET message sent.
- */
-void sym_xpt_async_sent_bdr(struct sym_hcb *np, int target)
-{
-	printf_notice("%s: TARGET %d has been reset.\n", sym_name(np), target);
-}
-
-/*
  *  Choose the more appropriate CAM status if 
  *  the IO encountered an extended error.
  */
diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.h b/drivers/scsi/sym53c8xx_2/sym_glue.h
index e5bd0af..567fbe0 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.h
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.h
@@ -263,7 +263,6 @@ void sym_set_cam_result_error(struct sym_hcb *np, struct sym_ccb *cp, int resid)
 void sym_xpt_done(struct sym_hcb *np, struct scsi_cmnd *ccb);
 #define sym_print_addr(cmd, arg...) dev_info(&cmd->device->sdev_gendev , ## arg)
 void sym_xpt_async_bus_reset(struct sym_hcb *np);
-void sym_xpt_async_sent_bdr(struct sym_hcb *np, int target);
 int  sym_setup_data_and_start (struct sym_hcb *np, struct scsi_cmnd *csio, struct sym_ccb *cp);
 void sym_log_bus_error(struct Scsi_Host *);
 void sym_dump_registers(struct Scsi_Host *);
diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.c b/drivers/scsi/sym53c8xx_2/sym_hipd.c
index 3cf1209..463f119 100644
--- a/drivers/scsi/sym53c8xx_2/sym_hipd.c
+++ b/drivers/scsi/sym53c8xx_2/sym_hipd.c
@@ -3543,7 +3543,8 @@ static void sym_sir_task_recovery(struct sym_hcb *np, int num)
 		 *  If we sent a BDR, make upper layer aware of that.
  		 */
 		if (np->abrt_msg[0] == M_RESET)
-			sym_xpt_async_sent_bdr(np, target);
+			starget_printk(KERN_NOTICE, starget,
+							"has been reset\n");
 		break;
 	}
 
-- 
1.4.4.2

Re: [PATCH 17/17] sym53c8xx: Remove sym_xpt_async_sent_bdr

[Jeff Garzik]

Matthew Wilcox wrote:
> This function just printed a message to the user; move the print to its
> only caller, and turn it into an starget_printk.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  drivers/scsi/sym53c8xx_2/sym_glue.c |    8 --------
>  drivers/scsi/sym53c8xx_2/sym_glue.h |    1 -
>  drivers/scsi/sym53c8xx_2/sym_hipd.c |    3 ++-
>  3 files changed, 2 insertions(+), 10 deletions(-)

ACK