Skip to contentSkip to conversation

sym53c8xx_2 pci_remove cleanup

PATCH In review Brian King 2 participants 3 messages Oct 8, 2003 – Oct 10, 2003

Subsystems (2)
lore
Share & tools
lore

Brian King

author
#0 lore raw

PATCH 1/1 sym53c8xx_2 pci_remove cleanup

This is a patch to the sym2 driver to properly cleanup midlayer resources both on module unload and pci hot remove.

Brian

Signature · 3 lines
Brian King
eServer Storage I/O
IBM Linux Technology Center
drivers/scsi/sym53c8xx_2/sym_glue.c +6 -1
@@ -1605,6 +1605,8 @@
 	sym_hcb_free(np);
 
 	sym_mfree_dma(np, sizeof(*np), "HCB");
+
+	scsi_host_put(np->s.host);
 }
 
 /*
@@ -1861,7 +1863,8 @@
 	printf_info("%s: giving up ...\n", sym_name(np));
 	if (np)
 		sym_free_resources(np);
-	scsi_host_put(instance);
+	else
+		scsi_host_put(instance);
 
 	return -1;
  }
@@ -2213,6 +2216,8 @@
 {
 	printk("%s: detaching ...\n", sym_name(np));
 
+	scsi_remove_host(np->s.host);
+
 	del_timer_sync(&np->s.timer);
 
 	/*
#1 lore raw
Quoted 3 lines · Brian King

This is a patch to the sym2 driver to properly cleanup midlayer resources both on module unload and pci hot remove.

Here's the patch I currently have in my tree for doing the same thing. I wanted to test it, but I don't have any machines that don't have their root on it ... and so far I've failed to set up NFSroot on any of them.

I'm not sure whether it applies to the public tree or not, but it also handles scsi_add_host() failure. If it doesn't apply to your tree, let me know and I'll send you the latest version.

Index: drivers/scsi/sym53c8xx_2/sym_glue.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/scsi/sym53c8xx_2/sym_glue.c,v retrieving revision 1.34

scsi/sym53c8xx_2/sym_glue.c +25 -22
diff -u -p -r1.34 sym_glue.c
@@ -1630,8 +1630,8 @@ out_err32:
  *  If all is OK, install interrupt handling and
  *  start the timer daemon.
  */
