Skip to contentSkip to conversation

PC300 pci_enable_device fix

PATCH In review Bjorn Helgaas 3 participants 5 messages Apr 13, 2005 – Apr 14, 2005

Subsystems (2)
Network drivers Cyclades PC300 driver
lore
Share & tools
lore

Bjorn Helgaas

author
#0 lore raw

PATCH 1/1 PC300 pci_enable_device fix

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 <[email protected]>

===== drivers/net/wan/pc300_drv.c 1.24 vs edited =====

drivers/net/wan/pc300_drv.c +3 -3
@@ -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);
#1 lore raw
Quoted 4 lines · Bjorn Helgaas

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?

Quoted 42 lines
   Reported and tested by Artur Lipowski.

   Signed-off-by: Bjorn Helgaas <[email protected]>

   ===== 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 [email protected]
   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/

Signature · 3 lines
Cheers,
Ashok Raj
- Open Source Technology Center
#2 lore raw
Quoted 10 lines · Ashok Raj
Quoted 4 lines · Bjorn Helgaas

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.

Quoted 29 lines
Quoted 29 lines
   Reported and tested by Artur Lipowski.

   Signed-off-by: Bjorn Helgaas <[email protected]>

   ===== 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);
#3 lore raw

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

Quoted 10 lines · Marcelo Tosatti
Quoted 9 lines · Ashok Raj
Quoted 4 lines · Bjorn Helgaas

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()?

Quoted 7 lines
Quoted 1 line

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 <[email protected]>
Signed-off-by Marcelo Tosatti <[email protected]>
drivers/net/wan/pc300_drv.c renamed from drivers/net/wan/pc300_drv.c.orig +5 -2
@@ -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;
 }
author
#4 lore raw
In reply to Ashok Raj
Quoted 8 lines · Ashok Raj
Quoted 4 lines · Bjorn Helgaas

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 <[email protected]>

===== drivers/net/wan/pc300_drv.c 1.24 vs edited =====

drivers/net/wan/pc300_drv.c +11 -10
@@ -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)