Re: [PATCH RFC 1/2] [media] v4l2: add V4L2_INPUT_TYPE_DEFAULT

[Mauro Carvalho Chehab <mchehab@s-opensource.com>]

Em Fri, 31 Mar 2017 00:55:12 -0300
Helen Koike <helen.koike@collabora.com> escreveu:

> On 2017-03-30 11:39 PM, Helen Koike wrote:
> > Hi Laurent,
> >
> > Thanks for reviewing
> >
> > On 2017-03-30 04:56 PM, Laurent Pinchart wrote:  
> >> Hi Helen,
> >>
> >> Thank you for the patch.
> >>
> >> On Thursday 30 Mar 2017 13:02:17 Helen Koike wrote:  
> >>> Add V4L2_INPUT_TYPE_DEFAULT and helpers functions for input ioctls to be
> >>> used when no inputs are available in the device
> >>>
> >>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> >>> ---
> >>>  drivers/media/v4l2-core/v4l2-ioctl.c | 27 +++++++++++++++++++++++++++
> >>>  include/media/v4l2-ioctl.h           | 26 ++++++++++++++++++++++++++
> >>>  include/uapi/linux/videodev2.h       |  1 +
> >>>  3 files changed, 54 insertions(+)
> >>>
> >>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> >>> b/drivers/media/v4l2-core/v4l2-ioctl.c index 0c3f238..ccaf04b 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> >>> @@ -2573,6 +2573,33 @@ struct mutex *v4l2_ioctl_get_lock(struct
> >>> video_device
> >>> *vdev, unsigned cmd) return vdev->lock;
> >>>  }
> >>>
> >>> +int v4l2_ioctl_enum_input_default(struct file *file, void *priv,
> >>> +                  struct v4l2_input *i)
> >>> +{
> >>> +    if (i->index > 0)
> >>> +        return -EINVAL;
> >>> +
> >>> +    memset(i, 0, sizeof(*i));
> >>> +    i->type = V4L2_INPUT_TYPE_DEFAULT;
> >>> +    strlcpy(i->name, "Default", sizeof(i->name));
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +EXPORT_SYMBOL(v4l2_ioctl_enum_input_default);  
> >>
> >> V4L2 tends to use EXPORT_SYMBOL_GPL.  
> >
> > The whole v4l2-ioctl.c file is using EXPORT_SYMBOL instead of
> > EXPORT_SYMBOL_GPL, should we change it all to EXPORT_SYMBOL_GPL then (in
> > another patch) ?
> >  
> >>
> >> What would you think about calling those default functions directly
> >> from the
> >> core when the input ioctl handlers are not set ? You wouldn't need to
> >> modify
> >> drivers.  
> >
> > Sure, I'll add them in ops inside __video_register_device when it
> > validates the ioctls  
> 
> I just realize I can not simply override struct v4l2_ioctl_ops as it is 
> declared as a const inside strut video_device. I'll call those default 
> functions only when the ioctls are handled to not modify vdev->ops.

You should not override it, anyway. What you should do, instead, is
something like:

static int v4l_ginput(const struct v4l2_ioctl_ops *ops, ...)
{
	if (ops->vidioc_ginput)
		return ops->vidioc_ginput(...);

	/* default code */
}

You should also make sure that the ioctls are alway valid, e. g. by
calling SET_VALID_IOCTL()

Thanks,
Mauro