From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751463AbdL2QKY (ORCPT ); Fri, 29 Dec 2017 11:10:24 -0500 Received: from mail-pl0-f67.google.com ([209.85.160.67]:32986 "EHLO mail-pl0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750869AbdL2QKW (ORCPT ); Fri, 29 Dec 2017 11:10:22 -0500 X-Google-Smtp-Source: ACJfBotw9x/VVZuMpVbH8kfDhO49rIznND/80kVOOyZWXOu717I/U9mj7LuDlo8Ds8pUU1ow/wR60A== Subject: Re: [PATCH] watchdog: imx2_wdt: restore previous timeout after suspend+resume To: Martin Kaiser , Wim Van Sebroeck , linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Xiubo Li , kernel@pengutronix.de, Fabio Estevam , stable@vger.kernel.org References: <1514555950-28743-1-git-send-email-martin@kaiser.cx> From: Guenter Roeck Message-ID: <080e499c-a31b-e805-f761-6564e02d1be9@roeck-us.net> Date: Fri, 29 Dec 2017 08:10:19 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <1514555950-28743-1-git-send-email-martin@kaiser.cx> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/29/2017 05:59 AM, Martin Kaiser wrote: > When the watchdog device is suspended, its timeout is set to the maximum > value. During resume, the previously set timeout should be restored. > This does not work at the moment. > > The suspend function calls > > imx2_wdt_set_timeout(wdog, IMX2_WDT_MAX_TIME); > > and resume reverts this by calling > > imx2_wdt_set_timeout(wdog, wdog->timeout); > > However, imx2_wdt_set_timeout() updates wdog->timeout. Therefore, > wdog->timeout is set to IMX2_WDT_MAX_TIME when we enter the resume > function. > > Fix this by setting wdog->timeout to the previous value at the end of > the suspend function. This manual update makes wdog->timeout different > from the actual setting in the hardware. This should be ok in our case > as kernel code is not running while we're suspended. > > Signed-off-by: Martin Kaiser > Cc: stable@vger.kernel.org That works, though it is a bit hackish. Better would be to have a function such as _imx2_wdt_set_timeout() which doesn't actually update wdt->timeout and could be called from all the other places doing the same. Separate patch, maybe. Reviewed-by: Guenter Roeck > --- > drivers/watchdog/imx2_wdt.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c > index 4874b0f..66efcc0 100644 > --- a/drivers/watchdog/imx2_wdt.c > +++ b/drivers/watchdog/imx2_wdt.c > @@ -368,6 +368,7 @@ static int imx2_wdt_suspend(struct device *dev) > { > struct watchdog_device *wdog = dev_get_drvdata(dev); > struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog); > + unsigned int resume_timeout = wdog->timeout; > > /* The watchdog IP block is running */ > if (imx2_wdt_is_running(wdev)) { > @@ -377,6 +378,7 @@ static int imx2_wdt_suspend(struct device *dev) > > clk_disable_unprepare(wdev->clk); > > + wdog->timeout = resume_timeout; > return 0; > } > >