Re: [PATCH 11/14] staging: comedi: multiq3: tidy up multiq3_encoder_insn_read()

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

 



On 03/10/15 01:30, H Hartley Sweeten wrote:
Encoders are not a "normal" subdevice in comedi. For aesthetics, tidy
up this function and add a couple comments to clarify the function.

Remove the strange munging of the data. The encoder data is decoded
in quadrature and used to increment or decrement a 24-bit counter.
Adding 0x800000 to the counter value doesn't make any sense.

I think it's because the counter starts at zero when reset, and moving it backwards would make it go negative, which the comedi user would see as a large jump in the unsigned value. The offset biases it to the middle of the range so moving it backwards just decreases the value. Of course if you keep moving it in the same direction for long enough, it will eventually wrap around, but that shouldn't happen if there are physical limits on how far the encoder can move forwards and backwards.


Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
  drivers/staging/comedi/drivers/multiq3.c | 26 +++++++++++++++++---------
  1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/drivers/multiq3.c b/drivers/staging/comedi/drivers/multiq3.c
index b5d2354..955f75a 100644
--- a/drivers/staging/comedi/drivers/multiq3.c
+++ b/drivers/staging/comedi/drivers/multiq3.c
@@ -185,22 +185,30 @@ static int multiq3_encoder_insn_read(struct comedi_device *dev,
  				     struct comedi_insn *insn,
  				     unsigned int *data)
  {
-	int chan = CR_CHAN(insn->chanspec);
-	int value;
-	int n;
+	unsigned int chan = CR_CHAN(insn->chanspec);
+	unsigned int val;
+	int i;

-	for (n = 0; n < insn->n; n++) {
+	for (i = 0; i < insn->n; i++) {
+		/* select encoder channel */
  		multiq3_set_ctrl(dev, MULTIQ3_CTRL_EN |
  				      MULTIQ3_CTRL_E_CHAN(chan));
+
+		/* reset the byte pointer */
  		outb(MULTIQ3_BP_RESET, dev->iobase + MULTIQ3_ENC_CTRL_REG);
+
+		/* latch the data */
  		outb(MULTIQ3_TRSFRCNTR_OL, dev->iobase + MULTIQ3_ENC_CTRL_REG);
-		value = inb(dev->iobase + MULTIQ3_ENC_DATA_REG);
-		value |= (inb(dev->iobase + MULTIQ3_ENC_DATA_REG) << 8);
-		value |= (inb(dev->iobase + MULTIQ3_ENC_DATA_REG) << 16);
-		data[n] = (value + 0x800000) & 0xffffff;
+
+		/* read the 24-bit encoder data (lsb/mid/msb) */
+		val = inb(dev->iobase + MULTIQ3_ENC_DATA_REG);
+		val |= (inb(dev->iobase + MULTIQ3_ENC_DATA_REG) << 8);
+		val |= (inb(dev->iobase + MULTIQ3_ENC_DATA_REG) << 16);
+
+		data[i] = val;
  	}

-	return n;
+	return insn->n;
  }

  static void multiq3_encoder_reset(struct comedi_device *dev,



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