Skip to contentSkip to conversation

misplaced pci_enable_device()s in drivers/sound/

PATCH In review Marcus Meissner 2 participants 3 messages Apr 18, 2001

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 misplaced pci_enable_device()s in drivers/sound/

Hi,

Several pci_enable_device()s in drivers/sound happen _after_ accessing PCI resources. I have moved them before the relevant first accesses.

(Untested, but should work.)

Against 2.4.3-ac9.

Ciao, Marcus

Index: es1370.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/es1370.c,v retrieving revision 1.13

es1370.c +3 -2
diff -u -r1.13 es1370.c
@@ -2569,6 +2569,9 @@
 	mm_segment_t fs;
 	int i, val;
 
+	if (pci_enable_device(pcidev))
+		return -1;
+
 	if (!RSRCISIOREGION(pcidev, 0))
 		return -1;
 	if (pcidev->irq == 0) 
@@ -2599,8 +2602,6 @@
 		printk(KERN_ERR "es1370: io ports %#lx-%#lx in use\n", s->io, s->io+ES1370_EXTENT-1);
 		goto err_region;
 	}
-	if (pci_enable_device(pcidev))
-		goto err_irq;
 	if (request_irq(s->irq, es1370_interrupt, SA_SHIRQ, "es1370", s)) {
 		printk(KERN_ERR "es1370: irq %u in use\n", s->irq);
 		goto err_irq;

Index: es1371.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/es1371.c,v retrieving revision 1.7

es1371.c +3 -2
diff -u -r1.7 es1371.c
@@ -2783,6 +2783,9 @@
 	signed long tmo2;
 	unsigned int cssr;
 
+	if (pci_enable_device(pcidev))
+		return -1;
+
 	if (!RSRCISIOREGION(pcidev, 0))
 		return -1;
 	if (pcidev->irq == 0) 
@@ -2822,8 +2825,6 @@
 		printk(KERN_ERR PFX "io ports %#lx-%#lx in use\n", s->io, s->io+ES1371_EXTENT-1);
 		goto err_region;
 	}
-	if (pci_enable_device(pcidev))
-		goto err_irq;
 	if (request_irq(s->irq, es1371_interrupt, SA_SHIRQ, "es1371", s)) {
 		printk(KERN_ERR PFX "irq %u in use\n", s->irq);
 		goto err_irq;

Index: esssolo1.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/esssolo1.c,v retrieving revision 1.7

esssolo1.c +2 -2
diff -u -r1.7 esssolo1.c
@@ -2297,6 +2297,8 @@
 	struct solo1_state *s;
 	struct pm_dev *pmdev;
 
+ 	if (pci_enable_device(pcidev))
+		return -1;
 	if (!RSRCISIOREGION(pcidev, 0) ||
 	    !RSRCISIOREGION(pcidev, 1) ||
 	    !RSRCISIOREGION(pcidev, 2) ||
@@ -2362,8 +2364,6 @@
 		printk(KERN_ERR "solo1: irq %u in use\n", s->irq);
 		goto err_irq;
 	}
- 	if (pci_enable_device(pcidev))
-		goto err_irq;
 	printk(KERN_INFO "solo1: joystick port at %#x\n", s->gameport.io+1);
 	/* register devices */
 	if ((s->dev_audio = register_sound_dsp(&solo1_audio_fops, -1)) < 0)

Index: maestro.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/maestro.c,v retrieving revision 1.4

maestro.c +3 -13
diff -u -r1.4 maestro.c
@@ -3321,6 +3321,9 @@
 	/* don't pick up weird modem maestros */
 	if(((pcidev->class >> 8) & 0xffff) != PCI_CLASS_MULTIMEDIA_AUDIO)
 		return 0;
+
+	if (pci_enable_device(pcidev))
+		return 0;
 			
 	iobase = SILLY_PCI_BASE_ADDRESS(pcidev); 
 
@@ -3406,19 +3409,6 @@
 	}
 	
 	ess = &card->channels[0];
-
-	if (pci_enable_device(pcidev)) {
-		printk (KERN_ERR "maestro: pci_enable_device() failed\n");
-		for (i = 0; i < NR_DSPS; i++) {
-			struct ess_state *s = &card->channels[i];
-			if (s->dev_audio != -1)
-				unregister_sound_dsp(s->dev_audio);
-		}
-		release_region(card->iobase, 256);
-		unregister_reboot_notifier(&maestro_nb);
-		kfree(card);
-		return 0;
-	}
 
 	/*
 	 *	Ok card ready. Begin setup proper

Index: sonicvibes.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/sonicvibes.c,v retrieving revision 1.7

sonicvibes.c +2 -2
diff -u -r1.7 sonicvibes.c
@@ -2508,6 +2508,8 @@
 	char *ddmaname;
 	unsigned ddmanamelen;
 
+	if (pci_enable_device(pcidev))
+		return -1;
 	if (!RSRCISIOREGION(pcidev, RESOURCE_SB) ||
 	    !RSRCISIOREGION(pcidev, RESOURCE_ENH) ||
 	    !RSRCISIOREGION(pcidev, RESOURCE_SYNTH) ||
@@ -2594,8 +2596,6 @@
 		printk(KERN_ERR "sv: gameport io ports in use\n");
 		s->gameport.io = s->gameport.size = 0;
 	}
-	if (pci_enable_device(pcidev))
-		goto err_irq;
 	/* initialize codec registers */
 	outb(0x80, s->ioenh + SV_CODEC_CONTROL); /* assert reset */
 	udelay(50);

