Hi Andrzej,

Thank you for the patch. I think we're nearly there :-)

On Friday 05 December 2014 15:16:36 Andrzej Pietrasiewicz wrote:
> Add support for using the uvc function as a component of USB gadgets
> composed with configfs.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrze...@samsung.com>
> ---
>  Documentation/ABI/testing/configfs-usb-gadget-uvc |  265 +++
>  drivers/usb/gadget/Kconfig                        |   11 +
>  drivers/usb/gadget/function/Makefile              |    2 +-
>  drivers/usb/gadget/function/f_uvc.c               |  107 +-
>  drivers/usb/gadget/function/u_uvc.h               |   46 +
>  drivers/usb/gadget/function/uvc_configfs.c        | 2439 ++++++++++++++++++
>  drivers/usb/gadget/function/uvc_configfs.h        |   22 +
>  7 files changed, 2888 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/ABI/testing/configfs-usb-gadget-uvc
>  create mode 100644 drivers/usb/gadget/function/uvc_configfs.c
>  create mode 100644 drivers/usb/gadget/function/uvc_configfs.h

[snip]

> diff --git a/drivers/usb/gadget/function/f_uvc.c
> b/drivers/usb/gadget/function/f_uvc.c index b14c599..76891ad 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c

[snip]

> @@ -508,6 +509,7 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum
> usb_device_speed speed) uvc_streaming_std = uvc_fs_streaming;
>               break;
>       }
> +

As mentioned in a reply to 2/3, this change belongs to 2/3.

>       if (!uvc_control_desc || !uvc_streaming_cls)
>               return ERR_PTR(-ENODEV);
> 
> @@ -787,25 +789,104 @@ static void uvc_free_inst(struct
> usb_function_instance *f)

[snip]

>  static void uvc_free(struct usb_function *f)
>  {
>       struct uvc_device *uvc = to_uvc(f);
> -
> +     struct f_uvc_opts *opts = container_of(f->fi, struct f_uvc_opts,
> +                                            func_inst);
> +     --opts->refcnt;

Ok, now refcnt is modified, but it's never tested :-)

>       kfree(uvc);
>  }

[snip]

> diff --git a/drivers/usb/gadget/function/u_uvc.h
> b/drivers/usb/gadget/function/u_uvc.h index c0706a3..e197b18 100644
> --- a/drivers/usb/gadget/function/u_uvc.h
> +++ b/drivers/usb/gadget/function/u_uvc.h

[snip]

> @@ -26,11 +27,56 @@ struct f_uvc_opts {
>       unsigned int                                    streaming_interval;
>       unsigned int                                    streaming_maxpacket;
>       unsigned int                                    streaming_maxburst;
> +
> +     /*
> +      * These are actually used in uvc_function_bind().
> +      * Set by uvc_alloc_inst(), in a legacy gadget
> +      * overwriten to use legacy gadget's own structures.
> +      */

How about

        /*
         * Control descriptors array pointers for full-/high-speed and
         * super-speed. They point by default to the uvc_fs_control_cls and
         * uvc_ss_control_cls arrays respectively. Legacy gadgets must
         * override them in their gadget bind callback.
         */

>       const struct uvc_descriptor_header * const      *fs_control;
>       const struct uvc_descriptor_header * const      *ss_control;
> +
> +     /*
> +      * These are actually used in uvc_function_bind().
> +      * A legacy gadget sets these after a call to
> +      * uvc_alloc_inst() to point to its own structures.
> +      * Overwritten by uvc_alloc() if uvc_fs_streaming_cls
> +      * below & friends are set, which takes place in a
> +      * configfs-based gadget
> +      */

        /*
         * Streaming descriptors array pointers for full-speed, high-speed and
         * super-speed. They will point to the uvc_[fhs]s_streaming_cls arrays
         * for configfs-based gadgets. Legacy gadgets must initialize them in
         * their gadget bind callback.
         */

>       const struct uvc_descriptor_header * const      *fs_streaming;
>       const struct uvc_descriptor_header * const      *hs_streaming;
>       const struct uvc_descriptor_header * const      *ss_streaming;
> +
> +     /* default descriptors for a configfs-based gadget */

        /* Default control descriptors for configfs-based gadgets. */

> +     struct uvc_camera_terminal_descriptor           uvc_camera_terminal;
> +     struct uvc_processing_unit_descriptor           uvc_processing;
> +     struct uvc_output_terminal_descriptor           uvc_output_terminal;
> +     struct uvc_color_matching_descriptor            uvc_color_matching;
> +
> +     /*
> +      * The the size of arrays can be fixed
> +      */

        /*
         * Control descriptors pointers arrays for full-/high-speed and
         * super-speed. The first element is a configurable control header
         * descriptor, the other elements point to the fixed default control
         * descriptors. Used by configfs only, must not be touched by legacy
         * gadgets.
         */

> +     struct uvc_descriptor_header                    *uvc_fs_control_cls[5];
> +     struct uvc_descriptor_header                    *uvc_ss_control_cls[5];
> +
> +     /*
> +      * With configfs the size of arrays is only known
> +      * at runtime, because the user can create a number
> +      * of formats, frames, etc.
> +      */

        /*
         * Streaming descriptors for full-speed, high-speed and super-speed.
         * Used by configfs only, must not be touched by legacy gadgets. The
         * arrays are allocated at runtime as the number of descriptors isn't
         * known in advance.
         */

> +     struct uvc_descriptor_header                    **uvc_fs_streaming_cls;
> +     struct uvc_descriptor_header                    **uvc_hs_streaming_cls;
> +     struct uvc_descriptor_header                    **uvc_ss_streaming_cls;
> +
> +     /*
> +      * Read/write access to configfs attributes is handled by configfs.
> +      *
> +      * This is to protect the data from concurrent access by read/write
> +      * and create symlink/remove symlink.

This lock protects the descriptors from concurrent access by read/write and 
symlink creation/removal.

> +      */
> +     struct mutex                    lock;
> +     int                             refcnt;
>  };
> 
>  void uvc_set_trace_param(unsigned int trace);

[snip]

-- 
Regards,

Laurent Pinchart

--
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

Reply via email to