Re: [PATCH] es1371 pci fix/cleanup

From: Marcus Meissner (Marcus.Meissner@caldera.de)
Date: Mon Apr 23 2001 - 11:06:53 EST


On Mon, Apr 23, 2001 at 11:58:25AM -0400, Jeff Garzik wrote:
> Marcus Meissner wrote:
> >
> > 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
--- drivers/sound/es1371.c 2001/04/17 17:26:05 1.7
+++ drivers/sound/es1371.c 2001/04/23 16:03:34
@@ -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");
@@ -2794,7 +2794,7 @@
         }
         if (!(s = kmalloc(sizeof(struct es1371_state), GFP_KERNEL))) {
                 printk(KERN_WARNING PFX "out of memory\n");
- return -1;
+ return -ENOMEM;
         }
         memset(s, 0, sizeof(struct es1371_state));
         init_waitqueue_head(&s->dma_adc.wait);
@@ -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;
@@ -2964,7 +2962,7 @@
         release_region(s->io, ES1371_EXTENT);
  err_region:
         kfree(s);
- return -1;
+ return -ENODEV;
 }
 
 static void __devinit es1371_remove(struct pci_dev *dev)
-
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 http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Apr 23 2001 - 21:00:46 EST