Hi Laurent, Thanks for the patch. On Mon, Nov 04, 2013 at 11:07:48AM +0100, Laurent Pinchart wrote: > The pad::get_fmt call must be protected by a mutex, but preparing its > arguments doesn't need to be. Move the non-critical code out of the > mutex-protected section. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > --- > drivers/media/platform/omap3isp/ispvideo.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c > index a908d00..f6304bb 100644 > --- a/drivers/media/platform/omap3isp/ispvideo.c > +++ b/drivers/media/platform/omap3isp/ispvideo.c > @@ -339,14 +339,11 @@ __isp_video_get_format(struct isp_video *video, struct v4l2_format *format) > if (subdev == NULL) > return -EINVAL; > > - mutex_lock(&video->mutex); > - > fmt.pad = pad; > fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE; > - ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt); > - if (ret == -ENOIOCTLCMD) > - ret = -EINVAL; By removing these lines, you're also returning -ENOIOCTLCMD to the caller. Is this intentional? That return value will end up to at least one place which seems to be isp_video_streamon() and, unless I'm mistaken, will cause ioctl(VIDIOC_STREAMON) also return ENOTTY. > + mutex_lock(&video->mutex); > + ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt); > mutex_unlock(&video->mutex); > > if (ret) -- Regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx -- 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