Hi Phil,
On Wed, Jun 15, 2022 at 4:32 PM Phil Edworthy <phil.edworthy@xxxxxxxxxxx> wrote:
> On 15 June 2022 10:41 Phil Edworthy wrote:
> > On Mon, Jun 13, 2022 at 5:06 PM Phil Edworthy wrote:
> > > The WDT on RZ/V2M devices is basically the same as RZ/G2L, but without
> > > the parity error registers. This means the driver has to reset the
> > > hardware plus set the minimum timeout in order to do a restart and has
> > > a single interrupt.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> > > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > --- a/drivers/watchdog/rzg2l_wdt.c
> > > +++ b/drivers/watchdog/rzg2l_wdt.c
> >
> > > @@ -139,14 +146,25 @@ static int rzg2l_wdt_restart(struct
> > watchdog_device *wdev,
> > > {
> > > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev);
> > >
> > > - clk_prepare_enable(priv->pclk);
> > > - clk_prepare_enable(priv->osc_clk);
> > > + if (priv->devtype == I2C_RZG2L) {
> > > + clk_prepare_enable(priv->pclk);
> > > + clk_prepare_enable(priv->osc_clk);
> > >
> > > - /* Generate Reset (WDTRSTB) Signal on parity error */
> > > - rzg2l_wdt_write(priv, 0, PECR);
> > > + /* Generate Reset (WDTRSTB) Signal on parity error */
> > > + rzg2l_wdt_write(priv, 0, PECR);
> > >
> > > - /* Force parity error */
> > > - rzg2l_wdt_write(priv, PEEN_FORCE, PEEN);
> > > + /* Force parity error */
> > > + rzg2l_wdt_write(priv, PEEN_FORCE, PEEN);
> > > + } else {
> > > + /* RZ/V2M doesn't have parity error registers */
> > > +
> > > + wdev->timeout = 0;
> > > + rzg2l_wdt_start(wdev);
> >
> > This will call pm_runtime_get_sync(), which is not allowed in this
> > context, cfr. commit e4cf89596c1f1e33 ("watchdog: rzg2l_wdt: Fix
> > 'BUG: Invalid wait context'").
> Ok, I see. I haven't been able to trigger that bug on rz/v2m. I'm
> Not sure what I can't trigger it though.
>
> > While you can call clk_prepare_enable() instead, that can only be
> > used as a temporary workaround, until you have implemented RZ/V2M
> > power domain support...
> Sorry, my knowledge of power domain is somewhat lacking...
>
> I followed the code into rpm_resume() and see from that commit msg
> that the problem arises in rpm_callback().
> Looking at the code is appears that if power domain doesn’t set any
> callbacks it's considered a success and so won’t call rpm_callback().
>
> Is that why power domain support will allow the driver to call
> pm_runtime_get_sync() without issue?
Not really.
You cannot call pm_runtime_get_sync() from a restart notifier.
Hence the RZ/G2L restart code works around that by enabling the
clocks manually, which works as the PM Domain on RZ/G2L is only a
clock domain.
However, unlike RZ/G2L, RV/V2M also has power areas[1]. Currently
Linux does not support the RZ/V2M power areas yet, so you can use
the same workaround as on RZ/G2L, i.e. enable the clocks manually.
When support for RZ/V2M power areas will be added, you will have
a problem, as you cannot enable power areas manually, only through
pm_runtime_get_sync().
Does RZ/V2M support alternative ways to reboot, e.g. PSCI reboot?
[1] Section 51, Internal Power Domain Controller (PMC).
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