Index: trident.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/trident.c,v retrieving revision 1.11

trident.c +3 -3
diff -u -r1.11 trident.c
@@ -3309,6 +3309,9 @@
 	struct trident_card *card;
 	u8 revision;
 
+	if (pci_enable_device(pci_dev))
+	    return -ENODEV;
+
 	if (pci_set_dma_mask(pci_dev, TRIDENT_DMA_MASK)) {
 		printk(KERN_ERR "trident: architecture does not support"
 		       " 30bit PCI busmaster DMA\n");
@@ -3322,9 +3325,6 @@
 		       iobase);
 		return -ENODEV;
 	}
-
-	if (pci_enable_device(pci_dev))
-	    return -ENODEV;
 
 	if ((card = kmalloc(sizeof(struct trident_card), GFP_KERNEL)) == NULL) {
 		printk(KERN_ERR "trident: out of memory\n");

Index: via82cxxx_audio.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/via82cxxx_audio.c,v retrieving revision 1.6

via82cxxx_audio.c +5 -5
diff -u -r1.6 via82cxxx_audio.c
@@ -3020,17 +3020,17 @@
 	if (printed_version++ == 0)
 		printk (KERN_INFO "Via 686a audio driver " VIA_VERSION "\n");
 
+	if (pci_enable_device (pdev)) {
+		rc = -EIO;
+		goto err_out_none;
+	}
+
 	if (!request_region (pci_resource_start (pdev, 0),
 	    		     pci_resource_len (pdev, 0),
 			     VIA_MODULE_NAME)) {
 		printk (KERN_ERR PFX "unable to obtain I/O resources, aborting\n");
 		rc = -EBUSY;
 		goto err_out;
-	}
-
-	if (pci_enable_device (pdev)) {
-		rc = -EIO;
-		goto err_out_none;
 	}
 
 	card = kmalloc (sizeof (*card), GFP_KERNEL);

Index: emu10k1/main.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/emu10k1/main.c,v retrieving revision 1.3

main.c +3 -5
diff -u -r1.3 main.c
@@ -613,6 +613,9 @@
 	struct emu10k1_card *card;
 	u32 subsysvid;
 
+	if (pci_enable_device(pci_dev))
+		return -ENODEV;
+
 	if ((card = kmalloc(sizeof(struct emu10k1_card), GFP_KERNEL)) == NULL) {
 		printk(KERN_ERR "emu10k1: out of memory\n");
 		return -ENOMEM;
@@ -621,11 +624,6 @@
 
 	if (pci_set_dma_mask(pci_dev, EMU10K1_DMA_MASK)) {
 		printk(KERN_ERR "emu10k1: architecture does not support 32bit PCI busmaster DMA\n");
-		kfree(card);
-		return -ENODEV;
-	}
-
-	if (pci_enable_device(pci_dev)) {
 		kfree(card);
 		return -ENODEV;
 	}
#1 lore raw
Quoted 2 lines · Marcus Meissner

Several pci_enable_device()s in drivers/sound happen _after_ accessing PCI resources. I have moved them before the relevant first accesses.

cool

Quoted 2 lines

if (!RSRCISIOREGION(pcidev, 0))
return -1;

can you replace this mess while you are cleaning stuff up. this, for example, should be
if (!(pci_resource_flags(pcidev,) & IORESOURCE_IO))

There is also pci_resource_start and pci_resource_len.

Quoted 1 line
        iobase = SILLY_PCI_BASE_ADDRESS(pcidev);

pci_resource_start

patch looks ok to me, but those would be nice additions...

Signature · 3 lines
Jeff Garzik       | "The universe is like a safe to which there is a
Building 1024     |  combination -- but the combination is locked up
MandrakeSoft      |  in the safe."    -- Peter DeVries
#2 lore raw
In reply to Marcus Meissner
Quoted 13 lines · Jeff Garzik
Quoted 2 lines · Marcus Meissner

Several pci_enable_device()s in drivers/sound happen _after_ accessing PCI resources. I have moved them before the relevant first accesses.

cool

Quoted 2 lines

if (!RSRCISIOREGION(pcidev, 0))
return -1;

can you replace this mess while you are cleaning stuff up. this, for example, should be
if (!(pci_resource_flags(pcidev,) & IORESOURCE_IO))

Quoted 5 lines

There is also pci_resource_start and pci_resource_len.

Quoted 1 line
        iobase = SILLY_PCI_BASE_ADDRESS(pcidev);

pci_resource_start

THere is also some pci dma stuff which probably got changed.

Quoted 1 line

patch looks ok to me, but those would be nice additions...

I am a bit afraid to break stuff doing this without testing, but I will do it for the cards I have access to.

Is there a 'HOW TO DO IT' PCI driver?

drivers/net/pci-skeleton.c looks up to date, is it?

Ciao, Marcus