Hi Biju,
On Thu, May 5, 2022 at 12:01 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Subject: Re: [PATCH] clk: renesas: cpg-mssr: Add a delay after deassert
> > 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>
> >
> > > 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.
I can confirm that accessing the VSP registers without the delay
causes a lock-up on R-Car M2-W, too.
I see no such lock-up on R-Car Gen3, but I cannot rule out that it
is mitigated by a handler in secure mode, and that VSP initialization
may actually fail (accessing registers of non-clocked modules usually
causes an imprecise external abort, which is caught by Linux on R-Car
Gen2, but turned into a no-op by secure firmware on R-Car Gen3).
Instead of adding the explicit delay, I tried added a polling loop
after the call to reset_control_deassert() in the vsp1 driver, to
wait until the reset is cleared, like is done in the i2c-rcar driver:
ret = read_poll_timeout_atomic(reset_control_status, ret, ret == 0, 1,
100, false, vsp1->rstc);
if (ret < 0) {
...
}
This also fixes the issue for me.
Adding more debug code shows that reset_control_status() is called
only once (both for i2c and vsp1), so the polling completes before
any call to udelay().
Note that at that time[1], we added the delay to the i2c-rcar driver
instead of the CPG/MSSR driver, as we were told that i2c reset was
special, and other modules do not need this.
Perhaps vsp reset is special, too?
Or perhaps it is time to revisit this, and add the polling to both
cpg_mssr_reset() and cpg_mssr_deassert(), so it can be removed from
the drivers?
[1] Commit 3b770017b03a4cdf ("i2c: rcar: handle RXDMA HW behaviour on Gen3").
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
