>On 4/14/21 4:48 AM, Wang Qing wrote:
>> Use the bark interrupt as the pretimeout notifier if available.
>>
>> By default, the pretimeout notification shall occur one second earlier
>> than the timeout.
>>
>> V2:
>> - panic() by default if WATCHDOG_PRETIMEOUT_GOV is not enabled.
>>
>> V3:
>> - Modify the pretimeout behavior, manually reset after the pretimeout
>> - is processed and wait until timeout.
>>
>> Signed-off-by: Wang Qing <wangqing@xxxxxxxx>
>> ---
>> drivers/watchdog/mtk_wdt.c | 62 ++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 57 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
>> index 97ca993..7bef1e3
>> --- a/drivers/watchdog/mtk_wdt.c
>> +++ b/drivers/watchdog/mtk_wdt.c
>> @@ -25,6 +25,7 @@
>> #include <linux/reset-controller.h>
>> #include <linux/types.h>
>> #include <linux/watchdog.h>
>> +#include <linux/interrupt.h>
>>
>> #define WDT_MAX_TIMEOUT 31
>> #define WDT_MIN_TIMEOUT 1
>> @@ -234,18 +235,46 @@ static int mtk_wdt_start(struct watchdog_device *wdt_dev)
>> void __iomem *wdt_base = mtk_wdt->wdt_base;
>> int ret;
>>
>> - ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
>> + ret = mtk_wdt_set_timeout(wdt_dev, wdt_dev->timeout - wdt_dev->pretimeout);
>> if (ret < 0)
>> return ret;
>>
>> reg = ioread32(wdt_base + WDT_MODE);
>> - reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN);
>> + reg &= ~WDT_MODE_IRQ_EN;
>> + if (wdt_dev->pretimeout)
>> + reg |= WDT_MODE_IRQ_EN;
>> + else
>> + reg &= ~WDT_MODE_IRQ_EN;
>> reg |= (WDT_MODE_EN | WDT_MODE_KEY);
>> iowrite32(reg, wdt_base + WDT_MODE);
>>
>> return 0;
>> }
>>
>> +static int mtk_wdt_set_pretimeout(struct watchdog_device *wdd,
>> + unsigned int timeout)
>> +{
>> + wdd->pretimeout = timeout;
>> + return mtk_wdt_start(wdd);
>
>The watchdog is not necessarily active here.
>
>> +}
>> +
>> +static irqreturn_t mtk_wdt_isr(int irq, void *arg)
>> +{
>> + struct watchdog_device *wdd = arg;
>> + struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdd);
>> + void __iomem *wdt_base = mtk_wdt->wdt_base;
>> +
>> + watchdog_notify_pretimeout(wdd);
>> + /*
>> + * Guaranteed to be reset when the timeout
>> + * expires under any situations
>> + */
>> + mdelay(1000*wdd->pretimeout);
>
>That is not how this is supposed to work. The idea with a pretimeout is that the
>real watchdog reset will happen under all circumstances, and that executing
>the pretimeout (and changing some hardware registers) is not a prerequisite
>for the real timeout to happen. After all, the system could be stuck hard, with
>interrupts disabled.
>
>On top of that, just sleeping here while waiting for the real timeout and
>then resetting the system isn't the idea either. On a single core system this
>will just hang. On a multi-core system, who knows if userspace managed to ping
>the watchdog in the meantime.
>
>Unless there is a means to trigger the watchdog twice, without intervention,
>the first time generating an interrupt and the second time resetting the system,
>there is no way for this to work. I don't see how this chip really supports
>pretimeout. It seems that it supports either a hard reset or generating an
>interrupt on watchdog timeout, and there is only a single timeout.
>
>If you have a use case for generating an interrupt and resetting the system via
>software (ie panic) _instead_ of having it generate a hard reset, please feel
>free to submit a patch along that line, together with a description of its use
>case.
>
>Thanks,
>Guenter
>
Yes, as mentioned before, the behavior of WDT_MODE_IRQ_EN is use irq instead of
reset, so we must use WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN if like you said
"the first time generating an interrupt and the second time resetting the system" .
The Dual mode is mentioned in the MTK datasheet:
In this mode, the watchdog will be AUTO-RESTART after interrupt is triggered.
AP need to clear WDT_STA after receiving interrupt from TOPRGU, or system reset
will be triggered after watchdog timer expires.
Instructions for use：
Set wdt_en = 1'b1.
Set dual_mode = 1'b1.
Set wdt_irq = 1'b1.
We can use Dual mode to achieve pretimeout behavior, only in this way can we
get more information during pretimeout processing, instead of directly resetting.
Thanks,
Qing
