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

[xiaolei wang <xiaolei.wang@windriver.com>]

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@windriver.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>   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@siliconsignals.io>
>
> Best Regards,
> Tarang