On 8/6/24 06:47, Lad, Prabhakar wrote:
Hi Guenter,
...
+ /*
+ * WDTCR
+ * - CKS[7:4] - Clock Division Ratio Select - 0101b: oscclk/256
+ * - RPSS[13:12] - Window Start Position Select - 11b: 100%
+ * - RPES[9:8] - Window End Position Select - 11b: 0%
+ * - TOPS[1:0] - Timeout Period Select - 11b: 16384 cycles (3FFFh)
+ */
+ rzv2h_wdt_setup(wdev, WDTCR_CKS_CLK_256 | WDTCR_RPSS_100 |
+ WDTCR_RPES_0 | WDTCR_TOPS_16384);
+
+ rzv2h_wdt_ping(wdev);
+
The need to ping the watchdog immediately after enabling it is unusual.
Please explain.
The down counting operation starts only after the ping operation, so
after starting the wdt a ping is issued here.
Please add that as comment to the code.
...
Ive now updated restart with below, so that we dont touch clocks if
they are already ON,
if (!watchdog_active(wdev)) {
ret = clk_enable(priv->pclk);
if (ret)
return ret;
ret = clk_enable(priv->oscclk);
if (ret) {
clk_disable(priv->pclk);
return ret;
}
}
if (!watchdog_active(wdev))
ret = reset_control_deassert(priv->rstc);
else
ret = reset_control_reset(priv->rstc);
Please rearrange to only require a single "if (!watchdog_active())".
Also, please add a comment explaining the need for calling
reset_control_reset() if the watchdog is active.
if (ret) {
clk_disable(priv->oscclk);
clk_disable(priv->pclk);
return ret;
}
/* delay to handle clock halt after de-assert operation */
udelay(3);
+ /*
+ * WDTCR
+ * - CKS[7:4] - Clock Division Ratio Select - 0000b: oscclk/1
+ * - RPSS[13:12] - Window Start Position Select - 00b: 25%
+ * - RPES[9:8] - Window End Position Select - 00b: 75%
+ * - TOPS[1:0] - Timeout Period Select - 00b: 1024 cycles (03FFh)
+ */
+ rzv2h_wdt_setup(wdev, WDTCR_CKS_CLK_1 | WDTCR_RPSS_25 |
+ WDTCR_RPES_75 | WDTCR_TOPS_1024);
+ rzv2h_wdt_ping(wdev);
+
Why is the ping here necessary ?
The down counting starts after the refresh operation, hence the WDT is pinged.
Should be covered with the explanation in rzv2h_wdt_start().
Thanks,
Guenter
