public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] PC300 pci_enable_device fix
  2005-04-13 22:02 ` Ashok Raj
@ 2005-04-13 19:45   ` Marcelo Tosatti
  2005-04-14 12:39     ` Marcelo Tosatti
  2005-04-14 18:17   ` Bjorn Helgaas
  1 sibling, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2005-04-13 19:45 UTC (permalink / raw)
  To: Ashok Raj; +Cc: Bjorn Helgaas, pc300, lkml

On Wed, Apr 13, 2005 at 03:02:43PM -0700, Ashok Raj wrote:
> On Wed, Apr 13, 2005 at 02:31:43PM -0700, Bjorn Helgaas wrote:
> > 
> >    Call pci_enable_device() before looking at IRQ and resources.
> >    The driver requires this fix or the "pci=routeirq" workaround
> >    on 2.6.10 and later kernels.
> 
> 
> the failure cases dont seem to worry about pci_disable_device()?
> 
> in err_release_ram: etc?

Yep the failure paths were wrong before, but Bjorn's patch moves 
pci_enable_device() way up to the beginning of the function. 

The failure path's err_release_ram etc. wont touch the resources
without pci_enable_pci(), with the fix.

> >    Reported and tested by Artur Lipowski.
> > 
> >    Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> > 
> >    ===== drivers/net/wan/pc300_drv.c 1.24 vs edited =====
> >    --- 1.24/drivers/net/wan/pc300_drv.c    2004-12-29 12:25:16 -07:00
> >    +++ edited/drivers/net/wan/pc300_drv.c  2005-04-13 13:35:21 -06:00
> >    @@ -3439,6 +3439,9 @@
> >     #endif
> >            }
> > 
> >    +       if ((err = pci_enable_device(pdev)) != 0)
> >    +               return err;
> >    +
> >            card = (pc300_t *) kmalloc(sizeof(pc300_t), GFP_KERNEL);
> >            if (card == NULL) {
> >                    printk("PC300 found at RAM 0x%08lx, "
> >    @@ -3526,9 +3529,6 @@
> >                    err = -ENODEV;
> >                    goto err_release_ram;
> >            }
> >    -
> >    -       if ((err = pci_enable_device(pdev)) != 0)
> >    -               goto err_release_sca;
> > 
> >                  card->hw.plxbase       =       ioremap(card->hw.plxphys,
> >    card->hw.plxsize);
> >                  card->hw.rambase       =       ioremap(card->hw.ramphys,
> >    card->hw.alloc_ramsize);

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

* [PATCH] PC300 pci_enable_device fix
@ 2005-04-13 21:31 Bjorn Helgaas
  2005-04-13 22:02 ` Ashok Raj
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2005-04-13 21:31 UTC (permalink / raw)
  To: pc300; +Cc: lkml

Call pci_enable_device() before looking at IRQ and resources.
The driver requires this fix or the "pci=routeirq" workaround
on 2.6.10 and later kernels.

Reported and tested by Artur Lipowski.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

===== drivers/net/wan/pc300_drv.c 1.24 vs edited =====
--- 1.24/drivers/net/wan/pc300_drv.c	2004-12-29 12:25:16 -07:00
+++ edited/drivers/net/wan/pc300_drv.c	2005-04-13 13:35:21 -06:00
@@ -3439,6 +3439,9 @@
 #endif
 	}
 
+	if ((err = pci_enable_device(pdev)) != 0)
+		return err;
+
 	card = (pc300_t *) kmalloc(sizeof(pc300_t), GFP_KERNEL);
 	if (card == NULL) {
 		printk("PC300 found at RAM 0x%08lx, "
@@ -3526,9 +3529,6 @@
 		err = -ENODEV;
 		goto err_release_ram;
 	}
-
-	if ((err = pci_enable_device(pdev)) != 0)
-		goto err_release_sca;
 
 	card->hw.plxbase = ioremap(card->hw.plxphys, card->hw.plxsize);
 	card->hw.rambase = ioremap(card->hw.ramphys, card->hw.alloc_ramsize);



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

* Re: [PATCH] PC300 pci_enable_device fix
  2005-04-13 21:31 [PATCH] PC300 pci_enable_device fix Bjorn Helgaas
@ 2005-04-13 22:02 ` Ashok Raj
  2005-04-13 19:45   ` Marcelo Tosatti
  2005-04-14 18:17   ` Bjorn Helgaas
  0 siblings, 2 replies; 6+ messages in thread
From: Ashok Raj @ 2005-04-13 22:02 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: pc300, lkml

On Wed, Apr 13, 2005 at 02:31:43PM -0700, Bjorn Helgaas wrote:
> 
>    Call pci_enable_device() before looking at IRQ and resources.
>    The driver requires this fix or the "pci=routeirq" workaround
>    on 2.6.10 and later kernels.


the failure cases dont seem to worry about pci_disable_device()?

in err_release_ram: etc?

> 
>    Reported and tested by Artur Lipowski.
> 
>    Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
> 
>    ===== drivers/net/wan/pc300_drv.c 1.24 vs edited =====
>    --- 1.24/drivers/net/wan/pc300_drv.c    2004-12-29 12:25:16 -07:00
>    +++ edited/drivers/net/wan/pc300_drv.c  2005-04-13 13:35:21 -06:00
>    @@ -3439,6 +3439,9 @@
>     #endif
>            }
> 
>    +       if ((err = pci_enable_device(pdev)) != 0)
>    +               return err;
>    +
>            card = (pc300_t *) kmalloc(sizeof(pc300_t), GFP_KERNEL);
>            if (card == NULL) {
>                    printk("PC300 found at RAM 0x%08lx, "
>    @@ -3526,9 +3529,6 @@
>                    err = -ENODEV;
>                    goto err_release_ram;
>            }
>    -
>    -       if ((err = pci_enable_device(pdev)) != 0)
>    -               goto err_release_sca;
> 
>                  card->hw.plxbase       =       ioremap(card->hw.plxphys,
>    card->hw.plxsize);
>                  card->hw.rambase       =       ioremap(card->hw.ramphys,
>    card->hw.alloc_ramsize);
> 
>    -
>    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  [1]http://vger.kernel.org/majordomo-info.html
>    Please read the FAQ at  [2]http://www.tux.org/lkml/
> 
> References
> 
>    1. http://vger.kernel.org/majordomo-info.html
>    2. http://www.tux.org/lkml/

-- 
Cheers,
Ashok Raj
- Open Source Technology Center

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

* Re: [PATCH] PC300 pci_enable_device fix
  2005-04-13 19:45   ` Marcelo Tosatti
@ 2005-04-14 12:39     ` Marcelo Tosatti
  0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Tosatti @ 2005-04-14 12:39 UTC (permalink / raw)
  To: Ashok Raj, germano.barreiro; +Cc: Bjorn Helgaas, pc300, lkml


Germano, can you give this patch a try before merging, please.

On Wed, Apr 13, 2005 at 04:45:31PM -0300, Marcelo Tosatti wrote:
> On Wed, Apr 13, 2005 at 03:02:43PM -0700, Ashok Raj wrote:
> > On Wed, Apr 13, 2005 at 02:31:43PM -0700, Bjorn Helgaas wrote:
> > > 
> > >    Call pci_enable_device() before looking at IRQ and resources.
> > >    The driver requires this fix or the "pci=routeirq" workaround
> > >    on 2.6.10 and later kernels.
> > 
> > 
> > the failure cases dont seem to worry about pci_disable_device()?
> > 
> > in err_release_ram: etc?
> 
> Yep the failure paths were wrong before, but Bjorn's patch moves 
> pci_enable_device() way up to the beginning of the function. 
> 
> The failure path's err_release_ram etc. wont touch the resources
> without pci_enable_pci(), with the fix.

Hi, 

I see what you mean now, Ashok. Yes, you are right, the error path lacks
pci_disable_device().

Reported by Artur Lipowski.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Marcelo Tosatti <marcelo.tosatti@cyclades.com>

--- a/drivers/net/wan/pc300_drv.c.orig	2005-04-14 13:56:04.000000000 -0300
+++ b/drivers/net/wan/pc300_drv.c	2005-04-14 13:58:56.000000000 -0300
@@ -3439,11 +3439,15 @@
 #endif
 	}
 
