>On 4/22/21 8:48 PM, 王擎 wrote: >> >>> On 4/22/21 7:53 PM, Wang Qing wrote: >>>> Use the bark interrupt as the pretimeout notifier if available. >>>> >>>> When the watchdog timer expires in dual mode, an interrupt will be >>>> triggered first, then the timing restarts. The reset signal will be >>>> initiated when the timer expires again. >>>> >>>> The pretimeout notification shall occur at timeout-sec/2. >>>> >>>> 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. >>>> >>>> V4: >>>> - Remove pretimeout related processing. >>>> - Add dual mode control separately. >>>> >>>> V5: >>>> - Fix some formatting and printing problems. >>>> >>>> V6: >>>> - Realize pretimeout processing through dualmode. >>>> >>>> V7: >>>> - Add set_pretimeout(). >>>> >>>> Signed-off-by: Wang Qing <wangqing@xxxxxxxx> >>>> --- >>>> drivers/watchdog/mtk_wdt.c | 76 +++++++++++++++++++++++++++++++++++++++++++--- >>>> 1 file changed, 71 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c >>>> index 97ca993..ab3ac5d >>>> --- 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 >>>> @@ -184,15 +185,23 @@ static int mtk_wdt_set_timeout(struct watchdog_device *wdt_dev, >>>> { >>>> struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev); >>>> void __iomem *wdt_base = mtk_wdt->wdt_base; >>>> + unsigned int timeout_interval = timeout; >>>> u32 reg; >>>> >>>> wdt_dev->timeout = timeout; >>>> - >>>> + /* >>>> + * In dual mode, irq will be triggered at timeout / 2 >>>> + * the real timeout occurs at timeout >>>> + */ >>>> + if (wdt_dev->pretimeout) { >>>> + wdt_dev->pretimeout = timeout / 2; >>> >>> min_timeout is set to 1. I don't this works well if timeout == 1. >>> You'll either need to set min_timeout to 2, or handle that case. >> >> It is appropriate to change min_timeout to 2. >> >>> >>>> + timeout_interval = wdt_dev->pretimeout; >>> >>> timeout_interval is unnecessary. Just update timeout accordingly. >>> It needs to take the situation of timeout == 1 into account, though. I plan to remove timeout_interval and directly use (timeout-pretimeout) update timeout. >> >> timeout represents the reset time. When the user calls timeout_show, >> He hopes to get the configured timeout, not the value changed >> by pre-timeout. >> I modify it like this more in line with the original intention. >> >>> >>>> + } >>>> /* >>>> * One bit is the value of 512 ticks >>>> * The clock has 32 KHz >>>> */ >>>> - reg = WDT_LENGTH_TIMEOUT(timeout << 6) | WDT_LENGTH_KEY; >>>> + reg = WDT_LENGTH_TIMEOUT(timeout_interval << 6) | WDT_LENGTH_KEY; >>>> iowrite32(reg, wdt_base + WDT_LENGTH); >>>> >>>> mtk_wdt_ping(wdt_dev); >>>> @@ -239,13 +248,46 @@ static int mtk_wdt_start(struct watchdog_device *wdt_dev) >>>> 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 | WDT_MODE_DUAL_EN); >>>> + else >>>> + reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_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) >>>> +{ >>>> + struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdd); >>>> + void __iomem *wdt_base = mtk_wdt->wdt_base; >>>> + u32 reg = ioread32(wdt_base + WDT_MODE); >>>> + >>>> + if (timeout && !wdd->pretimeout) { >>>> + wdd->pretimeout = wdd->timeout / 2; >>>> + reg |= (WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN); >>>> + } else if (!timeout && wdd->pretimeout) { >>>> + wdd->pretimeout = 0; >>>> + reg &= ~(WDT_MODE_IRQ_EN | WDT_MODE_DUAL_EN); >>>> + } else >>>> + return 0; >>>> + >>>> + iowrite32(reg, wdt_base + WDT_MODE); >>> >>> What is the point of setting the mode here ? It will >>> be set again in mtk_wdt_set_timeout(). Seems to me all >>> you need to do here is to set wdd->pretimeout, >>> then call mtk_wdt_set_timeout(). >> >> mtk_wdt_set_timeout() only set timeout and ping(). >> Here also need to config to the dualmode or not. >> >Ah, you are correct. Sorry, I confused this with the start function. >That makes me wonder if it would be better to extract a separate >function, mtk_wdt_set_mode(), for that purpose. Thoughts ? I have done this, but I found there is no good abstract method, because wdt mode is used in combination, for example: WDT_MODE_EN is included in start(), and here is not. And the judgment of pretimeout here is also different. Thanks, Qing > >Thanks, >Guenter > >> Thanks, >> Qing >>> >>> Guenter >>> >>>> + >>>> + return mtk_wdt_set_timeout(wdd, wdd->timeout); >>>> +} >>>> + >>>> +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 +295,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 +318,7 @@ static int mtk_wdt_probe(struct platform_device *pdev) >>>> struct device *dev = &pdev->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 +330,22 @@ 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 = &mtk_wdt_info; >>>> + irq = platform_get_irq(pdev, 0); >>>> + if (irq > 0) { >>>> + err = devm_request_irq(&pdev->dev, irq, mtk_wdt_isr, 0, "wdt_bark", >>>> + &mtk_wdt->wdt_dev); >>>> + if (err) >>>> + return err; >>>> + >>>> + mtk_wdt->wdt_dev.info = &mtk_wdt_pt_info; >>>> + mtk_wdt->wdt_dev.pretimeout = WDT_MAX_TIMEOUT / 2; >>>> + } else { >>>> + if (irq == -EPROBE_DEFER) >>>> + return -EPROBE_DEFER; >>>> + >>>> + mtk_wdt->wdt_dev.info = &mtk_wdt_info; >>>> + } >>>> + >>>> mtk_wdt->wdt_dev.ops = &mtk_wdt_ops; >>>> mtk_wdt->wdt_dev.timeout = WDT_MAX_TIMEOUT; >>>> mtk_wdt->wdt_dev.max_hw_heartbeat_ms = WDT_MAX_TIMEOUT * 1000; >>>> >>> >> >> >