Skip to contentSkip to conversation

USB: usbtmc: Add support for missing USBTMC-USB488 spec

PATCH Superseded dave penkler 3 participants 4 messages Oct 14, 2015 USB

v1 of 7 Superseded Latest is v7 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 USB: usbtmc: Add support for missing USBTMC-USB488 spec

Implement support for the USB488 defined READ_STATUS_BYTE and SRQ notifications with ioctl, fasync and poll/select in order to be able to synchronize with variable duration instrument operations.

Add ioctls for other USB488 requests: REN_CONTROL, GOTO_LOCAL and LOCAL_LOCKOUT.

Add convenience ioctl to return all device capabilities.

---
 drivers/usb/class/usbtmc.c   | 359 +++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/usb/tmc.h |  42 +++--
 2 files changed, 391 insertions(+), 10 deletions(-)
drivers/usb/class/usbtmc.c +359 -0
index 7a11a82..83c8416 100644
@@ -27,6 +27,7 @@
 #include <linux/uaccess.h>
 #include <linux/kref.h>
 #include <linux/slab.h>
+#include <linux/poll.h>
 #include <linux/mutex.h>
 #include <linux/usb.h>
 #include <linux/usb/tmc.h>
@@ -87,6 +88,24 @@ struct usbtmc_device_data {
 	u8 bTag_last_write;	/* needed for abort */
 	u8 bTag_last_read;	/* needed for abort */
 
+
+	/* data for interrupt in endpoint handling */
+	u8             bNotify1;
+	u8             bNotify2;
+	u16            ifnum;
+	u8             iin_bTag;
+	u8            *iin_buffer;
+	atomic_t       iin_data_valid;
+	unsigned int   iin_ep;
+	int            iin_ep_present;
+	int            iin_interval;
+	struct urb    *iin_urb;
+	u16            iin_wMaxPacketSize;
+	atomic_t       srq_asserted;
+
+	/* coalesced usb488_caps from usbtmc_dev_capabilities */
+	u8 usb488_caps;
+
 	u8 rigol_quirk;
 
 	/* attributes from the USB TMC spec for this device */
@@ -99,6 +118,8 @@ struct usbtmc_device_data {
 	struct usbtmc_dev_capabilities	capabilities;
 	struct kref kref;
 	struct mutex io_mutex;	/* only one i/o function running at a time */
+	wait_queue_head_t waitq;
+	struct fasync_struct *fasync;
 };
 #define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
 
@@ -373,6 +394,158 @@ exit:
 	return rv;
 }
 
