Re: [PATCH RFC] comedi: rtd520: if condition with no effect - if identical to else

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18/01/15 11:53, 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 <der.herr@xxxxxxx>
---

Patch is against 3.19.0-rc4 -next-20150116

This patch was only compile tested with x86_64_defconfig + CONFIG_STAGING=y
CONFIG_COMEDI=m, CONFIG_COMEDI_PCI_DRIVERS=m, CONFIG_COMEDI_RTD520=m

commit c5930d66 (staging: comedi: rtd520: use the comedi_device 'mmio'
member) introduced the current code, the probably bug was though
present before this commit:

         if (devpriv->xfer_count > 0)    /* transfer every N samples */
                 writew(IRQM_ADC_ABOUT_CNT, dev->mmio + LAS0_IT);
         else                            /* 1/2 FIFO transfers */
                 writew(IRQM_ADC_ABOUT_CNT, dev->mmio + LAS0_IT);

as the if and else branch are identical but the comments indicate that
they are serving different purposes this looks like a bug. The code has
though been in use in this form for more than 2 years - so maybe the
condition is simply unnecessary. In any case this needs a review
by someone who knows the details of the driver - the below patch
is only cleaning up the current situation and may thus be wrong.

...and how did this pass multiple modifications and reviews ?...

The two halves used to output different kernel debug messages which were removed in commit d62bc468. Prior to that, in commit 60aeb080, a whole bunch of unused #ifdef code was removed, which made the 'else' branch the same as the 'if' branch, apart from the debug messages, as can be seen here around line 1319 on the left side:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/staging/comedi/drivers/?id=60aeb08024dcd8a392917ab72a107ad451a59030&context=5&ignorews=0&dt=1


  drivers/staging/comedi/drivers/rtd520.c |    6 ++----
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/rtd520.c b/drivers/staging/comedi/drivers/rtd520.c
index 06ae436..3ccdade 100644
--- a/drivers/staging/comedi/drivers/rtd520.c
+++ b/drivers/staging/comedi/drivers/rtd520.c
@@ -1012,10 +1012,8 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
  	readw(dev->mmio + LAS0_CLEAR);

  	/* TODO: allow multiple interrupt sources */
-	if (devpriv->xfer_count > 0)	/* transfer every N samples */
-		writew(IRQM_ADC_ABOUT_CNT, dev->mmio + LAS0_IT);
-	else				/* 1/2 FIFO transfers */
-		writew(IRQM_ADC_ABOUT_CNT, dev->mmio + LAS0_IT);
+	/* transfer every N samples */
+	writew(IRQM_ADC_ABOUT_CNT, dev->mmio + LAS0_IT);

  	/* BUG: start_src is ASSUMED to be TRIG_NOW */
  	/* BUG? it seems like things are running before the "start" */


Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>

--
-=( 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



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux