staging: comedi: vmk80xx: wait for URBs to complete

For Velleman K8055 (aka VM110), `vmk80xx_read_packet()` and
`vmk8055_write_packet()` send an URB asynchronously and do not wait for
it complete.  However, callers of `vmk80xx_read_packet()` are assuming
the contents of the data buffer `devpriv->usb_rx_buf` are valid
immediately after that function returns.

For Velleman K8061 (aka VM140), `vmk80xx_read_packet()` and
`vmk80xx_write_packet()` punt the requests to `vmk80xx_do_bulk_msg()`
which *does* wait for the URBs to complete (albeit with no error
checking!).

Change `vmk80xx_read_packet()` and `vmk80xx_write_packet()` to use
`usb_interrupt_msg()` for the K8055, so the callers of
`vmk80xx_read_packet()` can assume the data buffer contents are valid
(if no error occurred).  Remove all the code for checking for transfers
in progress and busy waiting, as it's no longer needed.  Pretty much all
the callers of `vmk80xx_read_packet()` and `vmk80xx_write_packet()` hold
the same semaphore anyway, and the only caller that doesn't
(`vmk80xx_reset_device()` called during initialization of the device)
doesn't really matter.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/staging/comedi/drivers/vmk80xx.c

@@ -100,16 +100,9 @@ enum {
 #define VMK8061_CMD_RD_AO       0x0f
 #define VMK8061_CMD_RD_PWM      0x10
 
-#define TRANS_OUT_BUSY          1
-#define TRANS_IN_BUSY           2
-#define TRANS_IN_RUNNING        3
-
 #define IC3_VERSION             (1 << 0)
 #define IC6_VERSION             (1 << 1)
 
-#define URB_RCV_FLAG            (1 << 0)
-#define URB_SND_FLAG            (1 << 1)
-
 enum vmk80xx_model {
 	VMK8055_MODEL,
 	VMK8061_MODEL
@@ -170,60 +163,13 @@ struct vmk80xx_private {
 	struct usb_interface *intf;
 	struct usb_endpoint_descriptor *ep_rx;
 	struct usb_endpoint_descriptor *ep_tx;
-	struct usb_anchor rx_anchor;
-	struct usb_anchor tx_anchor;
 	struct firmware_version fw;
 	struct semaphore limit_sem;
-	wait_queue_head_t read_wait;
-	wait_queue_head_t write_wait;
 	unsigned char *usb_rx_buf;
 	unsigned char *usb_tx_buf;
-	unsigned long flags;
 	enum vmk80xx_model model;
 };
 
-static void vmk80xx_tx_callback(struct urb *urb)
-{
-	struct vmk80xx_private *devpriv = urb->context;
-	unsigned long *flags = &devpriv->flags;
-
-	if (!test_bit(TRANS_OUT_BUSY, flags))
-		return;
-
-	clear_bit(TRANS_OUT_BUSY, flags);
-
-	wake_up_interruptible(&devpriv->write_wait);
-}
-
-static void vmk80xx_rx_callback(struct urb *urb)
-{
-	struct vmk80xx_private *devpriv = urb->context;
-	unsigned long *flags = &devpriv->flags;
-	int stat = urb->status;
-
-	switch (stat) {
-	case 0:
-		break;
-	case -ENOENT:
-	case -ECONNRESET:
-	case -ESHUTDOWN:
-		break;
-	default:
-		/* Try to resubmit the urb */
-		if (test_bit(TRANS_IN_RUNNING, flags) && devpriv->intf) {
-			usb_anchor_urb(urb, &devpriv->rx_anchor);
-
-			if (usb_submit_urb(urb, GFP_KERNEL))
-				usb_unanchor_urb(urb);
-		}
-		break;
-	}
-
-	clear_bit(TRANS_IN_BUSY, flags);
-
-	wake_up_interruptible(&devpriv->read_wait);
-}
-
 static int vmk80xx_check_data_link(struct vmk80xx_private *devpriv)
 {
 	struct usb_device *usb = devpriv->usb;
@@ -277,50 +223,15 @@ static void vmk80xx_read_eeprom(struct vmk80xx_private *devpriv, int flag)
 		strncpy(devpriv->fw.ic6_vers, rx + 25, 24);
 }
 
-static void vmk80xx_build_int_urb(struct urb *urb, int flag)
-{
-	struct vmk80xx_private *devpriv = urb->context;
-	struct usb_device *usb = devpriv->usb;
-	__u8 rx_addr;
-	__u8 tx_addr;
-	unsigned int pipe;
-	unsigned char *buf;
-	size_t size;
-	void (*callback) (struct urb *);
-	int ival;
-
-	if (flag & URB_RCV_FLAG) {
-		rx_addr = devpriv->ep_rx->bEndpointAddress;
-		pipe = usb_rcvintpipe(usb, rx_addr);
-		buf = devpriv->usb_rx_buf;
-		size = le16_to_cpu(devpriv->ep_rx->wMaxPacketSize);
-		callback = vmk80xx_rx_callback;
-		ival = devpriv->ep_rx->bInterval;
-	} else {		/* URB_SND_FLAG */
-		tx_addr = devpriv->ep_tx->bEndpointAddress;
-		pipe = usb_sndintpipe(usb, tx_addr);
-		buf = devpriv->usb_tx_buf;
-		size = le16_to_cpu(devpriv->ep_tx->wMaxPacketSize);
-		callback = vmk80xx_tx_callback;
-		ival = devpriv->ep_tx->bInterval;
-	}
-
-	usb_fill_int_urb(urb, usb, pipe, buf, size, callback, devpriv, ival);
-}
-
 static void vmk80xx_do_bulk_msg(struct vmk80xx_private *devpriv)
 {
 	struct usb_device *usb = devpriv->usb;
-	unsigned long *flags = &devpriv->flags;
 	__u8 tx_addr;
 	__u8 rx_addr;
 	unsigned int tx_pipe;
 	unsigned int rx_pipe;
 	size_t size;
 
-	set_bit(TRANS_IN_BUSY, flags);
-	set_bit(TRANS_OUT_BUSY, flags);
-
 	tx_addr = devpriv->ep_tx->bEndpointAddress;
 	rx_addr = devpriv->ep_rx->bEndpointAddress;
 	tx_pipe = usb_sndbulkpipe(usb, tx_addr);
@@ -335,102 +246,52 @@ static void vmk80xx_do_bulk_msg(struct vmk80xx_private *devpriv)
 	usb_bulk_msg(usb, tx_pipe, devpriv->usb_tx_buf,
 		     size, NULL, devpriv->ep_tx->bInterval);
 	usb_bulk_msg(usb, rx_pipe, devpriv->usb_rx_buf, size, NULL, HZ * 10);
-
-	clear_bit(TRANS_OUT_BUSY, flags);
-	clear_bit(TRANS_IN_BUSY, flags);
 }
 
 static int vmk80xx_read_packet(struct vmk80xx_private *devpriv)
 {
-	unsigned long *flags = &devpriv->flags;
-	struct urb *urb;
-	int retval;
+	struct usb_device *usb;
+	struct usb_endpoint_descriptor *ep;
+	unsigned int pipe;
 
 	if (!devpriv->intf)
 		return -ENODEV;
 
-	/* Only useful for interrupt transfers */
-	if (test_bit(TRANS_IN_BUSY, flags))
-		if (wait_event_interruptible(devpriv->read_wait,
-					     !test_bit(TRANS_IN_BUSY, flags)))
-			return -ERESTART;
-
 	if (devpriv->model == VMK8061_MODEL) {
 		vmk80xx_do_bulk_msg(devpriv);
-
 		return 0;
 	}
 
-	urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!urb)
-		return -ENOMEM;
-
-	urb->context = devpriv;
-	vmk80xx_build_int_urb(urb, URB_RCV_FLAG);
-
-	set_bit(TRANS_IN_RUNNING, flags);
-	set_bit(TRANS_IN_BUSY, flags);
-
-	usb_anchor_urb(urb, &devpriv->rx_anchor);
-
-	retval = usb_submit_urb(urb, GFP_KERNEL);
-	if (!retval)
-		goto exit;
-
-	clear_bit(TRANS_IN_RUNNING, flags);
-	usb_unanchor_urb(urb);
-
-exit:
-	usb_free_urb(urb);
-
-	return retval;
+	usb = devpriv->usb;
+	ep = devpriv->ep_rx;
+	pipe = usb_rcvintpipe(usb, ep->bEndpointAddress);
+	return usb_interrupt_msg(usb, pipe, devpriv->usb_rx_buf,
+				 le16_to_cpu(ep->wMaxPacketSize), NULL,
+				 HZ * 10);
 }
 
 static int vmk80xx_write_packet(struct vmk80xx_private *devpriv, int cmd)
 {
-	unsigned long *flags = &devpriv->flags;
-	struct urb *urb;
-	int retval;
+	struct usb_device *usb;
+	struct usb_endpoint_descriptor *ep;
+	unsigned int pipe;
 
 	if (!devpriv->intf)
 		return -ENODEV;
 
-	if (test_bit(TRANS_OUT_BUSY, flags))
-		if (wait_event_interruptible(devpriv->write_wait,
-					     !test_bit(TRANS_OUT_BUSY, flags)))
-			return -ERESTART;
+	devpriv->usb_tx_buf[0] = cmd;
 
 	if (devpriv->model == VMK8061_MODEL) {
-		devpriv->usb_tx_buf[0] = cmd;
 		vmk80xx_do_bulk_msg(devpriv);
-
 		return 0;
 	}
 
-	urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!urb)
-		return -ENOMEM;
-
-	urb->context = devpriv;
-	vmk80xx_build_int_urb(urb, URB_SND_FLAG);
-
-	set_bit(TRANS_OUT_BUSY, flags);
-
-	usb_anchor_urb(urb, &devpriv->tx_anchor);
-
-	devpriv->usb_tx_buf[0] = cmd;
-
-	retval = usb_submit_urb(urb, GFP_KERNEL);
-	if (!retval)
-		goto exit;
-
-	clear_bit(TRANS_OUT_BUSY, flags);
-	usb_unanchor_urb(urb);
-
-exit:
-	usb_free_urb(urb);
-
-	return retval;
+	usb = devpriv->usb;
+	ep = devpriv->ep_tx;
+	pipe = usb_sndintpipe(usb, ep->bEndpointAddress);
+	return usb_interrupt_msg(usb, pipe, devpriv->usb_tx_buf,
+				 le16_to_cpu(ep->wMaxPacketSize), NULL,
+				 HZ * 10);
 }
 
 static int vmk80xx_reset_device(struct vmk80xx_private *devpriv)
