Re: [PATCH] clk: renesas: cpg-mssr: Add missing 1ms delay into reset toggle callback

["Niklas Söderlund" <niklas.soderlund@ragnatech.se>]

Hi Marek,

On 2025-10-05 06:00:15 +0200, Marek Vasut wrote:
> On 10/3/25 5:08 PM, Niklas Söderlund wrote:
> 
> Hello Niklas,
> 
> > On 2025-09-18 05:04:43 +0200, Marek Vasut wrote:
> > > R-Car V4H Reference Manual R19UH0186EJ0130 Rev.1.30 Apr. 21, 2025 page 583
> > > Figure 9.3.1(a) Software Reset flow (A) as well as flow (B) / (C) indicate
> > > after reset has been asserted by writing a matching reset bit into register
> > > SRCR, it is mandatory to wait 1ms.
> > > 
> > > This 1ms delay is documented on R-Car V4H and V4M, it is currently unclear
> > > whether S4 is affected as well. This patch does apply the extra delay on
> > > R-Car S4 as well.
> > > 
> > > Fix the reset driver to respect the additional delay when toggling resets.
> > > Drivers which use separate reset_control_(de)assert() must assure matching
> > > delay in their driver code.
> > > 
> > > Fixes: 0ab55cf18341 ("clk: renesas: cpg-mssr: Add support for R-Car V4H")
> > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > ---
> > > Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > Cc: Stephen Boyd <sboyd@kernel.org>
> > > Cc: linux-clk@vger.kernel.org
> > > Cc: linux-renesas-soc@vger.kernel.org
> > > ---
> > >   drivers/clk/renesas/renesas-cpg-mssr.c | 11 +++++++++--
> > >   1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
> > > index be9f59e6975d..65dfaceea71f 100644
> > > --- a/drivers/clk/renesas/renesas-cpg-mssr.c
> > > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> > > @@ -689,8 +689,15 @@ static int cpg_mssr_reset(struct reset_controller_dev *rcdev,
> > >   	/* Reset module */
> > >   	writel(bitmask, priv->pub.base0 + priv->reset_regs[reg]);
> > > -	/* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
> > > -	udelay(35);
> > > +	/*
> > > +	 * On R-Car Gen4, delay after SRCR has been written is 1ms.
> > > +	 * On older SoCs, delay after SRCR has been written is 35us
> > > +	 * (one cycle of the RCLK clock @ cca. 32 kHz).
> > > +	 */
> > > +	if (priv->reg_layout == CLK_REG_LAYOUT_RCAR_GEN4)
> > > +		usleep_range(1000, 2000);
> > > +	else
> > > +		usleep_range(35, 1000);
> > 
> > I rebased the R-Car ISP work to renesas-drivers today and it included
> > this change, and I seem to have hit an issue with the switch form
> > udelay() to usleep_range() I'm afraid. I can't find any other good
> > reproducer of the issue however.
> > 
> > THe core of the issue seems to be that if a reset is issued from an
> > atomic context bad things happen if you try to sleep. I get this splat
> > and the board is completer dead after it, needing a power cycle to
> > recover.
> > 
> > If I revert this patch things work as expected.
> Thank you for testing. Does it work well if you replace those
> usleep_range()s with plain udelay() ?

With this change it do work. I'm testing on V4H so I assume it's the 
Gen4 branch that is taken here?

diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
index a3d171ddaab9..2b45f0aa8d97 100644
--- a/drivers/clk/renesas/renesas-cpg-mssr.c
+++ b/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -709,9 +709,9 @@ static int cpg_mssr_reset(struct reset_controller_dev *rcdev,
         * (one cycle of the RCLK clock @ ca. 32 kHz).
         */
        if (priv->reg_layout == CLK_REG_LAYOUT_RCAR_GEN4)
-               usleep_range(1000, 2000);
+               udelay(1000);
        else
-               usleep_range(35, 1000);
+               udelay(35);

        /* Release module from reset state */
        return cpg_mssr_reset_operate(rcdev, NULL, false, id);

-- 
Kind Regards,
Niklas Söderlund