From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chanwoo Choi Subject: Re: [PATCH v2 3/5] devfreq: add devfreq_suspend/resume() functions Date: Tue, 04 Dec 2018 15:19:32 +0900 Message-ID: <5C061C74.808@samsung.com> References: <1543847475-7600-1-git-send-email-l.luba@partner.samsung.com> <1543847475-7600-4-git-send-email-l.luba@partner.samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8BIT Return-path: In-reply-to: <1543847475-7600-4-git-send-email-l.luba@partner.samsung.com> Sender: linux-kernel-owner@vger.kernel.org To: Lukasz Luba , linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, devicetree@vger.kernel.org Cc: tjakobi@math.uni-bielefeld.de, myungjoo.ham@samsung.com, kyungmin.park@samsung.com, rjw@rjwysocki.net, len.brown@intel.com, pavel@ucw.cz, gregkh@linuxfoundation.org, keescook@chromium.org, anton@enomsg.org, ccross@android.com, tony.luck@intel.com, robh+dt@kernel.org, mark.rutland@arm.com, kgene@kernel.org, krzk@kernel.org, m.szyprowski@samsung.com, b.zolnierkie@samsung.com List-Id: devicetree@vger.kernel.org Hi Lukasz, On 2018년 12월 03일 23:31, Lukasz Luba wrote: > This patch adds implementation for global suspend/resume for > devfreq framework. System suspend will next use these functions. > > The patch is based on earlier work by Tobias Jakobi. Please remove it from each patch description. > > Suggested-by: Tobias Jakobi > Suggested-by: Chanwoo Choi > Signed-off-by: Lukasz Luba > --- > drivers/devfreq/devfreq.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/devfreq.h | 6 ++++++ > 2 files changed, 48 insertions(+) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 36bed24..7d60423 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -935,6 +935,48 @@ 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"); When I checked the cpufreq_suspend(), cpufreq_suspend() prints message as 'err' level. I think that dev_err is more proper than dev_warn. I'm not sure what is more correct log. But, 'devfreq->dev' device has the separate suspend/resume function. So, I think that devfreq_suspend() should print error log containing that it is error by devfreq framework. "device suspend failed" -> "failed to suspend devfreq device" > + } > + mutex_unlock(&devfreq_list_lock); > +} > + > +/** > + * 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; > + > + 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"); ditto. "device resume failed" -> "failed to resume devfreq device" > + } > + 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 d985199..fbffa74 100644 > --- a/include/linux/devfreq.h > +++ b/include/linux/devfreq.h > @@ -205,6 +205,9 @@ 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); > > +extern void devfreq_suspend(void); > +extern void devfreq_resume(void); > + > /** > * update_devfreq() - Reevaluate the device and configure frequency > * @devfreq: the devfreq device > @@ -331,6 +334,9 @@ static inline int devfreq_resume_device(struct devfreq *devfreq) > return 0; > } > > +static inline void devfreq_suspend(void) {} > +static inline void devfreq_resume(void) {} > + > static inline struct dev_pm_opp *devfreq_recommended_opp(struct device *dev, > unsigned long *freq, u32 flags) > { > -- Best Regards, Chanwoo Choi Samsung Electronics