Hi Sakari Ailus,
Thanks for the feedback.
> -----Original Message-----
> Subject: Re: [PATCH 4/4] media: platform: rzg2l-cru: rzg2l-video:
> Restructure clk handling
>
> Hi Biju,
>
> Thanks for the patch.
>
> On Tue, Jan 23, 2024 at 11:58:21AM +0000, Biju Das wrote:
> > As per section 35.3.1 Starting Reception for the MIPI CSI-2 Input on
> > the latest hardware manual(R01UH0914EJ0140 Rev.1.40) it is mentioned
> > that we need to supply all CRU clks and we need to disable the vclk
> > before enabling the LINK reception and enable the vclk after enabling
> > the link Reception. So restructure clk handling as per the HW manual.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > .../platform/renesas/rzg2l-cru/rzg2l-cru.h | 3 -
> > .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 55 ++++++++++++---
> > .../platform/renesas/rzg2l-cru/rzg2l-ip.c | 15 +---
> > .../platform/renesas/rzg2l-cru/rzg2l-video.c | 69
> > ++++++++-----------
> > 4 files changed, 74 insertions(+), 68 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 c4609da9bf1a..f4c5cbb30bc9 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> > @@ -6,6 +6,7 @@
> > */
> >
> > #include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > #include <linux/delay.h>
> > #include <linux/interrupt.h>
> > #include <linux/io.h>
> > @@ -108,6 +109,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,10 +363,11 @@ 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;
> > + int ret, count;
> >
> > /* Select data lanes */
> > rzg2l_csi2_write(csi2, CSI2nMCT0, CSI2nMCT0_VDLN(csi2->lanes)); @@
> > -386,11 +389,40 @@ 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);
> > + for (count = 0; count < 5; count++) {
> > + if (!(__clk_is_enabled(csi2->vclk)))
> > + break;
> > + usleep_range(10, 20);
> > + }
> > +
> > + if (count == 5) {
> > + dev_err(csi2->dev, "Timeout, not able to turn OFF vclk\n");
> > + return -ETIMEDOUT;
> > + }
>
> It'd be nice to have a CCF function to do this. Either way, you can use
> read_poll_timeout().
OK, Will update rzg2l_mod_clock_endisable() [1] to handle this.
Currently it does status check for clk on but missing the same status check for clock off.
[1] https://elixir.bootlin.com/linux/latest/source/drivers/clk/renesas/rzg2l-cpg.c#L1201
Cheers,
Biju
