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

Signed-off-by: Vaibhav Bedia <redacted>
---
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(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 9cb5ced..46848f7 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c

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

-- 
1.7.0.4

On Thursday 19 July 2012 05:38 PM, Vaibhav Bedia wrote:

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.

On Fri, Jul 20, 2012 at 10:09:43, Nayak, Rajendra wrote:

On Thursday 19 July 2012 05:38 PM, Vaibhav Bedia wrote:

quoted

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.

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

On Friday 20 July 2012 10:31 AM, Bedia, Vaibhav wrote:

On Fri, Jul 20, 2012 at 10:09:43, Nayak, Rajendra wrote:

quoted

On Thursday 19 July 2012 05:38 PM, Vaibhav Bedia wrote:

quoted

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.

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?

Regards,
Vaibhav

On Fri, Jul 20, 2012 at 10:42:53, Nayak, Rajendra wrote:

On Friday 20 July 2012 10:31 AM, Bedia, Vaibhav wrote:

quoted

On Fri, Jul 20, 2012 at 10:09:43, Nayak, Rajendra wrote:

quoted

On Thursday 19 July 2012 05:38 PM, Vaibhav Bedia wrote:

quoted

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.

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?

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