* [v2,01/29] usb: usbtmc: Support Read Status Byte with SRQ per file
@ 2018-07-18 8:45 Guido Kiener
0 siblings, 0 replies; 2+ messages in thread
From: Guido Kiener @ 2018-07-18 8:45 UTC (permalink / raw)
To: gregkh, linux-usb, guido.kiener, steve_bayless, dpenkler; +Cc: Guido Kiener
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 since EPOLLIN is now reserved
for asynchronous reads
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 | 136 ++++++++++++++++++++++++++++---------
1 file changed, 105 insertions(+), 31 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 529295a17579..db58b84d43ee 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,7 +88,6 @@ 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;
@@ -104,9 +104,21 @@ 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;
+};
+
/* Forward declarations */
static struct usb_driver usbtmc_driver;
@@ -122,7 +134,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 +142,45 @@ 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;
+
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;
+
+ 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);
+ /* 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 +405,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 +419,25 @@ 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);
+ 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,
@@ -435,9 +483,8 @@ static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
stb = buffer[2];
}
- rv = copy_to_user(arg, &stb, sizeof(stb));
- if (rv)
- rv = -EFAULT;
+ rv = put_user(stb, (__u8 __user *)arg);
+ dev_dbg(dev, "stb:0x%02x received %d\n", (unsigned int)stb, rv);
exit:
/* bump interrupt bTag */
@@ -565,6 +612,7 @@ static int send_request_dev_dep_msg_in(struct usbtmc_device_data *data, size_t t
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 +624,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);
@@ -721,6 +770,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 +780,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)
@@ -1140,10 +1191,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 +1238,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 +1265,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 +1285,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 +1332,33 @@ static void usbtmc_interrupt(struct urb *urb)
}
/* check for SRQ notification */
if (data->iin_buffer[0] == 0x81) {
+ unsigned long flags;
+ 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_irqsave(&data->dev_lock, flags);
+ 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_irqrestore(&data->dev_lock, flags);
+
+ 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",
@@ -1295,6 +1369,7 @@ static void usbtmc_interrupt(struct urb *urb)
case -ESHUTDOWN:
case -EILSEQ:
case -ETIME:
+ case -EPIPE:
/* urb terminated, clean up */
dev_dbg(dev, "urb terminated, status: %d\n", status);
return;
@@ -1339,7 +1414,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 +1519,14 @@ 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");
-
- 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);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [v2,01/29] usb: usbtmc: Support Read Status Byte with SRQ per file
@ 2018-07-21 6:17 Greg Kroah-Hartman
0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2018-07-21 6:17 UTC (permalink / raw)
To: Guido Kiener; +Cc: linux-usb, guido.kiener, steve_bayless, dpenkler
On Wed, Jul 18, 2018 at 10:45:34AM +0200, Guido Kiener wrote:
> }
> - 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",
> @@ -1295,6 +1369,7 @@ static void usbtmc_interrupt(struct urb *urb)
> case -ESHUTDOWN:
> case -EILSEQ:
> case -ETIME:
> + case -EPIPE:
> /* urb terminated, clean up */
> dev_dbg(dev, "urb terminated, status: %d\n", status);
> return;
Minor nit, these two changes didn't have anything to do with the larger
patch. I'll let them slide for now, but please be more careful in the
future.
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-07-21 6:17 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-18 8:45 [v2,01/29] usb: usbtmc: Support Read Status Byte with SRQ per file Guido Kiener
2018-07-21 6:17 Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.