linux-kernel - Re: [PATCH 1/3] watchdog: sprd: should not disable watchdog in resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2691dbe4-0bff-fdb4-0871-60b491d740a4@roeck-us.net>
Date:   Mon, 26 Oct 2020 07:27:31 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Chunyan Zhang <zhang.lyra@...il.com>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>
Cc:     linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org,
        Orson Zhai <orsonzhai@...il.com>,
        Baolin Wang <baolin.wang7@...il.com>,
        Chunyan Zhang <chunyan.zhang@...soc.com>
Subject: Re: [PATCH 1/3] watchdog: sprd: should not disable watchdog in resume

On 10/26/20 1:09 AM, Chunyan Zhang wrote:
> From: Lingling Xu <ling_ling.xu@...soc.com>
> 
> Don't disable watchdog in resume process, otherwise system would crash
> once kick watchdog.
> 

This is a bit misleading: It is only disabled if the attempt to start it
has failed. Was this observed in practice ? If so, it might make sense
to identify and fix the underlying problem instead of trying to work
around it (or is this addressed with the second patch of the series ?)

Anyway, the patch itself is fine.

Reviewed-by: Guenter Roeck <linux@...ck-us.net>

Thanks,
Guenter

> Fixes: 477603467009 ("watchdog: Add Spreadtrum watchdog driver")
> Signed-off-by: Lingling Xu <ling_ling.xu@...soc.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@...soc.com>
> ---
>  drivers/watchdog/sprd_wdt.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/watchdog/sprd_wdt.c b/drivers/watchdog/sprd_wdt.c
> index 65cb55f3916f..f3c90b4afead 100644
> --- a/drivers/watchdog/sprd_wdt.c
> +++ b/drivers/watchdog/sprd_wdt.c
> @@ -345,15 +345,10 @@ static int __maybe_unused sprd_wdt_pm_resume(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	if (watchdog_active(&wdt->wdd)) {
> +	if (watchdog_active(&wdt->wdd))
>  		ret = sprd_wdt_start(&wdt->wdd);
> -		if (ret) {
> -			sprd_wdt_disable(wdt);
> -			return ret;
> -		}
> -	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static const struct dev_pm_ops sprd_wdt_pm_ops = {
>