All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
@ 2006-10-12 22:17 Jiri Kosina
  2006-10-12 22:25 ` Stephen Hemminger
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Kosina @ 2006-10-12 22:17 UTC (permalink / raw)
  To: mlindner, rroesler, Andrew Morton, Jeff Garzik; +Cc: linux-kernel, netdev

[PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly

Fix missing handling of pci_enable_device() return value in skge_resume() 

Signed-off-by: Jiri Kosina <jikos@jikos.cz>

--- 

 drivers/net/sk98lin/skge.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
index 99e9262..e12fb62 100644
--- a/drivers/net/sk98lin/skge.c
+++ b/drivers/net/sk98lin/skge.c
@@ -5070,7 +5070,11 @@ static int skge_resume(struct pci_dev *p
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	pci_enable_device(pdev);
+	if ((ret = pci_enable_device(pdev))) {
+		printk(KERN_ERR "sk98lin: Cannot enable PCI device during resume\n");
+		unregister_netdev(dev);
+		return ret;
+	}
 	pci_set_master(pdev);
 	if (pAC->GIni.GIMacsFound == 2)
 		ret = request_irq(dev->irq, SkGeIsr, IRQF_SHARED, "sk98lin", dev);

-- 
Jiri Kosina

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

* Re: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
  2006-10-12 22:17 [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly Jiri Kosina
@ 2006-10-12 22:25 ` Stephen Hemminger
  2006-10-12 22:38   ` Jiri Kosina
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2006-10-12 22:25 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: mlindner, rroesler, Andrew Morton, Jeff Garzik, linux-kernel, netdev

On Fri, 13 Oct 2006 00:17:50 +0200 (CEST)
Jiri Kosina <jikos@jikos.cz> wrote:

> [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
> 
> Fix missing handling of pci_enable_device() return value in skge_resume() 
> 
> Signed-off-by: Jiri Kosina <jikos@jikos.cz>
> 
> --- 
> 
>  drivers/net/sk98lin/skge.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
> index 99e9262..e12fb62 100644
> --- a/drivers/net/sk98lin/skge.c
> +++ b/drivers/net/sk98lin/skge.c
> @@ -5070,7 +5070,11 @@ static int skge_resume(struct pci_dev *p
>  
>  	pci_set_power_state(pdev, PCI_D0);
>  	pci_restore_state(pdev);
> -	pci_enable_device(pdev);
> +	if ((ret = pci_enable_device(pdev))) {
> +		printk(KERN_ERR "sk98lin: Cannot enable PCI device during resume\n");
> +		unregister_netdev(dev);
>

Having the device unregister seems harsh.
Why put condtional on same line?
Why not print device name dev->name.


-- 
Stephen Hemminger <shemminger@osdl.org>

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

* Re: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
  2006-10-12 22:25 ` Stephen Hemminger
@ 2006-10-12 22:38   ` Jiri Kosina
  2006-10-12 22:47     ` Stephen Hemminger
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Kosina @ 2006-10-12 22:38 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: mlindner, rroesler, Andrew Morton, Jeff Garzik, linux-kernel, netdev

On Thu, 12 Oct 2006, Stephen Hemminger wrote:

