Re: [PATCH 1/2] usb: gadget: f_fs: add poll for endpoint 0

[Michal Nazarewicz]

On Tue, Jan 14 2014, Robert Baldyga wrote:
> This patch adds poll function for file representing ep0.
>
> Ability of read from or write to ep0 file is related with actual state of ffs:
> - When desctiptors or strings are not written yet, POLLOUT flag is set.
> - If there is any event to read, POLLIN flag is set.
> - If setup request was read, POLLIN and POLLOUT flag is set, to allow
>   send response (by performing I/O operation consistent with setup request
>   direction) or set stall (by performing I/O operation opposite  setup
>   request direction).
>
> Signed-off-by: Robert Baldyga <r.bald...@samsung.com>
> ---
>  drivers/usb/gadget/f_fs.c |   43 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> @@ -726,6 +725,45 @@ static long ffs_ep0_ioctl(struct file *file, unsigned 
> code, unsigned long value)
>       return ret;
>  }
>  
> +static unsigned int ffs_ep0_poll(struct file *file, poll_table *wait)
> +{
> +     struct ffs_data *ffs = file->private_data;
> +     unsigned int mask = POLLWRNORM;
> +     int ret;
> +
> +     ret = ffs_mutex_lock(&ffs->mutex, file->f_flags & O_NONBLOCK);
> +     if (unlikely(ret < 0))
> +             return mask;
> +
> +     switch (ffs->state) {
> +     case FFS_READ_DESCRIPTORS:
> +     case FFS_READ_STRINGS:
> +             mask |= POLLOUT;
> +             break;
> +
> +     case FFS_ACTIVE:
> +             switch (FFS_SETUP_STATE(ffs)) {
> +             case FFS_NO_SETUP:
> +                     if(ffs->ev.count)
> +                             mask |= POLLIN;
> +                     break;
> +
> +             case FFS_SETUP_PENDING:

This should also be the case for:

                case FFS_SETUP_CANCELED:

After all, we are waiting for user space to write/read something to the
end point so that we can return -EIDRM.

> +                     mask |= (POLLIN | POLLOUT);
> +                     break;
> +
> +             default:
> +                     break;

This default is then unnecessary.

> +             }
> +     default:
> +             break;

Having an explicit:

        case FFS_CLOSING:

might assist the compiler in producing warnings later on if someone
decided to add some more values to ffs_state enum.

> +     }
> +
> +     mutex_unlock(&ffs->mutex);
> +
> +     return mask;
> +}
> +
>  static const struct file_operations ffs_ep0_operations = {
>       .llseek =       no_llseek,
>  

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<m...@google.com>--<xmpp:min...@jabber.org>--ooO--(_)--Ooo--

signature.asc
Description: PGP signature