Em 04-03-2011 17:49, Mauro Carvalho Chehab escreveu: > Em 03-03-2011 07:25, Laurent Pinchart escreveu: >> > Hi Mauro, >> > >> > The following changes since commit >> > 88a763df226facb74fdb254563e30e9efb64275c: >> > >> > [media] dw2102: prof 1100 corrected (2011-03-02 16:56:54 -0300) >> > >> > are available in the git repository at: >> > git://linuxtv.org/pinchartl/media.git media-2.6.39-0005-omap3isp > ... >> > Laurent Pinchart (36): > ... >> > v4l: subdev: Generic ioctl supportFrom >> > 57b36ef1b9733124f3e04e6e2c06cf358051e209 Mon Sep 17 00:00:00 2001 > From: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > Date: Fri, 26 Feb 2010 16:23:10 +0100 > Subject: v4l: subdev: Generic ioctl support > Cc: Linux Media Mailing List <linux-media@vger.kernel.org> > > Instead of returning an error when receiving an ioctl call with an > unsupported command, forward the call to the subdev core::ioctl handler. > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > Acked-by: Hans Verkuil <hverk...@xs4all.nl> > Signed-off-by: Mauro Carvalho Chehab <mche...@redhat.com> > --- > Documentation/video4linux/v4l2-framework.txt | 5 +++++ > drivers/media/video/v4l2-subdev.c | 2 +- > 2 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/Documentation/video4linux/v4l2-framework.txt > b/Documentation/video4linux/v4l2-framework.txt > index 77d96f4..f2df31b 100644 > --- a/Documentation/video4linux/v4l2-framework.txt > +++ b/Documentation/video4linux/v4l2-framework.txt > @@ -405,6 +405,11 @@ VIDIOC_UNSUBSCRIBE_EVENT > To properly support events, the poll() file operation is also > implemented. > > +Private ioctls > + > + All ioctls not in the above list are passed directly to the sub-device > + driver through the core::ioctl operation. > + > > I2C sub-device drivers > ---------------------- > diff --git a/drivers/media/video/v4l2-subdev.c > b/drivers/media/video/v4l2-subdev.c > index 6e76f73..0b80644 100644 > --- a/drivers/media/video/v4l2-subdev.c > +++ b/drivers/media/video/v4l2-subdev.c > @@ -276,7 +276,7 @@ static long subdev_do_ioctl(struct file *file, unsigned > int cmd, void *arg) > } > #endif > default: > - return -ENOIOCTLCMD; > + return v4l2_subdev_call(sd, core, ioctl, cmd, arg); > } > > return 0;
> I don't like to apply a patch like that without a very good explanation > about why it is needed and where it is used. Private ioctls are generally > a very bad idea, as they lack proper documentation. > > Also, we may quickly loose the control about what ioctl's are valid for > subdevs, as the same code could also be used to accept a video (or audio) > ioctl directly into a subdev. > > So, IMO, the better is to manually add ioctl's there as they're > needed inside subdevs. I'll not apply this patch on my tree for now. > > Is it currently needed for omap3isp? If so, what are the used ioctls > inside omap3isp? Ok, I got my answer at the omap3isp patches: # + * Private IOCTLs # + * # + * VIDIOC_OMAP3ISP_CCDC_CFG: Set CCDC configuration # + * VIDIOC_OMAP3ISP_PRV_CFG: Set preview engine configuration # + * VIDIOC_OMAP3ISP_AEWB_CFG: Set AEWB module configuration # + * VIDIOC_OMAP3ISP_HIST_CFG: Set histogram module configuration # + * VIDIOC_OMAP3ISP_AF_CFG: Set auto-focus module configuration # + * VIDIOC_OMAP3ISP_STAT_REQ: Read statistics (AEWB/AF/histogram) data # + * VIDIOC_OMAP3ISP_STAT_EN: Enable/disable a statistics module What are those ioctl's meant to do? In special, they seem to be needed to make the device work. As I don't have any device here to test, it is hard to be sure about that, so I'm tempted to nack this patch as-is. Also, where are those private ioctl's documented? Cheers, Mauro. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
