The "amplc_pc236" driver supports both ISA cards (Amplicon PC36AT) and PCI
cards (PCI236). I plan to split it into separate drivers with a module for
common code, but let's reorganise it a bit first.
The driver still supports manual attachment of PCI devices via the
`COMEDI_DEVCONFIG` ioctl and the comedi driver `attach` hook. That can go,
though the attach hook is still needed for ISA devices.
01) staging: comedi: amplc_pc236: reformat header comments
02) staging: comedi: amplc_pc236: remove some boilerplate comments
03) staging: comedi: amplc_pc236: remove manual configuration of PCI boards
04) staging: comedi: amplc_pc236: no need to manipulate PCI ref count
05) staging: comedi: amplc_pc236: no need to set hw_dev
06) staging: comedi: amplc_pc236: absorb pc236_pci_common_attach()
07) staging: comedi: amplc_pc236: remove 'model' member
08) staging: comedi: amplc_pc236: split pc236_boards[] into ISA & PCI
09) staging: comedi: amplc_pc236: don't check bus type in attach
10) staging: comedi: amplc_pc236: Simplify PCI board look-up
11) staging: comedi: amplc_pc236: remove PCI device ID macros
12) staging: comedi: amplc_pc236: set board_name before common attach
drivers/staging/comedi/drivers/amplc_pc236.c | 282 +++++++--------------------
1 file changed, 74 insertions(+), 208 deletions(-)
Use preferred style for copyright and driver description comments.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 92 ++++++++++++++--------------
1 file changed, 46 insertions(+), 46 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 243b0f4..9d64e45 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -1,51 +1,51 @@
/*
- comedi/drivers/amplc_pc236.c
- Driver for Amplicon PC36AT and PCI236 DIO boards.
-
- Copyright (C) 2002 MEV Ltd. <http://www.mev.co.uk/>
-
- COMEDI - Linux Control and Measurement Device Interface
- Copyright (C) 2000 David A. Schleef <[email protected]>
-
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-*/
+ * comedi/drivers/amplc_pc236.c
+ * Driver for Amplicon PC36AT and PCI236 DIO boards.
+ *
+ * Copyright (C) 2002 MEV Ltd. <http://www.mev.co.uk/>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 2000 David A. Schleef <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
/*
-Driver: amplc_pc236
-Description: Amplicon PC36AT, PCI236
-Author: Ian Abbott <[email protected]>
-Devices: [Amplicon] PC36AT (pc36at), PCI236 (pci236 or amplc_pc236)
-Updated: Wed, 01 Apr 2009 15:41:25 +0100
-Status: works
-
-Configuration options - PC36AT:
- [0] - I/O port base address
- [1] - IRQ (optional)
-
-Configuration options - PCI236:
- [0] - PCI bus of device (optional)
- [1] - PCI slot of device (optional)
- If bus/slot is not specified, the first available PCI device will be
- used.
-
-The PC36AT ISA board and PCI236 PCI board have a single 8255 appearing
-as subdevice 0.
-
-Subdevice 1 pretends to be a digital input device, but it always returns
-0 when read. However, if you run a command with scan_begin_src=TRIG_EXT,
-a rising edge on port C bit 3 acts as an external trigger, which can be
-used to wake up tasks. This is like the comedi_parport device, but the
-only way to physically disable the interrupt on the PC36AT is to remove
-the IRQ jumper. If no interrupt is connected, then subdevice 1 is
-unused.
-*/
+ * Driver: amplc_pc236
+ * Description: Amplicon PC36AT, PCI236
+ * Author: Ian Abbott <[email protected]>
+ * Devices: [Amplicon] PC36AT (pc36at), PCI236 (pci236 or amplc_pc236)
+ * Updated: Wed, 01 Apr 2009 15:41:25 +0100
+ * Status: works
+ *
+ * Configuration options - PC36AT:
+ * [0] - I/O port base address
+ * [1] - IRQ (optional)
+ *
+ * Configuration options - PCI236:
+ * [0] - PCI bus of device (optional)
+ * [1] - PCI slot of device (optional)
+ * If bus/slot is not specified, the first available PCI device will be
+ * used.
+ *
+ * The PC36AT ISA board and PCI236 PCI board have a single 8255 appearing
+ * as subdevice 0.
+ *
+ * Subdevice 1 pretends to be a digital input device, but it always returns
+ * 0 when read. However, if you run a command with scan_begin_src=TRIG_EXT,
+ * a rising edge on port C bit 3 acts as an external trigger, which can be
+ * used to wake up tasks. This is like the comedi_parport device, but the
+ * only way to physically disable the interrupt on the PC36AT is to remove
+ * the IRQ jumper. If no interrupt is connected, then subdevice 1 is
+ * unused.
+ */
#include <linux/module.h>
#include <linux/pci.h>
--
2.0.0
The `PCI_DEVICE_ID_AMPLICON_PCI236` macro is only used once, in the
module device table, so remove it and expand the macro in the table.
`The `PCI_DEVICE_ID_INVALID` macro is no longer used, so remove it.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 1aae066..1a7fa45 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -57,10 +57,6 @@
#define DO_ISA IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_ISA)
#define DO_PCI IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)
-/* PCI236 PCI configuration register information */
-#define PCI_DEVICE_ID_AMPLICON_PCI236 0x0009
-#define PCI_DEVICE_ID_INVALID 0xffff
-
/* PC36AT / PCI236 registers */
/* Disable, and clear, interrupts */
@@ -407,8 +403,8 @@ static struct comedi_driver amplc_pc236_driver = {
#if DO_PCI
static const struct pci_device_id pc236_pci_table[] = {
- { PCI_DEVICE(PCI_VENDOR_ID_AMPLICON, PCI_DEVICE_ID_AMPLICON_PCI236) },
- {0}
+ { PCI_DEVICE(PCI_VENDOR_ID_AMPLICON, 0x0009) },
+ { 0 }
};
MODULE_DEVICE_TABLE(pci, pc236_pci_table);
--
2.0.0
Since the legacy attach routine `pc236_attach()` is only called for
board names matching an entry in our array of ISA boards
`pc236_isa_boards[]`, and it is reasonable to expect all elements of
`pc236_isa_boards[]` to have their `bustype` member initialized
correctly to `isa_bustype`, don't bother checking the bus type in
`pc236_attach()`. Add `if (!DO_ISA) return -EINVAL` to optimize out the
remainder of the function if `CONFIG_COMEDI_AMPLC_PC236_ISA` is not
defined.
Similarly, don't bother checking the bus type in
`pc236_find_pci_board()` as it is reasonable to expect all elements of
`pc236_pci_boards[]` to have their `bustype` member initialized
correctly to `pci_bustype`.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 26 ++++++++------------------
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 0a5ba10..51c22f9 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -129,8 +129,7 @@ static const struct pc236_board *pc236_find_pci_board(struct pci_dev *pci_dev)
unsigned int i;
for (i = 0; i < ARRAY_SIZE(pc236_pci_boards); i++)
- if (is_pci_board(&pc236_pci_boards[i]) &&
- pci_dev->device == pc236_pci_boards[i].devid)
+ if (pci_dev->device == pc236_pci_boards[i].devid)
return &pc236_pci_boards[i];
return NULL;
}
@@ -349,30 +348,21 @@ static int pc236_common_attach(struct comedi_device *dev, unsigned long iobase,
static int pc236_attach(struct comedi_device *dev, struct comedi_devconfig *it)
{
- const struct pc236_board *thisboard = comedi_board(dev);
struct pc236_private *devpriv;
int ret;
+ if (!DO_ISA)
+ return -EINVAL;
+
devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
if (!devpriv)
return -ENOMEM;
- /* Process options according to bus type. */
- if (is_isa_board(thisboard)) {
- ret = comedi_request_region(dev, it->options[0], 0x4);
- if (ret)
- return ret;
-
- return pc236_common_attach(dev, dev->iobase, it->options[1], 0);
- } else if (is_pci_board(thisboard)) {
- dev_err(dev->class_dev,
- "Manual configuration of PCI board '%s' is not supported\n",
- thisboard->name);
- return -EIO;
- }
+ ret = comedi_request_region(dev, it->options[0], 0x4);
+ if (ret)
+ return ret;
- dev_err(dev->class_dev, "BUG! cannot determine board type!\n");
- return -EINVAL;
+ return pc236_common_attach(dev, dev->iobase, it->options[1], 0);
}
static int pc236_auto_attach(struct comedi_device *dev,
--
2.0.0
Since only a single PCI board is supported by the driver, there is no
need to call `pc236_find_pci_board()` to find the a board entry with
matching PCI device ID in `pc236_pci_boards[]`. Just point to the entry
directly and remove the look-up function. In fact, there is no reason
for `pc236_pci_boards[]` to be an array, so change it to a non-array
variable and rename it to `pc236_pci_board`. Also, the `devid` member
of `struct pc236_board` is no longer needed as it was only used by the
look-up function, so remove it.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 29 ++++------------------------
1 file changed, 4 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 51c22f9..1aae066 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -85,7 +85,6 @@ enum pc236_bustype { isa_bustype, pci_bustype };
struct pc236_board {
const char *name;
- unsigned short devid;
enum pc236_bustype bustype;
};
@@ -96,12 +95,9 @@ static const struct pc236_board pc236_isa_boards[] = {
},
};
-static const struct pc236_board pc236_pci_boards[] = {
- {
- .name = "pci236",
- .devid = PCI_DEVICE_ID_AMPLICON_PCI236,
- .bustype = pci_bustype,
- },
+static const struct pc236_board pc236_pci_board = {
+ .name = "pci236",
+ .bustype = pci_bustype,
};
struct pc236_private {
@@ -122,19 +118,6 @@ static inline bool is_pci_board(const struct pc236_board *board)
}
/*
- * This function looks for a board matching the supplied PCI device.
- */
-static const struct pc236_board *pc236_find_pci_board(struct pci_dev *pci_dev)
-{
- unsigned int i;
-
- for (i = 0; i < ARRAY_SIZE(pc236_pci_boards); i++)
- if (pci_dev->device == pc236_pci_boards[i].devid)
- return &pc236_pci_boards[i];
- return NULL;
-}
-
-/*
* This function is called to mark the interrupt as disabled (no command
* configured on subdevice 1) and to physically disable the interrupt
* (not possible on the PC36AT, except by removing the IRQ jumper!).
@@ -382,11 +365,7 @@ static int pc236_auto_attach(struct comedi_device *dev,
if (!devpriv)
return -ENOMEM;
- dev->board_ptr = pc236_find_pci_board(pci_dev);
- if (dev->board_ptr == NULL) {
- dev_err(dev->class_dev, "BUG! cannot determine board type!\n");
- return -EINVAL;
- }
+ dev->board_ptr = &pc236_pci_board;
ret = comedi_pci_enable(dev);
if (ret)
return ret;
--
2.0.0
For PCI boards, the `auto_attach` handler, `pc236_auto_attach()`,
initializes `dev->board_ptr` to point to a `struct pc236_board`, but
leaves `dev->board_name` unchanged. The Comedi core will have
initialized `dev->board_name` to the `driver_name` string member of
`amplc_pc236_driver`. For consistency with ISA boards manually
configured by the `COMEDI_DEVCONFIG` ioctl via the legacy `attach`
handler, `pc236_attach()`, set `dev->board_name` to the `name` member of
the `struct pc236_board` pointed to by `dev->board_ptr`.
Both `pc236_attach()` and `pc236_auto_attach()` call
`pc236_common_attach()`, which also sets `dev->board_name` to the `name`
member of the `struct pc236_board`. Since this assignment no longer
changes anything, remove it.
A nice side-effect of this change is that the same owner name string is
used for requesting I/O regions (before the call the
`pc236_common_attach()`) as is used for requesting the IRQ handler
(during the call to `pc236_common_attach()`). It was already the same
for (manually configured) ISA boards, but is now the same for
(automatically configured) PCI boards.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 1a7fa45..b92fc6f 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -284,11 +284,9 @@ static irqreturn_t pc236_interrupt(int irq, void *d)
static int pc236_common_attach(struct comedi_device *dev, unsigned long iobase,
unsigned int irq, unsigned long req_irq_flags)
{
- const struct pc236_board *thisboard = comedi_board(dev);
struct comedi_subdevice *s;
int ret;
- dev->board_name = thisboard->name;
dev->iobase = iobase;
ret = comedi_alloc_subdevices(dev, 2);
@@ -362,6 +360,7 @@ static int pc236_auto_attach(struct comedi_device *dev,
return -ENOMEM;
dev->board_ptr = &pc236_pci_board;
+ dev->board_name = pc236_pci_board.name;
ret = comedi_pci_enable(dev);
if (ret)
return ret;
--
2.0.0
Split `pc236_boards[]` into `pc236_isa_boards[]` for ISA cards and
`pc236_pci_boards[]` for PCI cards (there is only one of each). Only
initialize the board name look-up members of `struct comedi_driver
amplc_pc236_driver` if the ISA part of the driver is enabled in the
kernel config (`CONFIG_COMEDI_AMPLC_PC236_ISA`) using the array of ISA
boards (`pc236_isa_boards[]`). The driver doesn't allow manual
configuration of PCI devices, so there is no point having the comedi
core match the names of the PCI boards before it calls our driver's
legacy attach routine (`pc236_attach()`).
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 129578f..0a5ba10 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -88,20 +88,20 @@ struct pc236_board {
unsigned short devid;
enum pc236_bustype bustype;
};
-static const struct pc236_board pc236_boards[] = {
-#if DO_ISA
+
+static const struct pc236_board pc236_isa_boards[] = {
{
.name = "pc36at",
.bustype = isa_bustype,
},
-#endif
-#if DO_PCI
+};
+
+static const struct pc236_board pc236_pci_boards[] = {
{
.name = "pci236",
.devid = PCI_DEVICE_ID_AMPLICON_PCI236,
.bustype = pci_bustype,
},
-#endif
};
struct pc236_private {
@@ -128,10 +128,10 @@ static const struct pc236_board *pc236_find_pci_board(struct pci_dev *pci_dev)
{
unsigned int i;
- for (i = 0; i < ARRAY_SIZE(pc236_boards); i++)
- if (is_pci_board(&pc236_boards[i]) &&
- pci_dev->device == pc236_boards[i].devid)
- return &pc236_boards[i];
+ for (i = 0; i < ARRAY_SIZE(pc236_pci_boards); i++)
+ if (is_pci_board(&pc236_pci_boards[i]) &&
+ pci_dev->device == pc236_pci_boards[i].devid)
+ return &pc236_pci_boards[i];
return NULL;
}
@@ -429,9 +429,11 @@ static struct comedi_driver amplc_pc236_driver = {
.attach = pc236_attach,
.auto_attach = pc236_auto_attach,
.detach = pc236_detach,
- .board_name = &pc236_boards[0].name,
+#if DO_ISA
+ .board_name = &pc236_isa_boards[0].name,
.offset = sizeof(struct pc236_board),
- .num_names = ARRAY_SIZE(pc236_boards),
+ .num_names = ARRAY_SIZE(pc236_isa_boards),
+#endif
};
#if DO_PCI
--
2.0.0
The `model` member of `struct pc236_board` is no longer used since the
code to remove manual configuration of PCI devices was removed. Get rid
of it.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 22d3b6f..129578f 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -82,20 +82,17 @@
*/
enum pc236_bustype { isa_bustype, pci_bustype };
-enum pc236_model { pc36at_model, pci236_model };
struct pc236_board {
const char *name;
unsigned short devid;
enum pc236_bustype bustype;
- enum pc236_model model;
};
static const struct pc236_board pc236_boards[] = {
#if DO_ISA
{
.name = "pc36at",
.bustype = isa_bustype,
- .model = pc36at_model,
},
#endif
#if DO_PCI
@@ -103,7 +100,6 @@ static const struct pc236_board pc236_boards[] = {
.name = "pci236",
.devid = PCI_DEVICE_ID_AMPLICON_PCI236,
.bustype = pci_bustype,
- .model = pci236_model,
},
#endif
};
--
2.0.0
Absorb `pc236_pci_common_attach()` into `pc236_auto_attach()` since
that's the only place it is called from.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index f99c7f1..22d3b6f 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -351,22 +351,6 @@ static int pc236_common_attach(struct comedi_device *dev, unsigned long iobase,
return 0;
}
-static int pc236_pci_common_attach(struct comedi_device *dev,
- struct pci_dev *pci_dev)
-{
- struct pc236_private *devpriv = dev->private;
- unsigned long iobase;
- int ret;
-
- ret = comedi_pci_enable(dev);
- if (ret)
- return ret;
-
- devpriv->lcr_iobase = pci_resource_start(pci_dev, 1);
- iobase = pci_resource_start(pci_dev, 2);
- return pc236_common_attach(dev, iobase, pci_dev->irq, IRQF_SHARED);
-}
-
static int pc236_attach(struct comedi_device *dev, struct comedi_devconfig *it)
{
const struct pc236_board *thisboard = comedi_board(dev);
@@ -400,6 +384,8 @@ static int pc236_auto_attach(struct comedi_device *dev,
{
struct pci_dev *pci_dev = comedi_to_pci_dev(dev);
struct pc236_private *devpriv;
+ unsigned long iobase;
+ int ret;
if (!DO_PCI)
return -EINVAL;
@@ -415,7 +401,13 @@ static int pc236_auto_attach(struct comedi_device *dev,
dev_err(dev->class_dev, "BUG! cannot determine board type!\n");
return -EINVAL;
}
- return pc236_pci_common_attach(dev, pci_dev);
+ ret = comedi_pci_enable(dev);
+ if (ret)
+ return ret;
+
+ devpriv->lcr_iobase = pci_resource_start(pci_dev, 1);
+ iobase = pci_resource_start(pci_dev, 2);
+ return pc236_common_attach(dev, iobase, pci_dev->irq, IRQF_SHARED);
}
static void pc236_detach(struct comedi_device *dev)
--
2.0.0
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 9d64e45..7600319 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -117,10 +117,6 @@ static const struct pc236_board pc236_boards[] = {
#endif
};
-/* this structure is for data unique to this hardware driver. If
- several hardware drivers keep similar information in this structure,
- feel free to suggest moving the variable to the struct comedi_device struct.
- */
struct pc236_private {
unsigned long lcr_iobase; /* PLX PCI9052 config registers in PCIBAR1 */
int enable_irq;
@@ -425,12 +421,6 @@ static int pc236_pci_common_attach(struct comedi_device *dev,
return pc236_common_attach(dev, iobase, pci_dev->irq, IRQF_SHARED);
}
-/*
- * Attach is called by the Comedi core to configure the driver
- * for a particular board. If you specified a board_name array
- * in the driver structure, dev->board_ptr contains that
- * address.
- */
static int pc236_attach(struct comedi_device *dev, struct comedi_devconfig *it)
{
const struct pc236_board *thisboard = comedi_board(dev);
@@ -461,11 +451,6 @@ static int pc236_attach(struct comedi_device *dev, struct comedi_devconfig *it)
return -EINVAL;
}
-/*
- * The auto_attach hook is called at PCI probe time via
- * comedi_pci_auto_config(). dev->board_ptr is NULL on entry.
- * There should be a board entry matching the supplied PCI device.
- */
static int pc236_auto_attach(struct comedi_device *dev,
unsigned long context_unused)
{
@@ -517,12 +502,6 @@ static void pc236_detach(struct comedi_device *dev)
}
}
-/*
- * The struct comedi_driver structure tells the Comedi core module
- * which functions to call to configure/deconfigure (attach/detach)
- * the board, and also about the kernel module that contains
- * the device code.
- */
static struct comedi_driver amplc_pc236_driver = {
.driver_name = "amplc_pc236",
.module = THIS_MODULE,
--
2.0.0
Remove the code that allows PCI boards to be manually attached by the
`COMEDI_DEVCONFIG` ioctl (or the "comedi_config" application).
Supported PCI boards (PCI236) will be attached automatically at probe
time via `comedi_pci_auto_config()` and the `auto_attach` hook in the
`struct comedi_driver`.
The "wildcard" entry in `pc236_boards[]` was only used when manually
attaching a PCI board using a driver name instead of a board name, so is
no longer needed. Remove it.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 72 ++++------------------------
1 file changed, 9 insertions(+), 63 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 7600319..7331ae3 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -21,19 +21,16 @@
* Driver: amplc_pc236
* Description: Amplicon PC36AT, PCI236
* Author: Ian Abbott <[email protected]>
- * Devices: [Amplicon] PC36AT (pc36at), PCI236 (pci236 or amplc_pc236)
- * Updated: Wed, 01 Apr 2009 15:41:25 +0100
+ * Devices: [Amplicon] PC36AT (pc36at), PCI236 (pci236)
+ * Updated: Thu, 24 Jul 2014 14:25:26 +0000
* Status: works
*
* Configuration options - PC36AT:
* [0] - I/O port base address
* [1] - IRQ (optional)
*
- * Configuration options - PCI236:
- * [0] - PCI bus of device (optional)
- * [1] - PCI slot of device (optional)
- * If bus/slot is not specified, the first available PCI device will be
- * used.
+ * Manual configuration of PCI board (PCI236) is not supported; it is
+ * configured automatically.
*
* The PC36AT ISA board and PCI236 PCI board have a single 8255 appearing
* as subdevice 0.
@@ -85,7 +82,7 @@
*/
enum pc236_bustype { isa_bustype, pci_bustype };
-enum pc236_model { pc36at_model, pci236_model, anypci_model };
+enum pc236_model { pc36at_model, pci236_model };
struct pc236_board {
const char *name;
@@ -108,12 +105,6 @@ static const struct pc236_board pc236_boards[] = {
.bustype = pci_bustype,
.model = pci236_model,
},
- {
- .name = "amplc_pc236",
- .devid = PCI_DEVICE_ID_INVALID,
- .bustype = pci_bustype,
- .model = anypci_model, /* wildcard */
- },
#endif
};
@@ -149,49 +140,6 @@ static const struct pc236_board *pc236_find_pci_board(struct pci_dev *pci_dev)
}
/*
- * This function looks for a PCI device matching the requested board name,
- * bus and slot.
- */
-static struct pci_dev *pc236_find_pci_dev(struct comedi_device *dev,
- struct comedi_devconfig *it)
-{
- const struct pc236_board *thisboard = comedi_board(dev);
- struct pci_dev *pci_dev = NULL;
- int bus = it->options[0];
- int slot = it->options[1];
-
- for_each_pci_dev(pci_dev) {
- if (bus || slot) {
- if (bus != pci_dev->bus->number ||
- slot != PCI_SLOT(pci_dev->devfn))
- continue;
- }
- if (pci_dev->vendor != PCI_VENDOR_ID_AMPLICON)
- continue;
-
- if (thisboard->model == anypci_model) {
- /* Wildcard board matches any supported PCI board. */
- const struct pc236_board *foundboard;
-
- foundboard = pc236_find_pci_board(pci_dev);
- if (foundboard == NULL)
- continue;
- /* Replace wildcard board_ptr. */
- dev->board_ptr = foundboard;
- } else {
- /* Match specific model name. */
- if (pci_dev->device != thisboard->devid)
- continue;
- }
- return pci_dev;
- }
- dev_err(dev->class_dev,
- "No supported board found! (req. bus %d, slot %d)\n",
- bus, slot);
- return NULL;
-}
-
-/*
* This function is called to mark the interrupt as disabled (no command
* configured on subdevice 1) and to physically disable the interrupt
* (not possible on the PC36AT, except by removing the IRQ jumper!).
@@ -439,12 +387,10 @@ static int pc236_attach(struct comedi_device *dev, struct comedi_devconfig *it)
return pc236_common_attach(dev, dev->iobase, it->options[1], 0);
} else if (is_pci_board(thisboard)) {
- struct pci_dev *pci_dev;
-
- pci_dev = pc236_find_pci_dev(dev, it);
- if (!pci_dev)
- return -EIO;
- return pc236_pci_common_attach(dev, pci_dev);
+ dev_err(dev->class_dev,
+ "Manual configuration of PCI board '%s' is not supported\n",
+ thisboard->name);
+ return -EIO;
}
dev_err(dev->class_dev, "BUG! cannot determine board type!\n");
--
2.0.0
Now that this driver no longer supports "manual" attachment of PCI
devices in its `attach` hook (`pc236_attach()`), it no longer has code
that searches for a suitable PCI device and increments its reference
count. Since the driver no longer has any reason for incrementing and
decrementing the PCI device's reference count, the calls to
`pci_dev_get()` and `pci_dev_put()` can be removed.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index 7331ae3..cf45c58 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -417,13 +417,6 @@ static int pc236_auto_attach(struct comedi_device *dev,
dev_err(dev->class_dev, "BUG! cannot determine board type!\n");
return -EINVAL;
}
- /*
- * Need to 'get' the PCI device to match the 'put' in pc236_detach().
- * TODO: Remove the pci_dev_get() and matching pci_dev_put() once
- * support for manual attachment of PCI devices via pc236_attach()
- * has been removed.
- */
- pci_dev_get(pci_dev);
return pc236_pci_common_attach(dev, pci_dev);
}
@@ -438,13 +431,9 @@ static void pc236_detach(struct comedi_device *dev)
if (is_isa_board(thisboard)) {
comedi_legacy_detach(dev);
} else if (is_pci_board(thisboard)) {
- struct pci_dev *pcidev = comedi_to_pci_dev(dev);
-
if (dev->irq)
free_irq(dev->irq, dev);
comedi_pci_disable(dev);
- if (pcidev)
- pci_dev_put(pcidev);
}
}
--
2.0.0
The call to `comedi_set_hw_dev()` from `pc236_pci_common_attach()` is
now unnecessary since `pc236_pci_common_attach()` is now only called
from this driver's `auto_attach` hook `pc236_auto_attach()` and the
comedi core now calls `comedi_set_hw_dev()` before calling that. Remove
the unnecessary call.
Signed-off-by: Ian Abbott <[email protected]>
---
drivers/staging/comedi/drivers/amplc_pc236.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
index cf45c58..f99c7f1 100644
--- a/drivers/staging/comedi/drivers/amplc_pc236.c
+++ b/drivers/staging/comedi/drivers/amplc_pc236.c
@@ -358,8 +358,6 @@ static int pc236_pci_common_attach(struct comedi_device *dev,
unsigned long iobase;
int ret;
- comedi_set_hw_dev(dev, &pci_dev->dev);
-
ret = comedi_pci_enable(dev);
if (ret)
return ret;
--
2.0.0
On Friday, July 25, 2014 2:05 AM, Ian Abbott wrote:
> Use preferred style for copyright and driver description comments.
>
> Signed-off-by: Ian Abbott <[email protected]>
> ---
> drivers/staging/comedi/drivers/amplc_pc236.c | 92 ++++++++++++++--------------
> 1 file changed, 46 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/amplc_pc236.c b/drivers/staging/comedi/drivers/amplc_pc236.c
> index 243b0f4..9d64e45 100644
> --- a/drivers/staging/comedi/drivers/amplc_pc236.c
> +++ b/drivers/staging/comedi/drivers/amplc_pc236.c
> @@ -1,51 +1,51 @@
> /*
> - comedi/drivers/amplc_pc236.c
> - Driver for Amplicon PC36AT and PCI236 DIO boards.
> -
> - Copyright (C) 2002 MEV Ltd. <http://www.mev.co.uk/>
> -
> - COMEDI - Linux Control and Measurement Device Interface
> - Copyright (C) 2000 David A. Schleef <[email protected]>
> -
> - This program is free software; you can redistribute it and/or modify
> - it under the terms of the GNU General Public License as published by
> - the Free Software Foundation; either version 2 of the License, or
> - (at your option) any later version.
> -
> - This program is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - GNU General Public License for more details.
> -*/
> + * comedi/drivers/amplc_pc236.c
> + * Driver for Amplicon PC36AT and PCI236 DIO boards.
> + *
> + * Copyright (C) 2002 MEV Ltd. <http://www.mev.co.uk/>
> + *
> + * COMEDI - Linux Control and Measurement Device Interface
> + * Copyright (C) 2000 David A. Schleef <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> /*
> -Driver: amplc_pc236
> -Description: Amplicon PC36AT, PCI236
> -Author: Ian Abbott <[email protected]>
> -Devices: [Amplicon] PC36AT (pc36at), PCI236 (pci236 or amplc_pc236)
> -Updated: Wed, 01 Apr 2009 15:41:25 +0100
> -Status: works
> -
> -Configuration options - PC36AT:
> - [0] - I/O port base address
> - [1] - IRQ (optional)
> -
> -Configuration options - PCI236:
> - [0] - PCI bus of device (optional)
> - [1] - PCI slot of device (optional)
> - If bus/slot is not specified, the first available PCI device will be
> - used.
> -
> -The PC36AT ISA board and PCI236 PCI board have a single 8255 appearing -as subdevice 0.
> -
> -Subdevice 1 pretends to be a digital input device, but it always returns
> -0 when read. However, if you run a command with scan_begin_src=TRIG_EXT, -a rising edge on port C bit 3 acts as an external trigger, which can be -used to wake up tasks. This is like the comedi_parport device, but the -only way to physically disable the interrupt on the PC36AT is to remove -the IRQ jumper. If no interrupt is connected, then subdevice 1 is -unused.
> -*/
Ian,
This patch appears to be corrupted.
> + * Driver: amplc_pc236
> + * Description: Amplicon PC36AT, PCI236
> + * Author: Ian Abbott <[email protected]>
> + * Devices: [Amplicon] PC36AT (pc36at), PCI236 (pci236 or amplc_pc236)
> + * Updated: Wed, 01 Apr 2009 15:41:25 +0100
> + * Status: works
> + *
> + * Configuration options - PC36AT:
> + * [0] - I/O port base address
> + * [1] - IRQ (optional)
> + *
> + * Configuration options - PCI236:
> + * [0] - PCI bus of device (optional)
> + * [1] - PCI slot of device (optional)
> + * If bus/slot is not specified, the first available PCI device will be
> + * used.
> + *
> + * The PC36AT ISA board and PCI236 PCI board have a single 8255
> + appearing
Patch corruption?
> + * as subdevice 0.
> + *
> + * Subdevice 1 pretends to be a digital input device, but it always
> + returns
Again?
> + * 0 when read. However, if you run a command with
> + scan_begin_src=TRIG_EXT,
Again?
> + * a rising edge on port C bit 3 acts as an external trigger, which can
> + be
Again?
> + * used to wake up tasks. This is like the comedi_parport device, but
> + the
Again?
> + * only way to physically disable the interrupt on the PC36AT is to
> + remove
Again?
> + * the IRQ jumper. If no interrupt is connected, then subdevice 1 is
> + * unused.
> + */
>
> #include <linux/module.h>
> #include <linux/pci.h>
Not sure what happened..
Hartley
On Friday, July 25, 2014 2:05 AM, Ian Abbott wrote:
> The "amplc_pc236" driver supports both ISA cards (Amplicon PC36AT) and PCI cards (PCI236). I plan to split it into separate drivers with a module for common code, but let's reorganise it a bit first.
>
> The driver still supports manual attachment of PCI devices via the `COMEDI_DEVCONFIG` ioctl and the comedi driver `attach` hook. That can go, though the attach hook is still needed for ISA devices.
>
> 01) staging: comedi: amplc_pc236: reformat header comments
> 02) staging: comedi: amplc_pc236: remove some boilerplate comments
> 03) staging: comedi: amplc_pc236: remove manual configuration of PCI boards
> 04) staging: comedi: amplc_pc236: no need to manipulate PCI ref count
> 05) staging: comedi: amplc_pc236: no need to set hw_dev
> 06) staging: comedi: amplc_pc236: absorb pc236_pci_common_attach()
> 07) staging: comedi: amplc_pc236: remove 'model' member
> 08) staging: comedi: amplc_pc236: split pc236_boards[] into ISA & PCI
> 09) staging: comedi: amplc_pc236: don't check bus type in attach
> 10) staging: comedi: amplc_pc236: Simplify PCI board look-up
> 11) staging: comedi: amplc_pc236: remove PCI device ID macros
> 12) staging: comedi: amplc_pc236: set board_name before common attach
>
> drivers/staging/comedi/drivers/amplc_pc236.c | 282 +++++++--------------------
> 1 file changed, 74 insertions(+), 208 deletions(-)
Ian,
I'm not sure what happened but this entire series appears to be corrupt.
Can you check it and repost?
Thanks,
Hartley
On Friday, July 25, 2014 10:37 AM, Hartley Sweeten wrote:
> On Friday, July 25, 2014 2:05 AM, Ian Abbott wrote:
>> The "amplc_pc236" driver supports both ISA cards (Amplicon PC36AT) and PCI
>> cards (PCI236). I plan to split it into separate drivers with a module for
>> common code, but let's reorganise it a bit first.
>>
>> The driver still supports manual attachment of PCI devices via the
>> `COMEDI_DEVCONFIG` ioctl and the comedi driver `attach` hook. That can go,
>> though the attach hook is still needed for ISA devices.
>>
>> 01) staging: comedi: amplc_pc236: reformat header comments
>> 02) staging: comedi: amplc_pc236: remove some boilerplate comments
>> 03) staging: comedi: amplc_pc236: remove manual configuration of PCI boards
>> 04) staging: comedi: amplc_pc236: no need to manipulate PCI ref count
>> 05) staging: comedi: amplc_pc236: no need to set hw_dev
>> 06) staging: comedi: amplc_pc236: absorb pc236_pci_common_attach()
>> 07) staging: comedi: amplc_pc236: remove 'model' member
>> 08) staging: comedi: amplc_pc236: split pc236_boards[] into ISA & PCI
>> 09) staging: comedi: amplc_pc236: don't check bus type in attach
>> 10) staging: comedi: amplc_pc236: Simplify PCI board look-up
>> 11) staging: comedi: amplc_pc236: remove PCI device ID macros
>> 12) staging: comedi: amplc_pc236: set board_name before common attach
>>
>> drivers/staging/comedi/drivers/amplc_pc236.c | 282 +++++++--------------------
>> 1 file changed, 74 insertions(+), 208 deletions(-)
>
> Ian,
>
> I'm not sure what happened but this entire series appears to be corrupt.
>
> Can you check it and repost?
Ignore that... The problem was on my end.
Reviewed-by: H Hartley Sweeten <[email protected]>