omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

PATCH New Santosh Shilimkar 5 participants 13 messages Jan 5, 2011 – Mar 10, 2011

Subsystems (5)

Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP hwmod data +1 more subsystems

lore

Share & tools

lore

Santosh Shilimkar

author

2011-01-05 13:40 UTC #0 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

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 <[email protected]>

Cc Paul Walmsley <[email protected]>

---
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(-)

arch/arm/configs/omap2plus_defconfig +1 -1

index ccedde1..2d711d5 100644
@@ -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

arch/arm/mach-omap2/Kconfig +0 -1

index 3e8c9e8..077ce26 100644
@@ -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

arch/arm/mach-omap2/io.c +4 -0

index e66687b..b879a16 100644
@@ -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);

arch/arm/mach-omap2/omap_hwmod.c +1 -0

index e282e35..ccdb3fd 100644
@@ -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;
 }

Signature · 1 line

1.6.0.4

Santosh Shilimkar

author

2011-01-05 13:53 UTC #1 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

Quoted 9 lines

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

Kevin Hilman

2011-01-05 17:53 UTC #2 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

Quoted 45 lines · 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 <[email protected]>
Cc Paul Walmsley <[email protected]>
---
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(-)
arch/arm/mach-omap2/io.c +0 -0
index e66687b..b879a16 100644

arch/arm/mach-omap2/io.c void __init omap2_init_common_infrastructure(void) · line 385

Quoted 8 lines

arch/arm/mach-omap2/io.c +4 -0

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

Quoted 3 lines

arch/arm/mach-omap2/io.c +0 -0

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

Santosh Shilimkar

author

2011-01-06 03:53 UTC #3 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

Quoted 72 lines

