Hi Joel and Paul,
On Friday, 8 June 2018 22:12:56 EEST Laurent Pinchart wrote:
> From: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
> ---
> 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 ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
