Skip to contentSkip to conversation

Observation.

POWER DOMAIN suspend callbacks Santosh 3 participants 9 messages Aug 23, 2011 – Aug 24, 2011

Subsystems (2)
Arm (32-bit) Arm sub-architectures
lore
Share & tools
lore

Santosh

author
#0 lore raw

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Who is suppose to call the driver suspend callback? Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Thanks for clarification.

Regards
Santosh

#1 lore raw
Quoted 9 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Quoted 7 lines

Who is suppose to call the driver suspend callback? Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Looks like from

_od_suspend_noirq

      -> pm_generic_suspend_noirq
             -> __pm_generic_call(dev, PM_EVENT_SUSPEND, true);
                  -->  [..]
                      case PM_EVENT_SUSPEND:
                      callback = noirq ? pm->suspend_noirq : pm->suspend;

shouldn't we call pm_generic_suspend from _od_suspend_noirq rather than calling pm_generic_suspend_noirq?

Since _noirq seems to call .suspend_noirq hook which most of the them have not populated.

pm_generic_suspend* calls may fail to call legacy platform .suspend hooks?

Looks like we need below change for driver .suspend hook to be called.

arch/arm/plat-omap/omap_device.c +4 -4
index b6b4097..c2a18ae 100644
@@ -584,9 +584,9 @@ static int _od_suspend_noirq(struct device *dev)
        int ret;
 
        if (od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND)
-               return pm_generic_suspend_noirq(dev);
+               return pm_generic_suspend(dev);
 
-       ret = pm_generic_suspend_noirq(dev);
+       ret = pm_generic_suspend(dev);
 
        if (!ret && !pm_runtime_status_suspended(dev)) {
                if (pm_generic_runtime_suspend(dev) == 0) {

@@ -604,7 +604,7 @@ static int _od_resume_noirq(struct device *dev)
        struct omap_device *od = to_omap_device(pdev);
 
        if (od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND)
-               return pm_generic_resume_noirq(dev);
+               return pm_generic_resume(dev);
 
        if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
            !pm_runtime_status_suspended(dev)) {

@@ -613,7 +613,7 @@ static int _od_resume_noirq(struct device *dev)
                pm_generic_runtime_resume(dev);
        }
 
-       return pm_generic_resume_noirq(dev);
+       return pm_generic_resume(dev);
 }
 #endif

--
Thanks,
Govindraj.R

#2 lore raw
In reply to Santosh
Quoted 9 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Quoted 7 lines

Who is suppose to call the driver suspend callback? Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Looks like from

_od_suspend_noirq

      -> pm_generic_suspend_noirq
             -> __pm_generic_call(dev, PM_EVENT_SUSPEND, true);
                  -->  [..]
                      case PM_EVENT_SUSPEND:
                      callback = noirq ? pm->suspend_noirq : pm->suspend;

shouldn't we call pm_generic_suspend from _od_suspend_noirq rather than calling pm_generic_suspend_noirq?

Since _noirq seems to call .suspend_noirq hook which most of the them have not populated.

pm_generic_suspend* calls may fail to call legacy platform .suspend hooks?

Looks like we need below change for driver .suspend hook to be called.

arch/arm/plat-omap/omap_device.c +4 -4
index b6b4097..c2a18ae 100644
@@ -584,9 +584,9 @@ static int _od_suspend_noirq(struct device *dev)
        int ret;
 
        if (od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND)
-               return pm_generic_suspend_noirq(dev);
+               return pm_generic_suspend(dev);
 
