All of lore.kernel.org
 help / color / mirror / Atom feed
* [KJ] [PATCH] drivers/net/* : pci_request_regions
@ 2005-02-14 15:08 Christophe Lucas
  2005-02-14 16:42 ` Mark Hollomon
  2005-02-14 19:09 ` Greg KH
  0 siblings, 2 replies; 3+ messages in thread
From: Christophe Lucas @ 2005-02-14 15:08 UTC (permalink / raw)
  To: kernel-janitors

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

Hi,

description:
If PCI request regions fails, then someone else is using the
hardware we wish to use. For that one case, calling
pci_disable_device() is rather rude.
See : http://www.ussg.iu.edu/hypermail/linux/kernel/0502.1/1061.html

Signed-off-by: Christophe Lucas <c.lucas@ifrance.com>


[-- Attachment #2: pci_disable-linux-2.6.11-rc4_drivers_net.patch --]
[-- Type: text/plain, Size: 8816 bytes --]

diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/amd8111e.c linux-2.6.11-rc4/drivers/net/amd8111e.c
--- linux-2.6.11-rc4-vanilla/drivers/net/amd8111e.c	2005-02-14 09:03:50.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/amd8111e.c	2005-02-14 15:27:05.000000000 +0100
@@ -1967,6 +1967,7 @@ static int __devinit amd8111e_probe_one(
 	unsigned long reg_addr,reg_len;
 	struct amd8111e_priv* lp;
 	struct net_device* dev;
+	int pci_dev_busy = 0 ;
 
 	err = pci_enable_device(pdev);
 	if(err){
@@ -1986,6 +1987,7 @@ static int __devinit amd8111e_probe_one(
 	if(err){
 		printk(KERN_ERR "amd8111e: Cannot obtain PCI resources, "
 		       "exiting.\n");
+		pci_dev_busy = 1 ;
 		goto err_disable_pdev;
 	}
 
@@ -2135,7 +2137,8 @@ err_free_reg:
 	pci_release_regions(pdev);
 
 err_disable_pdev:
-	pci_disable_device(pdev);
+	if(!pci_dev_busy) 
+		pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 	return err;
 
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/b44.c linux-2.6.11-rc4/drivers/net/b44.c
--- linux-2.6.11-rc4-vanilla/drivers/net/b44.c	2004-12-24 22:34:32.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/b44.c	2005-02-14 15:26:04.000000000 +0100
@@ -1737,6 +1737,7 @@ static int __devinit b44_init_one(struct
 	struct net_device *dev;
 	struct b44 *bp;
 	int err, i;
+	int pci_dev_busy = 0 ;
 
 	if (b44_version_printed++ == 0)
 		printk(KERN_INFO "%s", version);
@@ -1759,6 +1760,7 @@ static int __devinit b44_init_one(struct
 	if (err) {
 		printk(KERN_ERR PFX "Cannot obtain PCI resources, "
 		       "aborting.\n");
+		pci_dev_busy = 1 ;
 		goto err_out_disable_pdev;
 	}
 
@@ -1882,7 +1884,8 @@ err_out_free_res:
 	pci_release_regions(pdev);
 
 err_out_disable_pdev:
-	pci_disable_device(pdev);
+	if(!pci_dev_busy) 
+		pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 	return err;
 }
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/dl2k.c linux-2.6.11-rc4/drivers/net/dl2k.c
--- linux-2.6.11-rc4-vanilla/drivers/net/dl2k.c	2005-02-14 09:03:51.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/dl2k.c	2005-02-14 13:32:04.000000000 +0100
@@ -135,6 +135,7 @@ rio_probe1 (struct pci_dev *pdev, const 
 	static int version_printed;
 	void *ring_space;
 	dma_addr_t ring_dma;
+	int pci_dev_busy = 0 ;
 
 	if (!version_printed++)
 		printk ("%s", version);
@@ -145,8 +146,10 @@ rio_probe1 (struct pci_dev *pdev, const 
 
 	irq = pdev->irq;
 	err = pci_request_regions (pdev, "dl2k");
-	if (err)
+	if (err) {
+		pci_dev_busy = 1 ;
 		goto err_out_disable;
+	}
 
 	pci_set_master (pdev);
 	dev = alloc_etherdev (sizeof (*np));
@@ -327,7 +330,8 @@ rio_probe1 (struct pci_dev *pdev, const 
 	pci_release_regions (pdev);
 
       err_out_disable:
-	pci_disable_device (pdev);
+	if(!pci_dev_busy) 
+		pci_disable_device (pdev);
 	return err;
 }
 
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/e100.c linux-2.6.11-rc4/drivers/net/e100.c
--- linux-2.6.11-rc4-vanilla/drivers/net/e100.c	2005-02-14 09:03:51.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/e100.c	2005-02-14 15:25:28.000000000 +0100
@@ -2153,6 +2153,7 @@ static int __devinit e100_probe(struct p
 	struct net_device *netdev;
 	struct nic *nic;
 	int err;
+	int pci_dev_busy = 0 ;
 
 	if(!(netdev = alloc_etherdev(sizeof(struct nic)))) {
 		if(((1 << debug) - 1) & NETIF_MSG_PROBE)
@@ -2198,6 +2199,7 @@ static int __devinit e100_probe(struct p
 
 	if((err = pci_request_regions(pdev, DRV_NAME))) {
 		DPRINTK(PROBE, ERR, "Cannot obtain PCI resources, aborting.\n");
+		pci_dev_busy = 1 ;
 		goto err_out_disable_pdev;
 	}
 
@@ -2286,7 +2288,8 @@ err_out_iounmap:
 err_out_free_res:
 	pci_release_regions(pdev);
 err_out_disable_pdev:
-	pci_disable_device(pdev);
+	if(!pci_dev_busy)
+		pci_disable_device(pdev);
 err_out_free_dev:
 	pci_set_drvdata(pdev, NULL);
 	free_netdev(netdev);
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/epic100.c linux-2.6.11-rc4/drivers/net/epic100.c
--- linux-2.6.11-rc4-vanilla/drivers/net/epic100.c	2005-02-14 09:03:51.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/epic100.c	2005-02-14 14:30:11.000000000 +0100
@@ -384,7 +384,8 @@ static int __devinit epic_init_one (stru
 	int i, ret, option = 0, duplex = 0;
 	void *ring_space;
 	dma_addr_t ring_dma;
-
+	int pci_dev_busy = 0 ;
+	
 /* when built into the kernel, we only print version if device is found */
 #ifndef MODULE
 	static int printed_version;
