| Message ID | 20230914181704.4811-23-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: Reduce cargo cult | expand |
On 2023-09-14 at 21:16 +03, Laurent Pinchart <laurent.pinchart@ideasonboard.com> 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. > > The streaming field of the driver's private structure is now unused, > drop it as well. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/ov4689.c | 7 ------- > 1 file changed, 7 deletions(-) > > diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c > index fda217d2cb10..cd8d7131e747 100644 > --- a/drivers/media/i2c/ov4689.c > +++ b/drivers/media/i2c/ov4689.c > @@ -100,7 +100,6 @@ struct ov4689 { > u32 clock_rate; > > struct mutex mutex; /* lock to protect streaming, ctrls and cur_mode */ > - bool streaming; We can now drop the mention of "streaming" from the comment one line above. Aside from that, Reviewed-by: Mikhail Rudenko <mike.rudenko@gmail.com> > struct v4l2_ctrl_handler ctrl_handler; > struct v4l2_ctrl *exposure; > > @@ -468,10 +467,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on) > > mutex_lock(&ov4689->mutex); > > - on = !!on; > - if (on == ov4689->streaming) > - goto unlock_and_return; > - > if (on) { > ret = pm_runtime_resume_and_get(&client->dev); > if (ret < 0) > @@ -504,8 +499,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on) > pm_runtime_put(&client->dev); > } > > - ov4689->streaming = on; > - > unlock_and_return: > mutex_unlock(&ov4689->mutex); -- Best regards, Mikhail Rudenko
Hi Mikhail, Laurent, On Fri, Sep 15, 2023 at 10:26:28PM +0300, Mikhail Rudenko wrote: > > On 2023-09-14 at 21:16 +03, Laurent Pinchart <laurent.pinchart@ideasonboard.com> 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. > > > > The streaming field of the driver's private structure is now unused, > > drop it as well. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/media/i2c/ov4689.c | 7 ------- > > 1 file changed, 7 deletions(-) > > > > diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c > > index fda217d2cb10..cd8d7131e747 100644 > > --- a/drivers/media/i2c/ov4689.c > > +++ b/drivers/media/i2c/ov4689.c > > @@ -100,7 +100,6 @@ struct ov4689 { > > u32 clock_rate; > > > > struct mutex mutex; /* lock to protect streaming, ctrls and cur_mode */ > > - bool streaming; > > We can now drop the mention of "streaming" from the comment one line > above. Aside from that, I'll remove that while applying. > > Reviewed-by: Mikhail Rudenko <mike.rudenko@gmail.com> Thanks! > > > struct v4l2_ctrl_handler ctrl_handler; > > struct v4l2_ctrl *exposure; > > > > @@ -468,10 +467,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on) > > > > mutex_lock(&ov4689->mutex); > > > > - on = !!on; > > - if (on == ov4689->streaming) > > - goto unlock_and_return; > > - > > if (on) { > > ret = pm_runtime_resume_and_get(&client->dev); > > if (ret < 0) > > @@ -504,8 +499,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on) > > pm_runtime_put(&client->dev); > > } > > > > - ov4689->streaming = on; > > - > > unlock_and_return: > > mutex_unlock(&ov4689->mutex); > > > -- > Best regards, > Mikhail Rudenko
diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c index fda217d2cb10..cd8d7131e747 100644 --- a/drivers/media/i2c/ov4689.c +++ b/drivers/media/i2c/ov4689.c @@ -100,7 +100,6 @@ struct ov4689 { u32 clock_rate; struct mutex mutex; /* lock to protect streaming, ctrls and cur_mode */ - bool streaming; struct v4l2_ctrl_handler ctrl_handler; struct v4l2_ctrl *exposure; @@ -468,10 +467,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on) mutex_lock(&ov4689->mutex); - on = !!on; - if (on == ov4689->streaming) - goto unlock_and_return; - if (on) { ret = pm_runtime_resume_and_get(&client->dev); if (ret < 0) @@ -504,8 +499,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on) pm_runtime_put(&client->dev); } - ov4689->streaming = on; - unlock_and_return: mutex_unlock(&ov4689->mutex);
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. The streaming field of the driver's private structure is now unused, drop it as well. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/i2c/ov4689.c | 7 ------- 1 file changed, 7 deletions(-)