Hi, On 2018년 11월 22일 03:01, Lukasz Luba wrote: > This patch adds implementation for global suspend/resume for > devfreq framework. System suspend will next use these functions. > > The patch draws on Tobias Jakobi's work posted ~2 years ago, who tried to > solve issue with devfreq device's frequency during suspend/resume. > During the discussion on LKML some corner cases and comments appeared > related to the design. This patch address them keeping in mind suggestions > from Chanwoo Choi.
Please remove the duplicate information about patch history. > > Suggested-by: Tobias Jakobi <[email protected]> > Suggested-by: Chanwoo Choi <[email protected]> > Signed-off-by: Lukasz Luba <[email protected]> > --- > drivers/devfreq/devfreq.c | 49 > +++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/devfreq.h | 7 +++++++ > 2 files changed, 56 insertions(+) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 7e09de8..2f4391c 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -44,6 +44,9 @@ static LIST_HEAD(devfreq_governor_list); > static LIST_HEAD(devfreq_list); > static DEFINE_MUTEX(devfreq_list_lock); > > +/* Flag showing state of suspend/resume */ > +static bool devfreq_suspended; Is it necessary? This patch just saves the 'true' or 'false' to this variable. And there are no any point to check the value of this variable. Please remove it. > + > /** > * find_device_devfreq() - find devfreq struct using device pointer > * @dev: device pointer used to lookup device devfreq. > @@ -938,6 +941,52 @@ int devfreq_resume_device(struct devfreq *devfreq) > EXPORT_SYMBOL(devfreq_resume_device); > > /** > + * devfreq_suspend() - Suspend devfreq governors and devices > + * > + * Called during system wide Suspend/Hibernate cycles for suspending > governors > + * and devices preserving the state for resume. On some platforms the devfreq > + * device must have precise state (frequency) after resume in order to > provide > + * fully operating setup. > + */ > +void devfreq_suspend(void) > +{ > + struct devfreq *devfreq; > + int ret; > + > + mutex_lock(&devfreq_list_lock); > + list_for_each_entry(devfreq, &devfreq_list, node) { > + ret = devfreq_suspend_device(devfreq); > + if (ret) > + dev_warn(&devfreq->dev, "device suspend failed\n"); > + } > + mutex_unlock(&devfreq_list_lock); > + > + devfreq_suspended = true; Remove it. > +} > + > +/** > + * devfreq_resume() - Resume devfreq governors and devices > + * > + * Called during system wide Suspend/Hibernate cycle for resuming governors > and > + * devices that are suspended with devfreq_suspend(). > + */ > +void devfreq_resume(void) > +{ > + struct devfreq *devfreq; > + int ret; > + > + devfreq_suspended = false; Remove it. > + > + mutex_lock(&devfreq_list_lock); > + list_for_each_entry(devfreq, &devfreq_list, node) { > + ret = devfreq_resume_device(devfreq); > + if (ret) > + dev_warn(&devfreq->dev, "device resume failed\n"); > + } > + mutex_unlock(&devfreq_list_lock); > +} > + > +/** > * devfreq_add_governor() - Add devfreq governor > * @governor: the devfreq governor to be added > */ > diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h > index 7fe96f9..4f0fea8 100644 > --- a/include/linux/devfreq.h > +++ b/include/linux/devfreq.h > @@ -202,6 +202,10 @@ extern void devm_devfreq_remove_device(struct device > *dev, > extern int devfreq_suspend_device(struct devfreq *devfreq); > extern int devfreq_resume_device(struct devfreq *devfreq); > > + Remove the blank line. > +extern void devfreq_suspend(void); > +extern void devfreq_resume(void); > + > /** > * update_devfreq() - Reevaluate the device and configure frequency > * @devfreq: the devfreq device > @@ -396,6 +400,9 @@ static inline int devfreq_update_stats(struct devfreq *df) > { > return -EINVAL; > } > + > +static inline void devfreq_suspend(void) {} > +static inline void devfreq_resume(void) {} You better to move the inline definitions under 'devfreq_resume_device' inline function. > #endif /* CONFIG_PM_DEVFREQ */ > > #endif /* __LINUX_DEVFREQ_H__ */ > -- Best Regards, Chanwoo Choi Samsung Electronics
