[PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Varadarajan, Charulatha]

With OMAP2PLUS watchdog implemented in hwmod fw way, the
module is reset during init.

After a watchdog timer module reset, the WDTs are enabled. The
default time for a system reset after a watchdog module reset
is ~10s as per the default value of the WDT registers. Hence
the system would be reset after 10s, if watchdog is not disabled
within 10s.

This patch fixes the above issue by disabling the watchdog timer
after reset during initialization of devices.

Signed-off-by: Charulatha V <charu@ti.com>
Reported-by: Kevin Hilman <khilman@deeprootsystems.com>
---
This patch is dependent on the below patch series (wdt hwmod) and
is created on top of pm-core branch.
http://www.spinics.net/lists/linux-omap/msg37043.html

 arch/arm/mach-omap2/devices.c |   44 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 8e2f0aa..9f44fc6 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -916,11 +916,55 @@ static inline void omap_init_vout(void) {}
 
 /*-------------------------------------------------------------------------*/
 
+/*
+ * WDT mdoule is reset during init which enables the watchdog.
+ * Hence it is required to disable the watchdog after the WDT reset
+ * during init. Otherwise the system would reboot as per the default
+ * watchdog timer registers settings.
+ */
+#define OMAP_WDT_WPS	(0x34)
+#define OMAP_WDT_SPR	(0x48)
+
+static int omap2_disable_wdt(struct omap_hwmod *oh, void *user)
+{
+	void __iomem *base;
+
+	if (!oh)
+		pr_err("Could not look up wdtimer_hwmod\n");
+
+	base = oh->_mpu_rt_va;
+
+	/* Enable the clocks before accessing the WDT registers */
+	omap_hwmod_enable(oh);
+
+	/* sequence required to disable watchdog */
+	__raw_writel(0xAAAA, base + OMAP_WDT_SPR);	/* TIMER_MODE */
+	while (__raw_readl(base + OMAP_WDT_WPS) & 0x10)
+		cpu_relax();
+
+	__raw_writel(0x5555, base + OMAP_WDT_SPR);	/* TIMER_MODE */
+	while (__raw_readl(base + OMAP_WDT_WPS) & 0x10)
+		cpu_relax();
+
+	omap_hwmod_idle(oh);
+
+	return 0;
+}
+
+static void __init omap_disable_wdt(void)
+{
+	if (cpu_class_is_omap2())
+		omap_hwmod_for_each_by_class("wd_timer",
+						omap2_disable_wdt, NULL);
+	return;
+}
+
 static int __init omap2_init_devices(void)
 {
 	/* please keep these calls, and their implementations above,
 	 * in alphabetical order so they're easier to sort through.
 	 */
+	omap_disable_wdt();
 	omap_hsmmc_reset();
 	omap_init_camera();
 	omap_init_mbox();
-- 
1.7.0.4

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Cousson, Benoit]

Hi Charu,

On 9/30/2010 10:11 AM, Varadarajan, Charulatha wrote:
> With OMAP2PLUS watchdog implemented in hwmod fw way, the
> module is reset during init.

In that case hwmod fw just highlighted the real behavior that was hidden 
so far by the X-loader.

You should as well add a link to the email thread with Kevin that raised 
the issue.

> After a watchdog timer module reset, the WDTs are enabled. The
> default time for a system reset after a watchdog module reset
> is ~10s as per the default value of the WDT registers. Hence
> the system would be reset after 10s, if watchdog is not disabled
> within 10s.
>
> This patch fixes the above issue by disabling the watchdog timer
> after reset during initialization of devices.

I'm still wondering as well what is the expected behavior of the 
watchdog in a real product. If it is started by default at boot time, 
this is probably for a good reason (or maybe not...).
So, disabling it all the time is maybe not the best solution.

Nishanth (M),
Do you have an idea on that topic?

>
> Signed-off-by: Charulatha V<charu@ti.com>
> Reported-by: Kevin Hilman<khilman@deeprootsystems.com>
> ---
> This patch is dependent on the below patch series (wdt hwmod) and
> is created on top of pm-core branch.
> http://www.spinics.net/lists/linux-omap/msg37043.html
>
>   arch/arm/mach-omap2/devices.c |   44 +++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 44 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 8e2f0aa..9f44fc6 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -916,11 +916,55 @@ static inline void omap_init_vout(void) {}
>
>   /*-------------------------------------------------------------------------*/
>
> +/*
> + * WDT mdoule is reset during init which enables the watchdog.

typo: module

The real explanation is that you should not assume anything from the 
boot loader at that time, so always stop the wdt.

> + * Hence it is required to disable the watchdog after the WDT reset
> + * during init. Otherwise the system would reboot as per the default
> + * watchdog timer registers settings.
> + */
> +#define OMAP_WDT_WPS	(0x34)
> +#define OMAP_WDT_SPR	(0x48)
> +
> +static int omap2_disable_wdt(struct omap_hwmod *oh, void *user)