> >  	pci_set_power_state(pdev, PCI_D0);
> >  	pci_restore_state(pdev);
> > -	pci_enable_device(pdev);
> > +	if ((ret = pci_enable_device(pdev))) {
> > +		printk(KERN_ERR "sk98lin: Cannot enable PCI device during resume\n");
> > +		unregister_netdev(dev);
> >
> Having the device unregister seems harsh.

What would be the proper way? As the initialization failed, accessing the 
device would not make sense any more (therefore I don't think that calling 
skge_remove_one() would be OK, as it issues calls to SkEventQueue() and 
SkEventDispatcher(), trying to send something to the card).

> Why put condtional on same line?

Pardon me?

> Why not print device name dev->name.

Thanks.

[PATCH] fix sk98lin driver, ignoring return value from pci_enable_device()

add check of return value to _resume() function of sk98lin driver.

Signed-off-by: Jiri Kosina <jikos@jikos.cz>

--- 

 drivers/net/sk98lin/skge.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
index d4913c3..1f03cf8 100644
--- a/drivers/net/sk98lin/skge.c
+++ b/drivers/net/sk98lin/skge.c
@@ -5070,7 +5070,11 @@ static int skge_resume(struct pci_dev *p
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	pci_enable_device(pdev);
+	if ((ret = pci_enable_device(pdev))) {
+		printk(KERN_ERR "sk98lin: Cannot enable PCI device %s during resume\n", 
+				dev->name);
+		return ret;
+	}
 	pci_set_master(pdev);
 	if (pAC->GIni.GIMacsFound == 2)
 		ret = request_irq(dev->irq, SkGeIsr, IRQF_SHARED, "sk98lin", dev);

-- 
Jiri Kosina

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

* Re: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
  2006-10-12 22:38   ` Jiri Kosina
@ 2006-10-12 22:47     ` Stephen Hemminger
  2006-10-12 22:57       ` Jiri Kosina
  0 siblings, 1 reply; 9+ messages in thread
From: Stephen Hemminger @ 2006-10-12 22:47 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: mlindner, rroesler, Andrew Morton, Jeff Garzik, linux-kernel, netdev

On Fri, 13 Oct 2006 00:38:20 +0200 (CEST)
Jiri Kosina <jikos@jikos.cz> wrote:

> On Thu, 12 Oct 2006, Stephen Hemminger wrote:
> 
> > >  	pci_set_power_state(pdev, PCI_D0);
> > >  	pci_restore_state(pdev);
> > > -	pci_enable_device(pdev);
> > > +	if ((ret = pci_enable_device(pdev))) {
> > > +		printk(KERN_ERR "sk98lin: Cannot enable PCI device during resume\n");
> > > +		unregister_netdev(dev);
> > >
> > Having the device unregister seems harsh.
> 
> What would be the proper way? As the initialization failed, accessing the 
> device would not make sense any more (therefore I don't think that calling 
> skge_remove_one() would be OK, as it issues calls to SkEventQueue() and 
> SkEventDispatcher(), trying to send something to the card).

I guess, its just not clear what the state of the machine is anyway
if you can't enable the device something is hosed (or the device was
hot removed).

> > Why put condtional on same line?
> 
> Pardon me?

I prefer:
	ret = pci_enable_device(pdev);
	if (ret) {



> 
> > Why not print device name dev->name.
> 
> Thanks.
> 
> [PATCH] fix sk98lin driver, ignoring return value from pci_enable_device()
> 
> add check of return value to _resume() function of sk98lin driver.
> 
> Signed-off-by: Jiri Kosina <jikos@jikos.cz>
> 
> --- 
> 
>  drivers/net/sk98lin/skge.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
> index d4913c3..1f03cf8 100644
> --- a/drivers/net/sk98lin/skge.c
> +++ b/drivers/net/sk98lin/skge.c
> @@ -5070,7 +5070,11 @@ static int skge_resume(struct pci_dev *p
>  
>  	pci_set_power_state(pdev, PCI_D0);
>  	pci_restore_state(pdev);
> -	pci_enable_device(pdev);
> +	if ((ret = pci_enable_device(pdev))) {
> +		printk(KERN_ERR "sk98lin: Cannot enable PCI device %s during resume\n", 
> +				dev->name);
> +		return ret;
> +	}
>  	pci_set_master(pdev);
>  	if (pAC->GIni.GIMacsFound == 2)
>  		ret = request_irq(dev->irq, SkGeIsr, IRQF_SHARED, "sk98lin", dev);
> 


-- 
Stephen Hemminger <shemminger@osdl.org>

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

* Re: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
  2006-10-12 22:47     ` Stephen Hemminger
@ 2006-10-12 22:57       ` Jiri Kosina
  2006-10-13  0:50         ` Andrew Morton
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Kosina @ 2006-10-12 22:57 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: mlindner, rroesler, Andrew Morton, Jeff Garzik, linux-kernel, netdev

On Thu, 12 Oct 2006, Stephen Hemminger wrote:

> > > Having the device unregister seems harsh.
> > What would be the proper way? As the initialization failed, accessing 
> > the device would not make sense any more (therefore I don't think that 
> > calling skge_remove_one() would be OK, as it issues calls to 
> > SkEventQueue() and SkEventDispatcher(), trying to send something to 
> > the card).
> I guess, its just not clear what the state of the machine is anyway
> if you can't enable the device something is hosed (or the device was
> hot removed).

Well, it depends on definition of 'hot'. What would for example happen in 
the case suspend-to-disk -> remove the card when the machine is switched 
off -> resume-from-disk? I guess that exactly this pci_enable_device() 
will fail, so we definitely have to handle this case, as it can easily 
happen.

> > > Why put condtional on same line?
> > Pardon me?
> I prefer:
> 	ret = pci_enable_device(pdev);

As you wish. 

[PATCH] fix sk98lin driver, ignoring return value from pci_enable_device()

add check of return value to _resume() function of sk98lin driver.

Signed-off-by: Jiri Kosina <jikos@jikos.cz>

---

 drivers/net/sk98lin/skge.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
index d4913c3..d691811 100644
--- a/drivers/net/sk98lin/skge.c
+++ b/drivers/net/sk98lin/skge.c
@@ -5070,7 +5070,13 @@ static int skge_resume(struct pci_dev *p
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	pci_enable_device(pdev);
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		printk(KERN_ERR "sk98lin: Cannot enable PCI device %s during resume\n", 
+				dev->name);
+		unregister_netdev(dev);
+		return ret;
+	}
 	pci_set_master(pdev);
 	if (pAC->GIni.GIMacsFound == 2)
 		ret = request_irq(dev->irq, SkGeIsr, IRQF_SHARED, "sk98lin", dev);

-- 
Jiri Kosina

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

* Re: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
  2006-10-12 22:57       ` Jiri Kosina
@ 2006-10-13  0:50         ` Andrew Morton
  2006-10-13  1:12           ` Jiri Kosina
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2006-10-13  0:50 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Stephen Hemminger, mlindner, rroesler, Jeff Garzik, linux-kernel, netdev

On Fri, 13 Oct 2006 00:57:18 +0200 (CEST)
Jiri Kosina <jikos@jikos.cz> wrote:

> @@ -5070,7 +5070,13 @@ static int skge_resume(struct pci_dev *p
>  
>  	pci_set_power_state(pdev, PCI_D0);
>  	pci_restore_state(pdev);
> -	pci_enable_device(pdev);
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		printk(KERN_ERR "sk98lin: Cannot enable PCI device %s during resume\n", 
> +				dev->name);
> +		unregister_netdev(dev);

This looks rather wrong - skge_exit() will run unregister_netdev() again.

Look a few lines down, to where this function already handles request_irq()
failure, reuse that code path.  Hopefully it has been tested..

(Once we have an easy-to-use fault-injection framework we'll be able to
test all these things more easily)

(But it's possible to test them already, with a bit of ad-hoc testing code)

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

* Re: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly
  2006-10-13  0:50         ` Andrew Morton
@ 2006-10-13  1:12           ` Jiri Kosina
  2007-02-23 23:17             ` [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() Dmitriy Monakhov
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Kosina @ 2006-10-13  1:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Stephen Hemminger, mlindner, rroesler, Jeff Garzik, linux-kernel, netdev

On Thu, 12 Oct 2006, Andrew Morton wrote:

> >  	pci_set_power_state(pdev, PCI_D0);
> >  	pci_restore_state(pdev);
> > -	pci_enable_device(pdev);
> > +	ret = pci_enable_device(pdev);
> > +	if (ret) {
> > +		printk(KERN_ERR "sk98lin: Cannot enable PCI device %s during resume\n", 
> > +				dev->name);
> > +		unregister_netdev(dev);
> This looks rather wrong - skge_exit() will run unregister_netdev() again.

You are of course right (the problem was also spotted by Russell King). 
This I believe is the correct one for the sk98lin case.

[PATCH] fix sk98lin driver, ignoring return value from pci_enable_device()

add check of return value to _resume() function of sk98lin driver.

Signed-off-by: Jiri Kosina <jikos@jikos.cz>

---

 drivers/net/sk98lin/skge.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
index d4913c3..3a9323d 100644
--- a/drivers/net/sk98lin/skge.c
+++ b/drivers/net/sk98lin/skge.c
@@ -5070,7 +5070,12 @@ static int skge_resume(struct pci_dev *p
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	pci_enable_device(pdev);
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		printk(KERN_WARNING "sk98lin: unable to enable device %s in resume\n",
+				dev->name);
+		goto out_err;
+	}	
 	pci_set_master(pdev);
 	if (pAC->GIni.GIMacsFound == 2)
 		ret = request_irq(dev->irq, SkGeIsr, IRQF_SHARED, "sk98lin", dev);
@@ -5078,10 +5083,8 @@ static int skge_resume(struct pci_dev *p
 		ret = request_irq(dev->irq, SkGeIsrOnePort, IRQF_SHARED, "sk98lin", dev);
 	if (ret) {
 		printk(KERN_WARNING "sk98lin: unable to acquire IRQ %d\n", dev->irq);
-		pAC->AllocFlag &= ~SK_ALLOC_IRQ;
-		dev->irq = 0;
-		pci_disable_device(pdev);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto out_err;
 	}
 
 	netif_device_attach(dev);
@@ -5098,6 +5101,13 @@ static int skge_resume(struct pci_dev *p
 	}
 
 	return 0;
+out_err:
+	pAC->AllocFlag &= ~SK_ALLOC_IRQ;
+	dev->irq = 0;
+	pci_disable_device(pdev);
+
+	return ret;
+
 }
 #else
 #define skge_suspend NULL

-- 
Jiri Kosina

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

* [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume()
  2006-10-13  1:12           ` Jiri Kosina
@ 2007-02-23 23:17             ` Dmitriy Monakhov
  2007-02-24 11:41               ` Dmitriy Monakhov
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitriy Monakhov @ 2007-02-23 23:17 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Andrew Morton, Stephen Hemminger, mlindner, rroesler,
	Jeff Garzik, linux-kernel, netdev

This thread looks dead but issue was't fixed.

Jiri Kosina <jikos@jikos.cz> writes:
>> > -	pci_enable_device(pdev);
>> > +	ret = pci_enable_device(pdev);
>> > +	if (ret) {
>> > +		printk(KERN_ERR "sk98lin: Cannot enable PCI device %s during resume\n", 
>> > +				dev->name);
>> > +		unregister_netdev(dev);
>> This looks rather wrong - skge_exit() will run unregister_netdev() again.
>
> You are of course right (the problem was also spotted by Russell King). 
> This I believe is the correct one for the sk98lin case.
>
> [PATCH] fix sk98lin driver, ignoring return value from pci_enable_device()
>
> add check of return value to _resume() function of sk98lin driver.
>
> Signed-off-by: Jiri Kosina <jikos@jikos.cz>
>
> ---
>
> --- a/drivers/net/sk98lin/skge.c
> +++ b/drivers/net/sk98lin/skge.c
> @@ -5070,7 +5070,12 @@ static int skge_resume(struct pci_dev *p
>  
>  	pci_set_power_state(pdev, PCI_D0);
>  	pci_restore_state(pdev);
> -	pci_enable_device(pdev);
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		printk(KERN_WARNING "sk98lin: unable to enable device %s in resume\n",
> +				dev->name);
> +		goto out_err;
> +	}	
[snip]
> +out_err:
> +	pAC->AllocFlag &= ~SK_ALLOC_IRQ;
> +	dev->irq = 0;
> +	pci_disable_device(pdev);
<<<<< Ok what happend if we jump here right after pci_disable_device() has 
      failed, but pci_disable_device() was called anyway, this is wrong and 
      may be fatal because pdev->enable_cnt may becomes negative.  
> +
> +	return ret;
> +
>  }
>  #else
>  #define skge_suspend NULL
This is reworked Jiri's patch:

[PATCH] sk98lin: handle pci_enable_device() return value in skge_resume()

Signed-off-by: Monakhov Dmitriy <dmonakhov@openvz.org>
---
 drivers/net/sk98lin/skge.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
index e94ab25..eea753a 100644
--- a/drivers/net/sk98lin/skge.c
+++ b/drivers/net/sk98lin/skge.c
@@ -5125,7 +5125,12 @@ static int skge_resume(struct pci_dev *pdev)
 
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
-	pci_enable_device(pdev);
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		printk(KERN_WARNING "sk98lin: unable to enable device %s "
+				"in resume\n", dev->name);
+		goto err_out;
+	}	
 	pci_set_master(pdev);
 	if (pAC->GIni.GIMacsFound == 2)
 		ret = request_irq(dev->irq, SkGeIsr, IRQF_SHARED, "sk98lin", dev);
@@ -5133,10 +5138,8 @@ static int skge_resume(struct pci_dev *pdev)
 		ret = request_irq(dev->irq, SkGeIsrOnePort, IRQF_SHARED, "sk98lin", dev);
 	if (ret) {
 		printk(KERN_WARNING "sk98lin: unable to acquire IRQ %d\n", dev->irq);
-		pAC->AllocFlag &= ~SK_ALLOC_IRQ;
-		dev->irq = 0;
-		pci_disable_device(pdev);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto err_out_disable_pdev;
 	}
 
 	netif_device_attach(dev);
@@ -5153,6 +5156,13 @@ static int skge_resume(struct pci_dev *pdev)
 	}
 
 	return 0;
+
+err_out_disable_pdev:
+	pci_disable_device(pdev);
+err_out:
+	pAC->AllocFlag &= ~SK_ALLOC_IRQ;
+	dev->irq = 0;
+	return ret;
 }
 #else
 #define skge_suspend NULL
-- 
1.4.4.4




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

* Re: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume()
  2007-02-23 23:17             ` [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() Dmitriy Monakhov
@ 2007-02-24 11:41               ` Dmitriy Monakhov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitriy Monakhov @ 2007-02-24 11:41 UTC (permalink / raw)
  To: Dmitriy Monakhov
  Cc: Jiri Kosina, Andrew Morton, Stephen Hemminger, mlindner,
	rroesler, Jeff Garzik, linux-kernel, netdev

Dmitriy Monakhov <dmonakhov@openvz.org> writes:

> This thread looks dead but issue was't fixed.
>
> Jiri Kosina <jikos@jikos.cz> writes:
>>> > -	pci_enable_device(pdev);
>>> > +	ret = pci_enable_device(pdev);
>>> > +	if (ret) {
>>> > +		printk(KERN_ERR "sk98lin: Cannot enable PCI device %s during resume\n", 
>>> > +				dev->name);
>>> > +		unregister_netdev(dev);
>>> This looks rather wrong - skge_exit() will run unregister_netdev() again.
>>
>> You are of course right (the problem was also spotted by Russell King). 
>> This I believe is the correct one for the sk98lin case.
>>
>> [PATCH] fix sk98lin driver, ignoring return value from pci_enable_device()
>>
>> add check of return value to _resume() function of sk98lin driver.
>>
>> Signed-off-by: Jiri Kosina <jikos@jikos.cz>
>>
>> ---
>>
>> --- a/drivers/net/sk98lin/skge.c
>> +++ b/drivers/net/sk98lin/skge.c
>> @@ -5070,7 +5070,12 @@ static int skge_resume(struct pci_dev *p
>>  
>>  	pci_set_power_state(pdev, PCI_D0);
>>  	pci_restore_state(pdev);
>> -	pci_enable_device(pdev);
>> +	ret = pci_enable_device(pdev);
>> +	if (ret) {
>> +		printk(KERN_WARNING "sk98lin: unable to enable device %s in resume\n",
>> +				dev->name);
>> +		goto out_err;
>> +	}	
> [snip]
>> +out_err:
>> +	pAC->AllocFlag &= ~SK_ALLOC_IRQ;
>> +	dev->irq = 0;
>> +	pci_disable_device(pdev);
> <<<<< Ok what happend if we jump here right after pci_disable_device() has 
 OOPs.....Of course i mean pci_enable_device() here^^^^^^^^^^^^^^^^^^^^^
 I'm sorry about this. (Option complete_word not always good, some times 
 brain work required too :) )
 So correct comment looks like:
  <<<<< Ok what happend if we jump here right after pci_enable_device() has
        failed, but pci_disable_device() was called anyway, this is wrong and 
        may be fatal because pdev->enable_cnt may becomes negative.  
>> +
>> +	return ret;
>> +
>>  }
>>  #else
>>  #define skge_suspend NULL
> This is reworked Jiri's patch:
>
> [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume()
>
> Signed-off-by: Monakhov Dmitriy <dmonakhov@openvz.org>
> ---
>  drivers/net/sk98lin/skge.c |   20 +++++++++++++++-----
>  1 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
> index e94ab25..eea753a 100644
> --- a/drivers/net/sk98lin/skge.c
> +++ b/drivers/net/sk98lin/skge.c
> @@ -5125,7 +5125,12 @@ static int skge_resume(struct pci_dev *pdev)
>  
>  	pci_set_power_state(pdev, PCI_D0);
>  	pci_restore_state(pdev);
> -	pci_enable_device(pdev);
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		printk(KERN_WARNING "sk98lin: unable to enable device %s "
> +				"in resume\n", dev->name);
> +		goto err_out;
> +	}	
>  	pci_set_master(pdev);
>  	if (pAC->GIni.GIMacsFound == 2)
>  		ret = request_irq(dev->irq, SkGeIsr, IRQF_SHARED, "sk98lin", dev);
> @@ -5133,10 +5138,8 @@ static int skge_resume(struct pci_dev *pdev)
>  		ret = request_irq(dev->irq, SkGeIsrOnePort, IRQF_SHARED, "sk98lin", dev);
>  	if (ret) {
>  		printk(KERN_WARNING "sk98lin: unable to acquire IRQ %d\n", dev->irq);
> -		pAC->AllocFlag &= ~SK_ALLOC_IRQ;
> -		dev->irq = 0;
> -		pci_disable_device(pdev);
> -		return -EBUSY;
> +		ret = -EBUSY;
> +		goto err_out_disable_pdev;
>  	}
>  
>  	netif_device_attach(dev);
> @@ -5153,6 +5156,13 @@ static int skge_resume(struct pci_dev *pdev)
>  	}
>  
>  	return 0;
> +
> +err_out_disable_pdev:
> +	pci_disable_device(pdev);
> +err_out:
> +	pAC->AllocFlag &= ~SK_ALLOC_IRQ;
> +	dev->irq = 0;
> +	return ret;
>  }
>  #else
>  #define skge_suspend NULL
> -- 
> 1.4.4.4
>
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


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

end of thread, other threads:[~2007-02-24 11:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-12 22:17 [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() properly Jiri Kosina
2006-10-12 22:25 ` Stephen Hemminger
2006-10-12 22:38   ` Jiri Kosina
2006-10-12 22:47     ` Stephen Hemminger
2006-10-12 22:57       ` Jiri Kosina
2006-10-13  0:50         ` Andrew Morton
2006-10-13  1:12           ` Jiri Kosina
2007-02-23 23:17             ` [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() Dmitriy Monakhov
2007-02-24 11:41               ` Dmitriy Monakhov

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.