[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<PN3P287MB18293AE98858CE9B30DB6C0E8BBDA@PN3P287MB1829.INDP287.PROD.OUTLOOK.COM>
Date: Wed, 31 Dec 2025 11:03:55 +0000
From: Tarang Raval <tarang.raval@...iconsignals.io>
To: Xiaolei Wang <xiaolei.wang@...driver.com>,
"laurent.pinchart@...asonboard.com" <laurent.pinchart@...asonboard.com>,
"sakari.ailus@...ux.intel.com" <sakari.ailus@...ux.intel.com>,
"dave.stevenson@...pberrypi.com" <dave.stevenson@...pberrypi.com>,
"jacopo@...ndi.org" <jacopo@...ndi.org>, "mchehab@...nel.org"
<mchehab@...nel.org>, "prabhakar.mahadev-lad.rj@...renesas.com"
<prabhakar.mahadev-lad.rj@...renesas.com>, "hverkuil+cisco@...nel.org"
<hverkuil+cisco@...nel.org>, "johannes.goede@....qualcomm.com"
<johannes.goede@....qualcomm.com>, "hverkuil-cisco@...all.nl"
<hverkuil-cisco@...all.nl>, "jai.luthra@...asonboard.com"
<jai.luthra@...asonboard.com>
CC: "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/3] media: i2c: ov5647: switch to
{enable,disable}_streams
Hi Xiaolei,
> Switch from s_stream to enable_streams and disable_streams callbacks.
>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.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.
> + 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@...iconsignals.io>
Best Regards,
Tarang
Powered by blists - more mailing lists