On 2013-01-29 23:56, H Hartley Sweeten wrote:
On Tuesday, January 29, 2013 4:42 PM, Peter Hüwe wrote:
Hi,
while analyzing the comedi drivers, I noticed that quite a lot of them use a
more or less similar find_boardinfo function.
<snip>
The names and the exact implementation differ slightly, but in most cases it
boils down to:
unsigned int i;
for (i = 0; i < ARRAY_SIZE(__BOARD_ARRAY__); i++)
if (pcidev->device == __BOARD_ARRAY__[i].device_id)
return &__BOARD_ARRAY__[i];
return NULL;
unfortunately the __BOARD_ARRAY__ is always of a different type (but all
contain the device_id field) and size.
---> is there a way to consolidate these functions into one function (which
can operate on the different types) ? It's almost a bit like 'templates'.
Maybe with some gcc extensions or kernel magic functions ?
I already thought about passing a void pointer to the __BOARD_ARRAY__ and the
size of one element of the __BOARD_ARRAY__ and doing pointer calculations -
but I guess there must be a better way.
Or is the only option to write a macro ?
As you noticed, the problem is the driver specific definition of the struct used
to hold the boardinfo.
In drivers.c, the comedi_recognize() function actually access the boardinfo
in order to support the COMEDI_DEVCONFIG ioctl. There is a comment above
it giving a description of how it works.
There might be a way to do this in a generic way. The problem is that the
drivers use different names for "common" information and the data is
packed in the structs differently so accessing it generically is a bit difficult,
if not impossible.
I have been trying to remove as much of this boardinfo stuff from the drivers
as possible. For now I think the current implementation is fairly clean.
For the PnP bus drivers that use the auto_attach mechanism I have some ideas
to get rid of the find_boardinfo functions but I need to work out the kinks first.
Please wait on "fixing" any of this until a good solution is worked out.
One way is to enumerate the possible indices of the custom board array
as a set of enum constants, initialize the custom board array using
designated element initializers (indexed by the enum constants) and
include the same enum constant in the 'driver_data' member of the struct
pci_device_id elements of the module's PCI device table. Then the
module's PCI driver's probe function can index directly into the custom
board array without searching.
The missing link in the above is passing the index from the
'driver_data' through to the modules' comedi_driver's 'auto_attach'
function. The 'comedi_pci_auto_config()' function does not currently
have a parameter for passing this information, but the underlying
'comedi_auto_config()' function does. Either the existing
'comedi_pci_auto_config()' function could be extended, or a separate
extended version of the function could be added (maybe as an inline
function in comedidev.h), or the modules could call
'comedi_auto_config()' directly.
We have posted patches to add extra context to
'comedi_pci_auto_config()' before, but they weren't applied because we
didn't have a clear use case for them. Now we have, but I wouldn't mind
leaving the existing function alone and adding a new one.
The nice thing is that it's all under the control of the individual drivers.
Here's some code to illustrate what I'm on about in the above description:
struct foobar_board {
const char *name;
unsigned int ai_chans;
unsigned int ai_bits;
};
enum foobar_board_nums {
bn_foo,
bn_bar,
bn_baz
};
static const struct foobar_board foobar_boards[] = {
[bn_foo] = {
.name = "foo",
.ai_chans = 4,
.ai_bits = 12,
},
[bn_bar] = {
.name = "bar",
.ai_chans = 4,
.ai_bits = 16,
},
[bn_baz] = {
.name = "baz",
.ai_chans = 8,
.ai_bits = 16,
},
};
static int foobar_auto_attach(struct comedi_device *dev,
unsigned long context_bn)
{
struct pci_dev *pcidev = comedi_to_pci_dev(dev);
struct foobar_board *thisboard = &foobar_boards[bn_foo];
dev->board_ptr = thisboard; /* no searching! */
/* just return error for now */
return -ENODEV;
}
static void foobar_detach(struct comedi_device *dev)
{
/* nothing to do for now */
}
static struct comedi_driver foobar_comedi_driver = {
.driver_name = "foobar",
.module = THIS_MODULE,
.auto_attach = foobar_auto_attach,
.detach = foobar_detach,
};
static DEFINE_PCI_DEVICE_TABLE(foobar_pci_table) = {
{
PCI_DEVICE(PCI_VENDOR_ID_FOOBAR,
PCI_DEVICE_ID_FOOBAR_FOO),
.driver_data = bn_foo,
},
{
PCI_DEVICE(PCI_VENDOR_ID_FOOBAR,
PCI_DEVICE_ID_FOOBAR_BAR),
.driver_data = bn_bar,
},
{
PCI_DEVICE(PCI_VENDOR_ID_FOOBAR,
PCI_DEVICE_ID_FOOBAR_BAZ),
.driver_data = bn_baz,
},
{ 0 }
};
MODULE_DEVICE_TABLE(pci, foobar_pci_table);
static int foobar_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
/* could use a variant of comedi_pci_auto_config() here */
return comedi_auto_config(&pdev->dev, &foobar_comedi_driver,
ent->driver_data);
}
static struct pci_driver foobar_pci_driver = {
.name = "foobar",
.id_table = foobar_pci_table,
.probe = foobar_pci_probe,
.remove = comedi_pci_auto_unconfig,
};
module_comedi_pci_driver(foobar_comedi_driver, foobar_pci_driver);
--
-=( 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
