Ian Abbott <abbotti@xxxxxxxxx> writes:
> commit 677a31565692d596ef42ea589b53ba289abf4713 upstream.
>
> The `insn_bits` handler `ni_65xx_dio_insn_bits()` has a `for` loop that
> currently writes (optionally) and reads back up to 5 "ports" consisting
> of 8 channels each. It reads up to 32 1-bit channels but can only read
> and write a whole port at once - it needs to handle up to 5 ports as the
> first channel it reads might not be aligned on a port boundary. It
> breaks out of the loop early if the next port it handles is beyond the
> final port on the card. It also breaks out early on the 5th port in the
> loop if the first channel was aligned. Unfortunately, it doesn't check
> that the current port it is dealing with belongs to the comedi subdevice
> the `insn_bits` handler is acting on. That's a bug.
>
> Redo the `for` loop to terminate after the final port belonging to the
> subdevice, changing the loop variable in the process to simplify things
> a bit. The `for` loop could now try and handle more than 5 ports if the
> subdevice has more than 40 channels, but the test `if (bitshift >= 32)`
> ensures it will break out early after 4 or 5 ports (depending on whether
> the first channel is aligned on a port boundary). (`bitshift` will be
> between -7 and 7 inclusive on the first iteration, increasing by 8 for
> each subsequent operation.)
>
> Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> ---
> This patch applies to kernels 2.6.34.y through to 3.5.y inclusive.
> Similar patch already queued for 3.10.y and 3.11.y by Greg-KH.
> I can post similar patches for kernels 3.6.y to 3.9.y on request.
> Similar patch for 2.6.32.y
Thanks Ian, I'm queuing it for the 3.5.y kernel.
Cheers,
--
Luis
> --- drivers/staging/comedi/drivers/ni_65xx.c | 26
> +++++++++++--------------- 1 file changed, 11 insertions(+), 15
> deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/ni_65xx.c b/drivers/staging/comedi/drivers/ni_65xx.c
> index 0d27a93..c36efe0 100644
> --- a/drivers/staging/comedi/drivers/ni_65xx.c
> +++ b/drivers/staging/comedi/drivers/ni_65xx.c
> @@ -411,29 +411,25 @@ static int ni_65xx_dio_insn_bits(struct comedi_device *dev,
> struct comedi_subdevice *s,
> struct comedi_insn *insn, unsigned int *data)
> {
> - unsigned base_bitfield_channel;
> - const unsigned max_ports_per_bitfield = 5;
> + int base_bitfield_channel;
> unsigned read_bits = 0;
> - unsigned j;
> + int last_port_offset = ni_65xx_port_by_channel(s->n_chan - 1);
> + int port_offset;
> +
> if (insn->n != 2)
> return -EINVAL;
> base_bitfield_channel = CR_CHAN(insn->chanspec);
> - for (j = 0; j < max_ports_per_bitfield; ++j) {
> - const unsigned port_offset =
> - ni_65xx_port_by_channel(base_bitfield_channel) + j;
> - const unsigned port =
> - sprivate(s)->base_port + port_offset;
> - unsigned base_port_channel;
> + for (port_offset = ni_65xx_port_by_channel(base_bitfield_channel);
> + port_offset <= last_port_offset; port_offset++) {
> + unsigned port = sprivate(s)->base_port + port_offset;
> + int base_port_channel = port_offset * ni_65xx_channels_per_port;
> unsigned port_mask, port_data, port_read_bits;
> - int bitshift;
> - if (port >= ni_65xx_total_num_ports(board(dev)))
> + int bitshift = base_port_channel - base_bitfield_channel;
> +
> + if (bitshift >= 32)
> break;
> - base_port_channel = port_offset * ni_65xx_channels_per_port;
> port_mask = data[0];
> port_data = data[1];
> - bitshift = base_port_channel - base_bitfield_channel;
> - if (bitshift >= 32 || bitshift <= -32)
> - break;
> if (bitshift > 0) {
> port_mask >>= bitshift;
> port_data >>= bitshift;
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html