On 2014-04-24 00:07, H Hartley Sweeten wrote:
The chanlist is checked in Step 5 of the (*do_cmdtest) there is no
reason to check it again in the (*do_cmd). The only reasonm its done
is to get the actual 'seglen', the non-repeating length of the chanlist.

Save the 'seglen' found by pci171x_ai_check_chanlist() in the private
data and use that in the (*do_cmd).

Since do_cmdtest can be done while a command is running, one needs to be careful to make sure that any private data modified by do_cmdtest is not used by the running command, and only used by do_cmd when setting up the next command.

It's fine in this case though as devpriv->act_chanlist_len is only used while setting up the next command, although perhaps it ought to be renamed to devpriv->saved_chanlist_len, possibly by a subsequent patch. (Also, devpriv->act_chanlist_pos isn't used so can be removed.)


Refactor the error handling in pci171x_ai_check_chanlist() to work like
the cfc_check_*() helpers.

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/adv_pci1710.c | 29 +++++++++++++++-------------
  1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c 
b/drivers/staging/comedi/drivers/adv_pci1710.c
index 59c8bc4..6698d3c 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -330,6 +330,7 @@ static int pci171x_ai_check_chanlist(struct comedi_device 
*dev,
                                     struct comedi_subdevice *s,
                                     struct comedi_cmd *cmd)
  {
+       struct pci1710_private *devpriv = dev->private;
        unsigned int chan0 = CR_CHAN(cmd->chanlist[0]);
        unsigned int last_aref = CR_AREF(cmd->chanlist[0]);
        unsigned int next_chan = (chan0 + 1) & s->n_chan;
@@ -337,8 +338,10 @@ static int pci171x_ai_check_chanlist(struct comedi_device 
*dev,
        unsigned int seglen;
        int i;

-       if (cmd->chanlist_len == 1)
-               return 1; /* seglen=1 */
+       if (cmd->chanlist_len == 1) {
+               devpriv->act_chanlist_len = cmd->chanlist_len;
+               return 0;
+       }

        /* first channel is always ok */
        chansegment[0] = cmd->chanlist[0];
@@ -353,7 +356,7 @@ static int pci171x_ai_check_chanlist(struct comedi_device 
*dev,
                if (aref == AREF_DIFF && (chan & 1)) {
                        dev_err(dev->class_dev,
                                "Odd channel cannot be differential input!\n");
-                       return 0;
+                       return -EINVAL;
                }

                if (last_aref == AREF_DIFF)
@@ -362,7 +365,7 @@ static int pci171x_ai_check_chanlist(struct comedi_device 
*dev,
                        dev_err(dev->class_dev,
                                "channel list must be continuous! chanlist[%i]=%d 
but must be %d or %d!\n",
                                i, chan, next_chan, chan0);
-                       return 0;
+                       return -EINVAL;
                }

                /* next correct channel in list */
@@ -381,10 +384,12 @@ static int pci171x_ai_check_chanlist(struct comedi_device 
*dev,
                                CR_CHAN(cmd->chanlist[i % seglen]),
                                CR_RANGE(cmd->chanlist[i % seglen]),
                                CR_AREF(chansegment[i % seglen]));
-                       return 0;
+                       return -EINVAL;
                }
        }
-       return seglen;
+       devpriv->act_chanlist_len = seglen;
+
+       return 0;
  }

  static void setup_channel_list(struct comedi_device *dev,
@@ -396,7 +401,6 @@ static void setup_channel_list(struct comedi_device *dev,
        struct pci1710_private *devpriv = dev->private;
        unsigned int i, range, chanprog;

-       devpriv->act_chanlist_len = seglen;
        devpriv->act_chanlist_pos = 0;

        for (i = 0; i < seglen; i++) {       /*  store range list to card */
@@ -953,7 +957,6 @@ static int pci171x_ai_cmd(struct comedi_device *dev, struct 
comedi_subdevice *s)
        struct pci1710_private *devpriv = dev->private;
        struct comedi_cmd *cmd = &s->async->cmd;
        unsigned int divisor1 = 0, divisor2 = 0;
-       unsigned int seglen;
        int mode;

        if (cmd->convert_src == TRIG_TIMER) {
@@ -967,10 +970,8 @@ static int pci171x_ai_cmd(struct comedi_device *dev, 
struct comedi_subdevice *s)

        start_pacer(dev, -1, 0, 0);     /*  stop pacer */

-       seglen = pci171x_ai_check_chanlist(dev, s, cmd);
-       if (seglen < 1)
-               return -EINVAL;
-       setup_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len, seglen);
+       setup_channel_list(dev, s, cmd->chanlist, cmd->chanlist_len,
+                          devpriv->act_chanlist_len);

        outb(0, dev->iobase + PCI171x_CLRFIFO);
        outb(0, dev->iobase + PCI171x_CLRINT);
@@ -1104,7 +1105,9 @@ static int pci171x_ai_cmdtest(struct comedi_device *dev,

        /* Step 5: check channel list */

-       if (!pci171x_ai_check_chanlist(dev, s, cmd))
+       err |= pci171x_ai_check_chanlist(dev, s, cmd);
+
+       if (err)
                return 5;

        return 0;



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