diff mbox

[v2,2/2] watchdog: add wdt shutdown callback to disable wdt if enabled

Message ID 1437630551-39650-3-git-send-email-eddie.huang@mediatek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eddie Huang (黃智傑) July 23, 2015, 5:49 a.m. UTC 
From: Greta Zhang <greta.zhang@mediatek.com>

Without .shutdown(), watchdog might reset the system during power off.
For example, if watchdog's timeout is set to 30s, then it is reset to
zero by mtk_wdt_ping(). During power off, no app will ping watchdog,
but watchdog is still running and may trigger reset.

Signed-off-by: Greta Zhang <greta.zhang@mediatek.com>
Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
---
 drivers/watchdog/mtk_wdt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Guenter Roeck July 23, 2015, 4:12 p.m. UTC | #1 
On 07/22/2015 10:49 PM, Eddie Huang wrote:
> From: Greta Zhang <greta.zhang@mediatek.com>
>
> Without .shutdown(), watchdog might reset the system during power off.
> For example, if watchdog's timeout is set to 30s, then it is reset to
> zero by mtk_wdt_ping(). During power off, no app will ping watchdog,
> but watchdog is still running and may trigger reset.
>
> Signed-off-by: Greta Zhang <greta.zhang@mediatek.com>
> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>

Except for the subject line,

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

