Skip to contentSkip to conversation

ARM: OMAP2+: PM: Register suspend ops even in the presence of DT blob

RFC PATCH New Vaibhav Bedia 2 participants 5 messages Jul 19, 2012 – Jul 20, 2012

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

RFC PATCH v2 1/1 ARM: OMAP2+: PM: Register suspend ops even in the presence of DT blob

As per the comment in omap2_common_late_init() looks like the original intent of the DT check was to treat only the PMIC and SR initialization differently. Recent changes to consolidate the suspend-resume code across OMAP3/4 resulted into the registration of suspend ops also being dependent on the check for DT blob. Since the suspend-resume operation should not really be dependent on the usage of DT remove this dependency by wrapping the PMIC and SR init under the DT check.

---
v2->v1
- As suggested by Paul, Instead of moving around the suspend ops registration just wrap the PMIC and SR init under the DT check.
- Fixed up Kevin's email address :\

 arch/arm/mach-omap2/pm.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)
arch/arm/mach-omap2/pm.c +10 -11
index 9cb5ced..46848f7 100644
@@ -302,19 +302,18 @@ int __init omap2_common_pm_late_init(void)
 	 * a completely different mechanism.
 	 * Disable this part if a DT blob is available.
 	 */
-	if (of_have_populated_dt())
-		return 0;
+	if (!of_have_populated_dt()) {
+		/* Init the voltage layer */
+		omap_pmic_late_init();
+		omap_voltage_late_init();
 
-	/* Init the voltage layer */
-	omap_pmic_late_init();
-	omap_voltage_late_init();
+		/* Initialize the voltages */
+		omap3_init_voltages();
+		omap4_init_voltages();
 
-	/* Initialize the voltages */
-	omap3_init_voltages();
-	omap4_init_voltages();
-
-	/* Smartreflex device init */
-	omap_devinit_smartreflex();
+		/* Smartreflex device init */
+		omap_devinit_smartreflex();
+	}
 
 #ifdef CONFIG_SUSPEND
 	suspend_set_ops(&omap_pm_ops);
Signature · 1 line
1.7.0.4
#1 lore raw

RFC PATCH v2 1/1 ARM: OMAP2+: PM: Register suspend ops even in the presence of DT blob

Quoted 8 lines · Vaibhav Bedia

As per the comment in omap2_common_late_init() looks like the original intent of the DT check was to treat only the PMIC and SR initialization differently. Recent changes to consolidate the suspend-resume code across OMAP3/4 resulted into the registration of suspend ops also being dependent on the check for DT blob. Since the suspend-resume operation should not really be dependent on the usage of DT remove this dependency by wrapping the PMIC and SR init under the DT check.

So I am guessing you also tested suspend/resume on your hardware with this patch, when booting with a DT blob, and it passed.

Quoted 45 lines

---
v2->v1
- As suggested by Paul, Instead of moving around the suspend ops registration just wrap the PMIC and SR init under the DT check.
- Fixed up Kevin's email address :\

  arch/arm/mach-omap2/pm.c |   21 ++++++++++-----------
  1 files changed, 10 insertions(+), 11 deletions(-)
arch/arm/mach-omap2/pm.c +10 -11
index 9cb5ced..46848f7 100644
@@ -302,19 +302,18 @@ int __init omap2_common_pm_late_init(void)
  	 * a completely different mechanism.
  	 * Disable this part if a DT blob is available.
  	 */
-	if (of_have_populated_dt())
-		return 0;
+	if (!of_have_populated_dt()) {
+		/* Init the voltage layer */
+		omap_pmic_late_init();
+		omap_voltage_late_init();
 
-	/* Init the voltage layer */
-	omap_pmic_late_init();
-	omap_voltage_late_init();
+		/* Initialize the voltages */
+		omap3_init_voltages();
+		omap4_init_voltages();
 
-	/* Initialize the voltages */
-	omap3_init_voltages();
-	omap4_init_voltages();
-
-	/* Smartreflex device init */
-	omap_devinit_smartreflex();
+		/* Smartreflex device init */
+		omap_devinit_smartreflex();
+	}
 
  #ifdef CONFIG_SUSPEND
  	suspend_set_ops(&omap_pm_ops);
#2 lore raw

RFC PATCH v2 1/1 ARM: OMAP2+: PM: Register suspend ops even in the presence of DT blob