+static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
+				unsigned long arg)
+{
+	u8 *buffer;
+	struct device *dev;
+	int rv;
+	u8 tag, stb;
+
+	dev = &data->intf->dev;
+
+	dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
+		data->iin_ep_present);
+
+	buffer = kmalloc(8, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+
+	atomic_set(&data->iin_data_valid, 0);
+
+	/* must issue read_stb before using poll or select */
+	atomic_set(&data->srq_asserted, 0);
+
+	rv = usb_control_msg(data->usb_dev,
+			usb_rcvctrlpipe(data->usb_dev, 0),
+			USBTMC488_REQUEST_READ_STATUS_BYTE,
+			USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			data->iin_bTag,
+			data->ifnum,
+			buffer, 0x03, USBTMC_TIMEOUT);
+
+	if (rv < 0) {
+		dev_err(dev, "stb usb_control_msg returned %d\n", rv);
+		goto exit;
+	}
+
+	if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+		dev_err(dev, "control status returned %x\n",
+			buffer[0]);
+		rv = -EIO;
+		goto exit;
+	}
+
+	if (data->iin_ep_present) {
+
+		rv = wait_event_interruptible_timeout(
+			data->waitq,
+			(atomic_read(&data->iin_data_valid) != 0),
+			USBTMC_TIMEOUT
+			);
+
+		if (rv < 0) {
+			dev_dbg(dev, "wait interrupted %d\n", rv);
+			goto exit;
+		}
+
+		if (rv == 0) {
+			dev_dbg(dev, "wait timed out\n");
+			rv = -ETIME;
+			goto exit;
+		}
+
+		tag = data->bNotify1 & 0x7f;
+
+		if (tag != data->iin_bTag) {
+			dev_err(dev, "expected bTag %x got %x\n",
+				data->iin_bTag, tag);
+		}
+
+		stb = data->bNotify2;
+	} else {
+		stb = buffer[2];
+	}
+
+	/* bump interrupt bTag */
+	data->iin_bTag += 1;
+	if (data->iin_bTag > 127)
+		data->iin_bTag = 2;
+
+	rv = copy_to_user((void *)arg, &stb, sizeof(stb));
+	if (rv)
+		rv = -EFAULT;
+
+ exit:
+	kfree(buffer);
+	return rv;
+
+}
+
+static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
+				unsigned long arg,
+				unsigned int cmd)
+{
+	u8 *buffer;
+	struct device *dev;
+	int rv;
+	unsigned int val;
+	u16 wValue;
+
+	dev = &data->intf->dev;
+
+	if (0 == (data->usb488_caps & USBTMC488_CAPABILITY_SIMPLE))
+		return -EINVAL;
+
+	buffer = kmalloc(8, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+
+	if (cmd == USBTMC488_REQUEST_REN_CONTROL) {
+		rv = copy_from_user(&val, (void *)arg, sizeof(val));
+		if (rv) {
+			rv = -EFAULT;
+			goto exit;
+		}
+		wValue = (val) ? 1 : 0;
+	} else {
+		wValue = 0;
+	}
+
+	rv = usb_control_msg(data->usb_dev,
+			usb_rcvctrlpipe(data->usb_dev, 0),
+			cmd,
+			USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			wValue,
+			data->ifnum,
+			buffer, 0x01, USBTMC_TIMEOUT);
+
+	if (rv < 0) {
+		dev_err(dev, "simple usb_control_msg failed %d\n", rv);
+		goto exit;
+	} else if (rv != 1) {
+		dev_warn(dev, "simple usb_control_msg returned %d\n", rv);
+		rv = -EIO;
+		goto exit;
+	}
+
+	if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+		dev_err(dev, "simple control status returned %x\n",
+			buffer[0]);
+		rv = -EIO;
+		goto exit;
+	} else {
+		rv = 0;
+	}
+
+ exit:
+	kfree(buffer);
+	return rv;
+}
+
+
 /*
  * Sends a REQUEST_DEV_DEP_MSG_IN message on the Bulk-IN endpoint.
  * @transfer_size: number of bytes to request from the device.
@@ -895,6 +1068,7 @@ static int get_capabilities(struct usbtmc_device_data *data)
 	data->capabilities.device_capabilities = buffer[5];
 	data->capabilities.usb488_interface_capabilities = buffer[14];
 	data->capabilities.usb488_device_capabilities = buffer[15];
+	data->usb488_caps = (buffer[14] & 0x07) | ((buffer[15] & 0x0f) << 4);
 	rv = 0;
 
 err_out:
@@ -1069,6 +1243,33 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case USBTMC_IOCTL_ABORT_BULK_IN:
 		retval = usbtmc_ioctl_abort_bulk_in(data);
 		break;
+
+	case USBTMC488_IOCTL_GET_CAPABILITIES:
+		retval = copy_to_user((void *)arg,
+				&data->usb488_caps,
+				sizeof(data->usb488_caps));
+		if (retval)
+			retval = -EFAULT;
+		break;
+
+	case USBTMC488_IOCTL_READ_STB:
+		retval = usbtmc488_ioctl_read_stb(data, arg);
+		break;
+
+	case USBTMC488_IOCTL_REN_CONTROL:
+		retval = usbtmc488_ioctl_simple(data, arg,
+						USBTMC488_REQUEST_REN_CONTROL);
+		break;
+
+	case USBTMC488_IOCTL_GOTO_LOCAL:
+		retval = usbtmc488_ioctl_simple(data, arg,
+						USBTMC488_REQUEST_GOTO_LOCAL);
+		break;
+
+	case USBTMC488_IOCTL_LOCAL_LOCKOUT:
+		retval = usbtmc488_ioctl_simple(data, arg,
+						USBTMC488_REQUEST_LOCAL_LOCKOUT);
+		break;
 	}
 
 skip_io_on_zombie:
@@ -1076,6 +1277,33 @@ skip_io_on_zombie:
 	return retval;
 }
 
+static int usbtmc_fasync(int fd, struct file *file, int on)
+{
+	struct usbtmc_device_data *data = file->private_data;
+
+	return fasync_helper(fd, file, on, &data->fasync);
+}
+
+static unsigned int usbtmc_poll(struct file *file, poll_table *wait)
+{
+	struct usbtmc_device_data *data = file->private_data;
+	unsigned int mask = 0;
+
+	mutex_lock(&data->io_mutex);
+
+	if (data->zombie)
+		goto no_poll;
+
+	poll_wait(file, &data->waitq, wait);
+
+	mask = data->zombie ? POLLHUP | POLLERR :
+		(atomic_read(&data->srq_asserted)) ? POLLIN | POLLRDNORM : 0;
+
+no_poll:
+	mutex_unlock(&data->io_mutex);
+	return mask;
+}
+
 static const struct file_operations fops = {
 	.owner		= THIS_MODULE,
 	.read		= usbtmc_read,
@@ -1083,6 +1311,8 @@ static const struct file_operations fops = {
 	.open		= usbtmc_open,
 	.release	= usbtmc_release,
 	.unlocked_ioctl	= usbtmc_ioctl,
+	.fasync         = usbtmc_fasync,
+	.poll           = usbtmc_poll,
 	.llseek		= default_llseek,
 };
 
@@ -1092,6 +1322,79 @@ static struct usb_class_driver usbtmc_class = {
 	.minor_base =	USBTMC_MINOR_BASE,
 };
 
+static void usbtmc_interrupt(struct urb *urb)
+{
+	struct usbtmc_device_data *data = urb->context;
+	int status = urb->status;
+	int rv;
+
+	dev_dbg(&data->intf->dev, "int status: %d len %d\n",
+		status, urb->actual_length);
+
+	switch (status) {
+	case 0: /* SUCCESS */
+
+		if (data->iin_buffer[0] & 0x80) {
+			/* check for valid STB notification */
+			if ((data->iin_buffer[0] & 0x7f) > 1) {
+				data->bNotify1 = data->iin_buffer[0];
+				data->bNotify2 = data->iin_buffer[1];
+				atomic_set(&data->iin_data_valid, 1);
+				wake_up_interruptible(&data->waitq);
+				goto exit;
+			/* check for SRQ notification */
+			}
+			if ((data->iin_buffer[0] & 0x7f) == 1) {
+				if (data->fasync)
+					kill_fasync(&data->fasync,
+						SIGIO, POLL_IN);
+
+				atomic_set(&data->srq_asserted, 1);
+				wake_up_interruptible(&data->waitq);
+				goto exit;
+			}
+		}
+		dev_warn(&data->intf->dev, "invalid notification: %x\n",
+			data->iin_buffer[0]);
+		break;
+	case -EOVERFLOW:
+		dev_err(&data->intf->dev,
+			"%s - overflow with length %d, actual length is %d\n",
+			__func__, data->iin_wMaxPacketSize, urb->actual_length);
+	case -ECONNRESET:
+	case -ENOENT:
+	case -ESHUTDOWN:
+	case -EILSEQ:
+	case -ETIME:
+		/* urb terminated, clean up */
+		dev_dbg(&data->intf->dev,
+			"%s - urb terminated, status: %d\n",
+			__func__, status);
+		return;
+	default:
+		dev_err(&data->intf->dev,
+			"%s - unknown status received: %d\n",
+			__func__, status);
+	}
+exit:
+	rv = usb_submit_urb(urb, GFP_ATOMIC);
+	if (rv) {
+		dev_err(&data->intf->dev, "%s - usb_submit_urb failed: %d\n",
+			__func__, rv);
+	}
+}
+
+static void usbtmc_free_int(struct usbtmc_device_data *data)
+{
+	if (data->iin_ep_present) {
+		if (data->iin_urb) {
+			usb_kill_urb(data->iin_urb);
+			kfree(data->iin_buffer);
+			usb_free_urb(data->iin_urb);
+			kref_put(&data->kref, usbtmc_delete);
+		}
+	}
+}
 
 static int usbtmc_probe(struct usb_interface *intf,
 			const struct usb_device_id *id)
