Hi Tobias,
I like your suggestion. But I have some comment on below.
On 2016년 11월 23일 22:51, Tobias Jakobi wrote:
> This suspend/resume operations works analogous to the
> cpufreq_{suspend,resume}() calls in the CPUFreq subsystem.
>
> Signed-off-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/devfreq/devfreq.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/devfreq.h | 10 +++++++
> 2 files changed, 85 insertions(+)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index bf3ea76..2f1aa83 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -785,6 +785,81 @@ int devfreq_resume_device(struct devfreq *devfreq)
> EXPORT_SYMBOL(devfreq_resume_device);
>
> /**
> + * devfreq_suspend() - Suspend DevFreq governors
> + *
> + * Called during system wide Suspend/Hibernate cycles for suspending governors
> + * in the same fashion as cpufreq_suspend().
> + */
> +void devfreq_suspend(void)
> +{
> + struct devfreq *devfreq;
> + unsigned long freq;
> + int ret;
> +
> + mutex_lock(&devfreq_list_lock);
> +
> + list_for_each_entry(devfreq, &devfreq_list, node) {
> + if (!devfreq->suspend_freq)
> + continue;
> +
> + ret = devfreq_suspend_device(devfreq);
> + if (ret < 0) {
> + dev_warn(&devfreq->dev, "%s: governor suspend failed\n", __func__);
> + continue;
> + }
> +
> + devfreq->resume_freq = 0;
> + if (devfreq->profile->get_cur_freq) {
> + ret = devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq);
> + if (ret >= 0)
> + devfreq->resume_freq = freq;
> + }
> +
> + freq = devfreq->suspend_freq;
> + ret = devfreq->profile->target(devfreq->dev.parent, &freq, 0);
DEVFREQ subsystem has the passive governor[2] and devfreq device using passive[1]
governor depends on the parent devfreq device. The passive governor uses
'DEVFREQ_TRANSITION_NOTIFIER' notifier to track the changed state of
parent devfreq device.
When changing the clock/voltage of parent devfreq device,
DEVFREQ subsystem have to notify the changed frequency
to the passive devfreq device.
So, you should call the devfreq_notifiy_transition()
before/after 'devfreq->profile->target' as following:
You can refer the update_devfreq() function
that is how to use devfreq_notifiy_transition().
For example,
struct devfreq_freq freqs;
if (devfreq->profile->get_cur_freq) {
ret = devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq);
if (ret >= 0)
devfreq->resume_freq = freq;
} else {
devfreq->resume_freq = devfreq_previous_freq;
}
freqs.old = devfreq->resume_freq;
freqs.new = devfreq->suspend_freq;
devfreq_notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE);
freq = devfreq->suspend_freq;
ret = devfreq->profile->target(devfreq->dev.parent, &freq, 0);
freqs.new = freq;
devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE);
if (ret < 0)
dev_warn(&devfreq->dev, "%s: setting suspend frequency failed\n", __func__);
}
[1] DEVFREQ_TRANSITION_NOTIFIER notifier
- https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0fe3a66410a3ba96679be903f1e287d7a0a264a9
[2] DEVFREQ passive governor
- https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=996133119f57334c38b020dbfaaac5b5eb127e29
> +
> + if (ret < 0)
> + dev_warn(&devfreq->dev, "%s: setting suspend frequency failed\n", __func__);
> + }
> +
> + mutex_unlock(&devfreq_list_lock);
> +}
> +
> +/**
> + * devfreq_resume() - Resume DevFreq governors
> + *
> + * Called during system wide Suspend/Hibernate cycle for resuming governors that
> + * are suspended with devfreq_suspend().
> + */
> +void devfreq_resume(void)
> +{
> + struct devfreq *devfreq;
> + unsigned long freq;
> + int ret;
> +
> + mutex_lock(&devfreq_list_lock);
> +
> + list_for_each_entry(devfreq, &devfreq_list, node) {
> + if (!devfreq->suspend_freq)
> + continue;
> +
> + freq = devfreq->resume_freq;
> + ret = devfreq->profile->target(devfreq->dev.parent, &freq, 0);
ditto.
> +
> + if (ret < 0) {
> + dev_warn(&devfreq->dev, "%s: setting resume frequency failed\n", __func__);
> + continue;
> + }
> +
> + ret = devfreq_resume_device(devfreq);
> + if (ret < 0)
> + dev_warn(&devfreq->dev, "%s: governor resume failed\n", __func__);
> + }
> +
> + 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 2de4e2e..555d024 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -171,6 +171,9 @@ struct devfreq {
> struct notifier_block nb;
> struct delayed_work work;
>
> + unsigned long suspend_freq; /* freq during devfreq suspend */
> + unsigned long resume_freq; /* freq restored after suspend cycle */
> +
> unsigned long previous_freq;
> struct devfreq_dev_status last_status;
>
> @@ -211,6 +214,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);
>
> +/* Suspend/resume the entire Devfreq subsystem. */
> +void devfreq_suspend(void);
> +void devfreq_resume(void);
> +
> /* Helper functions for devfreq user device driver with OPP. */
> extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
> unsigned long *freq, u32 flags);
> @@ -410,6 +417,9 @@ static inline int devfreq_update_stats(struct devfreq *df)
> {
> return -EINVAL;
> }
> +
> +static inline void devfreq_suspend(void) {}
> +static inline void devfreq_resume(void) {}
> #endif /* CONFIG_PM_DEVFREQ */
>
> #endif /* __LINUX_DEVFREQ_H__ */
>
--
Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