-----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
Quoted 1 line · Santosh Shilimkar
Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
mechanism'
Quoted 1 line
introduced watchdog timer state state management using
postsetup_state.
Quoted 1 line
This was done to allow some board files to support watchdog
coverage
Quoted 1 line
throughout kernel initialization and it work as intended when
RUNTIME_PM
Quoted 3 lines
is enabled.
With !CONFIG_RUNTIME_PM and no board is specifically requests
watchdog
Quoted 1 line
to remain enabled the omap_wdt_probe crashesh. This is because
hwmod
Quoted 1 line
in absense of runtime PM unable to turn watchdog clocks because
it's
Quoted 4 lines
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
Quoted 9 lines
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
Quoted 3 lines
[    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
Quoted 1 line
[    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
df87dde0
Quoted 2 lines
This patch make the default watchdog state to be enabled in case
of
Quoted 6 lines
!CONFIG_RUNTIME_PM. This fixes the crash
Signed-off-by Santosh Shilimkar <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Paul, I am not too sure if it breaks your _shutdown idea of
watchdog
arch/arm/mach-omap2/io.c void __init
Quoted 11 lines
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(-)
arch/arm/mach-omap2/io.c +0 -0
index e66687b..b879a16 100644
@@ -378,7 +378,11 @@ void __init
omap2_init_common_infrastructure(void)
arch/arm/mach-omap2/io.c void __init · line 378
Quoted 1 line
arch/arm/mach-omap2/io.c +0 -0
 	 * XXX ideally we could detect whether the MPU WDT was
currently

Quoted 12 lines

arch/arm/mach-omap2/io.c void __init omap2_init_common_infrastructure(void) · line 385
Quoted 7 lines
arch/arm/mach-omap2/io.c +4 -0
 	 * 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.

Russell King - ARM Linux

2011-01-05 22:18 UTC #4 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

In reply to Santosh Shilimkar

Quoted 11 lines · 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

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.

Santosh Shilimkar

author

2011-01-06 03:57 UTC #5 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when!CONFIG_RUNTIME_PM

Quoted 29 lines

-----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
Quoted 1 line · Santosh Shilimkar
Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
mechanism'
Quoted 1 line
introduced watchdog timer state state management using
postsetup_state.
Quoted 1 line
This was done to allow some board files to support watchdog
coverage
Quoted 1 line
throughout kernel initialization and it work as intended when
RUNTIME_PM
Quoted 3 lines
is enabled.
With !CONFIG_RUNTIME_PM and no board is specifically requests
watchdog
Quoted 1 line
to remain enabled the omap_wdt_probe crashesh. This is because
hwmod
Quoted 1 line
in absense of runtime PM unable to turn watchdog clocks because
it's
Quoted 2 lines
state is set to be disabled. For rest of the device, the state is set as enabled in absense of RUNTIME_PM

Quoted 9 lines

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

Russell King - ARM Linux

2011-01-06 15:26 UTC #6 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when!CONFIG_RUNTIME_PM

Quoted 39 lines · Santosh Shilimkar

Quoted 29 lines
-----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
Quoted 1 line · Santosh Shilimkar
Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
mechanism'
Quoted 1 line
introduced watchdog timer state state management using
postsetup_state.
Quoted 1 line
This was done to allow some board files to support watchdog
coverage
Quoted 1 line
throughout kernel initialization and it work as intended when
RUNTIME_PM
Quoted 3 lines
is enabled.
With !CONFIG_RUNTIME_PM and no board is specifically requests
watchdog
Quoted 1 line
to remain enabled the omap_wdt_probe crashesh. This is because
hwmod
Quoted 1 line
in absense of runtime PM unable to turn watchdog clocks because
it's
Quoted 2 lines
state is set to be disabled. For rest of the device, the state is set as enabled in absense of RUNTIME_PM
Quoted 9 lines
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.

Quoted 3 lines

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?

Santosh Shilimkar

author

2011-01-06 16:35 UTC #7 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probewhen!CONFIG_RUNTIME_PM

Quoted 9 lines

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

[.....]

Quoted 10 lines

Quoted 2 lines
Yes. That's managed through the shutdown part. Apart from that
there
Quoted 1 line
is another enhancement done in case some one wants to have WDT
running
Quoted 1 line
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.

Paul Walmsley

maintainer

2011-01-06 18:25 UTC #8 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

In reply to Santosh Shilimkar

Hi Santosh,

Quoted 25 lines · 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

Quoted 8 lines

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 <[email protected]>
Cc Paul Walmsley <[email protected]>
---
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

Santosh Shilimkar

author

2011-01-07 08:51 UTC #9 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

Quoted 58 lines

-----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,
Quoted 1 line · Santosh Shilimkar
Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
mechanism'
Quoted 1 line
introduced watchdog timer state state management using
postsetup_state.
Quoted 1 line
This was done to allow some board files to support watchdog
coverage
Quoted 1 line
throughout kernel initialization and it work as intended when
RUNTIME_PM
Quoted 3 lines
is enabled.
With !CONFIG_RUNTIME_PM and no board is specifically requests
watchdog
Quoted 1 line
to remain enabled the omap_wdt_probe crashesh. This is because
hwmod
Quoted 1 line
in absense of runtime PM unable to turn watchdog clocks because
it's
Quoted 4 lines
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
Quoted 4 lines
[    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)
Quoted 3 lines
[    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
Quoted 3 lines
[    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
Quoted 1 line
[    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
df87dde0
Quoted 2 lines
This patch make the default watchdog state to be enabled in case
of
Quoted 6 lines
!CONFIG_RUNTIME_PM. This fixes the crash
Signed-off-by Santosh Shilimkar <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Paul, I am not too sure if it breaks your _shutdown idea of
watchdog

Quoted 9 lines

Quoted 1 line
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

Santosh Shilimkar

author

2011-01-17 16:38 UTC #10 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

In reply to Paul Walmsley

Quoted 58 lines

-----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,
Quoted 1 line · Santosh Shilimkar
Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
mechanism'
Quoted 1 line
introduced watchdog timer state state management using
postsetup_state.
Quoted 1 line
This was done to allow some board files to support watchdog
coverage
Quoted 1 line
throughout kernel initialization and it work as intended when
RUNTIME_PM
Quoted 3 lines
is enabled.
With !CONFIG_RUNTIME_PM and no board is specifically requests
watchdog
Quoted 1 line
to remain enabled the omap_wdt_probe crashesh. This is because
hwmod
Quoted 1 line
in absense of runtime PM unable to turn watchdog clocks because
it's
Quoted 4 lines
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
Quoted 4 lines
[    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)
Quoted 3 lines
[    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
Quoted 3 lines
[    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
Quoted 1 line
[    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
df87dde0
Quoted 2 lines
This patch make the default watchdog state to be enabled in case
of
Quoted 6 lines
!CONFIG_RUNTIME_PM. This fixes the crash
Signed-off-by Santosh Shilimkar <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Paul, I am not too sure if it breaks your _shutdown idea of
watchdog

Quoted 9 lines

Quoted 1 line
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

Sricharan R

2011-02-24 16:13 UTC #11 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

Hi paul,

Quoted 1 line

-----Original Message-----

From: [email protected] [mailto:linux-omap-

Quoted 74 lines

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
Quoted 58 lines
-----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,
Quoted 1 line · Santosh Shilimkar
Commit ff2516fb 'wd_timer: disable on boot via hwmod postsetup
mechanism'
Quoted 1 line
introduced watchdog timer state state management using
postsetup_state.
Quoted 1 line
This was done to allow some board files to support watchdog
coverage
Quoted 1 line
throughout kernel initialization and it work as intended when
RUNTIME_PM
Quoted 3 lines
is enabled.
With !CONFIG_RUNTIME_PM and no board is specifically requests
watchdog
Quoted 1 line
to remain enabled the omap_wdt_probe crashesh. This is because
hwmod
Quoted 1 line
in absense of runtime PM unable to turn watchdog clocks because
it's
Quoted 4 lines
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
Quoted 4 lines
[    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)
Quoted 3 lines
[    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
Quoted 3 lines
[    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
Quoted 1 line
[    1.432189] r3 : fc314000  r2 : 00005555  r1 : fc314034  r0 :
df87dde0
Quoted 2 lines
This patch make the default watchdog state to be enabled in case
of
Quoted 6 lines
!CONFIG_RUNTIME_PM. This fixes the crash
Signed-off-by Santosh Shilimkar <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Paul, I am not too sure if it breaks your _shutdown idea of
watchdog
Quoted 9 lines
Quoted 1 line
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...

Quoted 7 lines

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 ?

arch/arm/mach-omap2/io.c +0 ~~-18~~

index 657f3c8..2641d73 100644
@@ -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())

(quoted diff) +0 -0

b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 879f55f..d665ee5 100644

arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +2 -1

@@ -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 */

(quoted diff) +0 -0

b/arch/arm/mach-omap2/wd_timer.c
index 4067669..e11c5f3 100644

arch/arm/mach-omap2/wd_timer.c +31 -1

@@ -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;
+}
+

(quoted diff) +0 -0

b/arch/arm/mach-omap2/wd_timer.h
index e0054a2..f6bbba7 100644

arch/arm/mach-omap2/wd_timer.h +1 -0

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

----

Quoted 3 lines

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

Paul Walmsley

maintainer

2011-03-10 08:54 UTC #12 lore raw

PATCH 1/1 omap: wd_timer: Fix crash frm wdt_probe when !CONFIG_RUNTIME_PM

Quoted 29 lines · Sricharan R

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.

Quoted 8 lines

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