Hi Geert,
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>
> > ---
> > v2:
> > - Replace use of parity error registers in restart
> > - Commit msg modified to reflect different contents
>
> Thanks for the update!
>
> > --- 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?
> > +
> > + /* Wait 2 consecutive overflow cycles for reset */
> > + udelay(DIV64_U64_ROUND_UP(2 * 0xFFFFF * 1000000ULL,
> > + priv->osc_clk_rate));
>
> DIV64_U64_ROUND_UP() does a 64-by-64 division, while priv->osc_clk_rate
> is "unsigned long" (yes, that is 64-bit on RZ/G2L and RZ/V2M ;-)
> Unfortunately there is no rounding version of div64_ul() yet.
>
> However, there is no need to use a 64-bit dividend, as the resulting
> delay will be multiple ms anyway, so you can just use mdelay() instead:
>
> mdelay(DIV_ROUNDUP(2 * 0xFFFFF * 1000, priv->osc_clk_rate));
Will fix, thanks for the suggestion.
Thanks
Phil