You should call it unused if the parameter is not used.

> +{
> +	void __iomem *base;
> +
> +	if (!oh)
> +		pr_err("Could not look up wdtimer_hwmod\n");
> +
> +	base = oh->_mpu_rt_va;

Paul added an hwmod API to get that va (something like *_get_mpu_rt_va).

> +
> +	/* Enable the clocks before accessing the WDT registers */
> +	omap_hwmod_enable(oh);

The enable can fail, so you should check the return value.

> +
> +	/* sequence required to disable watchdog */
> +	__raw_writel(0xAAAA, base + OMAP_WDT_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WDT_WPS)&  0x10)
> +		cpu_relax();
> +
> +	__raw_writel(0x5555, base + OMAP_WDT_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WDT_WPS)&  0x10)
> +		cpu_relax();
> +
> +	omap_hwmod_idle(oh);
> +
> +	return 0;
> +}
> +
> +static void __init omap_disable_wdt(void)
> +{
> +	if (cpu_class_is_omap2())

This code is already in mach-omap2/devices.c, so that test should be 
useless.

Regards,
Benoit

> +		omap_hwmod_for_each_by_class("wd_timer",
> +						omap2_disable_wdt, NULL);
> +	return;
> +}
> +
>   static int __init omap2_init_devices(void)
>   {
>   	/* please keep these calls, and their implementations above,
>   	 * in alphabetical order so they're easier to sort through.
>   	 */
> +	omap_disable_wdt();
>   	omap_hsmmc_reset();
>   	omap_init_camera();
>   	omap_init_mbox();

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Kevin Hilman]

"Cousson, Benoit" <b-cousson@ti.com> writes:

> Hi Charu,
>
> On 9/30/2010 10:11 AM, Varadarajan, Charulatha wrote:
>> With OMAP2PLUS watchdog implemented in hwmod fw way, the
>> module is reset during init.
>
> In that case hwmod fw just highlighted the real behavior that was
> hidden so far by the X-loader.
>
> You should as well add a link to the email thread with Kevin that
> raised the issue.
>
>> After a watchdog timer module reset, the WDTs are enabled. The
>> default time for a system reset after a watchdog module reset
>> is ~10s as per the default value of the WDT registers. Hence
>> the system would be reset after 10s, if watchdog is not disabled
>> within 10s.
>>
>> This patch fixes the above issue by disabling the watchdog timer
>> after reset during initialization of devices.
>
> I'm still wondering as well what is the expected behavior of the
> watchdog in a real product. If it is started by default at boot time,
> this is probably for a good reason (or maybe not...).
>
> So, disabling it all the time is maybe not the best solution.

I'm not sure what the other options are.  If you don't have a watchdog
driver, and the watchdog is armed, it will reboot the system.

The approach in this patch is just to continue the behavior that all
bootloaders currently do, but make it explicit in the kernel.

Kevin

> Nishanth (M),
> Do you have an idea on that topic?
>
>>
>> Signed-off-by: Charulatha V<charu@ti.com>
>> Reported-by: Kevin Hilman<khilman@deeprootsystems.com>
>> ---
>> This patch is dependent on the below patch series (wdt hwmod) and
>> is created on top of pm-core branch.
>> http://www.spinics.net/lists/linux-omap/msg37043.html
>>
>>   arch/arm/mach-omap2/devices.c |   44 +++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 44 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
>> index 8e2f0aa..9f44fc6 100644
>> --- a/arch/arm/mach-omap2/devices.c
>> +++ b/arch/arm/mach-omap2/devices.c
>> @@ -916,11 +916,55 @@ static inline void omap_init_vout(void) {}
>>
>>   /*-------------------------------------------------------------------------*/
>>
>> +/*
>> + * WDT mdoule is reset during init which enables the watchdog.
>
> typo: module
>
> The real explanation is that you should not assume anything from the
> boot loader at that time, so always stop the wdt.
>
>> + * Hence it is required to disable the watchdog after the WDT reset
>> + * during init. Otherwise the system would reboot as per the default
>> + * watchdog timer registers settings.
>> + */
>> +#define OMAP_WDT_WPS	(0x34)
>> +#define OMAP_WDT_SPR	(0x48)
>> +
>> +static int omap2_disable_wdt(struct omap_hwmod *oh, void *user)
>
> You should call it unused if the parameter is not used.
>
>> +{
>> +	void __iomem *base;
>> +
>> +	if (!oh)
>> +		pr_err("Could not look up wdtimer_hwmod\n");
>> +
>> +	base = oh->_mpu_rt_va;
>
> Paul added an hwmod API to get that va (something like *_get_mpu_rt_va).
>
>> +
>> +	/* Enable the clocks before accessing the WDT registers */
>> +	omap_hwmod_enable(oh);
>
> The enable can fail, so you should check the return value.
>
>> +
>> +	/* sequence required to disable watchdog */
>> +	__raw_writel(0xAAAA, base + OMAP_WDT_SPR);	/* TIMER_MODE */
>> +	while (__raw_readl(base + OMAP_WDT_WPS)&  0x10)
>> +		cpu_relax();
>> +
>> +	__raw_writel(0x5555, base + OMAP_WDT_SPR);	/* TIMER_MODE */
>> +	while (__raw_readl(base + OMAP_WDT_WPS)&  0x10)
>> +		cpu_relax();
>> +
>> +	omap_hwmod_idle(oh);
>> +
>> +	return 0;
>> +}
>> +
>> +static void __init omap_disable_wdt(void)
>> +{
>> +	if (cpu_class_is_omap2())
>
> This code is already in mach-omap2/devices.c, so that test should be
> useless.
>
> Regards,
> Benoit
>
>> +		omap_hwmod_for_each_by_class("wd_timer",
>> +						omap2_disable_wdt, NULL);
>> +	return;
>> +}
>> +
>>   static int __init omap2_init_devices(void)
>>   {
>>   	/* please keep these calls, and their implementations above,
>>   	 * in alphabetical order so they're easier to sort through.
>>   	 */
>> +	omap_disable_wdt();
>>   	omap_hsmmc_reset();
>>   	omap_init_camera();
>>   	omap_init_mbox();

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Cousson, Benoit]