@@ -1114,6 +1417,9 @@ static int usbtmc_probe(struct usb_interface *intf,
 	usb_set_intfdata(intf, data);
 	kref_init(&data->kref);
 	mutex_init(&data->io_mutex);
+	init_waitqueue_head(&data->waitq);
+	atomic_set(&data->iin_data_valid, 0);
+	atomic_set(&data->srq_asserted, 0);
 	data->zombie = 0;
 
 	/* Determine if it is a Rigol or not */
@@ -1134,9 +1440,12 @@ static int usbtmc_probe(struct usb_interface *intf,
 	data->bTag	= 1;
 	data->TermCharEnabled = 0;
 	data->TermChar = '\n';
+	/*  2 <= bTag <= 127   USBTMC-USB488 subclass specification 4.3.1 */
+	data->iin_bTag = 2;
 
 	/* USBTMC devices have only one setting, so use that */
 	iface_desc = data->intf->cur_altsetting;
+	data->ifnum = iface_desc->desc.bInterfaceNumber;
 
 	/* Find bulk in endpoint */
 	for (n = 0; n < iface_desc->desc.bNumEndpoints; n++) {
@@ -1161,6 +1470,20 @@ static int usbtmc_probe(struct usb_interface *intf,
 			break;
 		}
 	}
+	/* Find int endpoint */
+	for (n = 0; n < iface_desc->desc.bNumEndpoints; n++) {
+		endpoint = &iface_desc->endpoint[n].desc;
+
+		if (usb_endpoint_is_int_in(endpoint)) {
+			data->iin_ep_present = 1;
+			data->iin_ep = endpoint->bEndpointAddress;
+			data->iin_wMaxPacketSize = usb_endpoint_maxp(endpoint);
+			data->iin_interval = endpoint->bInterval;
+			dev_dbg(&intf->dev, "Found Int in endpoint at %u\n",
+				data->iin_ep);
+			break;
+		}
+	}
 
 	retcode = get_capabilities(data);
 	if (retcode)
@@ -1169,6 +1492,40 @@ static int usbtmc_probe(struct usb_interface *intf,
 		retcode = sysfs_create_group(&intf->dev.kobj,
 					     &capability_attr_grp);
 
+
+	if (data->iin_ep_present) {
+		/* allocate int urb */
+		data->iin_urb = usb_alloc_urb(0, GFP_KERNEL);
+		if (!data->iin_urb) {
+			dev_err(&intf->dev, "Failed to allocate int urb\n");
+			goto error_register;
+		}
+
+		/* will reference data in int urb */
+		kref_get(&data->kref);
+
+		/* allocate buffer for interrupt in */
+		data->iin_buffer = kmalloc(data->iin_wMaxPacketSize,
+					GFP_KERNEL);
+		if (!data->iin_buffer) {
+			dev_err(&intf->dev, "Failed to allocate int buf\n");
+			goto error_register;
+		}
+
+		/* fill interrupt urb */
+		usb_fill_int_urb(data->iin_urb, data->usb_dev,
+				usb_rcvintpipe(data->usb_dev, data->iin_ep),
+				data->iin_buffer, data->iin_wMaxPacketSize,
+				usbtmc_interrupt,
+				data, data->iin_interval);
+
+		if (usb_submit_urb(data->iin_urb, GFP_KERNEL)) {
+			retcode = -EIO;
+			dev_err(&intf->dev, "Failed to submit iin_urb\n");
+			goto error_register;
+		}
+	}
+
 	retcode = sysfs_create_group(&intf->dev.kobj, &data_attr_grp);
 
 	retcode = usb_register_dev(intf, &usbtmc_class);
@@ -1185,6 +1542,7 @@ static int usbtmc_probe(struct usb_interface *intf,
 error_register:
 	sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
 	sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
+	usbtmc_free_int(data);
 	kref_put(&data->kref, usbtmc_delete);
 	return retcode;
 }
@@ -1196,6 +1554,7 @@ static void usbtmc_disconnect(struct usb_interface *intf)
 	dev_dbg(&intf->dev, "usbtmc_disconnect called\n");
 
 	data = usb_get_intfdata(intf);
+	usbtmc_free_int(data);
 	usb_deregister_dev(intf, &usbtmc_class);
 	sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
 	sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
include/uapi/linux/usb/tmc.h +32 -10

58 lines not on this page · raw message

Signature · 1 line
2.5.1
#1 lore raw

On Wed, 2015-10-14 at 15:13 +0200, dave penkler wrote:

Hi,

just a few remarks.

Quoted 46 lines
drivers/usb/class/usbtmc.c +45 -0
+static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
+				unsigned long arg)
+{
+	u8 *buffer;
+	struct device *dev;
+	int rv;
+	u8 tag, stb;
+
+	dev = &data->intf->dev;
+
+	dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
+		data->iin_ep_present);
+
+	buffer = kmalloc(8, GFP_KERNEL);
+	if (!buffer)
+		return -ENOMEM;
+
+
+	atomic_set(&data->iin_data_valid, 0);
+
+	/* must issue read_stb before using poll or select */
+	atomic_set(&data->srq_asserted, 0);
+
+	rv = usb_control_msg(data->usb_dev,
+			usb_rcvctrlpipe(data->usb_dev, 0),
+			USBTMC488_REQUEST_READ_STATUS_BYTE,
+			USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			data->iin_bTag,
+			data->ifnum,
+			buffer, 0x03, USBTMC_TIMEOUT);
+
+	if (rv < 0) {
+		dev_err(dev, "stb usb_control_msg returned %d\n", rv);
+		goto exit;
+	}
+
+	if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+		dev_err(dev, "control status returned %x\n",
+			buffer[0]);
+		rv = -EIO;
+		goto exit;
+	}
+
+	if (data->iin_ep_present) {
+

drivers/usb/class/usbtmc.c exit: · line 451

Quoted 10 lines
drivers/usb/class/usbtmc.c +10 -0
+		rv = wait_event_interruptible_timeout(
+			data->waitq,
+			(atomic_read(&data->iin_data_valid) != 0),
+			USBTMC_TIMEOUT
+			);
+
+		if (rv < 0) {
+			dev_dbg(dev, "wait interrupted %d\n", rv);
+			goto exit;
+		}

If you do this, yet the transfer succeeds, how do you keep the tag between host and device consistent?

Quoted 35 lines
drivers/usb/class/usbtmc.c +35 -0
+
+		if (rv == 0) {
+			dev_dbg(dev, "wait timed out\n");
+			rv = -ETIME;
+			goto exit;
+		}
+
+		tag = data->bNotify1 & 0x7f;
+
+		if (tag != data->iin_bTag) {
+			dev_err(dev, "expected bTag %x got %x\n",
+				data->iin_bTag, tag);
+		}
+
+		stb = data->bNotify2;
+	} else {
+		stb = buffer[2];
+	}
+
+	/* bump interrupt bTag */
+	data->iin_bTag += 1;
+	if (data->iin_bTag > 127)
+		data->iin_bTag = 2;
+
+	rv = copy_to_user((void *)arg, &stb, sizeof(stb));
+	if (rv)
+		rv = -EFAULT;
+
+ exit:
+	kfree(buffer);
+	return rv;
+
+}
+
+static unsigned int usbtmc_poll(struct file *file, poll_table *wait)

