linux-kernel - RE: [RESEND PATCH v3 1/2] PM/devfreq: add suspend frequency support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Tue, 08 Nov 2016 02:06:58 +0000
From:   MyungJoo Ham <myungjoo.ham@...sung.com>
To:     Lin Huang <hl@...k-chips.com>
Cc:     Chanwoo Choi <cw00.choi@...sung.com>,
        dianders@...omium.org <dianders@...omium.org>,
        linux-rockchip@...ts.infradead.org 
        <linux-rockchip@...ts.infradead.org>,
        linux-pm@...r.kernel.org 
        <linux-pm@...r.kernel.org>,
        dbasehore@...omium.org 
        <dbasehore@...omium.org>,
        linux-arm-kernel@...ts.infradead.org 
        <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel@...r.kernel.org 
        <linux-kernel@...r.kernel.org>
Subject: RE: [RESEND PATCH v3 1/2] PM/devfreq: add suspend frequency support

> Add suspend frequency support and if needed set it to
> the frequency obtained from the suspend opp (can be defined
> using opp-v2 bindings and is optional).
> 
> Signed-off-by: Lin Huang <hl@...k-chips.com>
> ---
> Changes in v2:
> - use update_devfreq() instead devfreq_update_status()
> Changes in v3:
> - fix build error
> 
>  drivers/devfreq/devfreq.c                 | 18 ++++++++++++++++--
>  drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
>  include/linux/devfreq.h                   |  8 ++++++++
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 
>  
[]
> +void devfreq_opp_get_suspend_opp(struct device *dev, struct devfreq *devfreq)
> +{
> +	struct dev_pm_opp *suspend_opp;
> +
> +	rcu_read_lock();
> +	suspend_opp = dev_pm_opp_get_suspend_opp(dev);
> +	if (suspend_opp)
> +		devfreq->suspend_freq = dev_pm_opp_get_freq(suspend_opp);
> +	rcu_read_unlock();
> +}
> +EXPORT_SYMBOL(devfreq_opp_get_suspend_opp);
> +

Why do we need this function?

This could be done at the init time (devfreq_add_device).

Plus, when it does not have dev_pm_opp_get_suspend_opp() available,

>  /**
>   * devfreq_register_opp_notifier() - Helper function to get devfreq notified
>   *				   for any changes in the OPP availability
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index ae72ba5..a3efee0 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -29,6 +29,15 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  	struct devfreq_simple_ondemand_data *data = df->data;
>  	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>  
> +	/*
> +	 * if devfreq in suspend status and have suspend_freq,
> +	 * the frequency need to set to suspend_freq
> +	 */
> +	if (df->stop_polling && df->suspend_freq) {
> +		*freq =	df->suspend_freq;
> +		return 0;
> +	}
> +

Why are you adding this? "Stop polling" does not necessarily mean that
we are doing suspend-to-RAM/disk. It may really mean just to stop at
the current frequency.


Cheers,
MyungJoo