On 9/30/2010 3:55 PM, Kevin Hilman wrote:
> "Cousson, Benoit"<b-cousson@ti.com>  writes:
>
>> Hi Charu,
>>
>> On 9/30/2010 10:11 AM, Varadarajan, Charulatha wrote:
>>> With OMAP2PLUS watchdog implemented in hwmod fw way, the
>>> module is reset during init.
>>
>> In that case hwmod fw just highlighted the real behavior that was
>> hidden so far by the X-loader.
>>
>> You should as well add a link to the email thread with Kevin that
>> raised the issue.
>>
>>> After a watchdog timer module reset, the WDTs are enabled. The
>>> default time for a system reset after a watchdog module reset
>>> is ~10s as per the default value of the WDT registers. Hence
>>> the system would be reset after 10s, if watchdog is not disabled
>>> within 10s.
>>>
>>> This patch fixes the above issue by disabling the watchdog timer
>>> after reset during initialization of devices.
>>
>> I'm still wondering as well what is the expected behavior of the
>> watchdog in a real product. If it is started by default at boot time,
>> this is probably for a good reason (or maybe not...).
>>
>> So, disabling it all the time is maybe not the best solution.
>
> I'm not sure what the other options are.  If you don't have a watchdog
> driver, and the watchdog is armed, it will reboot the system.
>
> The approach in this patch is just to continue the behavior that all
> bootloaders currently do, but make it explicit in the kernel.

Yes, because we are not a building a product, and for us watchdog is a 
pain. But I'm not sure that a real product will disable that at all 
during the boot process.

I think that disabling it should be done only if the CONFIG_OMAP_WDT is 
not set.

But since I don't have a clue about a product can use that, it will be 
good to have such inputs to understand the usecase.

Benoit


