All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Ricardo Ribalda Delgado <ribalda@kernel.org>
Cc: linux-media@vger.kernel.org, Sakari Ailus <sakari.ailus@iki.fi>
Subject: Re: [PATCH 05/57] media: i2c: imx214: Drop check for reentrant .s_stream()
Date: Mon, 18 Sep 2023 11:07:17 +0300	[thread overview]
Message-ID: <20230918080717.GA28874@pendragon.ideasonboard.com> (raw)
In-Reply-To: <CAPybu_3=V9dGFA+pw-0dm_y650_jDsKc-MvvrX3J=BUZgEK1HQ@mail.gmail.com>

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@ideasonboard.com>
> 
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  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

  reply	other threads:[~2023-09-18  8:09 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-14 18:16 [PATCH 00/57] media: i2c: Reduce cargo cult Laurent Pinchart
2023-09-14 18:16 ` [PATCH 01/57] media: v4l2-subdev: Document .s_stream() operation requirements Laurent Pinchart
2023-09-18  7:49   ` Ricardo Ribalda Delgado
2023-09-18  8:08     ` Laurent Pinchart
2023-09-14 18:16 ` [PATCH 02/57] media: i2c: hi556: Drop check for reentrant .s_stream() Laurent Pinchart
2023-09-14 18:16 ` [PATCH 03/57] media: i2c: hi846: " Laurent Pinchart
2023-09-26  7:18   ` Martin Kepplinger
2023-09-14 18:16 ` [PATCH 04/57] media: i2c: imx208: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 05/57] media: i2c: imx214: " Laurent Pinchart
2023-09-18  7:52   ` Ricardo Ribalda Delgado
2023-09-18  8:07     ` Laurent Pinchart [this message]
2023-09-14 18:16 ` [PATCH 06/57] media: i2c: imx219: " Laurent Pinchart
2023-09-15 10:49   ` Dave Stevenson
2023-09-14 18:16 ` [PATCH 07/57] media: i2c: imx258: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 08/57] media: i2c: imx319: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 09/57] media: i2c: imx334: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 10/57] media: i2c: imx335: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 11/57] media: i2c: imx355: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 12/57] media: i2c: imx412: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 13/57] media: i2c: mt9m001: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 14/57] media: i2c: og01a1b: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 15/57] media: i2c: ov01a10: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 16/57] media: i2c: ov08d10: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 17/57] media: i2c: ov08x40: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 18/57] media: i2c: ov13858: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 19/57] media: i2c: ov13b10: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 20/57] media: i2c: ov2685: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 21/57] media: i2c: ov2740: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 22/57] media: i2c: ov4689: " Laurent Pinchart
2023-09-15 19:26   ` Mikhail Rudenko
2023-09-18  6:55     ` Sakari Ailus
2023-09-14 18:16 ` [PATCH 23/57] media: i2c: ov5647: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 24/57] media: i2c: ov5670: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 25/57] media: i2c: ov5675: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 26/57] media: i2c: ov5695: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 27/57] media: i2c: ov7740: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 28/57] media: i2c: ov8856: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 29/57] media: i2c: ov9282: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 30/57] media: i2c: ov9734: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 31/57] Documentation: media: camera-sensor: Fix typo and vocabulary selection Laurent Pinchart
2023-09-14 18:16 ` [PATCH 32/57] Documentation: media: camera-sensor: Use link to upstream DT bindings Laurent Pinchart
2023-09-14 18:16 ` [PATCH 33/57] Documentation: media: camera-sensor: Move power management section Laurent Pinchart
2023-09-14 18:16 ` [PATCH 34/57] Documentation: media: camera-sensor: Improve power management documentation Laurent Pinchart
2023-09-14 18:16 ` [PATCH 35/57] media: i2c: ar0521: Drop system suspend and resume handlers Laurent Pinchart
2023-09-15  4:23   ` Krzysztof Hałasa
2023-09-14 18:16 ` [PATCH 36/57] media: i2c: ccs: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 37/57] media: i2c: hi556: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 38/57] media: i2c: hi846: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 39/57] media: i2c: hi847: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 40/57] media: i2c: imx208: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 41/57] media: i2c: imx214: " Laurent Pinchart
2023-09-18  7:53   ` Ricardo Ribalda Delgado
2023-09-14 18:16 ` [PATCH 42/57] media: i2c: imx219: " Laurent Pinchart
2023-09-15 10:53   ` Dave Stevenson
2023-09-15 11:35     ` Laurent Pinchart
2023-09-14 18:16 ` [PATCH 43/57] media: i2c: imx258: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 44/57] media: i2c: imx319: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 45/57] media: i2c: imx355: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 46/57] media: i2c: og01a1b: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 47/57] media: i2c: ov01a10: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 48/57] media: i2c: ov02a10: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 49/57] media: i2c: ov08d10: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 50/57] media: i2c: ov08x40: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 51/57] media: i2c: ov13858: " Laurent Pinchart
2023-09-14 18:16 ` [PATCH 52/57] media: i2c: ov2740: " Laurent Pinchart
2023-09-14 18:17 ` [PATCH 53/57] media: i2c: ov13b10: Drop stream handling in runtime PM handlers Laurent Pinchart
2023-09-14 18:17 ` [PATCH 54/57] media: i2c: ov5670: Drop system suspend and resume handlers Laurent Pinchart
2023-09-14 18:17 ` [PATCH 55/57] media: i2c: ov5675: " Laurent Pinchart
2023-09-14 18:17 ` [PATCH 56/57] media: i2c: ov8856: " Laurent Pinchart
2023-09-14 18:17 ` [PATCH 57/57] media: i2c: ov9734: " Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230918080717.GA28874@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=ribalda@kernel.org \
    --cc=sakari.ailus@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.