All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Cousson, Benoit" <b-cousson@ti.com>
To: "Varadarajan, Charulatha" <charu@ti.com>
Cc: Tony Lindgren <tony@atomide.com>,
	Kevin Hilman <khilman@deeprootsystems.com>,
	"Menon, Nishanth" <nm@ti.com>, "wim@iguana.be" <wim@iguana.be>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"paul@pwsan.com" <paul@pwsan.com>,
	"Nayak, Rajendra" <rnayak@ti.com>,
	"Basak, Partha" <p-basak2@ti.com>
Subject: Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init
Date: Thu, 30 Sep 2010 18:57:35 +0200	[thread overview]
Message-ID: <4CA4C17F.7060106@ti.com> (raw)
In-Reply-To: <EAF47CD23C76F840A9E7FCE10091EFAB030CCF813E@dbde02.ent.ti.com>

On 9/30/2010 5:55 PM, Varadarajan, Charulatha wrote:
> Tony/ Benoit,
>
>>>
>>> I think that disabling it should be done only if the CONFIG_OMAP_WDT
>>> is not set.
>>
>> How about disabling is done always unless CONFIG_WATCHDOG_NOWAYOUT
>> is set?
>
> As given in the patch description, this patch does a disable of watchdog
> timer, during init, to avoid the system rebooting that happens due to
> enabling of watchdog timer after a reset of the module (during hwmod init).
>
> According to the default WDT registers values, the system reboot would
> happen in ~10s if watchdog is enabled with default values. Hence, after
> a WDT module reset during init, the watchdog has to be disabled within 10s
> otherwise the system will keep rebooting.
>
> Hence irrespective of CONFIG_WATCHDOG_NOWAYOUT/ CONFIG_OMAP_WDT,
> the watchdog timer needs to be disabled after a WDT reset has happened.

No, not necessarily, this is the whole point about a watchdog, you need 
just need to ping it to prove that the system is alive.

In case you didn't notice, every watchdogs are started during a cold 
reset since OMAP1610. Even Phoenix contains a watchdog that is started 
by default.
This is by construction... and this is done like that for a good reason. 
So stopping a watchdog just after the reset in a bootloader is not 
necessarily the behavior that user of a watchdog are expecting, 
otherwise it will not be started by default at boot time.

In your description, it looks like this behavior is a HW bug that we 
have to fix... It is just the way it is supposed to work.

Regards,
Benoit

>
> Later on, the watchdog timer probe would be called (if CONFIG_OMAP_WDT
> is defined) which takes care of doing the regular the watchdog timer
> settings. After this, normal operations like open, close, ioctl operations
> are supported (if CONFIG_OMAP_WDT is defined).
> Based on CONFIG_WATCHDOG_NOWAYOUT definition, disabling the watchdog
> may/may not be supported.
>
> Hence omap2_disable_wdt() introduced in this patch is not going to affect
> the watchdog operations in anyway execpt that it is fixing the reboot issue
> observed due to a watchdog timer reset. And this has to be done irrespective
> of any OMAP watchdog timer related flags.
>
> I guess, omap_wdt_disable() call during the WDT probe might be due to
> similar reasons.
>
>> That way product kernels can set CONFIG_WATCHDOG_NOWAYOUT
>> and for the rest of us we can let fsck run the standard Linux way.
>>
>> Tony


WARNING: multiple messages have this Message-ID (diff)
From: b-cousson@ti.com (Cousson, Benoit)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init
Date: Thu, 30 Sep 2010 18:57:35 +0200	[thread overview]
Message-ID: <4CA4C17F.7060106@ti.com> (raw)
In-Reply-To: <EAF47CD23C76F840A9E7FCE10091EFAB030CCF813E@dbde02.ent.ti.com>

On 9/30/2010 5:55 PM, Varadarajan, Charulatha wrote:
> Tony/ Benoit,
>
>>>
>>> I think that disabling it should be done only if the CONFIG_OMAP_WDT
>>> is not set.
>>
>> How about disabling is done always unless CONFIG_WATCHDOG_NOWAYOUT
>> is set?
>
> As given in the patch description, this patch does a disable of watchdog
> timer, during init, to avoid the system rebooting that happens due to
> enabling of watchdog timer after a reset of the module (during hwmod init).
>
> According to the default WDT registers values, the system reboot would
> happen in ~10s if watchdog is enabled with default values. Hence, after
> a WDT module reset during init, the watchdog has to be disabled within 10s
> otherwise the system will keep rebooting.
>
> Hence irrespective of CONFIG_WATCHDOG_NOWAYOUT/ CONFIG_OMAP_WDT,
> the watchdog timer needs to be disabled after a WDT reset has happened.

