es1371 pci fix/cleanup

Hi,

This moves pci_enable_device in the es1371 driver before any resource access and also replaces the RSRCISIOREGION by just pci_resource_flags as suggested by Jeff.

Tested and verified.

Ciao, Marcus

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

sound/es1371.c 2001/04/23 15:49:15 renamed from sound/es1371.c 2001/04/17 17:26:05 1.7 +0 -3

@@ -2771,9 +2771,6 @@
        { SOUND_MIXER_WRITE_IGAIN, 0x4040 }
 };
 
-#define RSRCISIOREGION(dev,num) (pci_resource_start((dev), (num)) != 0 && \
-                                (pci_resource_flags((dev), (num)) & IORESOURCE_IO))
-
 static int __devinit es1371_probe(struct pci_dev *pcidev, const struct pci_device_id *pciid)
 {
        struct es1371_state *s;

@@ -2783,8 +2780,11 @@
        signed long tmo2;

Quoted 8 lines · Marcus Meissner

Hi,

This moves pci_enable_device in the es1371 driver before any resource access and also replaces the RSRCISIOREGION by just pci_resource_flags as suggested by Jeff.

Tested and verified.

Looks ok except error returns.

pci_enable_device - obtain its return value, and return that.

no IORESOURCE_IO or pcidev->irq==0 - I guess -ENODEV would be appropriate. (basically look at errno.h and make a judgement call which error best fits the situation)

Hmm, I think I spotted all places in the probe function. I also return -ENODEV in case we can't request_region() or request_irq().

Some drivers use EBUSY, some ENOMEM, some ENODEV there, is there any standard return value?

Ciao, Marcus

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

sound/es1371.c 2001/04/23 16:03:34 renamed from sound/es1371.c 2001/04/17 17:26:05 1.7 +7 -7

@@ -2771,22 +2771,22 @@
        { SOUND_MIXER_WRITE_IGAIN, 0x4040 }
 };
 
-#define RSRCISIOREGION(dev,num) (pci_resource_start((dev), (num)) != 0 && \
-                                (pci_resource_flags((dev), (num)) & IORESOURCE_IO))
-
 static int __devinit es1371_probe(struct pci_dev *pcidev, const struct pci_device_id *pciid)
 {
        struct es1371_state *s;
        mm_segment_t fs;
-       int i, val;
+       int i, val, res = -1;
        unsigned long tmo;
        signed long tmo2;
        unsigned int cssr;
+
+       if ((res=pci_enable_device(pcidev)))
+               return res;
 
-       if (!RSRCISIOREGION(pcidev, 0))
-               return -1;
+       if (!(pci_resource_flags(pcidev, 0) & IORESOURCE_IO))
+               return -ENODEV;
        if (pcidev->irq == 0)
-               return -1;
+               return -ENODEV;
        i = pci_set_dma_mask(pcidev, 0xffffffff);
        if (i) {
                printk(KERN_WARNING "es1371: architecture does not support 32bit PCI busmaster DMA\n");

sound/es1371.c 2001/04/23 16:03:34 renamed from sound/es1371.c 2001/04/17 17:26:05 1.7 +0 -2

@@ -2822,8 +2822,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;