- 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@xxxxxxxxx> Reviewed-by: Steve Bayless <steve_bayless@xxxxxxxxxxxx> Signed-off-by: Guido Kiener <guido.kiener@xxxxxxxxxxxxxxxxx> --- 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); -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html