[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.cb/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_tuvcg_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 ?