diff mbox

[POWER,DOMAIN,suspend,callbacks] Observation.

Message ID 871uwcjbwi.fsf@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kevin Hilman Aug. 23, 2011, 5:06 p.m. UTC
Hi Santosh,

Santosh <santosh.shilimkar@ti.com> writes:

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

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

> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Santosh Shilimkar Aug. 24, 2011, 6:17 a.m. UTC | #1
On Tuesday 23 August 2011 10:36 PM, Kevin Hilman wrote:
> Hi Santosh,
>
> Santosh<santosh.shilimkar@ti.com>  writes:
>
>> 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.
>
>> 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.
>
>> 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.
>
The patch works like charm.

> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Aug. 24, 2011, 11:38 p.m. UTC | #2
Santosh <santosh.shilimkar@ti.com> writes:

> On Tuesday 23 August 2011 10:36 PM, Kevin Hilman wrote:
>> Hi Santosh,
>>
>> Santosh<santosh.shilimkar@ti.com>  writes:
>>
>>> 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.
>>
>>> 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.
>>
>>> 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.
>>
> The patch works like charm.

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

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index d8f2299..7a0d248 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -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,
 	}
 };