On Wed, Apr 19 2017, Jerry Zhang wrote:
> Also change the docs to reflect that ENODEV is no longer returned.
>
> This allows users to make an ioctl call as the first action on a
> connection. Ex, some functions might want to get endpoint size
> before making any i/os.
>
> Previously, calling ioctls before read/write would depending on the
> timing of endpoints being enabled.
>
> Signed-off-by: Jerry Zhang <zhangjerry@xxxxxxxxxx>
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
> ---
> Changelog since v1:
> - Guarded against epfile->ep changing after taking lock
>
> drivers/usb/gadget/function/f_fs.c | 93 +++++++++++++++++++++----------------
> include/uapi/linux/usb/functionfs.h | 5 +-
> 2 files changed, 56 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index a0085571824d..04425e6b457a 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -1190,6 +1190,7 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
> unsigned long value)
> {
> struct ffs_epfile *epfile = file->private_data;
> + struct ffs_ep *ep;
> int ret;
>
> ENTER();
> @@ -1197,50 +1198,64 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code,
> if (WARN_ON(epfile->ffs->state != FFS_ACTIVE))
> return -ENODEV;
>
> + /* Wait for endpoint to be enabled */
> + ep = epfile->ep;
> + if (!ep) {
> + if (file->f_flags & O_NONBLOCK)
> + return -EAGAIN;
> +
> + ret = wait_event_interruptible(epfile->wait, (ep = epfile->ep));
> + if (ret)
> + return -EINTR;
> + }
> +
> spin_lock_irq(&epfile->ffs->eps_lock);
> - if (likely(epfile->ep)) {
> - switch (code) {
> - case FUNCTIONFS_FIFO_STATUS:
> - ret = usb_ep_fifo_status(epfile->ep->ep);
> - break;
> - case FUNCTIONFS_FIFO_FLUSH:
> - usb_ep_fifo_flush(epfile->ep->ep);
> - ret = 0;
> - break;
> - case FUNCTIONFS_CLEAR_HALT:
> - ret = usb_ep_clear_halt(epfile->ep->ep);
> - break;
> - case FUNCTIONFS_ENDPOINT_REVMAP:
> - ret = epfile->ep->num;
> - break;
> - case FUNCTIONFS_ENDPOINT_DESC:
> - {
> - int desc_idx;
> - struct usb_endpoint_descriptor *desc;
>
> - switch (epfile->ffs->gadget->speed) {
> - case USB_SPEED_SUPER:
> - desc_idx = 2;
> - break;
> - case USB_SPEED_HIGH:
> - desc_idx = 1;
> - break;
> - default:
> - desc_idx = 0;
> - }
> - desc = epfile->ep->descs[desc_idx];
> + /* In the meantime, endpoint got disabled or changed. */
> + if (epfile->ep != ep) {
> + spin_unlock_irq(&epfile->ffs->eps_lock);
> + return -ESHUTDOWN;
> + }
>
> - spin_unlock_irq(&epfile->ffs->eps_lock);
> - ret = copy_to_user((void *)value, desc, desc->bLength);
> - if (ret)
> - ret = -EFAULT;
> - return ret;
> - }
> + switch (code) {
> + case FUNCTIONFS_FIFO_STATUS:
> + ret = usb_ep_fifo_status(epfile->ep->ep);
> + break;
> + case FUNCTIONFS_FIFO_FLUSH:
> + usb_ep_fifo_flush(epfile->ep->ep);
> + ret = 0;
> + break;
> + case FUNCTIONFS_CLEAR_HALT:
> + ret = usb_ep_clear_halt(epfile->ep->ep);
> + break;
> + case FUNCTIONFS_ENDPOINT_REVMAP:
> + ret = epfile->ep->num;
> + break;
> + case FUNCTIONFS_ENDPOINT_DESC:
> + {
> + int desc_idx;
> + struct usb_endpoint_descriptor *desc;
> +
> + switch (epfile->ffs->gadget->speed) {
> + case USB_SPEED_SUPER:
> + desc_idx = 2;
> + break;
> + case USB_SPEED_HIGH:
> + desc_idx = 1;
> + break;
> default:
> - ret = -ENOTTY;
> + desc_idx = 0;
> }
> - } else {
> - ret = -ENODEV;
> + desc = epfile->ep->descs[desc_idx];
> +
> + spin_unlock_irq(&epfile->ffs->eps_lock);
> + ret = copy_to_user((void *)value, desc, desc->bLength);
> + if (ret)
> + ret = -EFAULT;
> + return ret;
> + }
> + default:
> + ret = -ENOTTY;
> }
> spin_unlock_irq(&epfile->ffs->eps_lock);
>
> diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
> index b2a31a55a612..746f62379620 100644
> --- a/include/uapi/linux/usb/functionfs.h
> +++ b/include/uapi/linux/usb/functionfs.h
> @@ -275,13 +275,12 @@ struct usb_functionfs_event {
> #define FUNCTIONFS_INTERFACE_REVMAP _IO('g', 128)
>
> /*
> - * Returns real bEndpointAddress of an endpoint. If function is not
> - * active returns -ENODEV.
> + * Returns real bEndpointAddress of an endpoint.
Perhaps add something along the lines of:
If endpoint is disabled while the ioctl is running, returns
-ESHUTDOWN.
Same in other places.
> */
> #define FUNCTIONFS_ENDPOINT_REVMAP _IO('g', 129)
>
> /*
> - * Returns endpoint descriptor. If function is not active returns -ENODEV.
> + * Returns endpoint descriptor.
> */
> #define FUNCTIONFS_ENDPOINT_DESC _IOR('g', 130, \
> struct usb_endpoint_descriptor)
--
Best regards
ミハウ “𝓶𝓲𝓷𝓪86” ナザレヴイツ
«If at first you don’t succeed, give up skydiving»
--
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