+	if ((err = pci_enable_device(pdev)) != 0)
+		return err;
+
 	card = (pc300_t *) kmalloc(sizeof(pc300_t), GFP_KERNEL);
 	if (card == NULL) {
 		printk("PC300 found at RAM 0x%08lx, "
 		       "but could not allocate card structure.\n",
 		       pci_resource_start(pdev, 3));
+		pci_disable_device(pdev);
 		return -ENOMEM;
 	}
 	memset(card, 0, sizeof(pc300_t));
@@ -3527,8 +3531,6 @@
 		goto err_release_ram;
 	}
 
-	if ((err = pci_enable_device(pdev)) != 0)
-		goto err_release_sca;
 
 	card->hw.plxbase = ioremap(card->hw.plxphys, card->hw.plxsize);
 	card->hw.rambase = ioremap(card->hw.ramphys, card->hw.alloc_ramsize);
@@ -3628,6 +3630,7 @@
 err_release_io:
 	release_region(card->hw.iophys, card->hw.iosize);
 	kfree(card);
+	pci_disable_device(pdev);
 	return -ENODEV;
 }
 

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

* Re: [PATCH] PC300 pci_enable_device fix
  2005-04-13 22:02 ` Ashok Raj
  2005-04-13 19:45   ` Marcelo Tosatti
