Adding initial logs with just delay in deassert.
> Subject: RE: [PATCH] clk: renesas: cpg-mssr: Add a delay after deassert
>
> Hi Geert,
>
> Thanks for the feedback.
>
> > Subject: Re: [PATCH] clk: renesas: cpg-mssr: Add a delay after
> > deassert
> >
> > Hi Biju,
> >
> > On Wed, May 4, 2022 at 8:44 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> wrote:
> > > After adding reset support to vsp, it needs a delay of 32
> > > microseconds after reset operation, otherwise system hangs(due to
> > > register
> > read/write).
> > > This patch fixes the system hang issue by adding delay after
> > > deassert operation.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > Thanks for your patch!
> >
> > > After adding reset/deassert support for vsp based on [1], RZ/G1N
> > > board hangs. On debugging it found that it needs a delay of 35
> > > microseconds after deasserint reset. Wthout delay if there is any
> > > register read/write will lead to hang.
> > >
> > > This 35 microseconds value is picked from the reset().
> >
> > The 35 µs comes from the Hardware User's Manual: there should be at
> > least 1 RCLK cycle _in between_ asserting and deasserting reset.
> > The manual doesn't say anything about delays _after_ deasserting reset.
> >
> > Could it be that the VSP1 driver is actually deasserting reset too early?
>
> My test results on RZ/G1N shows, it needs 35 micro seconds after
> deasserting reset.
>
> Logs with just adding delay in assert()
> ------------------------------------
> [ 1.938991] renesas-cpg-mssr e6150000.clock-controller: deassert 131
> [ 1.945373] renesas-cpg-mssr e6150000.clock-controller: assert 131
> [ 1.956445] renesas-cpg-mssr e6150000.clock-controller: deassert 131
>
> It hangs after this. After this, there is a read register call which hangs
> the system.
>
> Looks like after deassert, it needs some delay for accessing registres.
>
> Logs with just adding delay in deassert()
> ------------------------------------
[ 1.938822] renesas-cpg-mssr e6150000.clock-controller: deassert 131
[ 1.945239] renesas-cpg-mssr e6150000.clock-controller: assert 131
[ 1.956273] renesas-cpg-mssr e6150000.clock-controller: deassert 131
[ 1.962874] renesas-cpg-mssr e6150000.clock-controller: deassert 128
[ 1.969271] renesas-cpg-mssr e6150000.clock-controller: assert 128
[ 1.975659] renesas-cpg-mssr e6150000.clock-controller: assert 131
[ 1.983799] renesas-cpg-mssr e6150000.clock-controller: deassert 128
[ 1.990344] renesas-cpg-mssr e6150000.clock-controller: deassert 127
[ 1.996760] renesas-cpg-mssr e6150000.clock-controller: assert 127
[ 2.003106] renesas-cpg-mssr e6150000.clock-controller: assert 128
[ 2.011224] renesas-cpg-mssr e6150000.clock-controller: deassert 127
[ 2.018421] renesas-cpg-mssr e6150000.clock-controller: assert 127
> [ 12.536298] renesas-cpg-mssr e6150000.clock-controller: deassert 131
> [ 12.563577] renesas-cpg-mssr e6150000.clock-controller: assert 131
> [ 12.571630] renesas-cpg-mssr e6150000.clock-controller: deassert 131
> [ 12.578412] renesas-cpg-mssr e6150000.clock-controller: assert 131
> [ 12.584702] renesas-cpg-mssr e6150000.clock-controller: deassert 131
> [ 12.609875] renesas-cpg-mssr e6150000.clock-controller: assert 131
> [ 12.618666] renesas-cpg-mssr e6150000.clock-controller: deassert 131
> [ 12.631736] renesas-cpg-mssr e6150000.clock-controller: assert 131
> [ 12.679240] renesas-cpg-mssr e6150000.clock-controller: deassert 128
> [ 12.689712] renesas-cpg-mssr e6150000.clock-controller: assert 128
> [ 12.697249] renesas-cpg-mssr e6150000.clock-controller: deassert 128
> [ 12.712796] renesas-cpg-mssr e6150000.clock-controller: assert 128
> [ 12.755191] renesas-cpg-mssr e6150000.clock-controller: deassert 127
> [ 12.762389] renesas-cpg-mssr e6150000.clock-controller: assert 127
> [ 12.775882] renesas-cpg-mssr e6150000.clock-controller: deassert 127
> [ 12.791329] renesas-cpg-mssr e6150000.clock-controller: assert 127
>
> It works Ok.
>
>
>
> >
> > > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > > @@ -637,6 +637,7 @@ static int cpg_mssr_assert(struct
> > reset_controller_dev *rcdev, unsigned long id)
> > > dev_dbg(priv->dev, "assert %u%02u\n", reg, bit);
> > >
> > > writel(bitmask, priv->base + priv->reset_regs[reg]);
> > > +
> >
> > Unrelated change.
>
> Oops, Added a debug message there. Will take out this.
>
> Cheers,
> Biju
> >
> > > return 0;
> > > }
> > >
> > > @@ -651,6 +652,10 @@ static int cpg_mssr_deassert(struct
> > reset_controller_dev *rcdev,
> > > dev_dbg(priv->dev, "deassert %u%02u\n", reg, bit);
> > >
> > > writel(bitmask, priv->base + priv->reset_clear_regs[reg]);
> > > +
> > > + /* Wait for at least one cycle of the RCLK clock (@ ca. 32
> > > + kHz)
> > */
> > > + udelay(35);
> > > +
> > > return 0;
> > > }
> >
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> > geert@linux- m68k.org
> >
> > In personal conversations with technical people, I call myself a hacker.
> > But when I'm talking to journalists I just say "programmer" or
> > something like that.
> > -- Linus Torvalds
