On 2012-05-29 15:45, Dan Carpenter wrote:
On Tue, May 29, 2012 at 02:49:35PM +0100, Ian Abbott wrote:
Implement the attach_pci() hook as function pc236_attach_pci(). This is
called bu comedi_pci_auto_config() in preference to the old attach()
hook (implemented by pc236_attach()) and avoids searching for the probed
PCI device.
Factor out code common to pc236_find_pci() and pc236_attach_pci() into
new function pc236_find_pci_board(). Factor out most code common to
pc236_attach() and pc236_attach_pci() into new functions
pc236_pci_common_attach() and pc236_common_attach().
Also #if out member 'devid' from struct pc236_board unless PCI boards
are supported as it is not used for ISA boards.
I can't tell you how much I hate these ifdefs. :( They're every
where... Who is it that cares about cares so much about 3 extra
unsigned shorts?
The rules are that #ifdefs are only ok in .h files and not in .c
files. We should be removing them and not adding them if we want
to move this code out of staging.
As an experiment, with the following patch (sorry for the line-wrapping
which prevents it being applied) and with CONFIG_COMEDI_AMPLC_PC236_ISA
defined and CONFIG_COMEDI_AMPLC_PC236_PCI not defined, the size of the
amplc_pc236.ko increased in size from 5785 bytes to 6215 bytes and there
was a compiler warning about pc236_attach_pci being defined but not used
(this is what the .attach_pci hook in struct comedi_driver
amplc_pc236_driver would get set to, but that is still conditionally
compiled out). If I set the .attach_pci hook to pc236_attach_pci
regardless, it gets rid of the warning, but the size of the
amplc_pc236.ko file increased to 6776 bytes as more code is reachable in
theory although not in practice as the PCI probe code that calls this
function indirectly is still conditionally compiled out.
(I'm building for i386 with a gcc 4.5.3 compiler.)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c
b/drivers/staging/comedi/drivers/amplc_pc236.c
index e0960ce..f72bdc4 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -100,9 +100,7 @@ enum pc236_model { pc36at_model, pci236_model,
anypci_model };
struct pc236_board {
const char *name;
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
unsigned short devid;
-#endif
enum pc236_bustype bustype;
enum pc236_model model;
};
@@ -135,18 +133,15 @@ static const struct pc236_board pc236_boards[] = {
feel free to suggest moving the variable to the struct
comedi_device struct.
*/
struct pc236_private {
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
/* PCI device */
struct pci_dev *pci_dev;
unsigned long lcr_iobase; /* PLX PCI9052 config registers in PCIBAR1 */
-#endif
int enable_irq;
};
/*
* This function looks for a board matching the supplied PCI device.
*/
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
static const struct pc236_board *pc236_find_pci_board(struct pci_dev
*pci_dev)
{
unsigned int i;
@@ -157,13 +152,11 @@ static const struct pc236_board
*pc236_find_pci_board(struct pci_dev *pci_dev)
return &pc236_boards[i];
return NULL;
}
-#endif
/*
* This function looks for a PCI device matching the requested board name,
* bus and slot.
*/
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
struct pci_dev *
pc236_find_pci(struct comedi_device *dev, int bus, int slot)
{
@@ -209,13 +202,11 @@ pc236_find_pci(struct comedi_device *dev, int bus,
int slot)
}
return NULL;
}
-#endif
/*
* This function checks and requests an I/O region, reporting an error
* if there is a conflict.
*/
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA)
static int pc236_request_region(struct comedi_device *dev, unsigned
long from,
unsigned long extent)
{
@@ -226,7 +217,6 @@ static int pc236_request_region(struct comedi_device
*dev, unsigned long from,
}
return 0;
}
-#endif
/*
* This function is called to mark the interrupt as disabled (no command
@@ -240,10 +230,8 @@ static void pc236_intr_disable(struct comedi_device
*dev)
spin_lock_irqsave(&dev->spinlock, flags);
devpriv->enable_irq = 0;
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
- if (devpriv->lcr_iobase)
+ if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI) && devpriv->lcr_iobase)
outl(PCI236_INTR_DISABLE, devpriv->lcr_iobase + PLX9052_INTCSR);
-#endif
spin_unlock_irqrestore(&dev->spinlock, flags);
}
@@ -259,10 +247,8 @@ static void pc236_intr_enable(struct comedi_device
*dev)
spin_lock_irqsave(&dev->spinlock, flags);
devpriv->enable_irq = 1;
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
- if (devpriv->lcr_iobase)
+ if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI) && devpriv->lcr_iobase)
outl(PCI236_INTR_ENABLE, devpriv->lcr_iobase + PLX9052_INTCSR);
-#endif
spin_unlock_irqrestore(&dev->spinlock, flags);
}
@@ -282,8 +268,8 @@ static int pc236_intr_check(struct comedi_device *dev)
spin_lock_irqsave(&dev->spinlock, flags);
if (devpriv->enable_irq) {
retval = 1;
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
- if (devpriv->lcr_iobase) {
+ if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI) &&
+ devpriv->lcr_iobase) {
if ((inl(devpriv->lcr_iobase + PLX9052_INTCSR)
& PLX9052_INTCSR_LI1STAT_MASK)
== PLX9052_INTCSR_LI1STAT_INACTIVE) {
@@ -294,7 +280,6 @@ static int pc236_intr_check(struct comedi_device *dev)
devpriv->lcr_iobase + PLX9052_INTCSR);
}
}
-#endif
}
spin_unlock_irqrestore(&dev->spinlock, flags);
@@ -439,26 +424,18 @@ static void pc236_report_attach(struct
comedi_device *dev, unsigned int irq)
char tmpbuf[60];
int tmplen;
- switch (thisboard->bustype) {
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA)
- case isa_bustype:
+ if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA) &&
+ thisboard->bustype == isa_bustype)
tmplen = scnprintf(tmpbuf, sizeof(tmpbuf),
"(base %#lx) ", dev->iobase);
- break;
-#endif
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
- case pci_bustype: {
- struct pc236_private *devpriv = dev->private;
- struct pci_dev *pci_dev = devpriv->pci_dev;
- tmplen = scnprintf(tmpbuf, sizeof(tmpbuf),
- "(pci %s) ", pci_name(pci_dev));
- }
- break;
-#endif
- default:
+ else if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI) &&
+ thisboard->bustype == pci_bustype) {
+ struct pc236_private *devpriv = dev->private;
+ struct pci_dev *pci_dev = devpriv->pci_dev;
+ tmplen = scnprintf(tmpbuf, sizeof(tmpbuf),
+ "(pci %s) ", pci_name(pci_dev));
+ } else
tmplen = 0;
- break;
- }
if (irq)
tmplen += scnprintf(&tmpbuf[tmplen], sizeof(tmpbuf) - tmplen,
"(irq %u%s) ", irq,
@@ -516,7 +493,6 @@ static int pc236_common_attach(struct comedi_device
*dev, unsigned long iobase,
return 1;
}
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
static int pc236_pci_common_attach(struct comedi_device *dev,
struct pci_dev *pci_dev)
{
@@ -535,7 +511,6 @@ static int pc236_pci_common_attach(struct
comedi_device *dev,
iobase = pci_resource_start(pci_dev, 2);
return pc236_common_attach(dev, iobase, pci_dev->irq, IRQF_SHARED);
}
-#endif
/*
* Attach is called by the Comedi core to configure the driver
@@ -555,37 +530,29 @@ static int pc236_attach(struct comedi_device *dev,
struct comedi_devconfig *it)
return ret;
}
/* Process options according to bus type. */
- switch (thisboard->bustype) {
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA)
- case isa_bustype: {
- unsigned long iobase = it->options[0];
- unsigned int irq = it->options[1];
- ret = pc236_request_region(dev, iobase, PC236_IO_SIZE);
- if (ret < 0)
- return ret;
- return pc236_common_attach(dev, iobase, irq, 0);
- }
- break;
-#endif
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
- case pci_bustype: {
- int bus = it->options[0];
- int slot = it->options[1];
- struct pci_dev *pci_dev;
-
- pci_dev = pc236_find_pci(dev, bus, slot);
- if (pci_dev == NULL)
- return -EIO;
- return pc236_pci_common_attach(dev, pci_dev);
- }
- break;
-#endif
- default:
+ if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA) &&
+ thisboard->bustype == isa_bustype) {
+ unsigned long iobase = it->options[0];
+ unsigned int irq = it->options[1];
+ ret = pc236_request_region(dev, iobase, PC236_IO_SIZE);
+ if (ret < 0)
+ return ret;
+ return pc236_common_attach(dev, iobase, irq, 0);
+ } else if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI) &&
+ thisboard->bustype == pci_bustype) {
+ int bus = it->options[0];
+ int slot = it->options[1];
+ struct pci_dev *pci_dev;
+
+ pci_dev = pc236_find_pci(dev, bus, slot);
+ if (pci_dev == NULL)
+ return -EIO;
+ return pc236_pci_common_attach(dev, pci_dev);
+ } else {
dev_err(dev->class_dev, PC236_DRIVER_NAME
": BUG! cannot determine board type!\n");
- break;
+ return -EINVAL;
}
- return -EINVAL;
}
/*
@@ -593,7 +560,6 @@ static int pc236_attach(struct comedi_device *dev,
struct comedi_devconfig *it)
* to the "manual" attach hook. dev->board_ptr is NULL on entry.
There should
* be a board entry matching the supplied PCI device.
*/
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
static int __devinit pc236_attach_pci(struct comedi_device *dev,
struct pci_dev *pci_dev)
{
@@ -613,7 +579,6 @@ static int __devinit pc236_attach_pci(struct
comedi_device *dev,
}
return pc236_pci_common_attach(dev, pci_dev);
}
-#endif
static void pc236_detach(struct comedi_device *dev)
{
@@ -626,18 +591,14 @@ static void pc236_detach(struct comedi_device *dev)
if (dev->subdevices)
subdev_8255_cleanup(dev, dev->subdevices + 0);
if (devpriv) {
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
- if (devpriv->pci_dev) {
+ if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI) &&
+ devpriv->pci_dev) {
if (dev->iobase)
comedi_pci_disable(devpriv->pci_dev);
pci_dev_put(devpriv->pci_dev);
- } else
-#endif
- {
-#if IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA)
+ } else if (IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA)) {
if (dev->iobase)
release_region(dev->iobase, PC236_IO_SIZE);
-#endif
}
}
}
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
