Hi Laurent,
Thanks for the feedback.
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Sent: Wednesday, February 14, 2024 2:56 PM
> Subject: Re: [PATCH v3 5/5] media: platform: rzg2l-cru: rzg2l-video: Fix
> start reception procedure
>
> 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.
Agreed. Will send a separate patch.
Cheers,
Biju
