On Wednesday, August 15, 2012 7:03 AM, Ian Abbott wrote:
> Some low-level Comedi drivers no longer support manual configuration of
> devices with the COMEDI_DEVCONFIG ioctl (used by the comedi_config
> program). For those drivers, the 'attach_pci' or 'attach_usb' handler
> will be set in the struct comedi_driver to configure devices
> automatically (via comedi_pci_auto_config() or
> comedi_usb_auto_config()). Their 'attach' handlers are redundant but
> the the comedi core module currently requires it to be set.
>
> Make the 'attach' handler optional and issue a warning if something
> wants to call it.
>
> Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> ---
> drivers/staging/comedi/drivers.c | 19 ++++++++++++++++---
> 1 files changed, 16 insertions(+), 3 deletions(-)
Ian,
Assuming this gets merged, I will remove the 'attach' callback from
the drivers that no longer need it.
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index c0fdb00..c8adc5e 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -186,6 +186,14 @@ int comedi_device_attach(struct comedi_device *dev, struct comedi_devconfig *it)
> }
> return -EIO;
> }
> + if (driv->attach == NULL) {
> + /* driver does not support manual configuration */
> + dev_warn(dev->class_dev,
> + "driver '%s' does not support attach using comedi_config\n",
> + driv->driver_name);
> + module_put(driv->module);
> + return -ENOSYS;
> + }
> /* initialize dev->driver here so
> * comedi_error() can be called from attach */
> dev->driver = driv;
> @@ -885,13 +893,18 @@ static int comedi_auto_config_wrapper(struct comedi_device *dev, void *context)
> dev->board_ptr = comedi_recognize(driv, it->board_name);
> if (dev->board_ptr == NULL) {
> printk(KERN_WARNING
> - "comedi: auto config failed to find board entry"
> - " '%s' for driver '%s'\n", it->board_name,
> - driv->driver_name);
> + "comedi: auto config failed to find board entry '%s' for driver '%s'\n",
> + it->board_name, driv->driver_name);
Since your modifying this printk, it could (should) also be changed to a dev_warn().
> comedi_report_boards(driv);
> return -EINVAL;
> }
> }
> + if (!driv->attach) {
> + printk(KERN_WARNING
> + "comedi: BUG! driver '%s' using old-style auto config but has no attach handler\n",
> + driv->driver_name);
This one should also be a dev_warn().
> + return -EINVAL;
> + }
> return driv->attach(dev, it);
> }
Side note about this file.
I would like to break the pci and usb specific functions out into
separate source files (comedi_pci.c and comedi_usb.c) and
conditionally compile them in based on the Kconfig
CONFIG_COMEDI_PCI_DRIVERS and CONFIG_COMEDI_USB_DRIVERS
options. This lets us get rid of the '#if IS_ENABLED(CONFIG_USB)' .
I also would like to break the comedi_buf_* functions out to
a separate source file (comedi_buf.c). It doesn't seem appropriate
for them to be in the "drivers.c" source.
Do you have any objections to this?
Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
