On 11/11/2013 12:16 PM, David Cohen wrote:
From: Michal Nazarewicz <mina86@xxxxxxxxxx>
When endpoint changes (due to it being disabled or alt setting changed),
mimic the action as if the change happened after the request has been
queued, instead of retrying with the new endpoint.
Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
Cc: David Cohen <david.a.cohen@xxxxxxxxxxxxxxx>
---
drivers/usb/gadget/f_fs.c | 94 +++++++++++++++++++++--------------------------
1 file changed, 41 insertions(+), 53 deletions(-)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 75e4b7846a8d..f1563c47e0c2 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -756,74 +756,61 @@ static ssize_t ffs_epfile_io(struct file *file,
{
struct ffs_epfile *epfile = file->private_data;
struct ffs_ep *ep;
- char *data = NULL;
ssize_t ret;
+ char *data;
int halt;
- goto first_try;
- do {
- spin_unlock_irq(&epfile->ffs->eps_lock);
- mutex_unlock(&epfile->mutex);
+ /* Are we still active? */
+ if (WARN_ON(epfile->ffs->state != FFS_ACTIVE)) {
+ ret = -ENODEV;
+ goto error;
+ }
-first_try:
- /* Are we still active? */
- if (WARN_ON(epfile->ffs->state != FFS_ACTIVE)) {
- ret = -ENODEV;
+ /* Wait for endpoint to be enabled */
+ ep = epfile->ep;
+ if (!ep) {
+ if (file->f_flags & O_NONBLOCK) {
+ ret = -EAGAIN;
goto error;
}
- /* Wait for endpoint to be enabled */
- ep = epfile->ep;
- if (!ep) {
- if (file->f_flags & O_NONBLOCK) {
- ret = -EAGAIN;
- goto error;
- }
-
- if (wait_event_interruptible(epfile->wait,
- (ep = epfile->ep))) {
- ret = -EINTR;
- goto error;
- }
- }
-
- /* Do we halt? */
- halt = !read == !epfile->in;
- if (halt && epfile->isoc) {
- ret = -EINVAL;
+ if (wait_event_interruptible(epfile->wait, (ep = epfile->ep))) {
+ ret = -EINTR;
goto error;
}
+ }
- /* Allocate & copy */
- if (!halt && !data) {
- data = kzalloc(len, GFP_KERNEL);
- if (unlikely(!data))
- return -ENOMEM;
+ /* Do we halt? */
+ halt = !read == !epfile->in;
One curiosity here. This patch prints the following warning:
In file included from (...)/drivers/usb/gadget/g_ffs.c:55:0:
(...)/drivers/usb/gadget/f_fs.c: In function 'ffs_epfile_io.isra.18':
(...)/drivers/usb/gadget/f_fs.c:837:15: warning: 'data_len' may be used
uninitialized in this function [-Wmaybe-uninitialized]
But if we do this dummy change on it:
@@ -782,7 +782,10 @@ static ssize_t ffs_epfile_io(struct file *file,
}
/* Do we halt? */
- halt = !read == !epfile->in;
+ if (!read == !epfile->in)
+ halt = 1;
+ else
+ halt = 0;
if (halt && epfile->isoc) {
ret = -EINVAL;
goto error;
The warning goes away. This false-positive warning comes out of this
gcc version:
$ i686-linux-android-gcc --version
i686-linux-android-gcc (GCC) 4.7
Br, David Cohen
+ if (halt && epfile->isoc) {
+ ret = -EINVAL;
+ goto error;
+ }
- if (!read &&
- unlikely(__copy_from_user(data, buf, len))) {
- ret = -EFAULT;
- goto error;
- }
- }
+ /* Allocate & copy */
+ if (!halt) {
+ data = kmalloc(len, GFP_KERNEL);
+ if (unlikely(!data))
+ return -ENOMEM;
- /* We will be using request */
- ret = ffs_mutex_lock(&epfile->mutex,
- file->f_flags & O_NONBLOCK);
- if (unlikely(ret))
+ if (!read && unlikely(copy_from_user(data, buf, len))) {
+ ret = -EFAULT;
goto error;
+ }
+ }
- /*
- * We're called from user space, we can use _irq rather then
- * _irqsave
- */
- spin_lock_irq(&epfile->ffs->eps_lock);
+ /* We will be using request */
+ ret = ffs_mutex_lock(&epfile->mutex, file->f_flags & O_NONBLOCK);
+ if (unlikely(ret))
+ goto error;
+ spin_lock_irq(&epfile->ffs->eps_lock);
- /*
- * While we were acquiring mutex endpoint got disabled
- * or changed?
- */
- } while (unlikely(epfile->ep != ep));
+ /* While we were acquiring mutex endpoint got disabled or changed. */
+ if (epfile->ep != ep) {
+ ret = -ESHUTDOWN;
+ spin_unlock_irq(&epfile->ffs->eps_lock);
+ goto error_unlock;
+ }
/* Halt */
if (unlikely(halt)) {
@@ -858,6 +845,7 @@ first_try:
}
}
+error_unlock:
mutex_unlock(&epfile->mutex);
error:
kfree(data);
--
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
