- 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 <dpenk...@gmail.com>
Reviewed-by: Steve Bayless <steve_bayl...@keysight.com>
Signed-off-by: Guido Kiener <guido.kie...@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);
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
