Hi Biju,
Thank you for the patch.
On Tue, Feb 13, 2024 at 06:12:33PM +0000, Biju Das wrote:
> As per section 35.3.1 Starting Reception for the MIPI CSI-2 Input on the
> latest hardware manual (R01UH0914EJ0145 Rev.1.45) we need to supply all
> the clocks and then release the CRU resets. Currently we are releasing
> the resets and then supplying the clocks. So, fix the start reception
> procedure.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> ---
> v3:
> * New patch.
> ---
> .../platform/renesas/rzg2l-cru/rzg2l-video.c | 59 +++++++++----------
> 1 file changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> index d15a9bfcc98b..b16b8af6e8f8 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> @@ -489,39 +489,24 @@ static int rzg2l_cru_set_stream(struct rzg2l_cru_dev *cru, int on)
>
> video_device_pipeline_stop(&cru->vdev);
>
> - pm_runtime_put_sync(cru->dev);
> - clk_disable_unprepare(cru->vclk);
> -
> return stream_off_ret;
> }
>
> - ret = pm_runtime_resume_and_get(cru->dev);
> - if (ret)
> - return ret;
> -
> - ret = clk_prepare_enable(cru->vclk);
> - if (ret)
> - goto err_pm_put;
> -
> ret = rzg2l_cru_mc_validate_format(cru, sd, pad);
> if (ret)
> - goto err_vclk_disable;
> + return ret;
>
> pipe = media_entity_pipeline(&sd->entity) ? : &cru->vdev.pipe;
> ret = video_device_pipeline_start(&cru->vdev, pipe);
> if (ret)
> - goto err_vclk_disable;
> + return ret;
>
> ret = v4l2_subdev_call(sd, video, pre_streamon, 0);
> - if (ret == -ENOIOCTLCMD)
> - ret = 0;
> - if (ret)
> + if (ret && ret != -ENOIOCTLCMD)
> goto pipe_line_stop;
>
> ret = v4l2_subdev_call(sd, video, s_stream, 1);
> - if (ret == -ENOIOCTLCMD)
> - ret = 0;
> - if (ret)
> + if (ret && ret != -ENOIOCTLCMD)
> goto err_s_stream;
>
> return 0;
> @@ -532,12 +517,6 @@ static int rzg2l_cru_set_stream(struct rzg2l_cru_dev *cru, int on)
> pipe_line_stop:
> video_device_pipeline_stop(&cru->vdev);
>
> -err_vclk_disable:
> - clk_disable_unprepare(cru->vclk);
> -
> -err_pm_put:
> - pm_runtime_put_sync(cru->dev);
> -
> return ret;
> }
>
> @@ -636,25 +615,33 @@ static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count
> struct rzg2l_cru_dev *cru = vb2_get_drv_priv(vq);
> int ret;
>
> + ret = pm_runtime_resume_and_get(cru->dev);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(cru->vclk);
> + if (ret)
> + goto err_pm_put;
> +
> /* Release reset state */
> ret = reset_control_deassert(cru->aresetn);
> if (ret) {
> dev_err(cru->dev, "failed to deassert aresetn\n");
> - return ret;
> + goto err_vclk_disable;
> }
>
> ret = reset_control_deassert(cru->presetn);
> if (ret) {
> reset_control_assert(cru->aresetn);
> dev_err(cru->dev, "failed to deassert presetn\n");
> - return ret;
> + goto assert_aresetn;
> }
>
> ret = request_irq(cru->image_conv_irq, rzg2l_cru_irq,
> IRQF_SHARED, KBUILD_MODNAME, cru);
Requesting the IRQ every time the device is started seems strange.
That's not related to this patch, but you may want to address it in a
separate series.
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> if (ret) {
> dev_err(cru->dev, "failed to request irq\n");
> - goto assert_resets;
> + goto assert_presetn;
> }
>
> /* Allocate scratch buffer. */
> @@ -686,10 +673,18 @@ static int rzg2l_cru_start_streaming_vq(struct vb2_queue *vq, unsigned int count
> free_image_conv_irq:
> free_irq(cru->image_conv_irq, cru);
>
> -assert_resets:
> +assert_presetn:
> reset_control_assert(cru->presetn);
> +
> +assert_aresetn:
> reset_control_assert(cru->aresetn);
>
> +err_vclk_disable:
> + clk_disable_unprepare(cru->vclk);
> +
> +err_pm_put:
> + pm_runtime_put_sync(cru->dev);
> +
> return ret;
> }
>
> @@ -704,9 +699,11 @@ static void rzg2l_cru_stop_streaming_vq(struct vb2_queue *vq)
> cru->scratch, cru->scratch_phys);
>
> free_irq(cru->image_conv_irq, cru);
> - reset_control_assert(cru->presetn);
> -
> return_unused_buffers(cru, VB2_BUF_STATE_ERROR);
> +
> + reset_control_assert(cru->presetn);
> + clk_disable_unprepare(cru->vclk);
> + pm_runtime_put_sync(cru->dev);
> }
>
> static const struct vb2_ops rzg2l_cru_qops = {
--
Regards,
Laurent Pinchart