> ---
>   drivers/watchdog/mtk_wdt.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 056412c..6ad9df9 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -210,6 +210,14 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   	return 0;
>   }
>
> +static void mtk_wdt_shutdown(struct platform_device *pdev)
> +{
> +	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
> +
> +	if (watchdog_active(&mtk_wdt->wdt_dev))
> +		mtk_wdt_stop(&mtk_wdt->wdt_dev);
> +}
> +
>   static int mtk_wdt_remove(struct platform_device *pdev)
>   {
>   	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
> @@ -259,6 +267,7 @@ static const struct dev_pm_ops mtk_wdt_pm_ops = {
>   static struct platform_driver mtk_wdt_driver = {
>   	.probe		= mtk_wdt_probe,
>   	.remove		= mtk_wdt_remove,
> +	.shutdown	= mtk_wdt_shutdown,
>   	.driver		= {
>   		.name		= DRV_NAME,
>   		.pm		= &mtk_wdt_pm_ops,
>
Matthias Brugger July 23, 2015, 5:37 p.m. UTC | #2 
On Thursday, July 23, 2015 01:49:11 PM Eddie Huang wrote:
> From: Greta Zhang <greta.zhang@mediatek.com>
> 
> Without .shutdown(), watchdog might reset the system during power off.
> For example, if watchdog's timeout is set to 30s, then it is reset to
> zero by mtk_wdt_ping(). During power off, no app will ping watchdog,
> but watchdog is still running and may trigger reset.
> 
> Signed-off-by: Greta Zhang <greta.zhang@mediatek.com>
> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
> ---
>  drivers/watchdog/mtk_wdt.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
Daniel Kurtz Aug. 11, 2015, 4:28 a.m. UTC | #3 
Hi Guenter, Matthias,

On Fri, Jul 24, 2015 at 1:37 AM, Matthias Brugger
<matthias.bgg@gmail.com> wrote:
> On Thursday, July 23, 2015 01:49:11 PM Eddie Huang wrote:
>> From: Greta Zhang <greta.zhang@mediatek.com>
>>
>> Without .shutdown(), watchdog might reset the system during power off.
>> For example, if watchdog's timeout is set to 30s, then it is reset to
>> zero by mtk_wdt_ping(). During power off, no app will ping watchdog,
>> but watchdog is still running and may trigger reset.
>>
>> Signed-off-by: Greta Zhang <greta.zhang@mediatek.com>
>> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
>> ---
>>  drivers/watchdog/mtk_wdt.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>
> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

is the plan to land these two patches via Guenter's watchdog tree [0]?
[0] git://www.linux-watchdog.org/linux-watchdog.git

In fact, though, I don't see any new patches in that repository.
The only branches I see are:
  remotes/wdog/master                                      b953c0d Linux 4.1
  remotes/wdog/winbond-superio                             dc89871
Sample Winbond Super-I/O MFD device consisting out of a lowel-level
driver that does the detection and creates the platform-data and a
watchdog driver.

Am I looking in the wrong place?

-Dan

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Guenter Roeck Aug. 11, 2015, 4:42 a.m. UTC | #4 
On 08/10/2015 09:28 PM, Daniel Kurtz wrote:
> Hi Guenter, Matthias,
>
> On Fri, Jul 24, 2015 at 1:37 AM, Matthias Brugger
> <matthias.bgg@gmail.com> wrote:
>> On Thursday, July 23, 2015 01:49:11 PM Eddie Huang wrote:
>>> From: Greta Zhang <greta.zhang@mediatek.com>
>>>
>>> Without .shutdown(), watchdog might reset the system during power off.
>>> For example, if watchdog's timeout is set to 30s, then it is reset to
>>> zero by mtk_wdt_ping(). During power off, no app will ping watchdog,
>>> but watchdog is still running and may trigger reset.
>>>
>>> Signed-off-by: Greta Zhang <greta.zhang@mediatek.com>
>>> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
>>> ---
>>>   drivers/watchdog/mtk_wdt.c | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>
>> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
>
> is the plan to land these two patches via Guenter's watchdog tree [0]?
> [0] git://www.linux-watchdog.org/linux-watchdog.git
>
That is Wim's tree, and Wim is the maintainer ;-).

I have the patches queued in my tree [1], and I plan to send a pull request
to Wim this week or early next week.

Guenter

---
[1] https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/log/?h=watchdog-next
Daniel Kurtz Aug. 11, 2015, 7:38 a.m. UTC | #5 
On Tue, Aug 11, 2015 at 12:42 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 08/10/2015 09:28 PM, Daniel Kurtz wrote:
>>
>> Hi Guenter, Matthias,
>>
>> On Fri, Jul 24, 2015 at 1:37 AM, Matthias Brugger
>> <matthias.bgg@gmail.com> wrote:
>>>
>>> On Thursday, July 23, 2015 01:49:11 PM Eddie Huang wrote:
>>>>
>>>> From: Greta Zhang <greta.zhang@mediatek.com>
>>>>
>>>> Without .shutdown(), watchdog might reset the system during power off.
>>>> For example, if watchdog's timeout is set to 30s, then it is reset to
>>>> zero by mtk_wdt_ping(). During power off, no app will ping watchdog,
>>>> but watchdog is still running and may trigger reset.
>>>>
>>>> Signed-off-by: Greta Zhang <greta.zhang@mediatek.com>
>>>> Signed-off-by: Eddie Huang <eddie.huang@mediatek.com>
>>>> ---
>>>>   drivers/watchdog/mtk_wdt.c | 9 +++++++++
>>>>   1 file changed, 9 insertions(+)
>>>
>>>
>>> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>
>>
>>
>> is the plan to land these two patches via Guenter's watchdog tree [0]?
>> [0] git://www.linux-watchdog.org/linux-watchdog.git
>>
> That is Wim's tree, and Wim is the maintainer ;-).

No wonder!  Thanks for cluing me in.

> I have the patches queued in my tree [1], and I plan to send a pull request
> to Wim this week or early next week.

Great!

Thanks,
-Dan

> Guenter
>
> ---
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/groeck/linux-staging.git/log/?h=watchdog-next
>
diff mbox

Patch

diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index 056412c..6ad9df9 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -210,6 +210,14 @@  static int mtk_wdt_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static void mtk_wdt_shutdown(struct platform_device *pdev)
+{
+	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
+
+	if (watchdog_active(&mtk_wdt->wdt_dev))
+		mtk_wdt_stop(&mtk_wdt->wdt_dev);
+}
+
 static int mtk_wdt_remove(struct platform_device *pdev)
 {
 	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
@@ -259,6 +267,7 @@  static const struct dev_pm_ops mtk_wdt_pm_ops = {
 static struct platform_driver mtk_wdt_driver = {
 	.probe		= mtk_wdt_probe,
 	.remove		= mtk_wdt_remove,
+	.shutdown	= mtk_wdt_shutdown,
 	.driver		= {
 		.name		= DRV_NAME,
 		.pm		= &mtk_wdt_pm_ops,