> Kevin
>
>> Nishanth (M),
>> Do you have an idea on that topic?
>>
>>>
>>> Signed-off-by: Charulatha V<charu@ti.com>
>>> Reported-by: Kevin Hilman<khilman@deeprootsystems.com>
>>> ---
>>> This patch is dependent on the below patch series (wdt hwmod) and
>>> is created on top of pm-core branch.
>>> http://www.spinics.net/lists/linux-omap/msg37043.html
>>>
>>>    arch/arm/mach-omap2/devices.c |   44 +++++++++++++++++++++++++++++++++++++++++
>>>    1 files changed, 44 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
>>> index 8e2f0aa..9f44fc6 100644
>>> --- a/arch/arm/mach-omap2/devices.c
>>> +++ b/arch/arm/mach-omap2/devices.c
>>> @@ -916,11 +916,55 @@ static inline void omap_init_vout(void) {}
>>>
>>>    /*-------------------------------------------------------------------------*/
>>>
>>> +/*
>>> + * WDT mdoule is reset during init which enables the watchdog.
>>
>> typo: module
>>
>> The real explanation is that you should not assume anything from the
>> boot loader at that time, so always stop the wdt.
>>
>>> + * Hence it is required to disable the watchdog after the WDT reset
>>> + * during init. Otherwise the system would reboot as per the default
>>> + * watchdog timer registers settings.
>>> + */
>>> +#define OMAP_WDT_WPS	(0x34)
>>> +#define OMAP_WDT_SPR	(0x48)
>>> +
>>> +static int omap2_disable_wdt(struct omap_hwmod *oh, void *user)
>>
>> You should call it unused if the parameter is not used.
>>
>>> +{
>>> +	void __iomem *base;
>>> +
>>> +	if (!oh)
>>> +		pr_err("Could not look up wdtimer_hwmod\n");
>>> +
>>> +	base = oh->_mpu_rt_va;
>>
>> Paul added an hwmod API to get that va (something like *_get_mpu_rt_va).
>>
>>> +
>>> +	/* Enable the clocks before accessing the WDT registers */
>>> +	omap_hwmod_enable(oh);
>>
>> The enable can fail, so you should check the return value.
>>
>>> +
>>> +	/* sequence required to disable watchdog */
>>> +	__raw_writel(0xAAAA, base + OMAP_WDT_SPR);	/* TIMER_MODE */
>>> +	while (__raw_readl(base + OMAP_WDT_WPS)&   0x10)
>>> +		cpu_relax();
>>> +
>>> +	__raw_writel(0x5555, base + OMAP_WDT_SPR);	/* TIMER_MODE */
>>> +	while (__raw_readl(base + OMAP_WDT_WPS)&   0x10)
>>> +		cpu_relax();
>>> +
>>> +	omap_hwmod_idle(oh);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void __init omap_disable_wdt(void)
>>> +{
>>> +	if (cpu_class_is_omap2())
>>
>> This code is already in mach-omap2/devices.c, so that test should be
>> useless.
>>
>> Regards,
>> Benoit
>>
>>> +		omap_hwmod_for_each_by_class("wd_timer",
>>> +						omap2_disable_wdt, NULL);
>>> +	return;
>>> +}
>>> +
>>>    static int __init omap2_init_devices(void)
>>>    {
>>>    	/* please keep these calls, and their implementations above,
>>>    	 * in alphabetical order so they're easier to sort through.
>>>    	 */
>>> +	omap_disable_wdt();
>>>    	omap_hsmmc_reset();
>>>    	omap_init_camera();
>>>    	omap_init_mbox();

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Kevin Hilman]

"Cousson, Benoit" <b-cousson@ti.com> writes:

> On 9/30/2010 3:55 PM, Kevin Hilman wrote:
>> "Cousson, Benoit"<b-cousson@ti.com>  writes:
>>
>>> Hi Charu,
>>>
>>> On 9/30/2010 10:11 AM, Varadarajan, Charulatha wrote:
>>>> With OMAP2PLUS watchdog implemented in hwmod fw way, the
>>>> module is reset during init.
>>>
>>> In that case hwmod fw just highlighted the real behavior that was
>>> hidden so far by the X-loader.
>>>
>>> You should as well add a link to the email thread with Kevin that
>>> raised the issue.
>>>
>>>> After a watchdog timer module reset, the WDTs are enabled. The
>>>> default time for a system reset after a watchdog module reset
>>>> is ~10s as per the default value of the WDT registers. Hence
>>>> the system would be reset after 10s, if watchdog is not disabled
>>>> within 10s.
>>>>
>>>> This patch fixes the above issue by disabling the watchdog timer
>>>> after reset during initialization of devices.
>>>
>>> I'm still wondering as well what is the expected behavior of the
>>> watchdog in a real product. If it is started by default at boot time,
>>> this is probably for a good reason (or maybe not...).
>>>
>>> So, disabling it all the time is maybe not the best solution.
>>
>> I'm not sure what the other options are.  If you don't have a watchdog
>> driver, and the watchdog is armed, it will reboot the system.
>>
>> The approach in this patch is just to continue the behavior that all
>> bootloaders currently do, but make it explicit in the kernel.
>
> Yes, because we are not a building a product, and for us watchdog is a
> pain. But I'm not sure that a real product will disable that at all
> during the boot process.

Maybe not.  But wouldn't a product just ensure the real watchdog
driver is loaded?

Or more likely, in addition to the hundreds of other out-of-tree
patches, they would just remove $SUBJECT patch.  ;)

> I think that disabling it should be done only if the CONFIG_OMAP_WDT
> is not set.

That's not easy either, as the watchdog driver can be built as a module,
and may (or may not) be loaded some unknown time from the kernel boot,
resulting in a likely

Kevin

> But since I don't have a clue about a product can use that, it will be
> good to have such inputs to understand the usecase.

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Tony Lindgren]

* Cousson, Benoit <b-cousson@ti.com> [100930 07:04]:
> 
> 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? 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

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Varadarajan, Charulatha]

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.

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

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Varadarajan, Charulatha]

Benoit,

<<snip>>

> > With OMAP2PLUS watchdog implemented in hwmod fw way, the
> > module is reset during init.
> 
> In that case hwmod fw just highlighted the real behavior that was hidden 
> so far by the X-loader.

Yes.

> 
> You should as well add a link to the email thread with Kevin that raised 
> the issue.

Okay.

<<snip>>

> > diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> > index 8e2f0aa..9f44fc6 100644
> > --- a/arch/arm/mach-omap2/devices.c
> > +++ b/arch/arm/mach-omap2/devices.c
> > @@ -916,11 +916,55 @@ static inline void omap_init_vout(void) {}
> >
> >   /*-------------------------------------------------------------------------*/
> >
> > +/*
> > + * WDT mdoule is reset during init which enables the watchdog.
> 
> typo: module

Okay will correct it.

> 
> The real explanation is that you should not assume anything from the 
> boot loader at that time, so always stop the wdt.

It is not an assumption from bootloader. After the WDT reset, the 
WDT is enabled and the reset values of WDT registers makes the system to
reboot (in ~10s) as WDT is enabled.

> 
> > + * Hence it is required to disable the watchdog after the WDT reset
> > + * during init. Otherwise the system would reboot as per the default
> > + * watchdog timer registers settings.
> > + */
> > +#define OMAP_WDT_WPS	(0x34)
> > +#define OMAP_WDT_SPR	(0x48)
> > +
> > +static int omap2_disable_wdt(struct omap_hwmod *oh, void *user)
> 
> You should call it unused if the parameter is not used.

Okay.

> 
> > +{
> > +	void __iomem *base;
> > +
> > +	if (!oh)
> > +		pr_err("Could not look up wdtimer_hwmod\n");
> > +
> > +	base = oh->_mpu_rt_va;
> 
> Paul added an hwmod API to get that va (something like *_get_mpu_rt_va).

Okay. Will use that.

> 
> > +
> > +	/* Enable the clocks before accessing the WDT registers */
> > +	omap_hwmod_enable(oh);
> 
> The enable can fail, so you should check the return value.

Okay.

> 
> > +
> > +	/* sequence required to disable watchdog */
> > +	__raw_writel(0xAAAA, base + OMAP_WDT_SPR);	/* TIMER_MODE */
> > +	while (__raw_readl(base + OMAP_WDT_WPS)&  0x10)
> > +		cpu_relax();
> > +
> > +	__raw_writel(0x5555, base + OMAP_WDT_SPR);	/* TIMER_MODE */
> > +	while (__raw_readl(base + OMAP_WDT_WPS)&  0x10)
> > +		cpu_relax();
> > +
> > +	omap_hwmod_idle(oh);
> > +
> > +	return 0;
> > +}
> > +
> > +static void __init omap_disable_wdt(void)
> > +{
> > +	if (cpu_class_is_omap2())
> 
> This code is already in mach-omap2/devices.c, so that test should be 
> useless.

I do not see a cpu_class_is_omap2() check in omap2_init_devices(). Please
point out where this check is done while/before calling omap_disable_wdt()?

> 
> 

<<snip>>

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Paul Walmsley]

Hello Charu,

On Thu, 30 Sep 2010, Varadarajan, Charulatha wrote:

> > > diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> > > index 8e2f0aa..9f44fc6 100644
> > > --- a/arch/arm/mach-omap2/devices.c
> > > +++ b/arch/arm/mach-omap2/devices.c
> > > +
> > > +static void __init omap_disable_wdt(void)
> > > +{
> > > +	if (cpu_class_is_omap2())
> > 
> > This code is already in mach-omap2/devices.c, so that test should be 
> > useless.
> 
> I do not see a cpu_class_is_omap2() check in omap2_init_devices(). Please
> point out where this check is done while/before calling omap_disable_wdt()?

It's implicit, due to the directory arch/arm/mach-omap2/ -- code in that 
directory is only built for OMAP2+ systems -- and right now there are no 
plans for OMAP1+ multi-arch booting.  So it's safe to assume that any code 
in arch/arm/mach-omap2 will only run on OMAP2+ boards.


- Paul

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [100930 09:34]:
> Hello Charu,
> 
> On Thu, 30 Sep 2010, Varadarajan, Charulatha wrote:
> 
> > > > diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> > > > index 8e2f0aa..9f44fc6 100644
> > > > --- a/arch/arm/mach-omap2/devices.c
> > > > +++ b/arch/arm/mach-omap2/devices.c
> > > > +
> > > > +static void __init omap_disable_wdt(void)
> > > > +{
> > > > +	if (cpu_class_is_omap2())
> > > 
> > > This code is already in mach-omap2/devices.c, so that test should be 
> > > useless.
> > 
> > I do not see a cpu_class_is_omap2() check in omap2_init_devices(). Please
> > point out where this check is done while/before calling omap_disable_wdt()?
> 
> It's implicit, due to the directory arch/arm/mach-omap2/ -- code in that 
> directory is only built for OMAP2+ systems -- and right now there are no 
> plans for OMAP1+ multi-arch booting.  So it's safe to assume that any code 
> in arch/arm/mach-omap2 will only run on OMAP2+ boards.

That might change pretty fast though. There are already experimental patches
to build in multiple ARM archs into a single kernel binary.

So we should already have these checks in place to avoid the initcalls running
on other ARM archs.

Regards,

Tony

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Cousson, Benoit]

On 9/30/2010 6:32 PM, Varadarajan, Charulatha wrote:
> Benoit,
>
> <<snip>>
>
>>> With OMAP2PLUS watchdog implemented in hwmod fw way, the
>>> module is reset during init.
>>
>> In that case hwmod fw just highlighted the real behavior that was hidden
>> so far by the X-loader.
>
> Yes.
>
>>
>> You should as well add a link to the email thread with Kevin that raised
>> the issue.
>
> Okay.
>
> <<snip>>
>
>>> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
>>> index 8e2f0aa..9f44fc6 100644
>>> --- a/arch/arm/mach-omap2/devices.c
>>> +++ b/arch/arm/mach-omap2/devices.c
>>> @@ -916,11 +916,55 @@ static inline void omap_init_vout(void) {}
>>>
>>>    /*-------------------------------------------------------------------------*/
>>>
>>> +/*
>>> + * WDT mdoule is reset during init which enables the watchdog.
>>
>> typo: module
>
> Okay will correct it.
>
>>
>> The real explanation is that you should not assume anything from the
>> boot loader at that time, so always stop the wdt.
>
> It is not an assumption from bootloader. After the WDT reset, the
> WDT is enabled and the reset values of WDT registers makes the system to
> reboot (in ~10s) as WDT is enabled.

The implicit assumption in the previous code is that the bootloader 
already stopped it. That exactly for that reason that hwmod is reseting 
every IPs, because we don't have a clue about what X-loader / u-boot can 
do between a cold-reset and the kernel boot.

>
>>
>>> + * Hence it is required to disable the watchdog after the WDT reset
>>> + * during init. Otherwise the system would reboot as per the default
>>> + * watchdog timer registers settings.
>>> + */
>>> +#define OMAP_WDT_WPS	(0x34)
>>> +#define OMAP_WDT_SPR	(0x48)
>>> +
>>> +static int omap2_disable_wdt(struct omap_hwmod *oh, void *user)
>>
>> You should call it unused if the parameter is not used.
>
> Okay.
>
>>
>>> +{
>>> +	void __iomem *base;
>>> +
>>> +	if (!oh)
>>> +		pr_err("Could not look up wdtimer_hwmod\n");
>>> +
>>> +	base = oh->_mpu_rt_va;
>>
>> Paul added an hwmod API to get that va (something like *_get_mpu_rt_va).
>
> Okay. Will use that.
>
>>
>>> +
>>> +	/* Enable the clocks before accessing the WDT registers */
>>> +	omap_hwmod_enable(oh);
>>
>> The enable can fail, so you should check the return value.
>
> Okay.
>
>>
>>> +
>>> +	/* sequence required to disable watchdog */
>>> +	__raw_writel(0xAAAA, base + OMAP_WDT_SPR);	/* TIMER_MODE */
>>> +	while (__raw_readl(base + OMAP_WDT_WPS)&   0x10)
>>> +		cpu_relax();
>>> +
>>> +	__raw_writel(0x5555, base + OMAP_WDT_SPR);	/* TIMER_MODE */
>>> +	while (__raw_readl(base + OMAP_WDT_WPS)&   0x10)
>>> +		cpu_relax();
>>> +
>>> +	omap_hwmod_idle(oh);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void __init omap_disable_wdt(void)
>>> +{
>>> +	if (cpu_class_is_omap2())
>>
>> This code is already in mach-omap2/devices.c, so that test should be
>> useless.
>
> I do not see a cpu_class_is_omap2() check in omap2_init_devices(). Please
> point out where this check is done while/before calling omap_disable_wdt()?
>

And Paul has just answered that one...

Benoit

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Cousson, Benoit]

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

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Varadarajan, Charulatha]

>> 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.
> 

Agreed

> 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.

Yes.

> 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.

But, how do we handle this? enable INIT_NO_RESET flag?

>
> In your description, it looks like this behavior is a HW bug that we
> have to fix...

Sorry if it sounded like that. 

> It is just the way it is supposed to work.

Yes. That's correct

Regards,
Benoit

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Shilimkar, Santosh]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Varadarajan, Charulatha
> Sent: Thursday, September 30, 2010 9:25 PM
> To: Tony Lindgren; Cousson, Benoit
> Cc: Kevin Hilman; Menon, Nishanth; wim@iguana.be; linux-
> omap@vger.kernel.org; linux-watchdog@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; paul@pwsan.com; Nayak, Rajendra; Basak, Partha
> Subject: RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during
> init
> 
> 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.
> 
One more option is to avoid the software reset using the CONFIG_OMAP_WDT
flag. Something like below.

