| Message ID | 20240910170610.226189-9-prabhakar.mahadev-lad.rj@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: ov5645: Add support for streams | expand |
Hi Prabhakar, Thank you for the patch. On Tue, Sep 10, 2024 at 06:06:07PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Switch from s_stream to enable_streams and disable_streams callbacks. Here too you should explain why. The "why" is the most important part of any commit message. > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/media/i2c/ov5645.c | 90 +++++++++++++++++++------------------- > 1 file changed, 46 insertions(+), 44 deletions(-) > > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c > index 9497ec737cb7..dc93514608ee 100644 > --- a/drivers/media/i2c/ov5645.c > +++ b/drivers/media/i2c/ov5645.c > @@ -923,71 +923,71 @@ static int ov5645_get_selection(struct v4l2_subdev *sd, > return 0; > } > > -static int ov5645_s_stream(struct v4l2_subdev *subdev, int enable) > +static int ov5645_enable_streams(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, u32 pad, > + u64 streams_mask) > { > - struct ov5645 *ov5645 = to_ov5645(subdev); > - struct v4l2_subdev_state *state; > + struct ov5645 *ov5645 = to_ov5645(sd); > int ret; > > - state = v4l2_subdev_lock_and_get_active_state(&ov5645->sd); > - > - if (enable) { > - ret = pm_runtime_resume_and_get(ov5645->dev); > - if (ret < 0) { > - v4l2_subdev_unlock_state(state); > - return ret; > - } > + ret = pm_runtime_resume_and_get(ov5645->dev); > + if (ret < 0) > + return ret; > > - ret = ov5645_set_register_array(ov5645, > + ret = ov5645_set_register_array(ov5645, > ov5645->current_mode->data, > ov5645->current_mode->data_size); > - if (ret < 0) { > - dev_err(ov5645->dev, "could not set mode %dx%d\n", > - ov5645->current_mode->width, > - ov5645->current_mode->height); > - goto err_rpm_put; > - } > - ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls); > - if (ret < 0) { > - dev_err(ov5645->dev, "could not sync v4l2 controls\n"); > - goto err_rpm_put; > - } > - > - ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45); > - if (ret < 0) > - goto err_rpm_put; > - > - ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, > - OV5645_SYSTEM_CTRL0_START); > - if (ret < 0) > - goto err_rpm_put; > - } else { > - ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40); > - if (ret < 0) > - goto stream_off_rpm_put; > + if (ret < 0) { > + dev_err(ov5645->dev, "could not set mode %dx%d\n", > + ov5645->current_mode->width, > + ov5645->current_mode->height); > + goto err_rpm_put; > + } > + ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls); > + if (ret < 0) { > + dev_err(ov5645->dev, "could not sync v4l2 controls\n"); > + goto err_rpm_put; > + } > > - ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, > - OV5645_SYSTEM_CTRL0_STOP); > + ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45); > + if (ret < 0) > + goto err_rpm_put; > > - goto stream_off_rpm_put; > - } > + ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, > + OV5645_SYSTEM_CTRL0_START); > + if (ret < 0) > + goto err_rpm_put; > > - v4l2_subdev_unlock_state(state); > return 0; > > err_rpm_put: > pm_runtime_put_sync(ov5645->dev); > return ret; > +} > + > +static int ov5645_disable_streams(struct v4l2_subdev *sd, > + struct v4l2_subdev_state *state, u32 pad, > + u64 streams_mask) > +{ > + struct ov5645 *ov5645 = to_ov5645(sd); > + int ret; > + > + ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40); > + if (ret < 0) > + goto rpm_put; > + > + ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, > + OV5645_SYSTEM_CTRL0_STOP); > > -stream_off_rpm_put: > +rpm_put: > pm_runtime_mark_last_busy(ov5645->dev); > pm_runtime_put_autosuspend(ov5645->dev); > - v4l2_subdev_unlock_state(state); > + > return ret; > } > > static const struct v4l2_subdev_video_ops ov5645_video_ops = { > - .s_stream = ov5645_s_stream, > + .s_stream = v4l2_subdev_s_stream_helper, > }; > > static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = { > @@ -996,6 +996,8 @@ static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = { > .get_fmt = v4l2_subdev_get_fmt, > .set_fmt = ov5645_set_format, > .get_selection = ov5645_get_selection, > + .enable_streams = ov5645_enable_streams, > + .disable_streams = ov5645_disable_streams, > }; > > static const struct v4l2_subdev_core_ops ov5645_core_ops = {
diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c index 9497ec737cb7..dc93514608ee 100644 --- a/drivers/media/i2c/ov5645.c +++ b/drivers/media/i2c/ov5645.c @@ -923,71 +923,71 @@ static int ov5645_get_selection(struct v4l2_subdev *sd, return 0; } -static int ov5645_s_stream(struct v4l2_subdev *subdev, int enable) +static int ov5645_enable_streams(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state, u32 pad, + u64 streams_mask) { - struct ov5645 *ov5645 = to_ov5645(subdev); - struct v4l2_subdev_state *state; + struct ov5645 *ov5645 = to_ov5645(sd); int ret; - state = v4l2_subdev_lock_and_get_active_state(&ov5645->sd); - - if (enable) { - ret = pm_runtime_resume_and_get(ov5645->dev); - if (ret < 0) { - v4l2_subdev_unlock_state(state); - return ret; - } + ret = pm_runtime_resume_and_get(ov5645->dev); + if (ret < 0) + return ret; - ret = ov5645_set_register_array(ov5645, + ret = ov5645_set_register_array(ov5645, ov5645->current_mode->data, ov5645->current_mode->data_size); - if (ret < 0) { - dev_err(ov5645->dev, "could not set mode %dx%d\n", - ov5645->current_mode->width, - ov5645->current_mode->height); - goto err_rpm_put; - } - ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls); - if (ret < 0) { - dev_err(ov5645->dev, "could not sync v4l2 controls\n"); - goto err_rpm_put; - } - - ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45); - if (ret < 0) - goto err_rpm_put; - - ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, - OV5645_SYSTEM_CTRL0_START); - if (ret < 0) - goto err_rpm_put; - } else { - ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40); - if (ret < 0) - goto stream_off_rpm_put; + if (ret < 0) { + dev_err(ov5645->dev, "could not set mode %dx%d\n", + ov5645->current_mode->width, + ov5645->current_mode->height); + goto err_rpm_put; + } + ret = __v4l2_ctrl_handler_setup(&ov5645->ctrls); + if (ret < 0) { + dev_err(ov5645->dev, "could not sync v4l2 controls\n"); + goto err_rpm_put; + } - ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, - OV5645_SYSTEM_CTRL0_STOP); + ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45); + if (ret < 0) + goto err_rpm_put; - goto stream_off_rpm_put; - } + ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, + OV5645_SYSTEM_CTRL0_START); + if (ret < 0) + goto err_rpm_put; - v4l2_subdev_unlock_state(state); return 0; err_rpm_put: pm_runtime_put_sync(ov5645->dev); return ret; +} + +static int ov5645_disable_streams(struct v4l2_subdev *sd, + struct v4l2_subdev_state *state, u32 pad, + u64 streams_mask) +{ + struct ov5645 *ov5645 = to_ov5645(sd); + int ret; + + ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40); + if (ret < 0) + goto rpm_put; + + ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0, + OV5645_SYSTEM_CTRL0_STOP); -stream_off_rpm_put: +rpm_put: pm_runtime_mark_last_busy(ov5645->dev); pm_runtime_put_autosuspend(ov5645->dev); - v4l2_subdev_unlock_state(state); + return ret; } static const struct v4l2_subdev_video_ops ov5645_video_ops = { - .s_stream = ov5645_s_stream, + .s_stream = v4l2_subdev_s_stream_helper, }; static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = { @@ -996,6 +996,8 @@ static const struct v4l2_subdev_pad_ops ov5645_subdev_pad_ops = { .get_fmt = v4l2_subdev_get_fmt, .set_fmt = ov5645_set_format, .get_selection = ov5645_get_selection, + .enable_streams = ov5645_enable_streams, + .disable_streams = ov5645_disable_streams, }; static const struct v4l2_subdev_core_ops ov5645_core_ops = {