[02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle

[Guido Kiener <guido@kiener-muenchen.de>]

- Add 'struct usbtmc_file_data' for each file handle to cache last
  srq_byte (=Status Byte with SRQ) received by usbtmc_interrupt(..)

- usbtmc488_ioctl_read_stb returns cached srq_byte when available for
  each file handle to avoid race conditions of concurrent applications.

- SRQ now sets EPOLLPRI instead of EPOLLIN

- Caches other values TermChar, TermCharEnabled, auto_abort in
  'struct usbtmc_file_data' will not be changed by sysfs device
  attributes during an open file session.
  Future ioctl functions can change these values.

- use consistent error return value ETIMEOUT instead of ETIME

Tested-by: Dave Penkler <dpenkler@gmail.com>
Reviewed-by: Steve Bayless <steve_bayless@keysight.com>
Signed-off-by: Guido Kiener <guido.kiener@rohde-schwarz.com>
---
 drivers/usb/class/usbtmc.c | 176 ++++++++++++++++++++++++++++---------
 1 file changed, 136 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 529295a17579..5754354429d8 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -67,6 +67,7 @@ struct usbtmc_device_data {
 	const struct usb_device_id *id;
 	struct usb_device *usb_dev;
 	struct usb_interface *intf;
+	struct list_head file_list;
 
 	unsigned int bulk_in;
 	unsigned int bulk_out;
@@ -87,12 +88,12 @@ struct usbtmc_device_data {
 	int            iin_interval;
 	struct urb    *iin_urb;
 	u16            iin_wMaxPacketSize;
-	atomic_t       srq_asserted;
 
 	/* coalesced usb488_caps from usbtmc_dev_capabilities */
 	__u8 usb488_caps;
 
 	/* attributes from the USB TMC spec for this device */
+	/* They are used as default values for file_data */
 	u8 TermChar;
 	bool TermCharEnabled;
 	bool auto_abort;
@@ -104,9 +105,26 @@ struct usbtmc_device_data {
 	struct mutex io_mutex;	/* only one i/o function running at a time */
 	wait_queue_head_t waitq;
 	struct fasync_struct *fasync;
+	spinlock_t dev_lock; /* lock for file_list */
 };
 #define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
 
+/*
+ * This structure holds private data for each USBTMC file handle.
+ */
+struct usbtmc_file_data {
+	struct usbtmc_device_data *data;
+	struct list_head file_elem;
+
+	u8             srq_byte;
+	atomic_t       srq_asserted;
+
+	/* These values are initialized with default values from device_data */
+	u8             TermChar;
+	bool           TermCharEnabled;
+	bool           auto_abort;
+};
+
 /* Forward declarations */
 static struct usb_driver usbtmc_driver;
 
@@ -114,6 +132,7 @@ static void usbtmc_delete(struct kref *kref)
 {
 	struct usbtmc_device_data *data = to_usbtmc_data(kref);
 
+	pr_debug("%s - called\n", __func__);
 	usb_put_dev(data->usb_dev);
 	kfree(data);
 }
@@ -122,7 +141,7 @@ static int usbtmc_open(struct inode *inode, struct file *filp)
 {
 	struct usb_interface *intf;
 	struct usbtmc_device_data *data;
-	int retval = 0;
+	struct usbtmc_file_data *file_data;
 
 	intf = usb_find_interface(&usbtmc_driver, iminor(inode));
 	if (!intf) {
@@ -130,21 +149,54 @@ static int usbtmc_open(struct inode *inode, struct file *filp)
 		return -ENODEV;
 	}
 
+	file_data = kzalloc(sizeof(*file_data), GFP_KERNEL);
+	if (!file_data)
+		return -ENOMEM;
+
+	pr_debug("%s - called\n", __func__);
+
 	data = usb_get_intfdata(intf);
 	/* Protect reference to data from file structure until release */
 	kref_get(&data->kref);
 
+	mutex_lock(&data->io_mutex);
+	file_data->data = data;
+
+	/* copy default values from device settings */
+	file_data->TermChar = data->TermChar;
+	file_data->TermCharEnabled = data->TermCharEnabled;
+	file_data->auto_abort = data->auto_abort;
+
+	INIT_LIST_HEAD(&file_data->file_elem);
+	spin_lock_irq(&data->dev_lock);
+	list_add_tail(&file_data->file_elem, &data->file_list);
+	spin_unlock_irq(&data->dev_lock);
+	mutex_unlock(&data->io_mutex);
+
 	/* Store pointer in file structure's private data field */
-	filp->private_data = data;
+	filp->private_data = file_data;
 
-	return retval;
+	return 0;
 }
 
 static int usbtmc_release(struct inode *inode, struct file *file)
 {
-	struct usbtmc_device_data *data = file->private_data;
+	struct usbtmc_file_data *file_data = file->private_data;
 
-	kref_put(&data->kref, usbtmc_delete);
+	pr_debug("%s - called\n", __func__);
+
+	/* prevent IO _AND_ usbtmc_interrupt */
+	mutex_lock(&file_data->data->io_mutex);
+	spin_lock_irq(&file_data->data->dev_lock);
+
+	list_del(&file_data->file_elem);
+
+	spin_unlock_irq(&file_data->data->dev_lock);
+	mutex_unlock(&file_data->data->io_mutex);
+
+	kref_put(&file_data->data->kref, usbtmc_delete);
+	file_data->data = NULL;
+	kfree(file_data);
 	return 0;
 }
 
@@ -369,10 +421,12 @@ static int usbtmc_ioctl_abort_bulk_out(struct usbtmc_device_data *data)
 	return rv;
 }
 
-static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
+static int usbtmc488_ioctl_read_stb(struct usbtmc_file_data *file_data,
 				void __user *arg)
 {
+	struct usbtmc_device_data *data = file_data->data;
 	struct device *dev = &data->intf->dev;
+	int srq_asserted = 0;
 	u8 *buffer;
 	u8 tag;
 	__u8 stb;
@@ -381,15 +435,27 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
 	dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
 		data->iin_ep_present);
 
+	spin_lock_irq(&data->dev_lock);
+	srq_asserted = atomic_xchg(&file_data->srq_asserted, srq_asserted);
+	if (srq_asserted) {
+		/* a STB with SRQ is already received */
+		stb = file_data->srq_byte;
+		spin_unlock_irq(&data->dev_lock);
+		rv = put_user(stb, (__u8 __user *)arg);
+		dev_dbg(dev, "stb:0x%02x with srq received %d\n",
+			(unsigned int)stb, rv);
+		if (rv)
+			return -EFAULT;
+		return rv;
+	}
+	spin_unlock_irq(&data->dev_lock);
+
 	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,
@@ -420,7 +486,7 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
 
 		if (rv == 0) {
 			dev_dbg(dev, "wait timed out\n");
-			rv = -ETIME;
+			rv = -ETIMEDOUT;
 			goto exit;
 		}
 
@@ -435,9 +501,11 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
 		stb = buffer[2];
 	}
 
-	rv = copy_to_user(arg, &stb, sizeof(stb));
-	if (rv)
+	if (put_user(stb, (__u8 __user *)arg))
 		rv = -EFAULT;
+	else
+		rv = 0;
+	dev_dbg(dev, "stb:0x%02x received %d\n", (unsigned int)stb, rv);
 
  exit:
 	/* bump interrupt bTag */
@@ -513,8 +581,10 @@ static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
  *
  * Also updates bTag_last_write.
  */
-static int send_request_dev_dep_msg_in(struct usbtmc_device_data *data, size_t transfer_size)
+static int send_request_dev_dep_msg_in(struct usbtmc_file_data *file_data,
+				       size_t transfer_size)
 {
+	struct usbtmc_device_data *data = file_data->data;
 	int retval;
 	u8 *buffer;
 	int actual;
@@ -533,9 +603,9 @@ static int send_request_dev_dep_msg_in(struct usbtmc_device_data *data, size_t t
 	buffer[5] = transfer_size >> 8;
 	buffer[6] = transfer_size >> 16;
 	buffer[7] = transfer_size >> 24;
-	buffer[8] = data->TermCharEnabled * 2;
+	buffer[8] = file_data->TermCharEnabled * 2;
 	/* Use term character? */
-	buffer[9] = data->TermChar;
+	buffer[9] = file_data->TermChar;
 	buffer[10] = 0; /* Reserved */
 	buffer[11] = 0; /* Reserved */
 
@@ -554,17 +624,17 @@ static int send_request_dev_dep_msg_in(struct usbtmc_device_data *data, size_t t
 		data->bTag++;
 
 	kfree(buffer);
-	if (retval < 0) {
-		dev_err(&data->intf->dev, "usb_bulk_msg in send_request_dev_dep_msg_in() returned %d\n", retval);
-		return retval;
-	}
+	if (retval < 0)
+		dev_err(&data->intf->dev, "%s returned %d\n",
+			__func__, retval);
 
-	return 0;
+	return retval;
 }
 
 static ssize_t usbtmc_read(struct file *filp, char __user *buf,
 			   size_t count, loff_t *f_pos)
 {
+	struct usbtmc_file_data *file_data;
 	struct usbtmc_device_data *data;
 	struct device *dev;
 	u32 n_characters;
@@ -576,7 +646,8 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
 	size_t this_part;
 
 	/* Get pointer to private data structure */
-	data = filp->private_data;
+	file_data = filp->private_data;
+	data = file_data->data;
 	dev = &data->intf->dev;
 
 	buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
@@ -591,7 +662,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
 
 	dev_dbg(dev, "usb_bulk_msg_in: count(%zu)\n", count);
 
-	retval = send_request_dev_dep_msg_in(data, count);
+	retval = send_request_dev_dep_msg_in(file_data, count);
 
 	if (retval < 0) {
 		if (data->auto_abort)
@@ -721,6 +792,7 @@ static ssize_t usbtmc_read(struct file *filp, char __user *buf,
 static ssize_t usbtmc_write(struct file *filp, const char __user *buf,
 			    size_t count, loff_t *f_pos)
 {
+	struct usbtmc_file_data *file_data;
 	struct usbtmc_device_data *data;
 	u8 *buffer;
 	int retval;
@@ -730,7 +802,8 @@ static ssize_t usbtmc_write(struct file *filp, const char __user *buf,
 	int done;
 	int this_part;
 
-	data = filp->private_data;
+	file_data = filp->private_data;
+	data = file_data->data;
 
 	buffer = kmalloc(USBTMC_SIZE_IOBUFFER, GFP_KERNEL);
 	if (!buffer)
@@ -1074,7 +1147,7 @@ static ssize_t name##_store(struct device *dev,				\
 	struct usb_interface *intf = to_usb_interface(dev);		\
 	struct usbtmc_device_data *data = usb_get_intfdata(intf);	\
 	ssize_t result;							\
-	unsigned val;							\
+	unsigned int val;						\
 									\
 	result = sscanf(buf, "%u\n", &val);				\
 	if (result != 1)						\
@@ -1140,10 +1213,13 @@ static int usbtmc_ioctl_indicator_pulse(struct usbtmc_device_data *data)
 
 static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
+	struct usbtmc_file_data *file_data;
 	struct usbtmc_device_data *data;
 	int retval = -EBADRQC;
 
-	data = file->private_data;
+	file_data = file->private_data;
+	data = file_data->data;
+
 	mutex_lock(&data->io_mutex);
 	if (data->zombie) {
 		retval = -ENODEV;
@@ -1184,7 +1260,8 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 
 	case USBTMC488_IOCTL_READ_STB:
-		retval = usbtmc488_ioctl_read_stb(data, (void __user *)arg);
+		retval = usbtmc488_ioctl_read_stb(file_data,
+						  (void __user *)arg);
 		break;
 
 	case USBTMC488_IOCTL_REN_CONTROL:
@@ -1210,14 +1287,15 @@ static long usbtmc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 static int usbtmc_fasync(int fd, struct file *file, int on)
 {
-	struct usbtmc_device_data *data = file->private_data;
+	struct usbtmc_file_data *file_data = file->private_data;
 
-	return fasync_helper(fd, file, on, &data->fasync);
+	return fasync_helper(fd, file, on, &file_data->data->fasync);
 }
 
 static __poll_t usbtmc_poll(struct file *file, poll_table *wait)
 {
-	struct usbtmc_device_data *data = file->private_data;
+	struct usbtmc_file_data *file_data = file->private_data;
+	struct usbtmc_device_data *data = file_data->data;
 	__poll_t mask;
 
 	mutex_lock(&data->io_mutex);
@@ -1229,7 +1307,7 @@ static __poll_t usbtmc_poll(struct file *file, poll_table *wait)
 
 	poll_wait(file, &data->waitq, wait);
 
-	mask = (atomic_read(&data->srq_asserted)) ? EPOLLIN | EPOLLRDNORM : 0;
+	mask = (atomic_read(&file_data->srq_asserted)) ? EPOLLPRI : 0;
 
 no_poll:
 	mutex_unlock(&data->io_mutex);
@@ -1276,15 +1354,32 @@ static void usbtmc_interrupt(struct urb *urb)
 		}
 		/* check for SRQ notification */
 		if (data->iin_buffer[0] == 0x81) {
+			struct list_head *elem;
+
 			if (data->fasync)
 				kill_fasync(&data->fasync,
-					SIGIO, POLL_IN);
+					SIGIO, POLL_PRI);
 
-			atomic_set(&data->srq_asserted, 1);
-			wake_up_interruptible(&data->waitq);
+			spin_lock(&data->dev_lock);
+			list_for_each(elem, &data->file_list) {
+				struct usbtmc_file_data *file_data;
+
+				file_data = list_entry(elem,
+						       struct usbtmc_file_data,
+						       file_elem);
+				file_data->srq_byte = data->iin_buffer[1];
+				atomic_set(&file_data->srq_asserted, 1);
+			}
+			spin_unlock(&data->dev_lock);
+
+			dev_dbg(dev, "srq received bTag %x stb %x\n",
+				(unsigned int)data->iin_buffer[0],
+				(unsigned int)data->iin_buffer[1]);
+			wake_up_interruptible_all(&data->waitq);
 			goto exit;
 		}
-		dev_warn(dev, "invalid notification: %x\n", data->iin_buffer[0]);
+		dev_warn(dev, "invalid notification: %x\n",
+			 data->iin_buffer[0]);
 		break;
 	case -EOVERFLOW:
 		dev_err(dev, "overflow with length %d, actual length is %d\n",
@@ -1339,7 +1434,9 @@ static int usbtmc_probe(struct usb_interface *intf,
 	mutex_init(&data->io_mutex);
 	init_waitqueue_head(&data->waitq);
 	atomic_set(&data->iin_data_valid, 0);
-	atomic_set(&data->srq_asserted, 0);
+	INIT_LIST_HEAD(&data->file_list);
+	spin_lock_init(&data->dev_lock);
+
 	data->zombie = 0;
 
 	/* Initialize USBTMC bTag and other fields */
@@ -1442,17 +1539,16 @@ static int usbtmc_probe(struct usb_interface *intf,
 
 static void usbtmc_disconnect(struct usb_interface *intf)
 {
-	struct usbtmc_device_data *data;
+	struct usbtmc_device_data *data  = usb_get_intfdata(intf);
 
-	dev_dbg(&intf->dev, "usbtmc_disconnect called\n");
+	dev_dbg(&intf->dev, "%s - called\n", __func__);
 
-	data = usb_get_intfdata(intf);
 	usb_deregister_dev(intf, &usbtmc_class);
 	sysfs_remove_group(&intf->dev.kobj, &capability_attr_grp);
 	sysfs_remove_group(&intf->dev.kobj, &data_attr_grp);
 	mutex_lock(&data->io_mutex);
 	data->zombie = 1;
-	wake_up_all(&data->waitq);
+	wake_up_interruptible_all(&data->waitq);
 	mutex_unlock(&data->io_mutex);
 	usbtmc_free_int(data);
 	kref_put(&data->kref, usbtmc_delete);