diff mbox series

[v2,6/8] usb: gadget: uvc: configfs: Add bFormatIndex attributes

Message ID 20180801215505.7658-7-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series usb: gadget: uvc: Improve configfs support | expand

Commit Message

Laurent Pinchart Aug. 1, 2018, 9:55 p.m. UTC
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@ideasonboard.com>
---

Changes since v1:

- Squash patch "usb: gadget: uvc: configfs: Document the bFormatIndex
  attribute".
---
 Documentation/ABI/testing/configfs-usb-gadget-uvc |  8 ++++++++
 drivers/usb/gadget/function/uvc_configfs.c        | 14 ++++++++------
 2 files changed, 16 insertions(+), 6 deletions(-)

Comments

Kieran Bingham Sept. 24, 2018, 12:09 p.m. UTC | #1
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@ideasonboard.com>
> ---
> 
> Changes since v1:
> 
> - Squash patch "usb: gadget: uvc: configfs: Document the bFormatIndex
>   attribute".

Thanks,

Reviewed-by: Kieran Bingham <kieran.bingham@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;
>  		}
>
diff mbox series

Patch

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