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.pinchart@xxxxxxxxxxxxxxxx>
> ---
>
> Changes since v1:
>
> - Squash patch "usb: gadget: uvc: configfs: Document the bFormatIndex
> attribute".
Thanks,
Reviewed-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> ---
> 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
