All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitriy Monakhov <dmonakhov@openvz.org>
To: Jiri Kosina <jikos@jikos.cz>
Cc: Andrew Morton <akpm@osdl.org>,
	Stephen Hemminger <shemminger@osdl.org>,
	mlindner@syskonnect.de, rroesler@syskonnect.de,
	Jeff Garzik <jeff@garzik.org>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume()
Date: Sat, 24 Feb 2007 02:17:19 +0300	[thread overview]
Message-ID: <87hctcv4zk.fsf_-_@sw.ru> (raw)
In-Reply-To: <Pine.LNX.4.64.0610130310120.29022@twin.jikos.cz> (Jiri Kosina's message of "Fri, 13 Oct 2006 03:12:55 +0200 (CEST)")

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




  reply	other threads:[~2007-02-23 23:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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             ` Dmitriy Monakhov [this message]
2007-02-24 11:41               ` [PATCH] sk98lin: handle pci_enable_device() return value in skge_resume() Dmitriy Monakhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87hctcv4zk.fsf_-_@sw.ru \
    --to=dmonakhov@openvz.org \
    --cc=akpm@osdl.org \
    --cc=jeff@garzik.org \
    --cc=jikos@jikos.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlindner@syskonnect.de \
    --cc=netdev@vger.kernel.org \
    --cc=rroesler@syskonnect.de \
    --cc=shemminger@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.