--------------
static struct omap_hwmod omap44xx_wd_timer2_hwmod = {
	.name		= "wd_timer2",
	.class		= &omap44xx_wd_timer_hwmod_class,
	.mpu_irqs	= omap44xx_wd_timer2_irqs,

+ #ifndef CONFIG_OMAP_WDT,
+ .flags		= HWMOD_INIT_NO_RESET,
+ #endif
	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_wd_timer2_irqs),
	.main_clk	= "wd_timer2_fck",
	.prcm = {
		.omap4 = {
			.clkctrl_reg = OMAP4430_CM_WKUP_WDT2_CLKCTRL,
		},
	},
	.slaves		= omap44xx_wd_timer2_slaves,
	.slaves_cnt	= ARRAY_SIZE(omap44xx_wd_timer2_slaves),
	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
};

Regards,
Santosh

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Kevin Hilman]

"Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:

>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Varadarajan, Charulatha
>> Sent: Thursday, September 30, 2010 9:25 PM
>> To: Tony Lindgren; Cousson, Benoit
>> Cc: Kevin Hilman; Menon, Nishanth; wim@iguana.be; linux-
>> omap@vger.kernel.org; linux-watchdog@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; paul@pwsan.com; Nayak, Rajendra; Basak, Partha
>> Subject: RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during
>> init
>> 
>> 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.
>> 
> One more option is to avoid the software reset using the CONFIG_OMAP_WDT
> flag. Something like below.

This was already proposed by Charu, and rejected.

Doing this means we have a dependency on particular bootloader init, and
we'd like to get rid of *all* assumptions about what the bootloader does
(or does not do.)

Kevin

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Shilimkar, Santosh]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
> Sent: Thursday, September 30, 2010 10:42 PM
> To: Shilimkar, Santosh
> Cc: Varadarajan, Charulatha; Tony Lindgren; Cousson, Benoit; Menon,
> Nishanth; wim@iguana.be; linux-omap@vger.kernel.org; linux-
> watchdog@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> paul@pwsan.com; Nayak, Rajendra; Basak, Partha
> Subject: Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during
> init
> 
> "Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:
> 
> >> -----Original Message-----
> >> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> owner@vger.kernel.org] On Behalf Of Varadarajan, Charulatha
> >> Sent: Thursday, September 30, 2010 9:25 PM
> >> To: Tony Lindgren; Cousson, Benoit
> >> Cc: Kevin Hilman; Menon, Nishanth; wim@iguana.be; linux-
> >> omap@vger.kernel.org; linux-watchdog@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org; paul@pwsan.com; Nayak, Rajendra; Basak,
> Partha
> >> Subject: RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset
> during
> >> init
> >>
> >> 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.
> >>
> > One more option is to avoid the software reset using the CONFIG_OMAP_WDT
> > flag. Something like below.
> 
> This was already proposed by Charu, and rejected.
> 
> Doing this means we have a dependency on particular bootloader init, and
> we'd like to get rid of *all* assumptions about what the bootloader does
> (or does not do.)
> 
you mean is not depeding on bootloader to disable WDT. Make sense.

Thanks

Regards,
Santosh

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Varadarajan, Charulatha]

Tony, Benoit, Kevin,

> > >> > >
> > >> > > 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.
> > >>
> > > One more option is to avoid the software reset using the
> CONFIG_OMAP_WDT
> > > flag. Something like below.
> >
> > This was already proposed by Charu, and rejected.
> >
> > Doing this means we have a dependency on particular bootloader init, and
> > we'd like to get rid of *all* assumptions about what the bootloader does
> > (or does not do.)
> >
> you mean is not depeding on bootloader to disable WDT. Make sense.
> 

Let me know your opinion on how to handle this issue.

- V Charulatha

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Kevin Hilman]

"Varadarajan, Charulatha" <charu@ti.com> writes:

> Tony, Benoit, Kevin,
>
>> > >> > >
>> > >> > > 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.
>> > >>
>> > > One more option is to avoid the software reset using the
>> CONFIG_OMAP_WDT
>> > > flag. Something like below.
>> >
>> > This was already proposed by Charu, and rejected.
>> >
>> > Doing this means we have a dependency on particular bootloader init, and
>> > we'd like to get rid of *all* assumptions about what the bootloader does
>> > (or does not do.)
>> >
>> you mean is not depeding on bootloader to disable WDT. Make sense.
>> 
>
> Let me know your opinion on how to handle this issue.

IMO, we should do what your original patch did: always disable the WDT,
just like most/all bootloaders do.

If we want to make changes later to support product needs (like no
disable on boot, etc.) that is a separate issue and should be done in a
separate patch.

Kevin

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Cousson, Benoit]

On 10/1/2010 4:43 PM, Kevin Hilman wrote:
> "Varadarajan, Charulatha"<charu@ti.com>  writes:
>
>> Tony, Benoit, Kevin,
>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>
>>>>> One more option is to avoid the software reset using the
>>> CONFIG_OMAP_WDT
>>>>> flag. Something like below.
>>>>
>>>> This was already proposed by Charu, and rejected.
>>>>
>>>> Doing this means we have a dependency on particular bootloader init, and
>>>> we'd like to get rid of *all* assumptions about what the bootloader does
>>>> (or does not do.)
>>>>
>>> you mean is not depeding on bootloader to disable WDT. Make sense.
>>>
>>
>> Let me know your opinion on how to handle this issue.
>
> IMO, we should do what your original patch did: always disable the WDT,
> just like most/all bootloaders do.
>
> If we want to make changes later to support product needs (like no
> disable on boot, etc.) that is a separate issue and should be done in a
> separate patch.

