On 11/02/17 10:37, Cheah Kok Cheong wrote:
Currently this module needs to be manually configured by COMEDI userspace tool before the test waveform can be read by a COMEDI compatible application. This patch adds auto-configuration capability and makes it the default loading option. This is achieved by creating a device during init to stand in for a real hardware device. This allows comedi_auto_config() to perform auto-configuration. With this patch, the test waveform can be read by a COMEDI compatible application without needing manual configuration. Previous behaviour is still selectable via module loading parameter. Module loading without passing any parameter will default to auto-configuration with the same default waveform amplitude and period values. For auto-configuration, different amplitude and period values can be set via module loading parameters. Tested on Xubuntu 16.04 using Xoscope ver: 2.0 which is available in the Ubuntu repository. Xoscope is a COMEDI compatible digital oscilloscope application. For manual configuration, only module loading/unloading is tested. Here are the truncated dmesg output. [sudo modprobe comedi_test] comedi_test: 1000000 microvolt, 100000 microsecond waveform attached driver 'comedi_test' has successfully auto-configured 'comedi_test'. [sudo modprobe comedi_test amplitude=2500000 period=150000] comedi_test: 2500000 microvolt, 150000 microsecond waveform attached driver 'comedi_test' has successfully auto-configured 'comedi_test'. [sudo modprobe comedi_test noauto=1] comedi_test: module is from the staging directory, the quality is unknown, you have been warned. For those without an actual hardware, the comedi_test module is as close as one can get to test the COMEDI system. Having both auto and manual configuration capability will broaden the test function of this module. Hopefully this will make it easier for people to check out the COMEDI system and contribute to its development. Signed-off-by: Cheah Kok Cheong <thrust73@xxxxxxxxx> --- V2: -Rename module param - Ian -Rename class - Ian -Tidy up init error handling - Ian -Allow module loading to continue when auto-configuration fails - Ian -Remove redundant "if" statement from module exit -Edit driver intro to reflect changes drivers/staging/comedi/drivers/comedi_test.c | 130 ++++++++++++++++++++++++--- 1 file changed, 118 insertions(+), 12 deletions(-) diff --git a/drivers/staging/comedi/drivers/comedi_test.c b/drivers/staging/comedi/drivers/comedi_test.c index ec5b9a2..027c972 100644 --- a/drivers/staging/comedi/drivers/comedi_test.c +++ b/drivers/staging/comedi/drivers/comedi_test.c @@ -36,7 +36,15 @@ * generate sample waveforms on systems that don't have data acquisition * hardware. * - * Configuration options: + * Auto-configuration is the default mode if no parameter is supplied during + * module loading. Manual configuration requires COMEDI userspace tool. + * To disable auto-configuration mode, pass "noauto=1" parameter for module + * loading. Refer modinfo or MODULE_PARM_DESC description below for details. + * + * Auto-configuration options: + * Refer modinfo or MODULE_PARM_DESC description below for details. + * + * Manual configuration options: * [0] - Amplitude in microvolts for fake waveforms (default 1 volt) * [1] - Period in microseconds for fake waveforms (default 0.1 sec) * @@ -53,8 +61,27 @@ #include <linux/timer.h> #include <linux/ktime.h> #include <linux/jiffies.h> +#include <linux/device.h> +#include <linux/kdev_t.h> #define N_CHANS 8 +#define DEV_NAME "comedi_testd" +#define CLASS_NAME "comedi_test" + +static bool config_mode; +static unsigned int set_amplitude; +static unsigned int set_period; +static struct class *ctcls; +static struct device *ctdev; + +module_param_named(noauto, config_mode, bool, 0444); +MODULE_PARM_DESC(noauto, "Disable auto-configuration: (1=disable [defaults to enable])"); + +module_param_named(amplitude, set_amplitude, uint, 0444); +MODULE_PARM_DESC(amplitude, "Set auto mode wave amplitude in microvolts: (defaults to 1 volt)"); + +module_param_named(period, set_period, uint, 0444); +MODULE_PARM_DESC(period, "Set auto mode wave period in microseconds: (defaults to 0.1 sec)"); /* Data unique to this driver */ struct waveform_private { @@ -607,13 +634,11 @@ static int waveform_ao_insn_write(struct comedi_device *dev, return insn->n; } -static int waveform_attach(struct comedi_device *dev, - struct comedi_devconfig *it) +static int waveform_common_attach(struct comedi_device *dev, + int amplitude, int period) { struct waveform_private *devpriv; struct comedi_subdevice *s; - int amplitude = it->options[0]; - int period = it->options[1]; int i; int ret; @@ -621,12 +646,6 @@ static int waveform_attach(struct comedi_device *dev, if (!devpriv) return -ENOMEM; - /* set default amplitude and period */ - if (amplitude <= 0) - amplitude = 1000000; /* 1 volt */ - if (period <= 0) - period = 100000; /* 0.1 sec */ - devpriv->wf_amplitude = amplitude; devpriv->wf_period = period; @@ -678,6 +697,36 @@ static int waveform_attach(struct comedi_device *dev, return 0; } +static int waveform_attach(struct comedi_device *dev, + struct comedi_devconfig *it) +{ + int amplitude = it->options[0]; + int period = it->options[1]; + + /* set default amplitude and period */ + if (amplitude <= 0) + amplitude = 1000000; /* 1 volt */ + if (period <= 0) + period = 100000; /* 0.1 sec */ + + return waveform_common_attach(dev, amplitude, period); +} + +static int waveform_auto_attach(struct comedi_device *dev, + unsigned long context_unused) +{ + int amplitude = set_amplitude; + int period = set_period; + + /* set default amplitude and period */ + if (!amplitude) + amplitude = 1000000; /* 1 volt */ + if (!period) + period = 100000; /* 0.1 sec */ + + return waveform_common_attach(dev, amplitude, period); +} + static void waveform_detach(struct comedi_device *dev) { struct waveform_private *devpriv = dev->private; @@ -692,9 +741,66 @@ static struct comedi_driver waveform_driver = { .driver_name = "comedi_test", .module = THIS_MODULE, .attach = waveform_attach, + .auto_attach = waveform_auto_attach, .detach = waveform_detach, }; -module_comedi_driver(waveform_driver); + +/* + * For auto-configuration, a device is created to stand in for a + * real hardware device. + */ +static int __init comedi_test_init(void) +{ + int ret; + + ret = comedi_driver_register(&waveform_driver); + if (ret) { + pr_err("comedi_test: unable to register driver\n"); + return ret; + } + + if (!config_mode) { + ctcls = class_create(THIS_MODULE, CLASS_NAME); + if (IS_ERR(ctcls)) { + pr_warn("comedi_test: unable to create class\n"); + return ret; + } + + ctdev = device_create(ctcls, NULL, MKDEV(0, 0), NULL, DEV_NAME); + if (IS_ERR(ctdev)) { + pr_warn("comedi_test: unable to create device\n"); + goto clean2; + } + + ret = comedi_auto_config(ctdev, &waveform_driver, 0); + if (ret) { + pr_warn("comedi_test: unable to auto-configure device\n"); + goto clean; + } + } + + return 0; + +clean: + device_destroy(ctcls, MKDEV(0, 0)); +clean2: + class_destroy(ctcls); + + return 0; +} +module_init(comedi_test_init); + +static void __exit comedi_test_exit(void) +{ + comedi_auto_unconfig(ctdev); + + device_destroy(ctcls, MKDEV(0, 0)); + + class_destroy(ctcls);
If the driver init returned successfully, but failed to set-up the auto-configured device, the device and class will not exist at this point, so those three calls need to go in an 'if' statement. Perhaps you could use 'if (ctcls) {', and set 'ctcls = NULL;' after calling 'class_destroy(ctcls);' in the init function.
Apart from that, it looks fine.
+ + comedi_driver_unregister(&waveform_driver); +} +module_exit(comedi_test_exit); MODULE_AUTHOR("Comedi http://www.comedi.org"); MODULE_DESCRIPTION("Comedi low-level driver");
-- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=- -=( Web: http://www.mev.co.uk/ )=- _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel