Re: [PATCH 33/53] staging: comedi: usbdux: tidy up usbdux_ai_inttrig()

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

 



On 2013-07-24 22:18, H Hartley Sweeten wrote:
Rename the local variable used for the private data pointer to the
comedi "norm".

Remove the unnecessary sanity check of the private data pointer. This
function can only be called is the private data was allocated during
the attach.

Tidy up the exit path using goto to ensure that the semaphore it
released.

Set the ai_cmd_running flag after submitting the urbs to remove the
need to clear the flag if the submit fails.

It's possible that (some of) the urbs could complete before you set the ai_cmd_running flag, and then usbdux_ai_urb_complete() would see the wrong value for ai_cmd_running. (I don't know if it needs memory barriers as well - there are probably enough memory barriers in the usb subsystem already.)


Return -EBUSY if an ai command is already running.

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

diff --git a/drivers/staging/comedi/drivers/usbdux.c b/drivers/staging/comedi/drivers/usbdux.c
index d0d683b..204867b 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -852,31 +852,31 @@ static int receive_dux_commands(struct comedi_device *dev, int command)
  }

  static int usbdux_ai_inttrig(struct comedi_device *dev,
-			     struct comedi_subdevice *s, unsigned int trignum)
+			     struct comedi_subdevice *s,
+			     unsigned int trignum)
  {
-	int ret;
-	struct usbdux_private *this_usbduxsub = dev->private;
-	if (!this_usbduxsub)
-		return -EFAULT;
+	struct usbdux_private *devpriv = dev->private;
+	int ret = -EINVAL;

-	down(&this_usbduxsub->sem);
+	down(&devpriv->sem);

-	if (trignum != 0) {
-		up(&this_usbduxsub->sem);
-		return -EINVAL;
-	}
-	if (!(this_usbduxsub->ai_cmd_running)) {
-		this_usbduxsub->ai_cmd_running = 1;
+	if (trignum != 0)
+		goto ai_trig_exit;
+
+	if (!devpriv->ai_cmd_running) {
  		ret = usbduxsub_submit_inurbs(dev);
-		if (ret < 0) {
-			this_usbduxsub->ai_cmd_running = 0;
-			up(&this_usbduxsub->sem);
-			return ret;
-		}
+		if (ret < 0)
+			goto ai_trig_exit;
+
+		devpriv->ai_cmd_running = 1;
  		s->async->inttrig = NULL;
+	} else {
+		ret = -EBUSY;
  	}
-	up(&this_usbduxsub->sem);
-	return 1;
+
+ai_trig_exit:
+	up(&devpriv->sem);
+	return ret;
  }

  static int usbdux_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)



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