No, not necessarily, this is the whole point about a watchdog, you need 
just need to ping it to prove that the system is alive.

In case you didn't notice, every watchdogs are started during a cold 
reset since OMAP1610. Even Phoenix contains a watchdog that is started 
by default.
This is by construction... and this is done like that for a good reason. 
So stopping a watchdog just after the reset in a bootloader is not 
necessarily the behavior that user of a watchdog are expecting, 
otherwise it will not be started by default at boot time.

In your description, it looks like this behavior is a HW bug that we 
have to fix... It is just the way it is supposed to work.

Regards,
Benoit

>
> Later on, the watchdog timer probe would be called (if CONFIG_OMAP_WDT
> is defined) which takes care of doing the regular the watchdog timer
> settings. After this, normal operations like open, close, ioctl operations
> are supported (if CONFIG_OMAP_WDT is defined).
> Based on CONFIG_WATCHDOG_NOWAYOUT definition, disabling the watchdog
> may/may not be supported.
>
> Hence omap2_disable_wdt() introduced in this patch is not going to affect
> the watchdog operations in anyway execpt that it is fixing the reboot issue
> observed due to a watchdog timer reset. And this has to be done irrespective
> of any OMAP watchdog timer related flags.
>
> I guess, omap_wdt_disable() call during the WDT probe might be due to
> similar reasons.
>
>> That way product kernels can set CONFIG_WATCHDOG_NOWAYOUT
>> and for the rest of us we can let fsck run the standard Linux way.
>>
>> Tony

  parent reply	other threads:[~2010-09-30 16:58 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-30  8:11 [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init Varadarajan, Charulatha
2010-09-30  8:11 ` Varadarajan, Charulatha
2010-09-30  9:07 ` Cousson, Benoit
2010-09-30  9:07   ` Cousson, Benoit
2010-09-30 13:55   ` Kevin Hilman
2010-09-30 13:55     ` Kevin Hilman
2010-09-30 14:12     ` Cousson, Benoit
2010-09-30 14:12       ` Cousson, Benoit
2010-09-30 14:51       ` Kevin Hilman
2010-09-30 14:51         ` Kevin Hilman
2010-09-30 15:07       ` Tony Lindgren
2010-09-30 15:07         ` Tony Lindgren
2010-09-30 15:55         ` Varadarajan, Charulatha
2010-09-30 15:55           ` Varadarajan, Charulatha
2010-09-30 16:32           ` Varadarajan, Charulatha
2010-09-30 16:32             ` Varadarajan, Charulatha
2010-09-30 16:43             ` Paul Walmsley
2010-09-30 16:43               ` Paul Walmsley
2010-09-30 16:51               ` Tony Lindgren
2010-09-30 16:51                 ` Tony Lindgren
2010-09-30 16:46             ` Cousson, Benoit
2010-09-30 16:46               ` Cousson, Benoit
2010-09-30 16:57           ` Cousson, Benoit [this message]
2010-09-30 16:57             ` Cousson, Benoit
2010-09-30 17:06             ` Varadarajan, Charulatha
2010-09-30 17:06               ` Varadarajan, Charulatha
2010-09-30 17:05           ` Shilimkar, Santosh
2010-09-30 17:05             ` Shilimkar, Santosh
2010-09-30 17:11             ` Kevin Hilman
2010-09-30 17:11               ` Kevin Hilman
2010-10-01  7:26               ` Shilimkar, Santosh
2010-10-01  7:26                 ` Shilimkar, Santosh
2010-10-01 13:33                 ` Varadarajan, Charulatha
2010-10-01 13:33                   ` Varadarajan, Charulatha
2010-10-01 14:43                   ` Kevin Hilman
2010-10-01 14:43                     ` Kevin Hilman
2010-10-01 17:12                     ` Cousson, Benoit
2010-10-01 17:12                       ` Cousson, Benoit
2010-09-30 13:57 ` Kevin Hilman
2010-09-30 13:57   ` Kevin Hilman
2010-09-30 13:57   ` Kevin Hilman
2010-09-30 16:36   ` Varadarajan, Charulatha
2010-09-30 16:36     ` Varadarajan, Charulatha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CA4C17F.7060106@ti.com \
    --to=b-cousson@ti.com \
    --cc=charu@ti.com \
    --cc=khilman@deeprootsystems.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=p-basak2@ti.com \
    --cc=paul@pwsan.com \
    --cc=rnayak@ti.com \
    --cc=tony@atomide.com \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.