Re: [PATCH v3 3/3] media: i2c: ov5647: switch to {enable,disable}_streams

[xiaolei wang]

On 12/31/25 19:03, Tarang Raval wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> Hi Xiaolei,
>
> > Switch from s_stream to enable_streams and disable_streams callbacks.
> >   
> > Signed-off-by: Xiaolei Wang <xiaolei.wang@xxxxxxxxxxxxx>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/media/i2c/ov5647.c | 89 ++++++++++++++++----------------------
> >   1 file changed, 38 insertions(+), 51 deletions(-)
> >   
> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> > index bc81f378436a..7091081a0828 100644
> > --- a/drivers/media/i2c/ov5647.c
> > +++ b/drivers/media/i2c/ov5647.c
> > @@ -637,23 +637,42 @@ static int ov5647_set_mode(struct v4l2_subdev *sd)
> >      return 0;
> >   }
> >
> > -static int ov5647_stream_on(struct v4l2_subdev *sd)
> > +static int ov5647_stream_stop(struct ov5647 *sensor)
> > +{
> > +   int ret = 0;
> > +
> > +   cci_write(sensor->regmap, OV5647_REG_MIPI_CTRL00,
> > +           MIPI_CTRL00_CLOCK_LANE_GATE | MIPI_CTRL00_BUS_IDLE |
> > +           MIPI_CTRL00_CLOCK_LANE_DISABLE, &ret);
> > +   cci_write(sensor->regmap, OV5647_REG_FRAME_OFF_NUMBER, 0x0f, &ret);
> > +   cci_write(sensor->regmap, OV5640_REG_PAD_OUT, 0x01, &ret);
> > +
> > +   return ret;
> > +}
> > +
> > +static int ov5647_enable_streams(struct v4l2_subdev *sd,
> > +                      struct v4l2_subdev_state *state, u32 pad,
> > +                      u64 streams_mask)
> >   {
> >      struct i2c_client *client = v4l2_get_subdevdata(sd);
> >      struct ov5647 *sensor = to_sensor(sd);
> >      u8 val = MIPI_CTRL00_BUS_IDLE;
> >      int ret = 0;
> No need for zero initialization.

I will correct this in the next version.

Best Regards,
Xiaolei

> > +   ret = pm_runtime_resume_and_get(&client->dev);
> > +   if (ret < 0)
> > +         return ret;
> > +
> >      ret = ov5647_set_mode(sd);
> >      if (ret) {
> >            dev_err(&client->dev, "Failed to program sensor mode: %d\n", ret);
> > -         return ret;
> > +         goto done;
> >      }
> >
> >      /* Apply customized values from user when stream starts. */
> >      ret =  __v4l2_ctrl_handler_setup(sd->ctrl_handler);
> >      if (ret)
> > -         return ret;
> > +         goto done;
> >
> >      if (sensor->clock_ncont)
> >            val |= MIPI_CTRL00_CLOCK_LANE_GATE |
> > @@ -663,19 +682,24 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
> >      cci_write(sensor->regmap, OV5647_REG_FRAME_OFF_NUMBER, 0x00, &ret);
> >      cci_write(sensor->regmap, OV5640_REG_PAD_OUT, 0x00, &ret);
> >
> > +done:
> > +   if (ret)
> > +         pm_runtime_put(&client->dev);
> > +
> >      return ret;
> >   }
> >
> > -static int ov5647_stream_off(struct v4l2_subdev *sd)
> > +static int ov5647_disable_streams(struct v4l2_subdev *sd,
> > +                       struct v4l2_subdev_state *state, u32 pad,
> > +                       u64 streams_mask)
> >   {
> > +   struct i2c_client *client = v4l2_get_subdevdata(sd);
> >      struct ov5647 *sensor = to_sensor(sd);
> > -   int ret = 0;
> > +   int ret;
> >
> > -   cci_write(sensor->regmap, OV5647_REG_MIPI_CTRL00,
> > -           MIPI_CTRL00_CLOCK_LANE_GATE | MIPI_CTRL00_BUS_IDLE |
> > -           MIPI_CTRL00_CLOCK_LANE_DISABLE, &ret);
> > -   cci_write(sensor->regmap, OV5647_REG_FRAME_OFF_NUMBER, 0x0f, &ret);
> > -   cci_write(sensor->regmap, OV5640_REG_PAD_OUT, 0x01, &ret);
> > +   ret = ov5647_stream_stop(sensor);
> > +
> > +   pm_runtime_put(&client->dev);
> >
> >      return ret;
> >   }
> > @@ -706,7 +730,7 @@ static int ov5647_power_on(struct device *dev)
> >      }
> >
> >      /* Stream off to coax lanes into LP-11 state. */
> > -   ret = ov5647_stream_off(&sensor->sd);
> > +   ret = ov5647_stream_stop(sensor);
> >      if (ret < 0) {
> >            dev_err(dev, "camera not available, check power\n");
> >            goto error_clk_disable;
> > @@ -803,47 +827,8 @@ __ov5647_get_pad_crop(struct ov5647 *ov5647,
> >      return NULL;
> >   }
> >
> > -static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
> > -{
> > -   struct i2c_client *client = v4l2_get_subdevdata(sd);
> > -   struct v4l2_subdev_state *state;
> > -   int ret;
> > -
> > -   state = v4l2_subdev_lock_and_get_active_state(sd);
> > -
> > -   if (enable) {
> > -         ret = pm_runtime_resume_and_get(&client->dev);
> > -         if (ret < 0)
> > -               goto error_unlock;
> > -
> > -         ret = ov5647_stream_on(sd);
> > -         if (ret < 0) {
> > -               dev_err(&client->dev, "stream start failed: %d\n", ret);
> > -               goto error_pm;
> > -         }
> > -   } else {
> > -         ret = ov5647_stream_off(sd);
> > -         if (ret < 0) {
> > -               dev_err(&client->dev, "stream stop failed: %d\n", ret);
> > -               goto error_pm;
> > -         }
> > -         pm_runtime_put(&client->dev);
> > -   }
> > -
> > -   v4l2_subdev_unlock_state(state);
> > -
> > -   return 0;
> > -
> > -error_pm:
> > -   pm_runtime_put(&client->dev);
> > -error_unlock:
> > -   v4l2_subdev_unlock_state(state);
> > -
> > -   return ret;
> > -}
> > -
> >   static const struct v4l2_subdev_video_ops ov5647_subdev_video_ops = {
> > -   .s_stream =       ov5647_s_stream,
> > +   .s_stream = v4l2_subdev_s_stream_helper,
> >   };
> >
> >   static int ov5647_enum_mbus_code(struct v4l2_subdev *sd,
> > @@ -986,6 +971,8 @@ static const struct v4l2_subdev_pad_ops ov5647_subdev_pad_ops = {
> >      .set_fmt          = ov5647_set_pad_fmt,
> >      .get_fmt          = ov5647_get_pad_fmt,
> >      .get_selection          = ov5647_get_selection,
> > +   .enable_streams         = ov5647_enable_streams,
> > +   .disable_streams  = ov5647_disable_streams,
> >   };
> >
> >   static const struct v4l2_subdev_ops ov5647_subdev_ops = {
> > --
> > 2.43.0
> Reviewed-by: Tarang Raval <tarang.raval@xxxxxxxxxxxxxxxxx>
>
> Best Regards,
> Tarang