On Tuesday, September 16, 2014 4:40 AM, Luca Ellero wrote:
> Add function for counter subdevice configuration.
>
> Signed-off-by: Luca Ellero <luca.ellero@xxxxxxxxxxxxxxxx>
> ---
> drivers/staging/comedi/drivers/ni_usb6501.c | 28 +++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/staging/comedi/drivers/ni_usb6501.c b/drivers/staging/comedi/drivers/ni_usb6501.c
> index 60fc1ee..31a798e 100644
> --- a/drivers/staging/comedi/drivers/ni_usb6501.c
> +++ b/drivers/staging/comedi/drivers/ni_usb6501.c
> @@ -439,6 +439,34 @@ static int ni6501_dio_insn_bits(struct comedi_device *dev,
> return insn->n;
> }
Again, this patch causes a build warning:
drivers/staging/comedi/drivers/ni_usb6501.c:442:12: warning: 'ni6501_cnt_insn_config' defined but not used [-Wunused-function]
But, it does fix the one caused by patch 5/9... ;-)
> +static int ni6501_cnt_insn_config(struct comedi_device *dev,
> + struct comedi_subdevice *s,
> + struct comedi_insn *insn,
> + unsigned int *data)
> +{
> + int ret;
> + u32 counter = 0;
> +
> + switch (data[0]) {
> + case INSN_CONFIG_ARM:
> + ret = ni6501_counter_command(dev, START_COUNTER, NULL);
> + break;
> + case INSN_CONFIG_DISARM:
> + ret = ni6501_counter_command(dev, STOP_COUNTER, NULL);
> + break;
> + case INSN_CONFIG_RESET:
> + ret = ni6501_counter_command(dev, STOP_COUNTER, NULL);
> + if (ret)
> + break;
> + ret = ni6501_counter_command(dev, WRITE_COUNTER, &counter);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
ni6501_counter_command() returns 0 or an errno.
Ian suggested that this if fine because it will prevent the comedi
core from attempting to write the data array back to the user.
This is not true because the INSN_CONFIG instruction is defined as:
#define INSN_CONFIG (3 | INSN_MASK_READ|INSN_MASK_WRITE)
And the do_insnlist_ioctl() function will copy back the data array
as long as the return value of parse_insn() is >= 0 and the insn has
INSN_MASK_READ set.
For aesthetics, I would prefer this function returns the insn->n value
if it succeeds. This would work and follows the norm for other comedi
drivers:
return ret ? ret : insn->n;
Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
