Hi Laurent,

On 01/08/18 22:55, Laurent Pinchart wrote:
> The UVC format description are numbered using the descriptor's
> bFormatIndex field. The index is used in UVC requests, and is thus
> needed to handle requests in userspace. Make it dynamically discoverable
> by exposing it in a bFormatIndex configfs attribute of the uncompressed
> and mjpeg format config items.
> 
> The bFormatIndex value exposed through the attribute is stored in the
> config item private data. However, that value is never set: the driver
> instead computes the bFormatIndex value when linking the stream class
> header in the configfs hierarchy and stores it directly in the class
> descriptors in a separate structure. In order to expose the value
> through the configfs attribute, store it in the config item private data
> as well. This results in a small code simplification.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> ---
> 
> Changes since v1:
> 
> - Squash patch "usb: gadget: uvc: configfs: Document the bFormatIndex
>   attribute".

Thanks,

Reviewed-by: Kieran Bingham <kieran.bing...@ideasonboard.com>

> ---
>  Documentation/ABI/testing/configfs-usb-gadget-uvc |  8 ++++++++
>  drivers/usb/gadget/function/uvc_configfs.c        | 14 ++++++++------
>  2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc 
> b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> index 490a0136fb02..a6cc8d6d398e 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> @@ -168,6 +168,10 @@ Description:     Specific MJPEG format descriptors
>  
>               All attributes read only,
>               except bmaControls and bDefaultFrameIndex:
> +             bFormatIndex            - unique id for this format descriptor;
> +                                     only defined after parent header is
> +                                     linked into the streaming class;
> +                                     read-only
>               bmaControls             - this format's data for bmaControls in
>                                       the streaming header
>               bmInterfaceFlags        - specifies interlace information,
> @@ -212,6 +216,10 @@ Date:            Dec 2014
>  KernelVersion:       4.0
>  Description: Specific uncompressed format descriptors
>  
> +             bFormatIndex            - unique id for this format descriptor;
> +                                     only defined after parent header is
> +                                     linked into the streaming class;
> +                                     read-only
>               bmaControls             - this format's data for bmaControls in
>                                       the streaming header
>               bmInterfaceFlags        - specifies interlace information,
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c 
> b/drivers/usb/gadget/function/uvc_configfs.c
> index fa8d2e1f54ba..5cee8aca3734 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -1538,6 +1538,7 @@ UVC_ATTR(uvcg_uncompressed_, cname, aname);
>  
>  #define identity_conv(x) (x)
>  
> +UVCG_UNCOMPRESSED_ATTR_RO(b_format_index, bFormatIndex, identity_conv);
>  UVCG_UNCOMPRESSED_ATTR(b_bits_per_pixel, bBitsPerPixel, identity_conv);
>  UVCG_UNCOMPRESSED_ATTR(b_default_frame_index, bDefaultFrameIndex,
>                      identity_conv);
> @@ -1568,6 +1569,7 @@ uvcg_uncompressed_bma_controls_store(struct config_item 
> *item,
>  UVC_ATTR(uvcg_uncompressed_, bma_controls, bmaControls);
>  
>  static struct configfs_attribute *uvcg_uncompressed_attrs[] = {
> +     &uvcg_uncompressed_attr_b_format_index,
>       &uvcg_uncompressed_attr_guid_format,
>       &uvcg_uncompressed_attr_b_bits_per_pixel,
>       &uvcg_uncompressed_attr_b_default_frame_index,
> @@ -1738,6 +1740,7 @@ UVC_ATTR(uvcg_mjpeg_, cname, aname)
>  
>  #define identity_conv(x) (x)
>  
> +UVCG_MJPEG_ATTR_RO(b_format_index, bFormatIndex, identity_conv);
>  UVCG_MJPEG_ATTR(b_default_frame_index, bDefaultFrameIndex,
>                      identity_conv);
>  UVCG_MJPEG_ATTR_RO(bm_flags, bmFlags, identity_conv);
> @@ -1768,6 +1771,7 @@ uvcg_mjpeg_bma_controls_store(struct config_item *item,
>  UVC_ATTR(uvcg_mjpeg_, bma_controls, bmaControls);
>  
>  static struct configfs_attribute *uvcg_mjpeg_attrs[] = {
> +     &uvcg_mjpeg_attr_b_format_index,
>       &uvcg_mjpeg_attr_b_default_frame_index,
>       &uvcg_mjpeg_attr_bm_flags,
>       &uvcg_mjpeg_attr_b_aspect_ratio_x,
> @@ -2079,24 +2083,22 @@ static int __uvcg_fill_strm(void *priv1, void *priv2, 
> void *priv3, int n,
>               struct uvcg_format *fmt = priv1;
>  
>               if (fmt->type == UVCG_UNCOMPRESSED) {
> -                     struct uvc_format_uncompressed *unc = *dest;
>                       struct uvcg_uncompressed *u =
>                               container_of(fmt, struct uvcg_uncompressed,
>                                            fmt);
>  
> +                     u->desc.bFormatIndex = n + 1;
> +                     u->desc.bNumFrameDescriptors = fmt->num_frames;
>                       memcpy(*dest, &u->desc, sizeof(u->desc));
>                       *dest += sizeof(u->desc);
> -                     unc->bNumFrameDescriptors = fmt->num_frames;
> -                     unc->bFormatIndex = n + 1;
>               } else if (fmt->type == UVCG_MJPEG) {
> -                     struct uvc_format_mjpeg *mjp = *dest;
>                       struct uvcg_mjpeg *m =
>                               container_of(fmt, struct uvcg_mjpeg, fmt);
>  
> +                     m->desc.bFormatIndex = n + 1;
> +                     m->desc.bNumFrameDescriptors = fmt->num_frames;
>                       memcpy(*dest, &m->desc, sizeof(m->desc));
>                       *dest += sizeof(m->desc);
> -                     mjp->bNumFrameDescriptors = fmt->num_frames;
> -                     mjp->bFormatIndex = n + 1;
>               } else {
>                       return -EINVAL;
>               }
> 

-- 
Regards
--
Kieran

Reply via email to