Hi Ricardo,
On Mon, Sep 18, 2023 at 09:52:32AM +0200, Ricardo Ribalda Delgado wrote:
> On Thu, Sep 14, 2023 at 8:17 PM Laurent Pinchart wrote:
> >
> > The subdev .s_stream() operation shall not be called to start streaming
> > on an already started subdev, or stop streaming on a stopped subdev.
> > Remove the check that guards against that condition.
> >
>
> In general I agree with the patch, but I think it would be safer to
> land it in two stages.
>
> first:
>
> if WARN_ON(imx214->streaming == enable)
> return 0;
>
> and in the next version replace it completely.
I don't want to send another series of 57 patches to remove the
WARN_ON() in a few kernel versions :-S Fortunately, it seems we have a
better option, I can centralize the check in call_s_stream() !
And, on a side note, how many kernel versions would we need before
removing the WARN_ON() ?
Something I have on my radar is replacing all direct .s_stream() calls
with v4l2_subdev_enable_streams() and v4l2_subdev_disable_streams(), but
that's more work than I can tackle at the moment.
> otherwise
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > ---
> > drivers/media/i2c/imx214.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> > index 2f9c8582f940..e2805173f4b1 100644
> > --- a/drivers/media/i2c/imx214.c
> > +++ b/drivers/media/i2c/imx214.c
> > @@ -775,9 +775,6 @@ static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
> > struct imx214 *imx214 = to_imx214(subdev);
> > int ret;
> >
> > - if (imx214->streaming == enable)
> > - return 0;
> > -
> > if (enable) {
> > ret = pm_runtime_resume_and_get(imx214->dev);
> > if (ret < 0)
--
Regards,
Laurent Pinchart