drivers/usb/class/usbtmc.c exit:

Quoted 10 lines
drivers/usb/class/usbtmc.c +10 -0
+{
+	struct usbtmc_device_data *data = file->private_data;
+	unsigned int mask = 0;
+
+	mutex_lock(&data->io_mutex);
+
+	if (data->zombie)
+		goto no_poll;
+
+	poll_wait(file, &data->waitq, wait);

Presumably the waiters should be woken when the device is disconnected.

drivers/usb/class/usbtmc.c skip_io_on_zombie: · line 1305

Quoted 8 lines
drivers/usb/class/usbtmc.c +8 -0
+
+	mask = data->zombie ? POLLHUP | POLLERR :
+		(atomic_read(&data->srq_asserted)) ? POLLIN | POLLRDNORM : 0;
+
+no_poll:
+	mutex_unlock(&data->io_mutex);
+	return mask;
+}

Regards
Oliver

#2 lore raw

Hi Oliver,

Quoted 5 lines · Oliver Neukum

On Wed, 2015-10-14 at 15:13 +0200, dave penkler wrote:

Hi,

just a few remarks.

thank you.

Quoted 52 lines
Quoted 52 lines
drivers/usb/class/usbtmc.c +50 -0
+static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
+                             unsigned long arg)
+{
+     u8 *buffer;
+     struct device *dev;
+     int rv;
+     u8 tag, stb;
+
+     dev = &data->intf->dev;
+
+     dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
+             data->iin_ep_present);
+
+     buffer = kmalloc(8, GFP_KERNEL);
+     if (!buffer)
+             return -ENOMEM;
+
+
+     atomic_set(&data->iin_data_valid, 0);
+
+     /* must issue read_stb before using poll or select */
+     atomic_set(&data->srq_asserted, 0);
+
+     rv = usb_control_msg(data->usb_dev,
+                     usb_rcvctrlpipe(data->usb_dev, 0),
+                     USBTMC488_REQUEST_READ_STATUS_BYTE,
+                     USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+                     data->iin_bTag,
+                     data->ifnum,
+                     buffer, 0x03, USBTMC_TIMEOUT);
+
+     if (rv < 0) {
+             dev_err(dev, "stb usb_control_msg returned %d\n", rv);
+             goto exit;
+     }
+
+     if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+             dev_err(dev, "control status returned %x\n",
+                     buffer[0]);
+             rv = -EIO;
+             goto exit;
+     }
+
+     if (data->iin_ep_present) {
+
+             rv = wait_event_interruptible_timeout(
+                     data->waitq,
+                     (atomic_read(&data->iin_data_valid) != 0),
+                     USBTMC_TIMEOUT
+                     );
Quoted 9 lines

drivers/usb/class/usbtmc.c exit: · line 451

Quoted 5 lines
drivers/usb/class/usbtmc.c +5 -0
+
+             if (rv < 0) {
+                     dev_dbg(dev, "wait interrupted %d\n", rv);
+                     goto exit;
+             }

If you do this, yet the transfer succeeds, how do you keep the tag between host and device consistent?

The next message will just use the same tag value as before if rv <= 0 which is not a problem - one could do it either way. I'll move the bump after exit: for V2

Quoted 39 lines
Quoted 39 lines
drivers/usb/class/usbtmc.c +39 -0
+
+             if (rv == 0) {
+                     dev_dbg(dev, "wait timed out\n");
+                     rv = -ETIME;
+                     goto exit;
+             }
+
+             tag = data->bNotify1 & 0x7f;
+
+             if (tag != data->iin_bTag) {
+                     dev_err(dev, "expected bTag %x got %x\n",
+                             data->iin_bTag, tag);
+             }
+
+             stb = data->bNotify2;
+     } else {
+             stb = buffer[2];
+     }
+
+     /* bump interrupt bTag */
+     data->iin_bTag += 1;
+     if (data->iin_bTag > 127)
+             data->iin_bTag = 2;
+
+     rv = copy_to_user((void *)arg, &stb, sizeof(stb));
+     if (rv)
+             rv = -EFAULT;
+
+ exit:
+     kfree(buffer);
+     return rv;
+
+}
+
+static unsigned int usbtmc_poll(struct file *file, poll_table *wait)
+{
+     struct usbtmc_device_data *data = file->private_data;
+     unsigned int mask = 0;
+
Quoted 9 lines

drivers/usb/class/usbtmc.c skip_io_on_zombie: · line 1297

Quoted 6 lines
drivers/usb/class/usbtmc.c +6 -0
+     mutex_lock(&data->io_mutex);
+
+     if (data->zombie)
+             goto no_poll;
+
+     poll_wait(file, &data->waitq, wait);

Presumably the waiters should be woken when the device is disconnected.

Yes the mask should be set to POLLHUP etc on the first zombie test above. After the poll_wait it should simply read

          mask = (atomic_read(&data->srq_asserted)) ? POLLIN | POLLRDNORM : 0;

Will revise for V2
thank you

Quoted 13 lines

drivers/usb/class/usbtmc.c skip_io_on_zombie: · line 1305

Quoted 8 lines
drivers/usb/class/usbtmc.c +8 -0
+
+     mask = data->zombie ? POLLHUP | POLLERR :
+             (atomic_read(&data->srq_asserted)) ? POLLIN | POLLRDNORM : 0;
+
+no_poll:
+     mutex_unlock(&data->io_mutex);
+     return mask;
+}

