On Friday 06 November 2009 06:30:42 Hiremath, Vaibhav wrote: > > -----Original Message----- > > From: linux-media-owner@xxxxxxxxxxxxxxx [mailto:linux-media- > > owner@xxxxxxxxxxxxxxx] On Behalf Of Hans Verkuil > > Sent: Thursday, November 05, 2009 9:48 PM > > To: Hiremath, Vaibhav > > Cc: linux-media@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH V2] Davinci VPFE Capture: Add support for > > Control ioctls > > > > On Thursday 29 October 2009 07:51:04 hvaibhav@xxxxxx wrote: > > > From: Vaibhav Hiremath <hvaibhav@xxxxxx> > > > > > > Added support for Control IOCTL, > > > - s_ctrl > > > - g_ctrl > > > - queryctrl > > > > > > Change from last patch: > > > - added room for error return in queryctrl function. > > > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@xxxxxx> > > > --- > > > drivers/media/video/davinci/vpfe_capture.c | 43 > > ++++++++++++++++++++++++++++ > > > 1 files changed, 43 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/media/video/davinci/vpfe_capture.c > > b/drivers/media/video/davinci/vpfe_capture.c > > > index abe21e4..8275d02 100644 > > > --- a/drivers/media/video/davinci/vpfe_capture.c > > > +++ b/drivers/media/video/davinci/vpfe_capture.c > > > @@ -1368,6 +1368,46 @@ static int vpfe_g_std(struct file *file, > > void *priv, v4l2_std_id *std_id) > > > return 0; > > > } > > > > > > +static int vpfe_queryctrl(struct file *file, void *priv, > > > + struct v4l2_queryctrl *qctrl) > > > +{ > > > + struct vpfe_device *vpfe_dev = video_drvdata(file); > > > + struct vpfe_subdev_info *sdinfo; > > > + int ret = 0; > > > + > > > + sdinfo = vpfe_dev->current_subdev; > > > + > > > + ret = v4l2_device_call_until_err(&vpfe_dev->v4l2_dev, sdinfo- > > >grp_id, > > > + core, queryctrl, qctrl); > > > + > > > + if (ret) > > > + qctrl->flags |= V4L2_CTRL_FLAG_DISABLED; > > > > Please remove this bogus flag. Just do: > > > [Hiremath, Vaibhav] Hans, while implementing this ioctl I was also thinking the same, but v4l2_ctrl_check do check this flag and also in V4L2 spec it has been mentioned that > "This control is permanently disabled and should be ignored by the application." > > So I thought this may be useful for standard applications, we still have some drivers do use this flag actually. The use of this flag is for very specific cases as is documented in a footnote in the v4l2 spec: "V4L2_CTRL_FLAG_DISABLED was intended for two purposes: Drivers can skip predefined controls not supported by the hardware (although returning EINVAL would do as well), or disable predefined and private controls after hardware detection without the trouble of reordering control arrays and indices (EINVAL cannot be used to skip private controls because it would prematurely end the enumeration)." In both cases you would only check for this flag if queryctrl returns 0, so setting the flag AND returning an error is unnecessary. Regards, Hans > > Thanks, > Vaibhav > > > return v4l2_device_call_until_err(&vpfe_dev->v4l2_dev, sdinfo- > > >grp_id, > > core, queryctrl, qctrl); > > > > Simple and effective. > > > > Regards, > > > > Hans > > > > > + > > > + return ret; > > > +} > > > + > > > +static int vpfe_g_ctrl(struct file *file, void *priv, struct > > v4l2_control *ctrl) > > > +{ > > > + struct vpfe_device *vpfe_dev = video_drvdata(file); > > > + struct vpfe_subdev_info *sdinfo; > > > + > > > + sdinfo = vpfe_dev->current_subdev; > > > + > > > + return v4l2_device_call_until_err(&vpfe_dev->v4l2_dev, sdinfo- > > >grp_id, > > > + core, g_ctrl, ctrl); > > > +} > > > + > > > +static int vpfe_s_ctrl(struct file *file, void *priv, struct > > v4l2_control *ctrl) > > > +{ > > > + struct vpfe_device *vpfe_dev = video_drvdata(file); > > > + struct vpfe_subdev_info *sdinfo; > > > + > > > + sdinfo = vpfe_dev->current_subdev; > > > + > > > + return v4l2_device_call_until_err(&vpfe_dev->v4l2_dev, sdinfo- > > >grp_id, > > > + core, s_ctrl, ctrl); > > > +} > > > + > > > /* > > > * Videobuf operations > > > */ > > > @@ -1939,6 +1979,9 @@ static const struct v4l2_ioctl_ops > > vpfe_ioctl_ops = { > > > .vidioc_querystd = vpfe_querystd, > > > .vidioc_s_std = vpfe_s_std, > > > .vidioc_g_std = vpfe_g_std, > > > + .vidioc_queryctrl = vpfe_queryctrl, > > > + .vidioc_g_ctrl = vpfe_g_ctrl, > > > + .vidioc_s_ctrl = vpfe_s_ctrl, > > > .vidioc_reqbufs = vpfe_reqbufs, > > > .vidioc_querybuf = vpfe_querybuf, > > > .vidioc_qbuf = vpfe_qbuf, > > > -- > > > 1.6.2.4 > > > > > > -- > > > 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 > > > > > > > > > > > -- > > Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom > > -- > > 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 > > > -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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