@@ -409,8 +410,10 @@ static int __devinit epic_init_one (stru
 	pci_set_master(pdev);
 
 	ret = pci_request_regions(pdev, DRV_NAME);
-	if (ret < 0)
+	if (ret < 0 ) {
+		pci_dev_busy = 1 ;
 		goto err_out_disable;
+	}
 
 	ret = -ENOMEM;
 
@@ -584,7 +587,8 @@ err_out_free_netdev:
 err_out_free_res:
 	pci_release_regions(pdev);
 err_out_disable:
-	pci_disable_device(pdev);
+	if(!pci_dev_busy ) 
+		pci_disable_device(pdev);
 	goto out;
 }
 \f
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/forcedeth.c linux-2.6.11-rc4/drivers/net/forcedeth.c
--- linux-2.6.11-rc4-vanilla/drivers/net/forcedeth.c	2005-02-14 09:03:51.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/forcedeth.c	2005-02-14 14:32:32.000000000 +0100
@@ -1885,6 +1885,7 @@ static int __devinit nv_probe(struct pci
 	unsigned long addr;
 	u8 __iomem *base;
 	int err, i;
+	int pci_dev_busy = 0 ;
 
 	dev = alloc_etherdev(sizeof(struct fe_priv));
 	err = -ENOMEM;
@@ -1914,8 +1915,10 @@ static int __devinit nv_probe(struct pci
 	pci_set_master(pci_dev);
 
 	err = pci_request_regions(pci_dev, DRV_NAME);
-	if (err < 0)
+	if (err < 0) {
+		pci_dev_busy = 1 ;
 		goto out_disable;
+	}
 
 	err = -EINVAL;
 	addr = 0;
@@ -2089,7 +2092,8 @@ out_unmap:
 out_relreg:
 	pci_release_regions(pci_dev);
 out_disable:
-	pci_disable_device(pci_dev);
+	if(!pci_dev_busy)
+		pci_disable_device(pci_dev);
 out_free:
 	free_netdev(dev);
 out:
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/saa9730.c linux-2.6.11-rc4/drivers/net/saa9730.c
--- linux-2.6.11-rc4-vanilla/drivers/net/saa9730.c	2004-12-24 22:34:58.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/saa9730.c	2005-02-14 14:40:22.000000000 +0100
@@ -1107,6 +1107,7 @@ static int __devinit saa9730_init_one(st
 	struct net_device *dev;
 	unsigned int pci_ioaddr;
 	int err;
+	int pci_dev_busy = 0 ;
 
 	if (lan_saa9730_debug > 1)
 		printk("saa9730.c: PCI bios is present, checking for devices...\n");
@@ -1127,6 +1128,7 @@ static int __devinit saa9730_init_one(st
 	err = pci_request_regions(pdev, DRV_MODULE_NAME);
 	if (err) {
 		printk(KERN_ERR "Cannot obtain PCI resources, aborting.\n");
+		pci_dev_busy = 1 ;
 		goto out2;
 	}
 
@@ -1150,7 +1152,8 @@ static int __devinit saa9730_init_one(st
 	return 0;
 	
 out2:
-	pci_disable_device(pdev);
+	if(!pci_dev_busy)
+		pci_disable_device(pdev);
 out1:
 	free_netdev(dev);
 out:
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/tg3.c linux-2.6.11-rc4/drivers/net/tg3.c
--- linux-2.6.11-rc4-vanilla/drivers/net/tg3.c	2005-02-14 09:03:54.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/tg3.c	2005-02-14 15:27:36.000000000 +0100
@@ -8610,6 +8610,7 @@ static int __devinit tg3_init_one(struct
 	struct net_device *dev;
 	struct tg3 *tp;
 	int i, err, pci_using_dac, pm_cap;
+	int pci_dev_busy = 0 ;
 
 	if (tg3_version_printed++ == 0)
 		printk(KERN_INFO "%s", version);
@@ -8632,6 +8633,7 @@ static int __devinit tg3_init_one(struct
 	if (err) {
 		printk(KERN_ERR PFX "Cannot obtain PCI resources, "
 		       "aborting.\n");
+		pci_dev_busy = 1;
 		goto err_out_disable_pdev;
 	}
 
@@ -8903,7 +8905,8 @@ err_out_free_res:
 	pci_release_regions(pdev);
 
 err_out_disable_pdev:
-	pci_disable_device(pdev);
+	if(!pci_dev_busy) 
+		pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 	return err;
 }
diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/via-velocity.c linux-2.6.11-rc4/drivers/net/via-velocity.c
--- linux-2.6.11-rc4-vanilla/drivers/net/via-velocity.c	2005-02-14 09:03:54.000000000 +0100
+++ linux-2.6.11-rc4/drivers/net/via-velocity.c	2005-02-14 14:50:32.000000000 +0100
@@ -692,6 +692,7 @@ static int __devinit velocity_found1(str
 	struct velocity_info *vptr;
 	struct mac_regs __iomem * regs;
 	int ret = -ENOMEM;
+	int pci_dev_busy = 0 ;
 
 	if (velocity_nics >= MAX_UNITS) {
 		printk(KERN_NOTICE VELOCITY_NAME ": already found %d NICs.\n", 
@@ -740,6 +741,7 @@ static int __devinit velocity_found1(str
 	ret = pci_request_regions(pdev, VELOCITY_NAME);
 	if (ret < 0) {
 		printk(KERN_ERR VELOCITY_NAME ": Failed to find PCI device.\n");
+		pci_dev_busy = 1 ;
 		goto err_disable;
 	}
 
@@ -823,7 +825,8 @@ err_iounmap:
 err_release_res:
 	pci_release_regions(pdev);
 err_disable:
-	pci_disable_device(pdev);
+	if(!pci_dev_busy)
+		pci_disable_device(pdev);
 err_free_dev:
 	free_netdev(dev);
 	goto out;

[-- Attachment #3: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [KJ] [PATCH] drivers/net/* : pci_request_regions
  2005-02-14 15:08 [KJ] [PATCH] drivers/net/* : pci_request_regions Christophe Lucas
@ 2005-02-14 16:42 ` Mark Hollomon
  2005-02-14 19:09 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Mark Hollomon @ 2005-02-14 16:42 UTC (permalink / raw)
  To: kernel-janitors

Christophe Lucas wrote:
> Hi,
> 
> description:
> If PCI request regions fails, then someone else is using the
> hardware we wish to use. For that one case, calling
> pci_disable_device() is rather rude.
> See : http://www.ussg.iu.edu/hypermail/linux/kernel/0502.1/1061.html
> 
> Signed-off-by: Christophe Lucas <c.lucas@ifrance.com>
> 
> 
> ------------------------------------------------------------------------
> 
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/amd8111e.c linux-2.6.11-rc4/drivers/net/amd8111e.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/amd8111e.c	2005-02-14 09:03:50.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/amd8111e.c	2005-02-14 15:27:05.000000000 +0100
> @@ -1967,6 +1967,7 @@ static int __devinit amd8111e_probe_one(
>  	unsigned long reg_addr,reg_len;
>  	struct amd8111e_priv* lp;
>  	struct net_device* dev;
> +	int pci_dev_busy = 0 ;
>  
>  	err = pci_enable_device(pdev);
>  	if(err){
> @@ -1986,6 +1987,7 @@ static int __devinit amd8111e_probe_one(
>  	if(err){
>  		printk(KERN_ERR "amd8111e: Cannot obtain PCI resources, "
>  		       "exiting.\n");
> +		pci_dev_busy = 1 ;
>  		goto err_disable_pdev;
>  	}
>  
> @@ -2135,7 +2137,8 @@ err_free_reg:
>  	pci_release_regions(pdev);
>  
>  err_disable_pdev:
> -	pci_disable_device(pdev);
> +	if(!pci_dev_busy) 
> +		pci_disable_device(pdev);

Why not add a "err_out_clear_drvdata" target that skips the pci_disable_dev ?

>  	pci_set_drvdata(pdev, NULL);
>  	return err;
>  
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/b44.c linux-2.6.11-rc4/drivers/net/b44.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/b44.c	2004-12-24 22:34:32.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/b44.c	2005-02-14 15:26:04.000000000 +0100
> @@ -1737,6 +1737,7 @@ static int __devinit b44_init_one(struct
>  	struct net_device *dev;
>  	struct b44 *bp;
>  	int err, i;
> +	int pci_dev_busy = 0 ;
>  
>  	if (b44_version_printed++ = 0)
>  		printk(KERN_INFO "%s", version);
> @@ -1759,6 +1760,7 @@ static int __devinit b44_init_one(struct
>  	if (err) {
>  		printk(KERN_ERR PFX "Cannot obtain PCI resources, "
>  		       "aborting.\n");
> +		pci_dev_busy = 1 ;
>  		goto err_out_disable_pdev;
>  	}
>  
> @@ -1882,7 +1884,8 @@ err_out_free_res:
>  	pci_release_regions(pdev);
>  
>  err_out_disable_pdev:
> -	pci_disable_device(pdev);
> +	if(!pci_dev_busy) 
> +		pci_disable_device(pdev);

Why not add a "err_out_clear_drvdata" target that skips the pci_disable_dev ?

>  	pci_set_drvdata(pdev, NULL);
>  	return err;
>  }
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/dl2k.c linux-2.6.11-rc4/drivers/net/dl2k.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/dl2k.c	2005-02-14 09:03:51.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/dl2k.c	2005-02-14 13:32:04.000000000 +0100
> @@ -135,6 +135,7 @@ rio_probe1 (struct pci_dev *pdev, const 
>  	static int version_printed;
>  	void *ring_space;
>  	dma_addr_t ring_dma;
> +	int pci_dev_busy = 0 ;
>  
>  	if (!version_printed++)
>  		printk ("%s", version);
> @@ -145,8 +146,10 @@ rio_probe1 (struct pci_dev *pdev, const 
>  
>  	irq = pdev->irq;
>  	err = pci_request_regions (pdev, "dl2k");
> -	if (err)
> +	if (err) {
> +		pci_dev_busy = 1 ;
>  		goto err_out_disable;
> +	}

If a simple "return err" isn't acceptable, why not a "err_out_return" label just 
before the return below?

>  
>  	pci_set_master (pdev);
>  	dev = alloc_etherdev (sizeof (*np));
> @@ -327,7 +330,8 @@ rio_probe1 (struct pci_dev *pdev, const 
>  	pci_release_regions (pdev);
>  
>        err_out_disable:
> -	pci_disable_device (pdev);
> +	if(!pci_dev_busy) 
> +		pci_disable_device (pdev);
>  	return err;
>  }
>  
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/e100.c linux-2.6.11-rc4/drivers/net/e100.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/e100.c	2005-02-14 09:03:51.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/e100.c	2005-02-14 15:25:28.000000000 +0100
> @@ -2153,6 +2153,7 @@ static int __devinit e100_probe(struct p
>  	struct net_device *netdev;
>  	struct nic *nic;
>  	int err;
> +	int pci_dev_busy = 0 ;
>  
>  	if(!(netdev = alloc_etherdev(sizeof(struct nic)))) {
>  		if(((1 << debug) - 1) & NETIF_MSG_PROBE)
> @@ -2198,6 +2199,7 @@ static int __devinit e100_probe(struct p
>  
>  	if((err = pci_request_regions(pdev, DRV_NAME))) {
>  		DPRINTK(PROBE, ERR, "Cannot obtain PCI resources, aborting.\n");
> +		pci_dev_busy = 1 ;
>  		goto err_out_disable_pdev;

Why not just "goto err_out_free_dev"?

>  	}
>  
> @@ -2286,7 +2288,8 @@ err_out_iounmap:
>  err_out_free_res:
>  	pci_release_regions(pdev);
>  err_out_disable_pdev:
> -	pci_disable_device(pdev);
> +	if(!pci_dev_busy)
> +		pci_disable_device(pdev);
>  err_out_free_dev:
>  	pci_set_drvdata(pdev, NULL);
>  	free_netdev(netdev);
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/epic100.c linux-2.6.11-rc4/drivers/net/epic100.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/epic100.c	2005-02-14 09:03:51.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/epic100.c	2005-02-14 14:30:11.000000000 +0100
> @@ -384,7 +384,8 @@ static int __devinit epic_init_one (stru
>  	int i, ret, option = 0, duplex = 0;
>  	void *ring_space;
>  	dma_addr_t ring_dma;
> -
> +	int pci_dev_busy = 0 ;
> +	
>  /* when built into the kernel, we only print version if device is found */
>  #ifndef MODULE
>  	static int printed_version;
> @@ -409,8 +410,10 @@ static int __devinit epic_init_one (stru
>  	pci_set_master(pdev);
>  
>  	ret = pci_request_regions(pdev, DRV_NAME);
> -	if (ret < 0)
> +	if (ret < 0 ) {
> +		pci_dev_busy = 1 ;
>  		goto err_out_disable;

Why not just "goto out"?

> +	}
>  
>  	ret = -ENOMEM;
>  
> @@ -584,7 +587,8 @@ err_out_free_netdev:
>  err_out_free_res:
>  	pci_release_regions(pdev);
>  err_out_disable:
> -	pci_disable_device(pdev);
> +	if(!pci_dev_busy ) 
> +		pci_disable_device(pdev);
>  	goto out;
>  }
>  \f
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/forcedeth.c linux-2.6.11-rc4/drivers/net/forcedeth.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/forcedeth.c	2005-02-14 09:03:51.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/forcedeth.c	2005-02-14 14:32:32.000000000 +0100
> @@ -1885,6 +1885,7 @@ static int __devinit nv_probe(struct pci
>  	unsigned long addr;
>  	u8 __iomem *base;
>  	int err, i;
> +	int pci_dev_busy = 0 ;
>  
>  	dev = alloc_etherdev(sizeof(struct fe_priv));
>  	err = -ENOMEM;
> @@ -1914,8 +1915,10 @@ static int __devinit nv_probe(struct pci
>  	pci_set_master(pci_dev);
>  
>  	err = pci_request_regions(pci_dev, DRV_NAME);
> -	if (err < 0)
> +	if (err < 0) {
> +		pci_dev_busy = 1 ;
>  		goto out_disable;

Why not just "goto out_free"?

> +	}
>  
>  	err = -EINVAL;
>  	addr = 0;
> @@ -2089,7 +2092,8 @@ out_unmap:
>  out_relreg:
>  	pci_release_regions(pci_dev);
>  out_disable:
> -	pci_disable_device(pci_dev);
> +	if(!pci_dev_busy)
> +		pci_disable_device(pci_dev);
>  out_free:
>  	free_netdev(dev);
>  out:
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/saa9730.c linux-2.6.11-rc4/drivers/net/saa9730.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/saa9730.c	2004-12-24 22:34:58.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/saa9730.c	2005-02-14 14:40:22.000000000 +0100
> @@ -1107,6 +1107,7 @@ static int __devinit saa9730_init_one(st
>  	struct net_device *dev;
>  	unsigned int pci_ioaddr;
>  	int err;
> +	int pci_dev_busy = 0 ;
>  
>  	if (lan_saa9730_debug > 1)
>  		printk("saa9730.c: PCI bios is present, checking for devices...\n");
> @@ -1127,6 +1128,7 @@ static int __devinit saa9730_init_one(st
>  	err = pci_request_regions(pdev, DRV_MODULE_NAME);
>  	if (err) {
>  		printk(KERN_ERR "Cannot obtain PCI resources, aborting.\n");
> +		pci_dev_busy = 1 ;
>  		goto out2;

Why not just change this to "goto out1"?

>  	}
>  
> @@ -1150,7 +1152,8 @@ static int __devinit saa9730_init_one(st
>  	return 0;
>  	
>  out2:
> -	pci_disable_device(pdev);
> +	if(!pci_dev_busy)
> +		pci_disable_device(pdev);
>  out1:
>  	free_netdev(dev);
>  out:
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/tg3.c linux-2.6.11-rc4/drivers/net/tg3.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/tg3.c	2005-02-14 09:03:54.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/tg3.c	2005-02-14 15:27:36.000000000 +0100
> @@ -8610,6 +8610,7 @@ static int __devinit tg3_init_one(struct
>  	struct net_device *dev;
>  	struct tg3 *tp;
>  	int i, err, pci_using_dac, pm_cap;
> +	int pci_dev_busy = 0 ;
>  
>  	if (tg3_version_printed++ = 0)
>  		printk(KERN_INFO "%s", version);
> @@ -8632,6 +8633,7 @@ static int __devinit tg3_init_one(struct
>  	if (err) {
>  		printk(KERN_ERR PFX "Cannot obtain PCI resources, "
>  		       "aborting.\n");
> +		pci_dev_busy = 1;
>  		goto err_out_disable_pdev;
>  	}
>  
> @@ -8903,7 +8905,8 @@ err_out_free_res:
>  	pci_release_regions(pdev);
>  
>  err_out_disable_pdev:
> -	pci_disable_device(pdev);
> +	if(!pci_dev_busy) 
> +		pci_disable_device(pdev);

Why not add a "err_out_clear_drvdata" target that skips the pci_disable_dev ?

>  	pci_set_drvdata(pdev, NULL);
>  	return err;
>  }
> diff -urpN -X /work/users/dontdiff linux-2.6.11-rc4-vanilla/drivers/net/via-velocity.c linux-2.6.11-rc4/drivers/net/via-velocity.c
> --- linux-2.6.11-rc4-vanilla/drivers/net/via-velocity.c	2005-02-14 09:03:54.000000000 +0100
> +++ linux-2.6.11-rc4/drivers/net/via-velocity.c	2005-02-14 14:50:32.000000000 +0100
> @@ -692,6 +692,7 @@ static int __devinit velocity_found1(str
>  	struct velocity_info *vptr;
>  	struct mac_regs __iomem * regs;
>  	int ret = -ENOMEM;
> +	int pci_dev_busy = 0 ;
>  
>  	if (velocity_nics >= MAX_UNITS) {
>  		printk(KERN_NOTICE VELOCITY_NAME ": already found %d NICs.\n", 
> @@ -740,6 +741,7 @@ static int __devinit velocity_found1(str
>  	ret = pci_request_regions(pdev, VELOCITY_NAME);
>  	if (ret < 0) {
>  		printk(KERN_ERR VELOCITY_NAME ": Failed to find PCI device.\n");
> +		pci_dev_busy = 1 ;
>  		goto err_disable;

Why not just change this to "goto err_free_dev" ? It would make the code here 
read better.

>  	}
>  
> @@ -823,7 +825,8 @@ err_iounmap:
>  err_release_res:
>  	pci_release_regions(pdev);
>  err_disable:
> -	pci_disable_device(pdev);
> +	if(!pci_dev_busy)
> +		pci_disable_device(pdev);
>  err_free_dev:
>  	free_netdev(dev);
>  	goto out;
> 

-- 
Mark Hollomon
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [KJ] [PATCH] drivers/net/* : pci_request_regions
  2005-02-14 15:08 [KJ] [PATCH] drivers/net/* : pci_request_regions Christophe Lucas
  2005-02-14 16:42 ` Mark Hollomon
@ 2005-02-14 19:09 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2005-02-14 19:09 UTC (permalink / raw)
  To: kernel-janitors

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

On Mon, Feb 14, 2005 at 04:08:15PM +0100, Christophe Lucas wrote:
> Hi,
> 
> description:
> If PCI request regions fails, then someone else is using the
> hardware we wish to use. For that one case, calling
> pci_disable_device() is rather rude.
> See : http://www.ussg.iu.edu/hypermail/linux/kernel/0502.1/1061.html
> 
> Signed-off-by: Christophe Lucas <c.lucas@ifrance.com>

And do these drivers work on devices that have problems like was
mentioned above (they live in dumb multi-function pci devices.)?

thanks,

greg k-h

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2005-02-14 19:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-14 15:08 [KJ] [PATCH] drivers/net/* : pci_request_regions Christophe Lucas
2005-02-14 16:42 ` Mark Hollomon
2005-02-14 19:09 ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.