Let's do that, since we still don't have a clue how it is supposed to be 
used by a product. If someone find out the answer, he'll fix that.

Benoit

Re: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Kevin Hilman]

"Varadarajan, Charulatha" <charu@ti.com> writes:

> With OMAP2PLUS watchdog implemented in hwmod fw way, the
> module is reset during init.

Please drop this sentence, as it's not directly related to hwmod, but
related to hardware reset, and was previously masked by most bootloaders.

> After a watchdog timer module reset, the WDTs are enabled. The
> default time for a system reset after a watchdog module reset
> is ~10s as per the default value of the WDT registers. Hence
> the system would be reset after 10s, if watchdog is not disabled
> within 10s.
>
> This patch fixes the above issue by disabling the watchdog timer
> after reset during initialization of devices.

Should also describe what happends if/when the watchdog driver is
loaded.  

Kevin

> Signed-off-by: Charulatha V <charu@ti.com>
> Reported-by: Kevin Hilman <khilman@deeprootsystems.com>
> ---
> This patch is dependent on the below patch series (wdt hwmod) and
> is created on top of pm-core branch.
> http://www.spinics.net/lists/linux-omap/msg37043.html
>
>  arch/arm/mach-omap2/devices.c |   44 +++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 44 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 8e2f0aa..9f44fc6 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -916,11 +916,55 @@ static inline void omap_init_vout(void) {}
>  
>  /*-------------------------------------------------------------------------*/
>  
> +/*
> + * WDT mdoule is reset during init which enables the watchdog.
> + * Hence it is required to disable the watchdog after the WDT reset
> + * during init. Otherwise the system would reboot as per the default
> + * watchdog timer registers settings.
> + */
> +#define OMAP_WDT_WPS	(0x34)
> +#define OMAP_WDT_SPR	(0x48)
> +
> +static int omap2_disable_wdt(struct omap_hwmod *oh, void *user)
> +{
> +	void __iomem *base;
> +
> +	if (!oh)
> +		pr_err("Could not look up wdtimer_hwmod\n");
> +
> +	base = oh->_mpu_rt_va;
> +
> +	/* Enable the clocks before accessing the WDT registers */
> +	omap_hwmod_enable(oh);
> +
> +	/* sequence required to disable watchdog */
> +	__raw_writel(0xAAAA, base + OMAP_WDT_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WDT_WPS) & 0x10)
> +		cpu_relax();
> +
> +	__raw_writel(0x5555, base + OMAP_WDT_SPR);	/* TIMER_MODE */
> +	while (__raw_readl(base + OMAP_WDT_WPS) & 0x10)
> +		cpu_relax();
> +
> +	omap_hwmod_idle(oh);
> +
> +	return 0;
> +}
> +
> +static void __init omap_disable_wdt(void)
> +{
> +	if (cpu_class_is_omap2())
> +		omap_hwmod_for_each_by_class("wd_timer",
> +						omap2_disable_wdt, NULL);
> +	return;
> +}
> +
>  static int __init omap2_init_devices(void)
>  {
>  	/* please keep these calls, and their implementations above,
>  	 * in alphabetical order so they're easier to sort through.
>  	 */
> +	omap_disable_wdt();
>  	omap_hsmmc_reset();
>  	omap_init_camera();
>  	omap_init_mbox();

RE: [PATCH] OMAP2PLUS: WDT: Fix: Disable WDT after reset during init

[Varadarajan, Charulatha]

Kevin,

> "Varadarajan, Charulatha" <charu@ti.com> writes:
> 

<<snip>>

> > With OMAP2PLUS watchdog implemented in hwmod fw way, the
> > module is reset during init.
> 
> Please drop this sentence, as it's not directly related to hwmod, but
> related to hardware reset, and was previously masked by most bootloaders.
> 

Okay, will modify accordingly.

> > After a watchdog timer module reset, the WDTs are enabled. The
> > default time for a system reset after a watchdog module reset
> > is ~10s as per the default value of the WDT registers. Hence
> > the system would be reset after 10s, if watchdog is not disabled
> > within 10s.
> >
> > This patch fixes the above issue by disabling the watchdog timer
> > after reset during initialization of devices.
> 
> Should also describe what happends if/when the watchdog driver is
> loaded.  

Okay. Will add it to the description.

> 
> Kevin
> 
> > Signed-off-by: Charulatha V <charu@ti.com>
> > Reported-by: Kevin Hilman <khilman@deeprootsystems.com>

<<snip>>