[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Santosh Shilimkar]

Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup mechanism'
introduced watchdog timer state state management using postsetup_state.
This was done to allow some board files to support watchdog coverage
throughout kernel initialization and it work as intended when RUNTIME_PM
is enabled.

With !CONFIG_RUNTIME_PM and no board is specifically requests watchdog
to remain enabled the omap_wdt_probe crashesh. This is because hwmod
in absense of runtime PM unable to turn watchdog clocks because it's
state is set to be disabled. For rest of the device, the state is
set as enabled in absense of RUNTIME_PM

[    1.372558] Unhandled fault: imprecise external abort (0x1406) at 0xad733eeb
[    1.379913] Internal error: : 1406 [#1] SMP
[    1.384277] last sysfs file:
[    1.387359] Modules linked in:
[    1.390563] CPU: 0    Tainted: G        W    (2.6.37-rc7-00265-g4298a4c-dirty #23)
[    1.398468] PC is at omap_wdt_disable+0x2c/0x3c
[    1.403198] LR is at omap_wdt_probe+0x124/0x1e0
[    1.407928] pc : [<c02f5bf4>]    lr : [<c03be10c>]    psr: 60000013
[    1.407958] sp : df833f00  ip : 00000000  fp : 00000000
[    1.419921] r10: c0ac57ac  r9 : df959e00  r8 : 00000000
[    1.425384] r7 : df959e08  r6 : df8000c0  r5 : df95bebc  r4 : df87dde0
[    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 : df87dde0

This patch make the default watchdog state to be enabled in case of
!CONFIG_RUNTIME_PM. This fixes the crash

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
Paul, I am not too sure if it breaks your _shutdown idea of watchdog
timer.
Patch generated against 'omap-for-linus' branch and boot tested on OMAP4
with and without CONFIG_OMAP_WATCHDOG.

 arch/arm/configs/omap2plus_defconfig |    2 +-
 arch/arm/mach-omap2/Kconfig          |    1 -
 arch/arm/mach-omap2/io.c             |    4 ++++
 arch/arm/mach-omap2/omap_hwmod.c     |    1 +
 4 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index ccedde1..2d711d5 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -71,7 +71,7 @@ CONFIG_NEON=y
 CONFIG_BINFMT_MISC=y
 CONFIG_PM=y
 CONFIG_PM_DEBUG=y
-CONFIG_PM_RUNTIME=y
+# CONFIG_PM_RUNTIME is not set
 CONFIG_NET=y
 CONFIG_PACKET=y
 CONFIG_UNIX=y
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 3e8c9e8..077ce26 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -8,7 +8,6 @@ config ARCH_OMAP2PLUS_TYPICAL
 	select AEABI
 	select REGULATOR
 	select PM
-	select PM_RUNTIME
 	select VFP
 	select NEON if ARCH_OMAP3 || ARCH_OMAP4
 	select SERIAL_OMAP
diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index e66687b..b879a16 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -378,7 +378,11 @@ void __init omap2_init_common_infrastructure(void)
 	 * XXX ideally we could detect whether the MPU WDT was currently
 	 * enabled here and make this conditional
 	 */
+#ifdef CONFIG_PM_RUNTIME
 	postsetup_state = _HWMOD_STATE_DISABLED;
+#else
+	postsetup_state = _HWMOD_STATE_ENABLED;
+#endif
 	omap_hwmod_for_each_by_class("wd_timer",
 				     _set_hwmod_postsetup_state,
 				     &postsetup_state);
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index e282e35..ccdb3fd 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1432,6 +1432,7 @@ static int _setup(struct omap_hwmod *oh, void *data)
 	else if (postsetup_state != _HWMOD_STATE_ENABLED)
 		WARN(1, "hwmod: %s: unknown postsetup state %d! defaulting to enabled\n",
 		     oh->name, postsetup_state);
+	printk("POSTSETUP state: %s = %d\n", oh->name, postsetup_state);
 
 	return 0;
 }
-- 
1.6.0.4

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Santosh Shilimkar]

> -----Original Message-----
> From: Santosh Shilimkar [mailto:santosh.shilimkar at ti.com]
> Sent: Wednesday, January 05, 2011 7:11 PM
> To: linux-omap at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org; Santosh Shilimkar; Paul
> Walmsley
> Subject: [PATCH] omap: wd_timer: Fix crash frm wdt_probe when
> !CONFIG_RUNTIME_PM
>
(Removing unnecessary changes which got committed by mistake)

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Kevin Hilman]

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

> Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup mechanism'
> introduced watchdog timer state state management using postsetup_state.
> This was done to allow some board files to support watchdog coverage
> throughout kernel initialization and it work as intended when RUNTIME_PM
> is enabled.
>
> With !CONFIG_RUNTIME_PM and no board is specifically requests watchdog
> to remain enabled the omap_wdt_probe crashesh. This is because hwmod
> in absense of runtime PM unable to turn watchdog clocks because it's
> state is set to be disabled. For rest of the device, the state is
> set as enabled in absense of RUNTIME_PM
>
> [    1.372558] Unhandled fault: imprecise external abort (0x1406) at
> 0xad733eeb
> [    1.379913] Internal error: : 1406 [#1] SMP
> [    1.384277] last sysfs file:
> [    1.387359] Modules linked in:
> [    1.390563] CPU: 0    Tainted: G        W
> (2.6.37-rc7-00265-g4298a4c-dirty #23)
> [    1.398468] PC is at omap_wdt_disable+0x2c/0x3c
> [    1.403198] LR is at omap_wdt_probe+0x124/0x1e0
> [    1.407928] pc : [<c02f5bf4>]    lr : [<c03be10c>]    psr: 60000013
> [    1.407958] sp : df833f00  ip : 00000000  fp : 00000000
> [    1.419921] r10: c0ac57ac  r9 : df959e00  r8 : 00000000
> [    1.425384] r7 : df959e08  r6 : df8000c0  r5 : df95bebc  r4 : df87dde0
> [    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 : df87dde0
>
> This patch make the default watchdog state to be enabled in case of
> !CONFIG_RUNTIME_PM. This fixes the crash
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> ---
> Paul, I am not too sure if it breaks your _shutdown idea of watchdog
> timer.  Patch generated against 'omap-for-linus' branch and boot
> tested on OMAP4 with and without CONFIG_OMAP_WATCHDOG.
>
>  arch/arm/mach-omap2/io.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> index e66687b..b879a16 100644
> --- a/arch/arm/mach-omap2/io.c
> +++ b/arch/arm/mach-omap2/io.c
> @@ -378,7 +378,11 @@ void __init omap2_init_common_infrastructure(void)
>  	 * XXX ideally we could detect whether the MPU WDT was currently
>  	 * enabled here and make this conditional
>  	 */
> +#ifdef CONFIG_PM_RUNTIME
>  	postsetup_state = _HWMOD_STATE_DISABLED;
> +#else
> +	postsetup_state = _HWMOD_STATE_ENABLED;
> +#endif

You shouldn't need the 'else' part of this since the default a few lines
above this code is already setting that for the !CONFIG_PM_RUNTIME case.

Kevin

>  	omap_hwmod_for_each_by_class("wd_timer",
>  				     _set_hwmod_postsetup_state,
>  				     &postsetup_state);

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Santosh Shilimkar]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman at ti.com]
> Sent: Wednesday, January 05, 2011 11:23 PM
> To: Santosh Shilimkar
> Cc: Paul Walmsley; linux-arm-kernel at lists.infradead.org; linux-
> omap at vger.kernel.org
> Subject: Re: [PATCH] omap: wd_timer: Fix crash frm wdt_probe when
> !CONFIG_RUNTIME_PM
>
> Santosh Shilimkar <santosh.shilimkar@ti.com> writes:
>
> > Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
> mechanism'
> > introduced watchdog timer state state management using
> postsetup_state.
> > This was done to allow some board files to support watchdog
> coverage
> > throughout kernel initialization and it work as intended when
> RUNTIME_PM
> > is enabled.
> >
> > With !CONFIG_RUNTIME_PM and no board is specifically requests
> watchdog
> > to remain enabled the omap_wdt_probe crashesh. This is because
> hwmod
> > in absense of runtime PM unable to turn watchdog clocks because
> it's
> > state is set to be disabled. For rest of the device, the state is
> > set as enabled in absense of RUNTIME_PM
> >
> > [    1.372558] Unhandled fault: imprecise external abort (0x1406)
> at
> > 0xad733eeb
> > [    1.379913] Internal error: : 1406 [#1] SMP
> > [    1.384277] last sysfs file:
> > [    1.387359] Modules linked in:
> > [    1.390563] CPU: 0    Tainted: G        W
> > (2.6.37-rc7-00265-g4298a4c-dirty #23)
> > [    1.398468] PC is at omap_wdt_disable+0x2c/0x3c
> > [    1.403198] LR is at omap_wdt_probe+0x124/0x1e0
> > [    1.407928] pc : [<c02f5bf4>]    lr : [<c03be10c>]    psr:
> 60000013
> > [    1.407958] sp : df833f00  ip : 00000000  fp : 00000000
> > [    1.419921] r10: c0ac57ac  r9 : df959e00  r8 : 00000000
> > [    1.425384] r7 : df959e08  r6 : df8000c0  r5 : df95bebc  r4 :
> df87dde0
> > [    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
> df87dde0
> >
> > This patch make the default watchdog state to be enabled in case
> of
> > !CONFIG_RUNTIME_PM. This fixes the crash
> >
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > ---
> > Paul, I am not too sure if it breaks your _shutdown idea of
> watchdog
> > timer.  Patch generated against 'omap-for-linus' branch and boot
> > tested on OMAP4 with and without CONFIG_OMAP_WATCHDOG.
> >
> >  arch/arm/mach-omap2/io.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
> > index e66687b..b879a16 100644
> > --- a/arch/arm/mach-omap2/io.c
> > +++ b/arch/arm/mach-omap2/io.c
> > @@ -378,7 +378,11 @@ void __init
> omap2_init_common_infrastructure(void)
> >  	 * XXX ideally we could detect whether the MPU WDT was
> currently
> >  	 * enabled here and make this conditional
> >  	 */
> > +#ifdef CONFIG_PM_RUNTIME
> >  	postsetup_state = _HWMOD_STATE_DISABLED;
> > +#else
> > +	postsetup_state = _HWMOD_STATE_ENABLED;
> > +#endif
>
> You shouldn't need the 'else' part of this since the default a few
> lines
> above this code is already setting that for the !CONFIG_PM_RUNTIME
> case.
Yep. Just 'CONFIG_PM_RUNTIME' wrapping is enough.

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Russell King - ARM Linux]

On Wed, Jan 05, 2011 at 07:10:55PM +0530, Santosh Shilimkar wrote:
> Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup mechanism'
> introduced watchdog timer state state management using postsetup_state.
> This was done to allow some board files to support watchdog coverage
> throughout kernel initialization and it work as intended when RUNTIME_PM
> is enabled.
> 
> With !CONFIG_RUNTIME_PM and no board is specifically requests watchdog
> to remain enabled the omap_wdt_probe crashesh. This is because hwmod
> in absense of runtime PM unable to turn watchdog clocks because it's
> state is set to be disabled. For rest of the device, the state is
> set as enabled in absense of RUNTIME_PM

Err... wasn't this provoked by an attempt to fix the LDP issue, that is
(I believe) because the boot loader enables the watchdog and pre-hwmod
kernels used to disable it.  Post-hwmod kernels stopped disabling the
watchdog, resulting in a few seconds booting userspace before the system
resets itself.

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when!CONFIG_RUNTIME_PM

[Santosh Shilimkar]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Thursday, January 06, 2011 3:49 AM
> To: Santosh Shilimkar
> Cc: linux-omap at vger.kernel.org; Paul Walmsley; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH] omap: wd_timer: Fix crash frm wdt_probe
> when!CONFIG_RUNTIME_PM
>
> On Wed, Jan 05, 2011 at 07:10:55PM +0530, Santosh Shilimkar wrote:
> > Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
> mechanism'
> > introduced watchdog timer state state management using
> postsetup_state.
> > This was done to allow some board files to support watchdog
> coverage
> > throughout kernel initialization and it work as intended when
> RUNTIME_PM
> > is enabled.
> >
> > With !CONFIG_RUNTIME_PM and no board is specifically requests
> watchdog
> > to remain enabled the omap_wdt_probe crashesh. This is because
> hwmod
> > in absense of runtime PM unable to turn watchdog clocks because
> it's
> > state is set to be disabled. For rest of the device, the state is
> > set as enabled in absense of RUNTIME_PM
>
> Err... wasn't this provoked by an attempt to fix the LDP issue, that
> is
> (I believe) because the boot loader enables the watchdog and pre-
> hwmod
> kernels used to disable it.  Post-hwmod kernels stopped disabling
> the
> watchdog, resulting in a few seconds booting userspace before the
> system
> resets itself.

Yes. That's managed through the shutdown part. Apart from that there
is another enhancement done in case some one wants to have WDT running
throughout the kernel boot.

Regards,
Santosh

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when!CONFIG_RUNTIME_PM

[Russell King - ARM Linux]

On Thu, Jan 06, 2011 at 09:27:32AM +0530, Santosh Shilimkar wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> > Sent: Thursday, January 06, 2011 3:49 AM
> > To: Santosh Shilimkar
> > Cc: linux-omap at vger.kernel.org; Paul Walmsley; linux-arm-
> > kernel at lists.infradead.org
> > Subject: Re: [PATCH] omap: wd_timer: Fix crash frm wdt_probe
> > when!CONFIG_RUNTIME_PM
> >
> > On Wed, Jan 05, 2011 at 07:10:55PM +0530, Santosh Shilimkar wrote:
> > > Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
> > mechanism'
> > > introduced watchdog timer state state management using
> > postsetup_state.
> > > This was done to allow some board files to support watchdog
> > coverage
> > > throughout kernel initialization and it work as intended when
> > RUNTIME_PM
> > > is enabled.
> > >
> > > With !CONFIG_RUNTIME_PM and no board is specifically requests
> > watchdog
> > > to remain enabled the omap_wdt_probe crashesh. This is because
> > hwmod
> > > in absense of runtime PM unable to turn watchdog clocks because
> > it's
> > > state is set to be disabled. For rest of the device, the state is
> > > set as enabled in absense of RUNTIME_PM
> >
> > Err... wasn't this provoked by an attempt to fix the LDP issue, that
> > is
> > (I believe) because the boot loader enables the watchdog and pre-
> > hwmod
> > kernels used to disable it.  Post-hwmod kernels stopped disabling
> > the
> > watchdog, resulting in a few seconds booting userspace before the
> > system
> > resets itself.
> 
> Yes. That's managed through the shutdown part. Apart from that there
> is another enhancement done in case some one wants to have WDT running
> throughout the kernel boot.

Good - and just to confirm, the fix for LDP watchdogging will be submitted
in the current merge window?

[PATCH] omap: wd_timer: Fix crash frm wdt_probewhen!CONFIG_RUNTIME_PM

[Santosh Shilimkar]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Thursday, January 06, 2011 8:57 PM
> To: Santosh Shilimkar
> Cc: linux-omap at vger.kernel.org; Paul Walmsley; linux-arm-
> kernel at lists.infradead.org
> Subject: Re: [PATCH] omap: wd_timer: Fix crash frm
> wdt_probewhen!CONFIG_RUNTIME_PM
>
[.....]

> >
> > Yes. That's managed through the shutdown part. Apart from that
> there
> > is another enhancement done in case some one wants to have WDT
> running
> > throughout the kernel boot.
>
> Good - and just to confirm, the fix for LDP watchdogging will be
> submitted
> in the current merge window?

Yes. It's in the queue already.

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Paul Walmsley]

Hi Santosh,

On Wed, 5 Jan 2011, Santosh Shilimkar wrote:

> Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup mechanism'
> introduced watchdog timer state state management using postsetup_state.
> This was done to allow some board files to support watchdog coverage
> throughout kernel initialization and it work as intended when RUNTIME_PM
> is enabled.
> 
> With !CONFIG_RUNTIME_PM and no board is specifically requests watchdog
> to remain enabled the omap_wdt_probe crashesh. This is because hwmod
> in absense of runtime PM unable to turn watchdog clocks because it's
> state is set to be disabled. For rest of the device, the state is
> set as enabled in absense of RUNTIME_PM
> 
> [    1.372558] Unhandled fault: imprecise external abort (0x1406) at 0xad733eeb
> [    1.379913] Internal error: : 1406 [#1] SMP
> [    1.384277] last sysfs file:
> [    1.387359] Modules linked in:
> [    1.390563] CPU: 0    Tainted: G        W    (2.6.37-rc7-00265-g4298a4c-dirty #23)
> [    1.398468] PC is at omap_wdt_disable+0x2c/0x3c
> [    1.403198] LR is at omap_wdt_probe+0x124/0x1e0
> [    1.407928] pc : [<c02f5bf4>]    lr : [<c03be10c>]    psr: 60000013
> [    1.407958] sp : df833f00  ip : 00000000  fp : 00000000
> [    1.419921] r10: c0ac57ac  r9 : df959e00  r8 : 00000000
> [    1.425384] r7 : df959e08  r6 : df8000c0  r5 : df95bebc  r4 : df87dde0
> [    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 : df87dde0
> 
> This patch make the default watchdog state to be enabled in case of
> !CONFIG_RUNTIME_PM. This fixes the crash
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> ---
> Paul, I am not too sure if it breaks your _shutdown idea of watchdog
> timer.

Maybe.  What happens in a case where the bootloader enables the watchdog, 
but the booting kernel is compiled with !CONFIG_OMAP_WATCHDOG and 
!CONFIG_PM_RUNTIME?  Won't the watchdog reset the MPU unexpectedly in that 
case?  Or am I missing something...


- Paul

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Santosh Shilimkar]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Thursday, January 06, 2011 11:56 PM
> To: Santosh Shilimkar
> Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] omap: wd_timer: Fix crash frm wdt_probe when
> !CONFIG_RUNTIME_PM
>
> Hi Santosh,
>
> On Wed, 5 Jan 2011, Santosh Shilimkar wrote:
>
> > Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
> mechanism'
> > introduced watchdog timer state state management using
> postsetup_state.
> > This was done to allow some board files to support watchdog
> coverage
> > throughout kernel initialization and it work as intended when
> RUNTIME_PM
> > is enabled.
> >
> > With !CONFIG_RUNTIME_PM and no board is specifically requests
> watchdog
> > to remain enabled the omap_wdt_probe crashesh. This is because
> hwmod
> > in absense of runtime PM unable to turn watchdog clocks because
> it's
> > state is set to be disabled. For rest of the device, the state is
> > set as enabled in absense of RUNTIME_PM
> >
> > [    1.372558] Unhandled fault: imprecise external abort (0x1406)
> at 0xad733eeb
> > [    1.379913] Internal error: : 1406 [#1] SMP
> > [    1.384277] last sysfs file:
> > [    1.387359] Modules linked in:
> > [    1.390563] CPU: 0    Tainted: G        W    (2.6.37-rc7-00265-
> g4298a4c-dirty #23)
> > [    1.398468] PC is at omap_wdt_disable+0x2c/0x3c
> > [    1.403198] LR is at omap_wdt_probe+0x124/0x1e0
> > [    1.407928] pc : [<c02f5bf4>]    lr : [<c03be10c>]    psr:
> 60000013
> > [    1.407958] sp : df833f00  ip : 00000000  fp : 00000000
> > [    1.419921] r10: c0ac57ac  r9 : df959e00  r8 : 00000000
> > [    1.425384] r7 : df959e08  r6 : df8000c0  r5 : df95bebc  r4 :
> df87dde0
> > [    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
> df87dde0
> >
> > This patch make the default watchdog state to be enabled in case
> of
> > !CONFIG_RUNTIME_PM. This fixes the crash
> >
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > ---
> > Paul, I am not too sure if it breaks your _shutdown idea of
> watchdog
> > timer.
>
> Maybe.  What happens in a case where the bootloader enables the
> watchdog,
> but the booting kernel is compiled with !CONFIG_OMAP_WATCHDOG and
> !CONFIG_PM_RUNTIME?  Won't the watchdog reset the MPU unexpectedly
> in that
> case?  Or am I missing something...
>
I had a same doubght. But the test I did with and without
"CONFIG_OMAP_WATCHDOG" just at kernel level. The TI bootloader
have MPU watchdog being always disabled.

Will do a test on this scenario by explicitly disabling the
MPU WDT in bootloader.

Regards,
Santosh

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Santosh Shilimkar]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Thursday, January 06, 2011 11:56 PM
> To: Santosh Shilimkar
> Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] omap: wd_timer: Fix crash frm wdt_probe when
> !CONFIG_RUNTIME_PM
>
> Hi Santosh,
>
> On Wed, 5 Jan 2011, Santosh Shilimkar wrote:
>
> > Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
> mechanism'
> > introduced watchdog timer state state management using
> postsetup_state.
> > This was done to allow some board files to support watchdog
> coverage
> > throughout kernel initialization and it work as intended when
> RUNTIME_PM
> > is enabled.
> >
> > With !CONFIG_RUNTIME_PM and no board is specifically requests
> watchdog
> > to remain enabled the omap_wdt_probe crashesh. This is because
> hwmod
> > in absense of runtime PM unable to turn watchdog clocks because
> it's
> > state is set to be disabled. For rest of the device, the state is
> > set as enabled in absense of RUNTIME_PM
> >
> > [    1.372558] Unhandled fault: imprecise external abort (0x1406)
> at 0xad733eeb
> > [    1.379913] Internal error: : 1406 [#1] SMP
> > [    1.384277] last sysfs file:
> > [    1.387359] Modules linked in:
> > [    1.390563] CPU: 0    Tainted: G        W    (2.6.37-rc7-00265-
> g4298a4c-dirty #23)
> > [    1.398468] PC is at omap_wdt_disable+0x2c/0x3c
> > [    1.403198] LR is at omap_wdt_probe+0x124/0x1e0
> > [    1.407928] pc : [<c02f5bf4>]    lr : [<c03be10c>]    psr:
> 60000013
> > [    1.407958] sp : df833f00  ip : 00000000  fp : 00000000
> > [    1.419921] r10: c0ac57ac  r9 : df959e00  r8 : 00000000
> > [    1.425384] r7 : df959e08  r6 : df8000c0  r5 : df95bebc  r4 :
> df87dde0
> > [    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
> df87dde0
> >
> > This patch make the default watchdog state to be enabled in case
> of
> > !CONFIG_RUNTIME_PM. This fixes the crash
> >
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > ---
> > Paul, I am not too sure if it breaks your _shutdown idea of
> watchdog
> > timer.
>
> Maybe.  What happens in a case where the bootloader enables the
> watchdog,
> but the booting kernel is compiled with !CONFIG_OMAP_WATCHDOG and
> !CONFIG_PM_RUNTIME?  Won't the watchdog reset the MPU unexpectedly
> in that
> case?  Or am I missing something...
>
I tried this scenario and some how the WDT isn't reseting MPU with my
patch.

Actually I was expecting it should reset but it didn't.

Regards,
Santosh

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Sricharan R]

Hi paul,
>-----Original Message-----
From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>owner at vger.kernel.org] On Behalf Of Santosh Shilimkar
>Sent: Monday, January 17, 2011 10:09 PM
>To: Paul Walmsley
>Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
>Subject: RE: [PATCH] omap: wd_timer: Fix crash frm wdt_probe when
>!CONFIG_RUNTIME_PM
>
>> -----Original Message-----
>> From: Paul Walmsley [mailto:paul at pwsan.com]
>> Sent: Thursday, January 06, 2011 11:56 PM
>> To: Santosh Shilimkar
>> Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
>> Subject: Re: [PATCH] omap: wd_timer: Fix crash frm wdt_probe when
>> !CONFIG_RUNTIME_PM
>>
>> Hi Santosh,
>>
>> On Wed, 5 Jan 2011, Santosh Shilimkar wrote:
>>
>> > Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
>> mechanism'
>> > introduced watchdog timer state state management using
>> postsetup_state.
>> > This was done to allow some board files to support watchdog
>> coverage
>> > throughout kernel initialization and it work as intended when
>> RUNTIME_PM
>> > is enabled.
>> >
>> > With !CONFIG_RUNTIME_PM and no board is specifically requests
>> watchdog
>> > to remain enabled the omap_wdt_probe crashesh. This is because
>> hwmod
>> > in absense of runtime PM unable to turn watchdog clocks because
>> it's
>> > state is set to be disabled. For rest of the device, the state is
>> > set as enabled in absense of RUNTIME_PM
>> >
>> > [    1.372558] Unhandled fault: imprecise external abort (0x1406)
>> at 0xad733eeb
>> > [    1.379913] Internal error: : 1406 [#1] SMP
>> > [    1.384277] last sysfs file:
>> > [    1.387359] Modules linked in:
>> > [    1.390563] CPU: 0    Tainted: G        W    (2.6.37-rc7-00265-
>> g4298a4c-dirty #23)
>> > [    1.398468] PC is at omap_wdt_disable+0x2c/0x3c
>> > [    1.403198] LR is at omap_wdt_probe+0x124/0x1e0
>> > [    1.407928] pc : [<c02f5bf4>]    lr : [<c03be10c>]    psr:
>> 60000013
>> > [    1.407958] sp : df833f00  ip : 00000000  fp : 00000000
>> > [    1.419921] r10: c0ac57ac  r9 : df959e00  r8 : 00000000
>> > [    1.425384] r7 : df959e08  r6 : df8000c0  r5 : df95bebc  r4 :
>> df87dde0
>> > [    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
>> df87dde0
>> >
>> > This patch make the default watchdog state to be enabled in case
>> of
>> > !CONFIG_RUNTIME_PM. This fixes the crash
>> >
>> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> > Cc: Paul Walmsley <paul@pwsan.com>
>> > ---
>> > Paul, I am not too sure if it breaks your _shutdown idea of
>> watchdog
>> > timer.
>>
>> Maybe.  What happens in a case where the bootloader enables the
>> watchdog,
>> but the booting kernel is compiled with !CONFIG_OMAP_WATCHDOG and
>> !CONFIG_PM_RUNTIME?  Won't the watchdog reset the MPU unexpectedly
>> in that
>> case?  Or am I missing something...
>>
>I tried this scenario and some how the WDT isn't reseting MPU with my
>patch.
>
>Actually I was expecting it should reset but it didn't.
>
>Regards,
>Santosh
In the case of !CONFIG_PM_RUNTIME and !CONFIG_OMAP_WATCHDOG, if
the boot loader or hwmod enables the watchdog, then it generates
an un wanted reset.

So to avoid this in the case of !CONFIG_OMAP_WATCHDOG then the
watchdog should always be disabled.

So there are two cases:
   Case 1: (!CONFIG_OMAP_WATCHDOG and !CONFIG_PM_RUNTIME)
             Watchdog should be disabled.

   Case 2: (!CONFIG_PM_RUNTIME) and ( driver is present and board file
enables it)
		 Watchdog should run.

   Solution1:
          Introduce a new hwmod watchdog reset function, by populating
          the .reset entry of the hwmod.
          This function is called during the hwmod init.
          This function resets the watchdog always and also
          disables it if !CONFIG_OMAP_WATCHDOG.

        ( or )

  Solution2:
          Introduce a state variable to differentiate if
          hwmod post_setup_state is set to enabled by
          by board file or io.c file.

          Use this state variable in the reset function as

          If it is enabled by io.c then disable watchdog
          Otherwise if it is set to enabled by board file
          then enable watchdog.

The below is the patch for solution1, that I have mentioned.
Please suggest which would be the right approach ?

diff --git a/arch/arm/mach-omap2/io.c b/arch/arm/mach-omap2/io.c
index 657f3c8..2641d73 100644
--- a/arch/arm/mach-omap2/io.c
+++ b/arch/arm/mach-omap2/io.c
@@ -382,24 +382,6 @@ void __init omap2_init_common_infrastructure(void)
 #endif
 	omap_hwmod_for_each(_set_hwmod_postsetup_state, &postsetup_state);

-	/*
-	 * Set the default postsetup state for unusual modules (like
-	 * MPU WDT).
-	 *
-	 * The postsetup_state is not actually used until
-	 * omap_hwmod_late_init(), so boards that desire full watchdog
-	 * coverage of kernel initialization can reprogram the
-	 * postsetup_state between the calls to
-	 * omap2_init_common_infra() and omap2_init_common_devices().
-	 *
-	 * XXX ideally we could detect whether the MPU WDT was currently
-	 * enabled here and make this conditional
-	 */
-	postsetup_state = _HWMOD_STATE_DISABLED;
-	omap_hwmod_for_each_by_class("wd_timer",
-				     _set_hwmod_postsetup_state,
-				     &postsetup_state);
-
 	omap_pm_if_early_init();

 	if (cpu_is_omap2420())
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 879f55f..d665ee5 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -545,7 +545,8 @@ static struct omap_hwmod_class_sysconfig i2c_sysc = {
 static struct omap_hwmod_class omap3xxx_wd_timer_hwmod_class = {
 	.name		= "wd_timer",
 	.sysc		= &omap3xxx_wd_timer_sysc,
-	.pre_shutdown	= &omap2_wd_timer_disable
+	.pre_shutdown	= &omap2_wd_timer_disable,
+	.reset		= &omap2_wd_timer_reset
 };

 /* wd_timer2 */
diff --git a/arch/arm/mach-omap2/wd_timer.c
b/arch/arm/mach-omap2/wd_timer.c
index 4067669..e11c5f3 100644
--- a/arch/arm/mach-omap2/wd_timer.c
+++ b/arch/arm/mach-omap2/wd_timer.c
@@ -24,7 +24,8 @@
  */
 #define OMAP_WDT_WPS		0x34
 #define OMAP_WDT_SPR		0x48
-
+#define OMAP_WDT_DSC		0x10
+#define OMAP_WDT_DST		0x14

 int omap2_wd_timer_disable(struct omap_hwmod *oh)
 {
@@ -54,3 +55,32 @@ int omap2_wd_timer_disable(struct omap_hwmod *oh)
 	return 0;
 }

+int omap2_wd_timer_reset(struct omap_hwmod *oh)
+{
+	void __iomem *base;
+	pr_err("\n\n\n\n\n\n\n\n\n omap2_wd_timer_reset \n\n\n\n\n");
+
+	if (!oh) {
+		pr_err("%s: could not look up wdtimer_hwmod\n",__func__);
+	}
+
+	base = omap_hwmod_get_mpu_rt_va(oh);
+	if(!base) {
+		pr_err("%s: Could not get the base address for %s\n",
+				oh->name, __func__);
+		return -EINVAL;
+	}
+
+	/* soft reset on watch dog timer */
+	__raw_writel(0x02, base + OMAP_WDT_DSC);
+	while (__raw_readl(base + OMAP_WDT_DST) & 0x1)
+		cpu_relax();
+
+#ifdef CONFIG_OMAP_WATCHDOG
+	/* Disable the watchdog */
+	omap2_wd_timer_disable(oh);
+#endif
+
+	return 0;
+}
+
diff --git a/arch/arm/mach-omap2/wd_timer.h
b/arch/arm/mach-omap2/wd_timer.h
index e0054a2..f6bbba7 100644
--- a/arch/arm/mach-omap2/wd_timer.h
+++ b/arch/arm/mach-omap2/wd_timer.h
@@ -13,5 +13,6 @@
 #include <plat/omap_hwmod.h>

 extern int omap2_wd_timer_disable(struct omap_hwmod *oh);
+extern int omap2_wd_timer_reset(struct omap_hwmod *oh);


Thanks,
  Sricharan


----
>To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>the body of a message to majordomo at vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

[Paul Walmsley]

On Thu, 24 Feb 2011, Sricharan R wrote:

> In the case of !CONFIG_PM_RUNTIME and !CONFIG_OMAP_WATCHDOG, if
> the boot loader or hwmod enables the watchdog, then it generates
> an un wanted reset.
> 
> So to avoid this in the case of !CONFIG_OMAP_WATCHDOG then the
> watchdog should always be disabled.
> 
> So there are two cases:
>    Case 1: (!CONFIG_OMAP_WATCHDOG and !CONFIG_PM_RUNTIME)
>              Watchdog should be disabled.
> 
>    Case 2: (!CONFIG_PM_RUNTIME) and ( driver is present and board file
> enables it)
> 		 Watchdog should run.
> 
>    Solution1:
>           Introduce a new hwmod watchdog reset function, by populating
>           the .reset entry of the hwmod.
>           This function is called during the hwmod init.
>           This function resets the watchdog always and also
>           disables it if !CONFIG_OMAP_WATCHDOG.
> 
>         ( or )
> 
>   Solution2:
>           Introduce a state variable to differentiate if
>           hwmod post_setup_state is set to enabled by
>           by board file or io.c file.
> 
>           Use this state variable in the reset function as
> 
>           If it is enabled by io.c then disable watchdog
>           Otherwise if it is set to enabled by board file
>           then enable watchdog.
> 
> The below is the patch for solution1, that I have mentioned.
> Please suggest which would be the right approach ?

I don't think either of these seem right.  At this point, I think the best 
way is to add a function pointer that can be used to quiesce IP blocks 
that become active after being reset, and to add a flag that board files 
can set to skip that quiescing process.  We're so close to the merge 
window opening, though, that I think the best time to deal with this will 
be after 2.6.40 is released.


- Paul