[PATCH v2] Staging: comedi: drivers: comedi_test: Add auto-configuration capability
Ian Abbott
abbotti at mev.co.uk
Mon Feb 13 11:14:14 UTC 2017
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 at gmail.com>
> ---
>
> 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 at mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
More information about the devel
mailing list