Hi Geert,
On 15 June 2022 15:43 Geert Uytterhoeven wrote:
> On Wed, Jun 15, 2022 at 4:32 PM Phil Edworthy 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().
Ok, makes sense, thank you for explaining it.
> Does RZ/V2M support alternative ways to reboot, e.g. PSCI reboot?
No, no other way.
Thanks
Phil