-       ret = pm_generic_suspend_noirq(dev);
+       ret = pm_generic_suspend(dev);
 
        if (!ret && !pm_runtime_status_suspended(dev)) {
                if (pm_generic_runtime_suspend(dev) == 0) {

@@ -604,7 +604,7 @@ static int _od_resume_noirq(struct device *dev)
        struct omap_device *od = to_omap_device(pdev);
 
        if (od->flags & OMAP_DEVICE_NO_IDLE_ON_SUSPEND)
-               return pm_generic_resume_noirq(dev);
+               return pm_generic_resume(dev);
 
        if ((od->flags & OMAP_DEVICE_SUSPENDED) &&
            !pm_runtime_status_suspended(dev)) {

@@ -613,7 +613,7 @@ static int _od_resume_noirq(struct device *dev)
                pm_generic_runtime_resume(dev);
        }
 
-       return pm_generic_resume_noirq(dev);
+       return pm_generic_resume(dev);
 }
 #endif

--
Thanks,
Govindraj.R

#3 lore raw
In reply to Santosh

Hi Santosh,

Quoted 8 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Correct.

Quoted 1 line

Who is suppose to call the driver suspend callback?

If populated, the PM domain callbacks should call the driver callbacks. If there are no PM domain callbacks, then the subsystem (in this case, the platform_bus) should be calling the driver callbacks.

Quoted 5 lines

Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Yup, there's a bug. They're not missing, just misplaced. ;)

When adding the noirq callbacks to ensure devices are idled late in suspend by omap_device, I the patch commited mistakenly uses SET_SYSTEM_SLEEP_PM_OPS(), which sets the "normal" suspend/resume handlers and not the noirq handlers.

Can you try the patch below? I only briefly tested it on omap3/n900 so far.

This populates most of the PM domain methods with the same ones used by the subystem (platform_bus) and only overrides the noirq methods with custom versions. This patch should make all the driver's suspend/resume methods be called as expected.

After a bit more sanitiy testing, I'll post a real patch for the -rc series.

Kevin

arch/arm/plat-omap/omap_device.c +2 -1
index d8f2299..7a0d248 100644
@@ -626,7 +626,8 @@ static struct dev_pm_domain omap_device_pm_domain = {
 		SET_RUNTIME_PM_OPS(_od_runtime_suspend, _od_runtime_resume,
 				   _od_runtime_idle)
 		USE_PLATFORM_PM_SLEEP_OPS
-		SET_SYSTEM_SLEEP_PM_OPS(_od_suspend_noirq, _od_resume_noirq)
+		.suspend_noirq = _od_suspend_noirq,
+		.resume_noirq = _od_resume_noirq,
 	}
 };
#4 lore raw
In reply to Santosh

Hi Santosh,

Quoted 8 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Correct.

Quoted 1 line

Who is suppose to call the driver suspend callback?

If populated, the PM domain callbacks should call the driver callbacks. If there are no PM domain callbacks, then the subsystem (in this case, the platform_bus) should be calling the driver callbacks.

Quoted 5 lines

Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Yup, there's a bug. They're not missing, just misplaced. ;)

When adding the noirq callbacks to ensure devices are idled late in suspend by omap_device, I the patch commited mistakenly uses SET_SYSTEM_SLEEP_PM_OPS(), which sets the "normal" suspend/resume handlers and not the noirq handlers.

Can you try the patch below? I only briefly tested it on omap3/n900 so far.

This populates most of the PM domain methods with the same ones used by the subystem (platform_bus) and only overrides the noirq methods with custom versions. This patch should make all the driver's suspend/resume methods be called as expected.

After a bit more sanitiy testing, I'll post a real patch for the -rc series.

Kevin

arch/arm/plat-omap/omap_device.c +2 -1
index d8f2299..7a0d248 100644
@@ -626,7 +626,8 @@ static struct dev_pm_domain omap_device_pm_domain = {
 		SET_RUNTIME_PM_OPS(_od_runtime_suspend, _od_runtime_resume,
 				   _od_runtime_idle)
 		USE_PLATFORM_PM_SLEEP_OPS
-		SET_SYSTEM_SLEEP_PM_OPS(_od_suspend_noirq, _od_resume_noirq)
+		.suspend_noirq = _od_suspend_noirq,
+		.resume_noirq = _od_resume_noirq,
 	}
 };
author
#5 lore raw
Quoted 29 lines · Kevin Hilman

Hi Santosh,

Quoted 8 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Correct.

Quoted 1 line

Who is suppose to call the driver suspend callback?

If populated, the PM domain callbacks should call the driver callbacks. If there are no PM domain callbacks, then the subsystem (in this case, the platform_bus) should be calling the driver callbacks.

Quoted 5 lines

Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Yup, there's a bug. They're not missing, just misplaced. ;)

Quoted 8 lines

