Skip to contentSkip to conversation

OMAP2+: PM: fix watchdog boot when runtime PM is disabled

PATCH New Kevin Hilman 3 participants 5 messages Jul 11, 2011 – Jul 13, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 OMAP2+: PM: fix watchdog boot when runtime PM is disabled

Fix boot crash in watchdog driver when runtime PM is disabled.

When runtime PM is disabled, devices should be left enabled so that all device accesses in drivers will succeed even though the runtime PM get/put calls are noops.

This is already the case for all devices, but the WDT init has its own hwmod postsetup which was not taking this into account.

This fixes a boot crash where the first device access in the WDT probe function would crash a booting kernel.

---
Tony, as this is a boot crash, it should probably be queued for v3.0. However, since OMAP2PLUS_TYPICAL always enables runtime PM, this is unlikely to be seen unless specifically testing with runtime PM disabled.

 arch/arm/mach-omap2/io.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/io.c +4 -0
index 441e79d..95c08aa 100644
@@ -395,7 +395,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);
Signature · 1 line
1.7.6
#1 lore raw

PATCH 1/1 OMAP2+: PM: fix watchdog boot when runtime PM is disabled

Quoted 12 lines · Kevin Hilman

Fix boot crash in watchdog driver when runtime PM is disabled.

When runtime PM is disabled, devices should be left enabled so that all device accesses in drivers will succeed even though the runtime PM get/put calls are noops.

This is already the case for all devices, but the WDT init has its own hwmod postsetup which was not taking this into account.

This fixes a boot crash where the first device access in the WDT probe function would crash a booting kernel.

Quoted 6 lines

---
Tony, as this is a boot crash, it should probably be queued for v3.0. However, since OMAP2PLUS_TYPICAL always enables runtime PM, this is unlikely to be seen unless specifically testing with runtime PM disabled.

I forgot to mention that I added this fix to my for_3.0/pm-fixes-3 branch.

Kevin

#2 lore raw

PATCH 1/1 OMAP2+: PM: fix watchdog boot when runtime PM is disabled

In reply to Kevin Hilman
Quoted 12 lines · Kevin Hilman

Fix boot crash in watchdog driver when runtime PM is disabled.

When runtime PM is disabled, devices should be left enabled so that all device accesses in drivers will succeed even though the runtime PM get/put calls are noops.

This is already the case for all devices, but the WDT init has its own hwmod postsetup which was not taking this into account.

This fixes a boot crash where the first device access in the WDT probe function would crash a booting kernel.

Quoted 7 lines

---
Tony, as this is a boot crash, it should probably be queued for v3.0. However, since OMAP2PLUS_TYPICAL always enables runtime PM, this is unlikely to be seen unless specifically testing with runtime PM disabled.

This patch is exactly same as the one I submitted on Jan 5, 2011 for the same issue. Here is the patchwork link for the same.

https://patchwork.kernel.org/patch/453481/

Paul had a valid concern about the boot-loader leaving the watchdog enabled. More on this in below thread.

http://www.spinics.net/lists/arm-kernel/msg110937.html

Quoted 19 lines
  arch/arm/mach-omap2/io.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/io.c +4 -0
index 441e79d..95c08aa 100644
@@ -395,7 +395,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);
#3 lore raw

PATCH 1/1 OMAP2+: PM: fix watchdog boot when runtime PM is disabled

Quoted 23 lines · Santosh Shilimkar
Quoted 12 lines · Kevin Hilman

Fix boot crash in watchdog driver when runtime PM is disabled.

When runtime PM is disabled, devices should be left enabled so that all device accesses in drivers will succeed even though the runtime PM get/put calls are noops.

This is already the case for all devices, but the WDT init has its own hwmod postsetup which was not taking this into account.

This fixes a boot crash where the first device access in the WDT probe function would crash a booting kernel.

Quoted 7 lines

---
Tony, as this is a boot crash, it should probably be queued for v3.0. However, since OMAP2PLUS_TYPICAL always enables runtime PM, this is unlikely to be seen unless specifically testing with runtime PM disabled.

This patch is exactly same as the one I submitted on Jan 5, 2011 for the same issue. Here is the patchwork link for the same.

Quoted 7 lines

https://patchwork.kernel.org/patch/453481/

Paul had a valid concern about the boot-loader leaving the watchdog enabled. More on this in below thread.

http://www.spinics.net/lists/arm-kernel/msg110937.html

Paul's concern is valid, and could be addressed by making that init code actually check if the WDT is currently active (as suggested by the comment there.)

In the mean time, this simple fix should be applied to prevent a boot crash, IMO.

Kevin

#4 lore raw

PATCH 1/1 OMAP2+: PM: fix watchdog boot when runtime PM is disabled

Quoted 6 lines · Kevin Hilman

Paul's concern is valid, and could be addressed by making that init code actually check if the WDT is currently active (as suggested by the comment there.)

In the mean time, this simple fix should be applied to prevent a boot crash, IMO.

Thing is, applying this fix will cause crashes if the bootloader enables the watchdog, but the kernel does not. It just trades one source of crashes for another.

The root cause of this problem is that the drivers are incorrectly relying on runtime PM to "enable" devices. My personal opinion is that we should be doing what SSB, PCI, etc. do, and have a omap_device_enable() that is called by the driver. Basically, we need a real omap_bus/omap_device.

Meanwhile, switching to lazy device reset as Tony suggested should mask the issue. The patches for this are 95% done; the intention is to post them for early 3.2. Or if people are really gung-ho, 3.1 rc.

- Paul