Hi Laurent,
Thanks for the review!!!
On Thu, Dec 15, 2011 at 11:18:53AM +0100, Laurent Pinchart wrote:
> Hi Sakari,
>
> Thanks for the patch.
>
> On Thursday 15 December 2011 10:50:32 Sakari Ailus wrote:
> > Validate pipeline of any external entity connected to the ISP driver.
> > The validation of the pipeline for the part that involves links inside the
> > domain of another driver must be done by that very driver.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
> > ---
> > drivers/media/video/omap3isp/ispvideo.c | 12 ++++++++++++
> > 1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/media/video/omap3isp/ispvideo.c
> > b/drivers/media/video/omap3isp/ispvideo.c index f229057..17bc03c 100644
> > --- a/drivers/media/video/omap3isp/ispvideo.c
> > +++ b/drivers/media/video/omap3isp/ispvideo.c
> > @@ -355,6 +355,18 @@ static int isp_video_validate_pipeline(struct
> > isp_pipeline *pipe) fmt_source.format.height != fmt_sink.format.height)
> > return -EPIPE;
> >
> > + if (subdev->host_priv) {
> > + /*
> > + * host_priv != NULL: this is a sensor. Issue
> > + * validate_pipeline. We're at our end of the
> > + * pipeline so we quit now.
> > + */
> > + ret = v4l2_subdev_call(subdev, pad, validate_pipeline);
> > + if (IS_ERR_VALUE(ret))
>
> Is the validate pipeline operation expected to return a value different than 0
> on success ? If not if (ret < 0) should do.
>
> Although there's another issue. Not all sensors will implement the
> validate_pipeline operation, so you shouldn't return an error if ret == -
> ENOIOCTLCMD.
>
> I will comment on the validate_pipeline approach itself in the "On controlling
> sensors" mail thread.
Good point. I'll fix this; same for your comment on the third patch.
--
Sakari Ailus
e-mail: sakari.ailus@xxxxxx jabber/XMPP/Gmail: 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
