On 24/03/2023 10:21, Dan Scally wrote: > > On 24/03/2023 09:20, Laurent Pinchart wrote: >> Hi Michael, >> >> (CC'ing Hans) >> >> Thank you for the patch. >> >> On Thu, Mar 23, 2023 at 12:41:11PM +0100, Michael Tretter wrote: >>> V4L2 OUTPUT devices should implement ENUM_OUTPUT, G_OUTPUT, and >>> S_OUTPUT. The UVC gadget provides only a single output. Therefore, allow >>> only a single output 0. >>> >>> According to the documentation, "_TYPE_ANALOG" is historical and should >>> be read as "_TYPE_VIDEO". >> I think v4l2-compliance should be fixed to not require those ioctls. As >> this patch clearly shows, they're useless :-) They are not useless. An application doesn't know how many outputs there are, and what type they are. Just because there is only one output, doesn't mean you can skip this. The application also has to know the capabilities of the output. Now, it can be useful to add some helper functions for this to v4l2-common.c, at least for g/s_output. Regards, Hans > > > +1 for this vote > >> >>> Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx> >>> --- >>> drivers/usb/gadget/function/uvc_v4l2.c | 28 ++++++++++++++++++++++++++++ >>> 1 file changed, 28 insertions(+) >>> >>> diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c >>> index 13c7ba06f994..4b8bf94e06fc 100644 >>> --- a/drivers/usb/gadget/function/uvc_v4l2.c >>> +++ b/drivers/usb/gadget/function/uvc_v4l2.c >>> @@ -377,6 +377,31 @@ uvc_v4l2_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *f) >>> return 0; >>> } >>> +static int >>> +uvc_v4l2_enum_output(struct file *file, void *priv_fh, struct v4l2_output *out) >>> +{ >>> + if (out->index != 0) >>> + return -EINVAL; >>> + >>> + out->type = V4L2_OUTPUT_TYPE_ANALOG; >>> + snprintf(out->name, sizeof(out->name), "UVC"); >>> + >>> + return 0; >>> +} >>> + >>> +static int >>> +uvc_v4l2_g_output(struct file *file, void *priv_fh, unsigned int *i) >>> +{ >>> + *i = 0; >>> + return 0; >>> +} >>> + >>> +static int >>> +uvc_v4l2_s_output(struct file *file, void *priv_fh, unsigned int i) >>> +{ >>> + return i ? -EINVAL : 0; >>> +} >>> + >>> static int >>> uvc_v4l2_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *b) >>> { >>> @@ -547,6 +572,9 @@ const struct v4l2_ioctl_ops uvc_v4l2_ioctl_ops = { >>> .vidioc_enum_frameintervals = uvc_v4l2_enum_frameintervals, >>> .vidioc_enum_framesizes = uvc_v4l2_enum_framesizes, >>> .vidioc_enum_fmt_vid_out = uvc_v4l2_enum_format, >>> + .vidioc_enum_output = uvc_v4l2_enum_output, >>> + .vidioc_g_output = uvc_v4l2_g_output, >>> + .vidioc_s_output = uvc_v4l2_s_output, >>> .vidioc_reqbufs = uvc_v4l2_reqbufs, >>> .vidioc_querybuf = uvc_v4l2_querybuf, >>> .vidioc_qbuf = uvc_v4l2_qbuf,