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 <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
  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: <abbo...@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to