On 03/02/15 12:38, Nicholas Mc Guire wrote:
The if and the else branch code are identical - so the condition has no
effect on the effective code - this patch removes the condition and the
duplicated code.
Signed-off-by: Nicholas Mc Guire <hofrat@xxxxxxxxx>
---
The if and else branch are identical code thus the condition has no effect
if (cmd->scan_begin_src == TRIG_FOLLOW) {
/* internal trigger */
err |= cfc_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
} else {
/* external trigger */
/* should be level/edge, hi/lo specification here */
err |= cfc_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
}
As the comments indicate that they are serving different purposes this
looks like a bug. In any case - if intentional - it would need some
comments on why.
This needs a review by someone that knows the details of this driver.
Also not sure about the retained comment string if that is still valid now.
Patch was only compile tested for x86_64_defconfig + CONFIG_STAGING=y
CONFIG_COMEDI=m, COMEDI_ISA_DRIVERS=y, CONFIG_COMEDI_DT282X=m
Patch is against 3.0.19-rc7 (localversion = -next-20150203)
drivers/staging/comedi/drivers/dt282x.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/dt282x.c b/drivers/staging/comedi/drivers/dt282x.c
index 051dfb2..22c59e5 100644
--- a/drivers/staging/comedi/drivers/dt282x.c
+++ b/drivers/staging/comedi/drivers/dt282x.c
@@ -685,14 +685,8 @@ static int dt282x_ai_cmdtest(struct comedi_device *dev,
err |= cfc_check_trigger_arg_is(&cmd->start_arg, 0);
- if (cmd->scan_begin_src == TRIG_FOLLOW) {
- /* internal trigger */
- err |= cfc_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
- } else {
- /* external trigger */
- /* should be level/edge, hi/lo specification here */
I think what that comment means is that it should allow scan_begin_arg
to have various combinations of the CR_EDGE and CR_INVERT bits set.
I.e. it ought to allow whatever combination of CR_EDGE and CR_INVERT
better describes the nature of the external trigger signal, in addition
to allowing the lazy default value 0.
I don't know what the nature of the external trigger signal is, as I
haven't seen the manual. I think Hartley might have seen one.
- err |= cfc_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
- }
+ /* internal trigger */
That comment would be misleading as it could be an internal or external
trigger.
If you want to apply this patch, remove that comment first. But I'd
rather leave the existing code there as a reminder.
+ err |= cfc_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
err |= cfc_check_trigger_arg_min(&cmd->convert_arg, 4000);
--
-=( 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