When adding the noirq callbacks to ensure devices are idled late in suspend by omap_device, I the patch commited mistakenly uses SET_SYSTEM_SLEEP_PM_OPS(), which sets the "normal" suspend/resume handlers and not the noirq handlers.

Can you try the patch below? I only briefly tested it on omap3/n900 so far.

The patch works like charm.

Quoted 8 lines

This populates most of the PM domain methods with the same ones used by the subystem (platform_bus) and only overrides the noirq methods with custom versions. This patch should make all the driver's suspend/resume methods be called as expected.

After a bit more sanitiy testing, I'll post a real patch for the -rc series.

Great.

Regards
Santosh

author
#6 lore raw
In reply to Kevin Hilman
Quoted 29 lines · Kevin Hilman

Hi Santosh,

Quoted 8 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Correct.

Quoted 1 line

Who is suppose to call the driver suspend callback?

If populated, the PM domain callbacks should call the driver callbacks. If there are no PM domain callbacks, then the subsystem (in this case, the platform_bus) should be calling the driver callbacks.

Quoted 5 lines

Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Yup, there's a bug. They're not missing, just misplaced. ;)

Quoted 8 lines

When adding the noirq callbacks to ensure devices are idled late in suspend by omap_device, I the patch commited mistakenly uses SET_SYSTEM_SLEEP_PM_OPS(), which sets the "normal" suspend/resume handlers and not the noirq handlers.

Can you try the patch below? I only briefly tested it on omap3/n900 so far.

The patch works like charm.

Quoted 8 lines

This populates most of the PM domain methods with the same ones used by the subystem (platform_bus) and only overrides the noirq methods with custom versions. This patch should make all the driver's suspend/resume methods be called as expected.

After a bit more sanitiy testing, I'll post a real patch for the -rc series.

Great.

Regards
Santosh

#7 lore raw
Quoted 35 lines · Santosh
Quoted 34 lines · Kevin Hilman

Hi Santosh,

Quoted 8 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Correct.

Quoted 1 line

Who is suppose to call the driver suspend callback?

If populated, the PM domain callbacks should call the driver callbacks. If there are no PM domain callbacks, then the subsystem (in this case, the platform_bus) should be calling the driver callbacks.

Quoted 5 lines

Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Yup, there's a bug. They're not missing, just misplaced. ;)

When adding the noirq callbacks to ensure devices are idled late in suspend by omap_device, I the patch commited mistakenly uses SET_SYSTEM_SLEEP_PM_OPS(), which sets the "normal" suspend/resume handlers and not the noirq handlers.

Quoted 4 lines
Quoted 3 lines

Can you try the patch below? I only briefly tested it on omap3/n900 so far.

The patch works like charm.

Thanks, I'll add a tested-by for you.

Kevin

#8 lore raw
In reply to Santosh
Quoted 35 lines · Santosh
Quoted 34 lines · Kevin Hilman

Hi Santosh,

Quoted 8 lines · Santosh

Rafael, Kevin,

On latest kernel( V3.1-rc1+), the subsystem(driver) suspend callbacks are not getting called because power domain callbcaks are populated.

And as per commit 4d27e9dc{PM: Make power domain callbacks take precedence over subsystem ones}, it's expected bahavior.

Correct.

Quoted 1 line

Who is suppose to call the driver suspend callback?

If populated, the PM domain callbacks should call the driver callbacks. If there are no PM domain callbacks, then the subsystem (in this case, the platform_bus) should be calling the driver callbacks.

Quoted 5 lines

Some drivers/subsystem would have state machine which needs to be suspended.

Is the power domain suspend callback, suppose to take care of it ? If yes, then that seems to be missing for OMAP.

Yup, there's a bug. They're not missing, just misplaced. ;)

When adding the noirq callbacks to ensure devices are idled late in suspend by omap_device, I the patch commited mistakenly uses SET_SYSTEM_SLEEP_PM_OPS(), which sets the "normal" suspend/resume handlers and not the noirq handlers.

Quoted 4 lines
Quoted 3 lines

Can you try the patch below? I only briefly tested it on omap3/n900 so far.

The patch works like charm.

Thanks, I'll add a tested-by for you.

Kevin