linux-kernel - Re: [PATCH v2 18/18] cpufreq: scmi: add support for fast frequency switching

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 9 Aug 2017 09:58:25 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Sudeep Holla <sudeep.holla@....com>
Cc:     ALKML <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        DTML <devicetree@...r.kernel.org>,
        Roy Franz <roy.franz@...ium.com>,
        Harb Abdulhamid <harba@...eaurora.org>,
        Nishanth Menon <nm@...com>, Arnd Bergmann <arnd@...db.de>,
        Loc Ho <lho@....com>, Alexey Klimov <alexey.klimov@....com>,
        Ryan Harkin <Ryan.Harkin@....com>,
        Jassi Brar <jassisinghbrar@...il.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>, linux-pm@...r.kernel.org
Subject: Re: [PATCH v2 18/18] cpufreq: scmi: add support for fast frequency
 switching

On 04-08-17, 15:31, Sudeep Holla wrote:
> The cpufreq core provides option for drivers to implement fast_switch
> callback which is invoked for frequency switching from interrupt context.
> 
> This patch adds support for fast_switch callback in SCMI cpufreq driver
> by making use of polling based SCMI transfer. It also sets the flag
> fast_switch_possible.
> 
> Cc: "Rafael J. Wysocki" <rjw@...ysocki.net>
> Cc: Viresh Kumar <viresh.kumar@...aro.org>
> Cc: linux-pm@...r.kernel.org
> Signed-off-by: Sudeep Holla <sudeep.holla@....com>
> ---
>  drivers/cpufreq/scmi-cpufreq.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> index 034359cafea5..cb1084cb1ef1 100644
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -61,6 +61,19 @@ scmi_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index)
>  	return perf_ops->freq_set(priv->handle, priv->domain_id, freq, false);
>  }
>  
> +static unsigned int scmi_cpufreq_fast_switch(struct cpufreq_policy *policy,
> +					     unsigned int target_freq)
> +{
> +	struct scmi_data *priv = policy->driver_data;
> +	struct scmi_perf_ops *perf_ops = priv->handle->perf_ops;
> +
> +	if (!perf_ops->freq_set(priv->handle, priv->domain_id,
> +				target_freq * 1000, true))
> +		return target_freq;
> +
> +	return CPUFREQ_ENTRY_INVALID;
> +}

This is very much similar to the target routine, perhaps we should write another
local routine which is used by both target and fast switch.

Do we guarantee that the frequency is changed by the time this routine returns?
Or we just send a SCMI request and return back ?

If we just send the request and don't wait for freq to get changed, what
protects another scmi_cpufreq_fast_switch() to get called before the first one
is finished? And what will happen on that call ?

>  static int
>  scmi_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask)
>  {
> @@ -164,6 +177,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)
>  
>  	policy->cpuinfo.transition_latency = latency;
>  
> +	policy->fast_switch_possible = true;
>  	return 0;
>  
>  out_free_cpufreq_table:
> @@ -180,6 +194,7 @@ static int scmi_cpufreq_exit(struct cpufreq_policy *policy)
>  {
>  	struct scmi_data *priv = policy->driver_data;
>  
> +	policy->fast_switch_possible = false;
>  	cpufreq_cooling_unregister(priv->cdev);
>  	dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table);
>  	dev_pm_opp_cpumask_remove_table(policy->related_cpus);
> @@ -228,6 +243,7 @@ static struct cpufreq_driver scmi_cpufreq_driver = {
>  	.init			= scmi_cpufreq_init,
>  	.exit			= scmi_cpufreq_exit,
>  	.ready			= scmi_cpufreq_ready,
> +	.fast_switch		= scmi_cpufreq_fast_switch,

Maybe add it right after target_index ?

-- 
viresh