Quoted 10 lines · Rajendra
Quoted 8 lines · Vaibhav Bedia

As per the comment in omap2_common_late_init() looks like the original intent of the DT check was to treat only the PMIC and SR initialization differently. Recent changes to consolidate the suspend-resume code across OMAP3/4 resulted into the registration of suspend ops also being dependent on the check for DT blob. Since the suspend-resume operation should not really be dependent on the usage of DT remove this dependency by wrapping the PMIC and SR init under the DT check.

Quoted 3 lines

So I am guessing you also tested suspend/resume on your hardware with this patch, when booting with a DT blob, and it passed.

I am getting there ;)

I am in the process of getting a real suspend/resume functional. As part of this I was just flow flushing the whole suspend process in the mainline kernel by putting in a dummy implementation for the .enter op and found that this change was needed.

Regards,
Vaibhav

#3 lore raw

RFC PATCH v2 1/1 ARM: OMAP2+: PM: Register suspend ops even in the presence of DT blob

Quoted 15 lines · Vaibhav
Quoted 10 lines · Rajendra
Quoted 8 lines · Vaibhav Bedia

As per the comment in omap2_common_late_init() looks like the original intent of the DT check was to treat only the PMIC and SR initialization differently. Recent changes to consolidate the suspend-resume code across OMAP3/4 resulted into the registration of suspend ops also being dependent on the check for DT blob. Since the suspend-resume operation should not really be dependent on the usage of DT remove this dependency by wrapping the PMIC and SR init under the DT check.

Quoted 3 lines

So I am guessing you also tested suspend/resume on your hardware with this patch, when booting with a DT blob, and it passed.

Quoted 6 lines

I am getting there ;)

I am in the process of getting a real suspend/resume functional. As part of this I was just flow flushing the whole suspend process in the mainline kernel by putting in a dummy implementation for the .enter op and found that this change was needed.

Ok, good to know you are working on this. It might then make sense for for you to also include this patch as part of that series? It doesn't make much sense to register suspend ops which then break suspend isn't it? Btw, whats the behavior on current mainline with just this patch. Does it hang/crash or just prevent the system from doing down?

Quoted 3 lines

Regards,
Vaibhav

#4 lore raw

RFC PATCH v2 1/1 ARM: OMAP2+: PM: Register suspend ops even in the presence of DT blob

Quoted 23 lines · Rajendra
Quoted 15 lines · Vaibhav
Quoted 10 lines · Rajendra
Quoted 8 lines · Vaibhav Bedia

As per the comment in omap2_common_late_init() looks like the original intent of the DT check was to treat only the PMIC and SR initialization differently. Recent changes to consolidate the suspend-resume code across OMAP3/4 resulted into the registration of suspend ops also being dependent on the check for DT blob. Since the suspend-resume operation should not really be dependent on the usage of DT remove this dependency by wrapping the PMIC and SR init under the DT check.

Quoted 3 lines

So I am guessing you also tested suspend/resume on your hardware with this patch, when booting with a DT blob, and it passed.

Quoted 6 lines

I am getting there ;)

I am in the process of getting a real suspend/resume functional. As part of this I was just flow flushing the whole suspend process in the mainline kernel by putting in a dummy implementation for the .enter op and found that this change was needed.

Quoted 7 lines

Ok, good to know you are working on this. It might then make sense for for you to also include this patch as part of that series? It doesn't make much sense to register suspend ops which then break suspend isn't it? Btw, whats the behavior on current mainline with just this patch. Does it hang/crash or just prevent the system from doing down?

Ok. I can keep this patch as part of the suspend-resume support for AM33XX.

With just this patch on top of the tree being maintained by Vaibhav Hiremath [1] I found a couple of issues that I am yet to fully root-cause.

1. The PM_SUSPEND_PREPARE handler in drivers/mmc/core/core.c never returns unless the MMC_UNSAFE_RESUME config option is selected. This might be due to the suspend and resume in mmc_bus_ops being NULL in the case where the config option is not set.

2. The suspend process hangs somewhere in the .suspend_noirq callback for the UART being used as the console when no_console_suspend is passed.

I am yet to check the behavior on an OMAP3 EVM so right now I am not sure whether these are AM33XX specific issues. If you have any pointers that will greatly help.

Regards,
Vaibhav B.

[1] https://github.com/hvaibhav/am335x-linux/tree/am335x-upstream-staging