[2/2] usb: gadget: uvc: configfs: Add bFrameIndex attributes

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2018-06-08 19:16:11

Hi Joel and Paul,

On Friday, 8 June 2018 22:12:56 EEST Laurent Pinchart wrote:
quoted hunk
From: Paul Elder <paul.elder@ideasonboard.com>

The UVC frame descriptions are numbered using the descriptor's
bFrameIndex 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 bFrameIndex configfs attribute of the frame
config item.

However, the bFrameIndex value is never set to begin with. Set these
values when linking the stream class header in the configfs hierarchy,
and store it in the config item private data.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 drivers/usb/gadget/function/uvc_configfs.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc_configfs.c
b/drivers/usb/gadget/function/uvc_configfs.c index
0ad6ea57d0b7..882d9963b244 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -1152,6 +1152,8 @@ UVC_ATTR(uvcg_frame_, cname, aname);

 #define noop_conversion(x) (x)

+UVCG_FRAME_ATTR_RO(b_frame_index, bFrameIndex, noop_conversion,
+		noop_conversion, 8);
 UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
 		noop_conversion, 8);
 UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
@@ -1298,6 +1300,7 @@ static ssize_t
uvcg_frame_dw_frame_interval_store(struct config_item *item,
UVC_ATTR(uvcg_frame_, dw_frame_interval, dwFrameInterval);

 static struct configfs_attribute *uvcg_frame_attrs[] = {
+	&uvcg_frame_attr_b_frame_index,
 	&uvcg_frame_attr_bm_capabilities,
 	&uvcg_frame_attr_w_width,
 	&uvcg_frame_attr_w_height,
@@ -2130,12 +2133,15 @@ static int __uvcg_fill_strm(void *priv1, void
*priv2, void *priv3, int n, sizeof(*frm->dw_frame_interval);
 		memcpy(*dest, frm->dw_frame_interval, sz);
 		*dest += sz;
-		if (frm->fmt_type == UVCG_UNCOMPRESSED)
+		if (frm->fmt_type == UVCG_UNCOMPRESSED) {
 			h->bLength = UVC_DT_FRAME_UNCOMPRESSED_SIZE(
 				frm->frame.b_frame_interval_type);
-		else if (frm->fmt_type == UVCG_MJPEG)
+			frm->frame.b_frame_index = n + 1;
+		} else if (frm->fmt_type == UVCG_MJPEG) {
 			h->bLength = UVC_DT_FRAME_MJPEG_SIZE(
 				frm->frame.b_frame_interval_type);
+			frm->frame.b_frame_index = n + 1;
+		}
 	}
 	break;
 	}
This approach looks simpler to me than the patch that Joel submitted. However, 
this patch lacks ABI documentation, which is included in Joel's patch.

Joel, as you were first, I propose taking your patch and modifying it with the 
approach followed here, using the UVCG_FRAME_ATTR_RO macro and setting 
b_frame_index in __uvcg_fill_strm(). Would that be OK with you ? Or is there a 
reason why the approach taken in your patch would be better ?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help