-static int __devinit 
-sym_attach (struct scsi_host_template *tpnt, int unit, struct sym_device *dev)
+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_hcb *np = NULL;
@@ -1821,13 +1821,7 @@ sym_attach (struct scsi_host_template *t
 
 	spin_unlock_irqrestore(instance->host_lock, flags);
 
-	/*
-	 *  Now let the generic SCSI driver
-	 *  look for the SCSI devices on the bus ..
-	 */
-	scsi_add_host(instance, &dev->pdev->dev); /* XXX: handle failure */
-	scsi_scan_host(instance);
-	return 0;
+	return instance;
 
  reset_failed:
 	printf_err("%s: FATAL ERROR: CHECK SCSI BUS - CABLES, "
@@ -1835,13 +1829,13 @@ sym_attach (struct scsi_host_template *t
 	spin_unlock_irqrestore(instance->host_lock, flags);
  attach_failed:
 	if (!instance)
-		return -1;
+		return NULL;
 	printf_info("%s: giving up ...\n", sym_name(np));
 	if (np)
 		sym_free_resources(np);
 	scsi_host_put(instance);
 
-	return -1;
+	return NULL;
  }
 
 
@@ -2180,12 +2174,9 @@ sym53c8xx_pci_init(struct pci_dev *pdev,
 
 
 /*
- *  Linux release module stuff.
- *
  *  Called before unloading the module.
  *  Detach the host.
  *  We have to free resources and halt the NCR chip.
- *
  */
 static int __devexit sym_detach(struct sym_hcb *np)
 {
@@ -2194,18 +2185,15 @@ static int __devexit sym_detach(struct s
 	del_timer_sync(&np->s.timer);
 
 	/*
-	 *  Reset NCR chip.
-	 *  We should use sym_soft_reset(), but we donnot want to do 
-	 *  so, since we may not be safe if interrupts occur.
+	 * Reset NCR chip.
+	 * We should use sym_soft_reset(), but we don't want to do 
+	 * so, since we may not be safe if interrupts occur.
 	 */
 	printk("%s: resetting chip\n", sym_name(np));
 	OUTB (nc_istat, SRST);
 	UDELAY (10);
 	OUTB (nc_istat, 0);
 
-	/*
-	 *  Free host resources
-	 */
 	sym_free_resources(np);
 
 	return 1;
@@ -2314,6 +2302,7 @@ static int __devinit sym2_probe(struct p
 {
 	struct sym_device sym_dev;
 	struct sym_nvram nvram;
+	struct Scsi_Host *instance;
 
 	memset(&sym_dev, 0, sizeof(sym_dev));
 	memset(&nvram, 0, sizeof(nvram));
@@ -2332,12 +2321,20 @@ static int __devinit sym2_probe(struct p
 
 	sym_get_nvram(&sym_dev, &nvram);
 
-	if (sym_attach(&sym2_template, attach_count, &sym_dev))
+	instance = sym_attach(&sym2_template, attach_count, &sym_dev);
+	if (!instance)
 		goto free;
 
+	if (scsi_add_host(instance, &pdev->dev))
+		goto detach;
+	scsi_scan_host(instance);
+
 	attach_count++;
+
 	return 0;
 
+ detach:
+	sym_detach(pci_get_drvdata(pdev));
  free:
 	pci_release_regions(pdev);
  disable:
@@ -2347,7 +2344,13 @@ static int __devinit sym2_probe(struct p
 
 static void __devexit sym2_remove(struct pci_dev *pdev)
 {
-	sym_detach(pci_get_drvdata(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);
 
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
Signature · 3 lines
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
author
#2 lore raw

Looks good. I loaded it on a machine and it works fine for me. I tried both hot plug and insmod/rmmod.

-Brian

Quoted 143 lines · Matthew Wilcox
Quoted 1 line · Brian King

This is a patch to the sym2 driver to properly cleanup midlayer resources both on module unload and pci hot remove.

Here's the patch I currently have in my tree for doing the same thing. I wanted to test it, but I don't have any machines that don't have their root on it ... and so far I've failed to set up NFSroot on any of them.

I'm not sure whether it applies to the public tree or not, but it also handles scsi_add_host() failure. If it doesn't apply to your tree, let me know and I'll send you the latest version.

Index: drivers/scsi/sym53c8xx_2/sym_glue.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/scsi/sym53c8xx_2/sym_glue.c,v retrieving revision 1.34
diff -u -p -r1.34 sym_glue.c

scsi/sym53c8xx_2/sym_glue.c +25 -22
@@ -1630,8 +1630,8 @@ out_err32:
  *  If all is OK, install interrupt handling and
  *  start the timer daemon.
  */
-static int __devinit 
-sym_attach (struct scsi_host_template *tpnt, int unit, struct sym_device *dev)
+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_hcb *np = NULL;
@@ -1821,13 +1821,7 @@ sym_attach (struct scsi_host_template *t
 
 	spin_unlock_irqrestore(instance->host_lock, flags);
 
-	/*
-	 *  Now let the generic SCSI driver
-	 *  look for the SCSI devices on the bus ..
-	 */
-	scsi_add_host(instance, &dev->pdev->dev); /* XXX: handle failure */
-	scsi_scan_host(instance);
-	return 0;
+	return instance;
 
  reset_failed:
 	printf_err("%s: FATAL ERROR: CHECK SCSI BUS - CABLES, "
@@ -1835,13 +1829,13 @@ sym_attach (struct scsi_host_template *t
 	spin_unlock_irqrestore(instance->host_lock, flags);
  attach_failed:
 	if (!instance)
-		return -1;
+		return NULL;
 	printf_info("%s: giving up ...\n", sym_name(np));
 	if (np)
 		sym_free_resources(np);
 	scsi_host_put(instance);
 
-	return -1;
+	return NULL;
  }
 
 
@@ -2180,12 +2174,9 @@ sym53c8xx_pci_init(struct pci_dev *pdev,
 
 
 /*
- *  Linux release module stuff.
- *
  *  Called before unloading the module.
  *  Detach the host.
  *  We have to free resources and halt the NCR chip.
- *
  */
 static int __devexit sym_detach(struct sym_hcb *np)
 {
@@ -2194,18 +2185,15 @@ static int __devexit sym_detach(struct s
 	del_timer_sync(&np->s.timer);
 
 	/*
-	 *  Reset NCR chip.
-	 *  We should use sym_soft_reset(), but we donnot want to do 
-	 *  so, since we may not be safe if interrupts occur.
+	 * Reset NCR chip.
+	 * We should use sym_soft_reset(), but we don't want to do 
+	 * so, since we may not be safe if interrupts occur.
 	 */
 	printk("%s: resetting chip\n", sym_name(np));
 	OUTB (nc_istat, SRST);
 	UDELAY (10);
 	OUTB (nc_istat, 0);
 
-	/*
-	 *  Free host resources
-	 */
 	sym_free_resources(np);
 
 	return 1;
@@ -2314,6 +2302,7 @@ static int __devinit sym2_probe(struct p
 {
 	struct sym_device sym_dev;
 	struct sym_nvram nvram;
+	struct Scsi_Host *instance;
 
 	memset(&sym_dev, 0, sizeof(sym_dev));
 	memset(&nvram, 0, sizeof(nvram));
@@ -2332,12 +2321,20 @@ static int __devinit sym2_probe(struct p
 
 	sym_get_nvram(&sym_dev, &nvram);
 
-	if (sym_attach(&sym2_template, attach_count, &sym_dev))
+	instance = sym_attach(&sym2_template, attach_count, &sym_dev);
+	if (!instance)
 		goto free;
 
+	if (scsi_add_host(instance, &pdev->dev))
+		goto detach;
+	scsi_scan_host(instance);
+
 	attach_count++;
+
 	return 0;
 
+ detach:
+	sym_detach(pci_get_drvdata(pdev));
  free:
 	pci_release_regions(pdev);
  disable:
@@ -2347,7 +2344,13 @@ static int __devinit sym2_probe(struct p
 
 static void __devexit sym2_remove(struct pci_dev *pdev)
 {
-	sym_detach(pci_get_drvdata(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);
 
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
Signature · 3 lines
Brian King
eServer Storage I/O
IBM Linux Technology Center