Message ID | 1371931874-3220-1-git-send-email-shuah.kh@samsung.com (mailing list archive) |
---|---|
State | Rejected, archived |
Headers | show |
On Saturday, June 22, 2013 02:11:14 PM Shuah Khan wrote: > Add a new interface get_pm_transition() to return pm_transition state. > This interface is intended to be used from dev_pm_ops class and type > suspend interfaces that call legacy pm ops driver suspend interfaces. > Legacy suspend pm_ops take pm_message_t as a parameter. > > e.g: drm_class_suspend() calls into driver suspend routines > via drm_dev->driver->suspend(drm_dev, state). > > Once drm_class_suspend() is converted to dev_pm_ops, it will no longer > have access to pm_transition which it has to pass into driver legacy > suspend calls. get_pm_transition() interface addresses this need. That shouldn't be necessary because each transition has its own callback in strict dev_pm_ops. Thanks, Rafael > Signed-off-by: Shuah Khan <shuah.kh@samsung.com> > --- > drivers/base/power/main.c | 17 +++++++++++++++++ > include/linux/pm.h | 3 +++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > index c2132b8..3e2e09d 100644 > --- a/drivers/base/power/main.c > +++ b/drivers/base/power/main.c > @@ -57,6 +57,23 @@ static pm_message_t pm_transition; > static int async_error; > > /** > + * get_pm_transition - return pm_transition state. > + * This interface is intended to be used from dev_pm_ops class and type > + * suspend interfaces that call legacy pm ops driver suspend interfaces. > + * Legacy suspend pm_ops take pm_message_t as a parameter. > + * e.g: drm_class_suspend() calls into driver suspend routines > + * via drm_dev->driver->suspend(drm_dev, state). > + * Once drm_class_suspend() is converted to dev_pm_ops, it will no longer > + * have access to pm_transition which it has to pass into driver legacy > + * suspend calls. get_pm_transition() interface addresses this need. > +*/ > +pm_message_t get_pm_transition() > +{ > + return pm_transition; > +} > +EXPORT_SYMBOL_GPL(get_pm_transition); > + > +/** > * device_pm_sleep_init - Initialize system suspend-related device fields. > * @dev: Device object being initialized. > */ > diff --git a/include/linux/pm.h b/include/linux/pm.h > index a224c7f..46f3252 100644 > --- a/include/linux/pm.h > +++ b/include/linux/pm.h > @@ -63,6 +63,9 @@ typedef struct pm_message { > int event; > } pm_message_t; > > +/* drivers/base/power/main.c */ > +extern pm_message_t get_pm_transition(void); > + > /** > * struct dev_pm_ops - device PM callbacks > * >
On 06/22/2013 03:17 PM, Rafael J. Wysocki wrote: > On Saturday, June 22, 2013 02:11:14 PM Shuah Khan wrote: >> Add a new interface get_pm_transition() to return pm_transition state. >> This interface is intended to be used from dev_pm_ops class and type >> suspend interfaces that call legacy pm ops driver suspend interfaces. >> Legacy suspend pm_ops take pm_message_t as a parameter. >> >> e.g: drm_class_suspend() calls into driver suspend routines >> via drm_dev->driver->suspend(drm_dev, state). >> >> Once drm_class_suspend() is converted to dev_pm_ops, it will no longer >> have access to pm_transition which it has to pass into driver legacy >> suspend calls. get_pm_transition() interface addresses this need. > > That shouldn't be necessary because each transition has its own callback > in strict dev_pm_ops. > > Thanks, > Rafael > Yes that is correct that there is no need pass in or know pm_transition with dev_pm_ops. The issue I am running into is the legacy pm_ops class suspend/resume routines call. In the example, I mentioned in my changelog, drm_class_suspend() calls legacy pm_ops and passes in state. It passes in the state to the driver legacy suspend routine. I have seen code paths in drivers that differentiate between PM_EVENT_FREEZE, PM_EVENT_SUSPEND, PM_EVENT_SLEEP etc. I considered passing PM_EVENT_SUSPEND to drm_dev->driver->suspend(drm_dev, state) from drm_class_suspend() which would eliminate the need for this new interface. However, I am concerned about breaking driver legacy suspend routines that key of off the state to execute different code paths for PM_EVENT_FREEZE vs. PM_EVENT_SUSPEND. Suspend routines get called when state is is PM_EVENT_FREEZE based on my testing. I would rather not add a new interface. Hoping you will have another idea on how to pass in the state to legacy suspend/resume without adding this new interface. My thinking is that this new interface is temporary measure until all of the legacy suspend routines get converted to dev_pm_ops and at the tile legacy interface gets removed, this new interface can go away as well. thanks, -- Shuah Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research America (Silicon Valley) shuah.kh@samsung.com | (970) 672-0658 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, June 24, 2013 04:20:06 PM Shuah Khan wrote: > On 06/22/2013 03:17 PM, Rafael J. Wysocki wrote: > > On Saturday, June 22, 2013 02:11:14 PM Shuah Khan wrote: > >> Add a new interface get_pm_transition() to return pm_transition state. > >> This interface is intended to be used from dev_pm_ops class and type > >> suspend interfaces that call legacy pm ops driver suspend interfaces. > >> Legacy suspend pm_ops take pm_message_t as a parameter. > >> > >> e.g: drm_class_suspend() calls into driver suspend routines > >> via drm_dev->driver->suspend(drm_dev, state). > >> > >> Once drm_class_suspend() is converted to dev_pm_ops, it will no longer > >> have access to pm_transition which it has to pass into driver legacy > >> suspend calls. get_pm_transition() interface addresses this need. > > > > That shouldn't be necessary because each transition has its own callback > > in strict dev_pm_ops. > > > > Thanks, > > Rafael > > > > Yes that is correct that there is no need pass in or know pm_transition > with dev_pm_ops. The issue I am running into is the legacy pm_ops class > suspend/resume routines call. In the example, I mentioned in my > changelog, drm_class_suspend() calls legacy pm_ops and passes in state. > It passes in the state to the driver legacy suspend routine. I have seen > code paths in drivers that differentiate between PM_EVENT_FREEZE, > PM_EVENT_SUSPEND, PM_EVENT_SLEEP etc. > > I considered passing PM_EVENT_SUSPEND to > drm_dev->driver->suspend(drm_dev, state) from drm_class_suspend() which > would eliminate the need for this new interface. However, I am concerned > about breaking driver legacy suspend routines that key of off the state > to execute different code paths for PM_EVENT_FREEZE vs. > PM_EVENT_SUSPEND. Suspend routines get called when state is is > PM_EVENT_FREEZE based on my testing. > > I would rather not add a new interface. Hoping you will have another > idea on how to pass in the state to legacy suspend/resume without adding > this new interface. My thinking is that this new interface is temporary > measure until all of the legacy suspend routines get converted to > dev_pm_ops and at the tile legacy interface gets removed, this new > interface can go away as well. There are multiple bus types with legacy callbacks. PCI and platform for two examples and USB does something similar. Please have a look at these. Thanks, Rafael
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index c2132b8..3e2e09d 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -57,6 +57,23 @@ static pm_message_t pm_transition; static int async_error; /** + * get_pm_transition - return pm_transition state. + * This interface is intended to be used from dev_pm_ops class and type + * suspend interfaces that call legacy pm ops driver suspend interfaces. + * Legacy suspend pm_ops take pm_message_t as a parameter. + * e.g: drm_class_suspend() calls into driver suspend routines + * via drm_dev->driver->suspend(drm_dev, state). + * Once drm_class_suspend() is converted to dev_pm_ops, it will no longer + * have access to pm_transition which it has to pass into driver legacy + * suspend calls. get_pm_transition() interface addresses this need. +*/ +pm_message_t get_pm_transition() +{ + return pm_transition; +} +EXPORT_SYMBOL_GPL(get_pm_transition); + +/** * device_pm_sleep_init - Initialize system suspend-related device fields. * @dev: Device object being initialized. */ diff --git a/include/linux/pm.h b/include/linux/pm.h index a224c7f..46f3252 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -63,6 +63,9 @@ typedef struct pm_message { int event; } pm_message_t; +/* drivers/base/power/main.c */ +extern pm_message_t get_pm_transition(void); + /** * struct dev_pm_ops - device PM callbacks *
Add a new interface get_pm_transition() to return pm_transition state. This interface is intended to be used from dev_pm_ops class and type suspend interfaces that call legacy pm ops driver suspend interfaces. Legacy suspend pm_ops take pm_message_t as a parameter. e.g: drm_class_suspend() calls into driver suspend routines via drm_dev->driver->suspend(drm_dev, state). Once drm_class_suspend() is converted to dev_pm_ops, it will no longer have access to pm_transition which it has to pass into driver legacy suspend calls. get_pm_transition() interface addresses this need. Signed-off-by: Shuah Khan <shuah.kh@samsung.com> --- drivers/base/power/main.c | 17 +++++++++++++++++ include/linux/pm.h | 3 +++ 2 files changed, 20 insertions(+)