Re: [PATCH 07/13] staging: comedi: pcmuio: make sure input channels stay inputs

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

 



On 2013-07-24 19:48, H Hartley Sweeten wrote:
When updating the output channels make sure the channels configured as
inputs stay inputs.

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

diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c
index 076a08a..5b0ade2 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -215,8 +215,14 @@ static int pcmuio_dio_insn_bits(struct comedi_device *dev,
  		s->state &= ~mask;
  		s->state |= (mask & bits);

-		/* invert the state and update the channels */
+		/*
+		 * Invert the state and update the channels.
+		 *
+		 * The s->io_bits mask makes sure inputs are '0' so
+		 * that the output pins stay in a high-z state.
+		 */
  		val = s->state ^ ((0x1 << s->n_chan) - 1);
+		val &= s->io_bits;
  		pcmuio_write(dev, val, asic, 0, port);
  	}



That's fine although the original function has other problems, for example the value returned in data[1] should reflect the modified data, not the original data and probably shouldn't be modifying s->state based on the data read from the hardware. I think the function should be something like this:

static int pcmuio_dio_insn_bits(struct comedi_device *dev,
				struct comedi_subdevice *s,
				struct comedi_insn *insn,
				unsigned int *data)
{
	unsigned int chanmask = ((1 << s->n_chan) - 1;
	unsigned int mask = data[0] & chanmask;
	unsigned int bits = data[1];
	int asic = s->index / 2;
	int port = (s->index % 2) ? 3 : 0;
	unsigned int val;

	if (mask) {
		s->state &= ~mask;
		s->state |= (mask & bits);
		/*
		 * Invert the state and update the channels.
		 *
		 * The s->io_bits mask makes sure inputs are '0' so
		 * that the output pins stay in a high-z state.
		 */
		val = ~s->state & s->io_bits & chanmask;
		pcmuio_write(dev, val, asic, 0, port);
	}
		
	/* get inverted state of the channels from the port */
	val = pcmuio_read(dev, asic, 0, port);

	/* get the true state of the channels */
	data[1] = ~val & chanmask;

	return insn->n;
}


--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
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