@@ -447,25 +308,6 @@ static int vmk80xx_reset_device(struct vmk80xx_private *devpriv)
 	return vmk80xx_write_packet(devpriv, VMK8055_CMD_WRT_AD);
 }
 
-#define DIR_IN  1
-#define DIR_OUT 2
-
-static int rudimentary_check(struct vmk80xx_private *devpriv, int dir)
-{
-	if (!devpriv)
-		return -EFAULT;
-	if (dir & DIR_IN) {
-		if (test_bit(TRANS_IN_BUSY, &devpriv->flags))
-			return -EBUSY;
-	}
-	if (dir & DIR_OUT) {
-		if (test_bit(TRANS_OUT_BUSY, &devpriv->flags))
-			return -EBUSY;
-	}
-
-	return 0;
-}
-
 static int vmk80xx_ai_insn_read(struct comedi_device *dev,
 				struct comedi_subdevice *s,
 				struct comedi_insn *insn,
@@ -476,10 +318,6 @@ static int vmk80xx_ai_insn_read(struct comedi_device *dev,
 	int reg[2];
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_IN);
-	if (n)
-		return n;
-
 	down(&devpriv->limit_sem);
 	chan = CR_CHAN(insn->chanspec);
 
@@ -529,10 +367,6 @@ static int vmk80xx_ao_insn_write(struct comedi_device *dev,
 	int reg;
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_OUT);
-	if (n)
-		return n;
-
 	down(&devpriv->limit_sem);
 	chan = CR_CHAN(insn->chanspec);
 
@@ -573,10 +407,6 @@ static int vmk80xx_ao_insn_read(struct comedi_device *dev,
 	int reg;
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_IN);
-	if (n)
-		return n;
-
 	down(&devpriv->limit_sem);
 	chan = CR_CHAN(insn->chanspec);
 
@@ -606,10 +436,6 @@ static int vmk80xx_di_insn_bits(struct comedi_device *dev,
 	int reg;
 	int retval;
 
-	retval = rudimentary_check(devpriv, DIR_IN);
-	if (retval)
-		return retval;
-
 	down(&devpriv->limit_sem);
 
 	rx_buf = devpriv->usb_rx_buf;
@@ -646,16 +472,10 @@ static int vmk80xx_do_insn_bits(struct comedi_device *dev,
 {
 	struct vmk80xx_private *devpriv = dev->private;
 	unsigned char *rx_buf, *tx_buf;
-	int dir, reg, cmd;
+	int reg, cmd;
 	int retval;
 
-	dir = 0;
-
-	if (data[0])
-		dir |= DIR_OUT;
-
 	if (devpriv->model == VMK8061_MODEL) {
-		dir |= DIR_IN;
 		reg = VMK8061_DO_REG;
 		cmd = VMK8061_CMD_DO;
 	} else { /* VMK8055_MODEL */
@@ -663,10 +483,6 @@ static int vmk80xx_do_insn_bits(struct comedi_device *dev,
 		cmd = VMK8055_CMD_WRT_AD;
 	}
 
-	retval = rudimentary_check(devpriv, dir);
-	if (retval)
-		return retval;
-
 	down(&devpriv->limit_sem);
 
 	rx_buf = devpriv->usb_rx_buf;
@@ -712,10 +528,6 @@ static int vmk80xx_cnt_insn_read(struct comedi_device *dev,
 	int reg[2];
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_IN);
-	if (n)
-		return n;
-
 	down(&devpriv->limit_sem);
 	chan = CR_CHAN(insn->chanspec);
 
@@ -762,10 +574,6 @@ static int vmk80xx_cnt_insn_config(struct comedi_device *dev,
 	int reg;
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_OUT);
-	if (n)
-		return n;
-
 	insn_cmd = data[0];
 	if (insn_cmd != INSN_CONFIG_RESET && insn_cmd != GPCT_RESET)
 		return -EINVAL;
@@ -809,10 +617,6 @@ static int vmk80xx_cnt_insn_write(struct comedi_device *dev,
 	int cmd;
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_OUT);
-	if (n)
-		return n;
-
 	down(&devpriv->limit_sem);
 	chan = CR_CHAN(insn->chanspec);
 
@@ -856,10 +660,6 @@ static int vmk80xx_pwm_insn_read(struct comedi_device *dev,
 	int reg[2];
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_IN);
-	if (n)
-		return n;
-
 	down(&devpriv->limit_sem);
 
 	tx_buf = devpriv->usb_tx_buf;
@@ -893,10 +693,6 @@ static int vmk80xx_pwm_insn_write(struct comedi_device *dev,
 	int cmd;
 	int n;
 
-	n = rudimentary_check(devpriv, DIR_OUT);
-	if (n)
-		return n;
-
 	down(&devpriv->limit_sem);
 
 	tx_buf = devpriv->usb_tx_buf;
@@ -1106,11 +902,6 @@ static int vmk80xx_auto_attach(struct comedi_device *dev,
 		return ret;
 
 	sema_init(&devpriv->limit_sem, 8);
-	init_waitqueue_head(&devpriv->read_wait);
-	init_waitqueue_head(&devpriv->write_wait);
-
-	init_usb_anchor(&devpriv->rx_anchor);
-	init_usb_anchor(&devpriv->tx_anchor);
 
 	usb_set_intfdata(intf, devpriv);
 
@@ -1141,9 +932,6 @@ static void vmk80xx_detach(struct comedi_device *dev)
 
 	usb_set_intfdata(devpriv->intf, NULL);
 
-	usb_kill_anchored_urbs(&devpriv->rx_anchor);
-	usb_kill_anchored_urbs(&devpriv->tx_anchor);
-
 	kfree(devpriv->usb_rx_buf);
 	kfree(devpriv->usb_tx_buf);