Hi Hans,
On Friday 26 November 2010 10:06:13 Hans Verkuil wrote:
> On Thursday, November 25, 2010 03:21:51 Laurent Pinchart wrote:
> > Pass the control-related ioctls to the subdev driver through the core
> > operations.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> >
> > Documentation/video4linux/v4l2-framework.txt | 16 ++++++++++++++++
> > drivers/media/video/v4l2-subdev.c | 24
> > ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 0
> > deletions(-)
> >
> > diff --git a/Documentation/video4linux/v4l2-framework.txt
> > b/Documentation/video4linux/v4l2-framework.txt index 4c9185a..f683f63
> > 100644
> > --- a/Documentation/video4linux/v4l2-framework.txt
> > +++ b/Documentation/video4linux/v4l2-framework.txt
> > @@ -336,6 +336,22 @@ argument to 0. Setting the argument to 1 will only
> > enable device node
> >
> > registration if the sub-device driver has set the
> > V4L2_SUBDEV_FL_HAS_DEVNODE flag.
> >
> > +The device node handles a subset of the V4L2 API.
> > +
> > +VIDIOC_QUERYCTRL
> > +VIDIOC_QUERYMENU
> > +VIDIOC_G_CTRL
> > +VIDIOC_S_CTRL
> > +VIDIOC_G_EXT_CTRLS
> > +VIDIOC_S_EXT_CTRLS
> > +VIDIOC_TRY_EXT_CTRLS
> > +
> > + The controls ioctls are identical to the ones defined in V4L2. They
> > + behave identically, with the only exception that they deal only with
> > + controls implemented in the sub-device. Depending on the driver, those
> > + controls can be also be accessed through one (or several) V4L2 device
> > + nodes.
> > +
> >
> > I2C sub-device drivers
> > ----------------------
> >
> > diff --git a/drivers/media/video/v4l2-subdev.c
> > b/drivers/media/video/v4l2-subdev.c index 0deff78..806ec30 100644
> > --- a/drivers/media/video/v4l2-subdev.c
> > +++ b/drivers/media/video/v4l2-subdev.c
> > @@ -45,7 +45,31 @@ static int subdev_close(struct file *file)
> >
> > static long subdev_do_ioctl(struct file *file, unsigned int cmd, void
> > *arg) {
> >
> > + struct video_device *vdev = video_devdata(file);
> > + struct v4l2_subdev *sd = vdev_to_v4l2_subdev(vdev);
> > +
> >
> > switch (cmd) {
> >
> > + case VIDIOC_QUERYCTRL:
> > + return v4l2_subdev_call(sd, core, queryctrl, arg);
> > +
> > + case VIDIOC_QUERYMENU:
> > + return v4l2_subdev_call(sd, core, querymenu, arg);
> > +
> > + case VIDIOC_G_CTRL:
> > + return v4l2_subdev_call(sd, core, g_ctrl, arg);
> > +
> > + case VIDIOC_S_CTRL:
> > + return v4l2_subdev_call(sd, core, s_ctrl, arg);
> > +
> > + case VIDIOC_G_EXT_CTRLS:
> > + return v4l2_subdev_call(sd, core, g_ext_ctrls, arg);
> > +
> > + case VIDIOC_S_EXT_CTRLS:
> > + return v4l2_subdev_call(sd, core, s_ext_ctrls, arg);
> > +
> > + case VIDIOC_TRY_EXT_CTRLS:
> > + return v4l2_subdev_call(sd, core, try_ext_ctrls, arg);
> > +
> >
> > default:
> > return -ENOIOCTLCMD;
> >
> > }
>
> I am very much in favor of replacing this with direct calls to the control
> framework:
>
> case VIDIOC_QUERYCTRL:
> return v4l2_queryctrl(sd->ctrl_handler, arg);
>
> This should promote the uptake of the control framework since that will
> simplify subdev drivers (and provide the full functionality of all the
> control-related ioctls).
>
> This is a new feature, so we can enforce this here without breaking
> anything.
Sounds good to me. Do all the control framework ioctl handlers return the
correct error code (and more importantly don't crash) when sd->ctrl_handler is
NULL ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