@ 2005-04-14 18:17   ` Bjorn Helgaas
  1 sibling, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2005-04-14 18:17 UTC (permalink / raw)
  To: Ashok Raj; +Cc: pc300, lkml, Marcelo Tosatti

On Wed, 2005-04-13 at 15:02 -0700, Ashok Raj wrote:
> On Wed, Apr 13, 2005 at 02:31:43PM -0700, Bjorn Helgaas wrote:
> > 
> >    Call pci_enable_device() before looking at IRQ and resources.
> >    The driver requires this fix or the "pci=routeirq" workaround
> >    on 2.6.10 and later kernels.

> the failure cases dont seem to worry about pci_disable_device()?
> in err_release_ram: etc?

True.  The failure paths were broken before, but here's a
revised patch that adds pci_disable_device() to them as
well.  Artur has tested this patch, too.


Call pci_enable_device() before looking at IRQ and resources.
The driver requires this fix or the "pci=routeirq" workaround
on 2.6.10 and later kernels.

Reported and tested by Artur Lipowski.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

===== drivers/net/wan/pc300_drv.c 1.24 vs edited =====
--- 1.24/drivers/net/wan/pc300_drv.c	2004-12-29 12:25:16 -07:00
+++ edited/drivers/net/wan/pc300_drv.c	2005-04-13 16:14:19 -06:00
@@ -3427,7 +3427,7 @@
 {
 	static int first_time = 1;
 	ucchar cpc_rev_id;
-	int err = 0, eeprom_outdated = 0;
+	int err, eeprom_outdated = 0;
 	ucshort device_id;
 	pc300_t *card;
 
@@ -3439,15 +3439,21 @@
 #endif
 	}
 
+	if ((err = pci_enable_device(pdev)) < 0)
+		return err;
+
 	card = (pc300_t *) kmalloc(sizeof(pc300_t), GFP_KERNEL);
 	if (card == NULL) {
 		printk("PC300 found at RAM 0x%08lx, "
 		       "but could not allocate card structure.\n",
 		       pci_resource_start(pdev, 3));
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto err_disable_dev;
 	}
 	memset(card, 0, sizeof(pc300_t));
 
+	err = -ENODEV;
+
 	/* read PCI configuration area */
 	device_id = ent->device;
 	card->hw.irq = pdev->irq;
@@ -3507,7 +3513,6 @@
 		printk("PC300 found at RAM 0x%08x, "
 		       "but could not allocate PLX mem region.\n",
 		       card->hw.ramphys);
-		err = -ENODEV;
 		goto err_release_io;
 	}
 	if (!request_mem_region(card->hw.ramphys, card->hw.alloc_ramsize,
@@ -3515,7 +3520,6 @@
 		printk("PC300 found at RAM 0x%08x, "
 		       "but could not allocate RAM mem region.\n",
 		       card->hw.ramphys);
-		err = -ENODEV;
 		goto err_release_plx;
 	}
 	if (!request_mem_region(card->hw.scaphys, card->hw.scasize,
@@ -3523,13 +3527,9 @@
 		printk("PC300 found at RAM 0x%08x, "
 		       "but could not allocate SCA mem region.\n",
 		       card->hw.ramphys);
-		err = -ENODEV;
 		goto err_release_ram;
 	}
 
-	if ((err = pci_enable_device(pdev)) != 0)
-		goto err_release_sca;
-
 	card->hw.plxbase = ioremap(card->hw.plxphys, card->hw.plxsize);
 	card->hw.rambase = ioremap(card->hw.ramphys, card->hw.alloc_ramsize);
 	card->hw.scabase = ioremap(card->hw.scaphys, card->hw.scasize);
@@ -3619,7 +3619,6 @@
 		iounmap(card->hw.falcbase);
 		release_mem_region(card->hw.falcphys, card->hw.falcsize);
 	}
-err_release_sca:
 	release_mem_region(card->hw.scaphys, card->hw.scasize);
 err_release_ram:
 	release_mem_region(card->hw.ramphys, card->hw.alloc_ramsize);
@@ -3628,7 +3627,9 @@
 err_release_io:
 	release_region(card->hw.iophys, card->hw.iosize);
 	kfree(card);
-	return -ENODEV;
+err_disable_dev:
+	pci_disable_device(pdev);
+	return err;
 }
 
 static void __devexit cpc_remove_one(struct pci_dev *pdev)



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

* [PATCH] PC300 pci_enable_device fix
@ 2005-04-26 14:41 Marcelo Tosatti
  0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Tosatti @ 2005-04-26 14:41 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, Bjorn Helgaas, Ashok Raj


Call pci_enable_device() before looking at IRQ and resources, 
and pci_disable_device() when shutting the interface down.

The driver requires this fix or the "pci=routeirq" workaround
on 2.6.10 and later kernels.
                                                                                                                                                                                   
Reported and tested by Artur Lipowski.
                                                                                                                                                                                   
From: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Marcelo Tosatti <marcelo.tosatti@cyclades.com>

Index: drivers/net/wan/pc300_drv.c
===================================================================
--- 5b6486ded5188e41ac9bc81ad4a5e2bd746f7ede/drivers/net/wan/pc300_drv.c  (mode:100644 sha1:d67be2587d4d33879d479f331ad4cb9d3ac33f75)
+++ uncommitted/drivers/net/wan/pc300_drv.c  (mode:100664)
@@ -3427,7 +3427,7 @@
 {
 	static int first_time = 1;
 	ucchar cpc_rev_id;
-	int err = 0, eeprom_outdated = 0;
+	int err, eeprom_outdated = 0;
 	ucshort device_id;
 	pc300_t *card;
 
@@ -3439,15 +3439,21 @@
 #endif
 	}
 
+	if ((err = pci_enable_device(pdev)) < 0)
+		return err;
+
 	card = (pc300_t *) kmalloc(sizeof(pc300_t), GFP_KERNEL);
 	if (card == NULL) {
 		printk("PC300 found at RAM 0x%08lx, "
 		       "but could not allocate card structure.\n",
 		       pci_resource_start(pdev, 3));
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto err_disable_dev;
 	}
 	memset(card, 0, sizeof(pc300_t));
 
+	err = -ENODEV;
+
 	/* read PCI configuration area */
 	device_id = ent->device;
 	card->hw.irq = pdev->irq;
@@ -3507,7 +3513,6 @@
 		printk("PC300 found at RAM 0x%08x, "
 		       "but could not allocate PLX mem region.\n",
 		       card->hw.ramphys);
-		err = -ENODEV;
 		goto err_release_io;
 	}
 	if (!request_mem_region(card->hw.ramphys, card->hw.alloc_ramsize,
@@ -3515,7 +3520,6 @@
 		printk("PC300 found at RAM 0x%08x, "
 		       "but could not allocate RAM mem region.\n",
 		       card->hw.ramphys);
-		err = -ENODEV;
 		goto err_release_plx;
 	}
 	if (!request_mem_region(card->hw.scaphys, card->hw.scasize,
@@ -3523,13 +3527,9 @@
 		printk("PC300 found at RAM 0x%08x, "
 		       "but could not allocate SCA mem region.\n",
 		       card->hw.ramphys);
-		err = -ENODEV;
 		goto err_release_ram;
 	}
 
-	if ((err = pci_enable_device(pdev)) != 0)
-		goto err_release_sca;
-
 	card->hw.plxbase = ioremap(card->hw.plxphys, card->hw.plxsize);
 	card->hw.rambase = ioremap(card->hw.ramphys, card->hw.alloc_ramsize);
 	card->hw.scabase = ioremap(card->hw.scaphys, card->hw.scasize);
@@ -3619,7 +3619,6 @@
 		iounmap(card->hw.falcbase);
 		release_mem_region(card->hw.falcphys, card->hw.falcsize);
 	}
-err_release_sca:
 	release_mem_region(card->hw.scaphys, card->hw.scasize);
 err_release_ram:
 	release_mem_region(card->hw.ramphys, card->hw.alloc_ramsize);
@@ -3628,7 +3627,9 @@
 err_release_io:
 	release_region(card->hw.iophys, card->hw.iosize);
 	kfree(card);
-	return -ENODEV;
+err_disable_dev:
+	pci_disable_device(pdev);
+	return err;
 }
 
 static void __devexit cpc_remove_one(struct pci_dev *pdev)
@@ -3662,6 +3663,7 @@
 		if (card->hw.irq)
 			free_irq(card->hw.irq, card);
 		kfree(card);
+		pci_disable_device(pdev);
 	}
 }
 



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

end of thread, other threads:[~2005-04-26 19:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-04-13 21:31 [PATCH] PC300 pci_enable_device fix Bjorn Helgaas
2005-04-13 22:02 ` Ashok Raj
2005-04-13 19:45   ` Marcelo Tosatti
2005-04-14 12:39     ` Marcelo Tosatti
2005-04-14 18:17   ` Bjorn Helgaas
  -- strict thread matches above, loose matches on Subject: below --
2005-04-26 14:41 Marcelo Tosatti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox