Hi Laurent,
> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Sent: Wednesday, February 14, 2024 2:50 PM
> Subject: Re: [PATCH v3 4/5] media: platform: rzg2l-cru: rzg2l-csi2:
> Restructure vclk handling
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Tue, Feb 13, 2024 at 06:12:32PM +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
> > disable the vclk before enabling the LINK reception and enable the
> > vclk after enabling the link Reception. So restructure vclk handling as
> per the HW manual.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > v2->v3:
> > * Updated commit header and description
> > * Split the patch into 2. Restructuring of vclk for link reception is
> > handled here and fixing start reception procedure is handled
> > in the next patch.
> > v1->v2:
> > * Dropped clk-provider.h and __clk_is_enabled() as consumer clk should
> > not use it. Plan to send RFC for clk_disable_unprepare_sync() in ccf.
> > ---
> > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 3 --
> > .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 28 +++++++++++--------
> > .../platform/renesas/rzg2l-cru/rzg2l-ip.c | 15 ++--------
> > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 10 -------
> > 4 files changed, 19 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > index 811603f18af0..a5a99b004322 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-cru.h
> > @@ -133,9 +133,6 @@ struct rzg2l_cru_dev {
> > struct v4l2_pix_format format;
> > };
> >
> > -void rzg2l_cru_vclk_unprepare(struct rzg2l_cru_dev *cru); -int
> > rzg2l_cru_vclk_prepare(struct rzg2l_cru_dev *cru);
> > -
> > int rzg2l_cru_start_image_processing(struct rzg2l_cru_dev *cru);
> > void rzg2l_cru_stop_image_processing(struct rzg2l_cru_dev *cru);
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > index e00d9379dd2c..e68fcdaea207 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > @@ -108,6 +108,7 @@ struct rzg2l_csi2 {
> > struct reset_control *presetn;
> > struct reset_control *cmn_rstb;
> > struct clk *sysclk;
> > + struct clk *vclk;
> > unsigned long vclk_rate;
> >
> > struct v4l2_subdev subdev;
> > @@ -361,7 +362,7 @@ static int rzg2l_csi2_dphy_setting(struct
> v4l2_subdev *sd, bool on)
> > return rzg2l_csi2_dphy_disable(csi2); }
> >
> > -static void rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
> > +static int rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
> > {
> > unsigned long vclk_rate = csi2->vclk_rate / HZ_PER_MHZ;
> > u32 frrskw, frrclk, frrskw_coeff, frrclk_coeff; @@ -386,11 +387,15
> > @@ static void rzg2l_csi2_mipi_link_enable(struct rzg2l_csi2 *csi2)
> > rzg2l_csi2_write(csi2, CSI2nDTEL, 0xf778ff0f);
> > rzg2l_csi2_write(csi2, CSI2nDTEH, 0x00ffff1f);
> >
> > + clk_disable_unprepare(csi2->vclk);
> > +
> > /* Enable LINK reception */
> > rzg2l_csi2_write(csi2, CSI2nMCT3, CSI2nMCT3_RXEN);
> > +
> > + return clk_prepare_enable(csi2->vclk);
> > }
> >
> > -static void rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
> > +static int rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
> > {
> > unsigned int timeout = VSRSTS_RETRIES;
> >
> > @@ -409,18 +414,21 @@ static void rzg2l_csi2_mipi_link_disable(struct
> > rzg2l_csi2 *csi2)
> >
> > if (!timeout)
> > dev_err(csi2->dev, "Clearing CSI2nRTST.VSRSTS timed out\n");
> > +
> > + return 0;
> > }
> >
> > static int rzg2l_csi2_mipi_link_setting(struct v4l2_subdev *sd, bool
> > on) {
> > struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> > + int ret;
> >
> > if (on)
> > - rzg2l_csi2_mipi_link_enable(csi2);
> > + ret = rzg2l_csi2_mipi_link_enable(csi2);
> > else
> > - rzg2l_csi2_mipi_link_disable(csi2);
> > + ret = rzg2l_csi2_mipi_link_disable(csi2);
> >
> > - return 0;
> > + return ret;
> > }
> >
> > static int rzg2l_csi2_s_stream(struct v4l2_subdev *sd, int enable) @@
> > -731,7 +739,6 @@ static const struct media_entity_operations
> > rzg2l_csi2_entity_ops = { static int rzg2l_csi2_probe(struct
> > platform_device *pdev) {
> > struct rzg2l_csi2 *csi2;
> > - struct clk *vclk;
> > int ret;
> >
> > csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL); @@
> > -757,12 +764,11 @@ static int rzg2l_csi2_probe(struct platform_device
> *pdev)
> > return dev_err_probe(&pdev->dev, PTR_ERR(csi2->sysclk),
> > "Failed to get system clk\n");
> >
> > - vclk = clk_get(&pdev->dev, "video");
> > - if (IS_ERR(vclk))
> > - return dev_err_probe(&pdev->dev, PTR_ERR(vclk),
> > + csi2->vclk = devm_clk_get(&pdev->dev, "video");
> > + if (IS_ERR(csi2->vclk))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(csi2->vclk),
> > "Failed to get video clock\n");
> > - csi2->vclk_rate = clk_get_rate(vclk);
> > - clk_put(vclk);
> > + csi2->vclk_rate = clk_get_rate(csi2->vclk);
> >
> > csi2->dev = &pdev->dev;
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
> > b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
> > index 8466b4e55909..2d22c373588b 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-ip.c
> > @@ -80,20 +80,9 @@ static int rzg2l_cru_ip_s_stream(struct v4l2_subdev
> *sd, int enable)
> > return ret;
> > }
> >
> > - rzg2l_cru_vclk_unprepare(cru);
> > -
> > ret = v4l2_subdev_call(cru->ip.remote, video, s_stream,
> enable);
> > - if (ret == -ENOIOCTLCMD)
> > - ret = 0;
> > - if (!ret) {
> > - ret = rzg2l_cru_vclk_prepare(cru);
> > - if (!ret)
> > - return 0;
> > - } else {
> > - /* enable back vclk so that s_stream in error path
> disables it */
> > - if (rzg2l_cru_vclk_prepare(cru))
> > - dev_err(cru->dev, "Failed to enable vclk\n");
> > - }
> > + if (!ret || (ret == -ENOIOCTLCMD))
>
> No need for the inner parentheses.
>
> I can fix this when applying, no need to send a v4 just for this.
Thank you,
Biju
