Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available
On 4/9/21 8:11 PM, 王擎 wrote: > >> On 4/9/21 7:42 PM, 王擎 wrote: >>> On 4/9/21 2:55 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. > > Signed-off-by: Wang Qing > --- > drivers/watchdog/mtk_wdt.c | 47 > +++--- > 1 file changed, 44 insertions(+), 3 deletions(-) > > diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c > index 97ca993..8b919cc > --- a/drivers/watchdog/mtk_wdt.c > +++ b/drivers/watchdog/mtk_wdt.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > > #define WDT_MAX_TIMEOUT 31 > #define WDT_MIN_TIMEOUT 1 > @@ -234,18 +235,35 @@ 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); That looks suspiciously like the real watchdog won't happen at all. What will happen if the pretimeout governor is set to none ? Guenter >>> The pretimeout governor is panic by default. If pretimeout is enabled and >>> the governor is >>> set to none, it means the timeout behavior does not need to be processed, >>> only printing. >>> >> >> That was not my question. My question was if the real timeout happens in >> that case. >> >> Guenter >> > Yes, the real timeout will happen. After WDT timeout, IRQ is sent out instead > of > reset signal first. In order to ensure that CPU does not get stuck after IRQ > is sent out, > WDT will time again and send reset signal to reset. > When will that be, or in other words how does the chip know when to time out ? After all, only a single timeout value is written into the chip. I don't see how it would know to reset the chip after wdt_dev->timeout. Guenter
Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available
On 4/9/21 7:42 PM, 王擎 wrote: > >> On 4/9/21 2:55 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. >>> >>> Signed-off-by: Wang Qing >>> --- >>> drivers/watchdog/mtk_wdt.c | 47 >>> +++--- >>> 1 file changed, 44 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c >>> index 97ca993..8b919cc >>> --- a/drivers/watchdog/mtk_wdt.c >>> +++ b/drivers/watchdog/mtk_wdt.c >>> @@ -25,6 +25,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #define WDT_MAX_TIMEOUT31 >>> #define WDT_MIN_TIMEOUT1 >>> @@ -234,18 +235,35 @@ 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); >> >> That looks suspiciously like the real watchdog won't happen at all. >> What will happen if the pretimeout governor is set to none ? >> >> Guenter >> > The pretimeout governor is panic by default. If pretimeout is enabled and the > governor is > set to none, it means the timeout behavior does not need to be processed, > only printing. > That was not my question. My question was if the real timeout happens in that case. Guenter
Re: [PATCH] watchdog: mtk: support pre-timeout when the bark irq is available
On 4/9/21 2:55 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.
>
> Signed-off-by: Wang Qing
> ---
> drivers/watchdog/mtk_wdt.c | 47
> +++---
> 1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 97ca993..8b919cc
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include
>
> #define WDT_MAX_TIMEOUT 31
> #define WDT_MIN_TIMEOUT 1
> @@ -234,18 +235,35 @@ 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);
That looks suspiciously like the real watchdog won't happen at all.
What will happen if the pretimeout governor is set to none ?
Guenter
> if (ret < 0)
> return ret;
>
> reg = ioread32(wdt_base + WDT_MODE);
> reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN);
> + if (wdt_dev->pretimeout)
> + 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);
> +}
> +
> +static irqreturn_t mtk_wdt_isr(int irq, void *arg)
> +{
> + struct watchdog_device *wdd = arg;
> + watchdog_notify_pretimeout(wdd);
> +
> + return IRQ_HANDLED;
> +}
> +
> static const struct watchdog_info mtk_wdt_info = {
> .identity = DRV_NAME,
> .options= WDIOF_SETTIMEOUT |
> @@ -253,12 +271,21 @@ static const struct watchdog_info mtk_wdt_info = {
> WDIOF_MAGICCLOSE,
> };
>
> +static const struct watchdog_info mtk_wdt_pt_info = {
> + .identity = DRV_NAME,
> + .options= WDIOF_SETTIMEOUT |
> + WDIOF_PRETIMEOUT |
> + WDIOF_KEEPALIVEPING |
> + WDIOF_MAGICCLOSE,
> +};
> +
> static const struct watchdog_ops mtk_wdt_ops = {
> .owner = THIS_MODULE,
> .start = mtk_wdt_start,
> .stop = mtk_wdt_stop,
> .ping = mtk_wdt_ping,
> .set_timeout= mtk_wdt_set_timeout,
> + .set_pretimeout = mtk_wdt_set_pretimeout,
> .restart= mtk_wdt_restart,
> };
>
> @@ -267,7 +294,7 @@ static int mtk_wdt_probe(struct platform_device *pdev)
> struct device *dev = >dev;
> struct mtk_wdt_dev *mtk_wdt;
> const struct mtk_wdt_data *wdt_data;
> - int err;
> + int err,irq;
>
> mtk_wdt = devm_kzalloc(dev, sizeof(*mtk_wdt), GFP_KERNEL);
> if (!mtk_wdt)
> @@ -279,7 +306,21 @@ static int mtk_wdt_probe(struct platform_device *pdev)
> if (IS_ERR(mtk_wdt->wdt_base))
> return PTR_ERR(mtk_wdt->wdt_base);
>
> - mtk_wdt->wdt_dev.info = _wdt_info;
> + irq = platform_get_irq(pdev, 0);
> + if (irq > 0) {
> + err = devm_request_irq(>dev, irq, mtk_wdt_isr, 0,
> "wdt_bark", _wdt->wdt_dev);
> + if (err)
> + return err;
> +
> + mtk_wdt->wdt_dev.info = _wdt_pt_info;
> + mtk_wdt->wdt_dev.pretimeout = 1;
> + } else {
> + if (irq == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + mtk_wdt->wdt_dev.info = _wdt_info;
> + }
> +
> mtk_wdt->wdt_dev.ops = _wdt_ops;
> mtk_wdt->wdt_dev.timeout = WDT_MAX_TIMEOUT;
> mtk_wdt->wdt_dev.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT * 1000;
>
