On Wednesday, January 30, 2013 4:04 AM, Ian Abbott wrote: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;
};
I would also like to make a common "boardinfo" struct that the comedi
core can then use in the comedi_recognize() and comedi_report_boards()
functions to remove the need for the pointer math. Something like:
struct comedi_board {
const char *name;
const void *private;
};
The comedi_driver then could be changed to:
+ const struct comedi_board *boards;
- /* number of elements in board_name and board_id arrays */
- unsigned int num_names;
- const char *const *board_name;
- /* offset in bytes from one board name pointer to the next */
- int offset;
};
The board_ptr in comedi_device would then change to:
+ const struct comedi_board *board_ptr;
- const void *board_ptr;
The comedi_board() helper would also need changed:
static inline const void *comedi_board(const struct comedi_device *dev)
{
+ return (dev->board_ptr) ? dev->board_ptr->private : NULL;
- return dev->board_ptr;
}
It still returns the driver specific boardinfo as a const void *.
The common comedi_board would also allow removing the board_name
from the comedi_device. A helper function could just fetch it:
static const char *comedi_board_name(struct comedi_device *dev)
{
return (dev->board_ptr) ? dev->board_ptr->name : dev->driver->driver_name;
}
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,
},
};
Using the common comedi_board would change this a bit:
static const struct foobar_board[] = {
[bn_foo] = {
.ai_chans = 4,
.ai_bits = 12,
},
[bn_bar] = {
.ai_chans = 4,
.ai_bits = 16,
},
[bn_baz] = {
.ai_chans = 8,
.ai_bits = 16,
},
};
static const struct comedi_board foobar_boards[] = {
[bn_foo] = {
.name = "foo",
.private = &foorbar_info[bn_foo],
},
[bn_bar] = {
.name = "bar",
.private = &foorbar_info[bn_bar],
},
[bn_baz] = {
.name = "baz",
.private = &foorbar_info[bn_baz],
},
};
Any other "common" information that the comedi core needs to
access could be added to comedi_board. All the driver specific
information stays in the private struct.
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! */
The dev->board_ptr should just be set in the comedi core before
calling the drivers auto_attach. Something like:
comedi_set_hw_dev(comedi_dev, hardware_device);
comedi_dev->driver = driver;
+ if (driver->boards)
+ comedi_dev->board_ptr = &driver->boards[context];
ret = driver->auto_attach(comedi_dev, context);
Actually, if we go this route, the context should not be required in the
auto_attach since the core has already taken care of it.
Basically, once the auto_attach is called the comedi_device already has
the following fields initialized correctly:
driver points to the comedi_driver
hw_dev points to the underlying device (pci/usb/pcmcia/etc.)
board_name no longer needed
board_ptr points to the correct comedi_board 'context'
Other than that, the rest of your code follows what I'm thinking.
Opinion?