Regards
Oliver

#3 lore raw
In reply to dave penkler
url: https://github.com/0day-ci/linux/commits/dave-penkler/USB-usbtmc-Add-support-for-missing-USBTMC-USB488-spec/20151014-211711 reproduce:

Hi dave,

[auto build test WARNING on usb/usb-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url: https://github.com/0day-ci/linux/commits/dave-penkler/USB-usbtmc-Add-support-for-missing-USBTMC-USB488-spec/20151014-211711
reproduce:

        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__

sparse warnings: (new ones prefixed by >>)

>> drivers/usb/class/usbtmc.c:476:28: sparse: incorrect type in argument 1 (different address spaces)
   drivers/usb/class/usbtmc.c:476:28:    expected void [noderef] <asn:1>*to
   drivers/usb/class/usbtmc.c:476:28:    got void *<noident>
>> drivers/usb/class/usbtmc.c:507:44: sparse: incorrect type in argument 2 (different address spaces)

drivers/usb/class/usbtmc.c:507:44: expected void const [noderef] <asn:1>*from
drivers/usb/class/usbtmc.c:507:44: got void *<noident>
drivers/usb/class/usbtmc.c:1248:40: sparse: incorrect type in argument 1 (different address spaces)
drivers/usb/class/usbtmc.c:1248:40: expected void [noderef] <asn:1>*to
drivers/usb/class/usbtmc.c:1248:40: got void *<noident>

vim +476 drivers/usb/class/usbtmc.c

470
471 /* bump interrupt bTag */
472 data->iin_bTag += 1;
473 if (data->iin_bTag > 127)
474 data->iin_bTag = 2;
475

 > 476		rv = copy_to_user((void *)arg, &stb, sizeof(stb));
   477		if (rv)
   478			rv = -EFAULT;
   479	
   480	 exit:
   481		kfree(buffer);
   482		return rv;
   483	
   484	}
   485	
   486	static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
   487					unsigned long arg,
   488					unsigned int cmd)
   489	{
   490		u8 *buffer;
   491		struct device *dev;
   492		int rv;
   493		unsigned int val;
   494		u16 wValue;
   495	
   496		dev = &data->intf->dev;
   497	
   498		if (0 == (data->usb488_caps & USBTMC488_CAPABILITY_SIMPLE))
   499			return -EINVAL;
   500	
   501		buffer = kmalloc(8, GFP_KERNEL);
   502		if (!buffer)
   503			return -ENOMEM;
   504	
   505	
   506		if (cmd == USBTMC488_REQUEST_REN_CONTROL) {
 > 507			rv = copy_from_user(&val, (void *)arg, sizeof(val));

508 if (rv) {
509 rv = -EFAULT;
510 goto exit;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation