Re: [PATCH] cpufreq: Make sure CPU is running on a freq from freq-table

["Rafael J. Wysocki" <rjw@rjwysocki.net>]

On Thursday, November 21, 2013 12:39:02 PM Viresh Kumar wrote:
> Sometimes boot loaders set CPU frequency to a value outside of frequency table
> present with cpufreq core. In such cases CPU might be unstable if it has to run
> on that frequency for long duration of time and so its better to set it to a
> frequency which is specified in freq-table. This also makes cpufreq stats
> inconsistent as cpufreq-stats would fail to register because current frequency
> of CPU isn't found in freq-table.
> 
> Because we don't want this change to effect boot process badly, we go for the
> next freq which is >= policy->cur ('cur' must be set by now, otherwise we will
> end up setting freq to lowest of the table as 'cur' is initialized to zero).
> 
> In case where CPU is already running on one of the frequencies present in
> freq-table, this would turn into a dummy call as __cpufreq_driver_target() would
> return early.
> 
> Reported-by: Carlos Hernandez <ceh@ti.com>
> Reported-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> After lots of discussion with Nishanth and others, I feel something like this..
> 
> @Nishanth: Please see if this works for you and I hope we don't need any of
> these patches anymore:
> 
> - https://lkml.org/lkml/2013/11/15/569 : cpufreq: cpufreq-cpu0: Use a sane boot
> frequency when booting with a mismatched bootloader configuration
> - https://lkml.org/lkml/2013/11/15/503 : cpufreq: stats: Do not populate stats
>   when policy->cur has no exact match
> - https://lkml.org/lkml/2013/11/19/16 : cpufreq/stats: Add "unknown" frequency
>   field in stats tables
> 
>  drivers/cpufreq/cpufreq.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 02d534d..d55c843 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1038,6 +1038,32 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif,
>  		}
>  	}
>  
> +	/*
> +	 * Sometimes boot loaders set CPU frequency to a value outside of
> +	 * frequency table present with cpufreq core. In such cases CPU might be
> +	 * unstable if it has to run on that frequency for long duration of time
> +	 * and so its better to set it to a frequency which is specified in
> +	 * freq-table. This also makes cpufreq stats inconsistent as
> +	 * cpufreq-stats would fail to register because current frequency of CPU
> +	 * isn't found in freq-table.
> +	 *
> +	 * Because we don't want this change to effect boot process badly, we go
> +	 * for the next freq which is >= policy->cur ('cur' must be set by now,
> +	 * otherwise we will end up setting freq to lowest of the table as 'cur'
> +	 * is initialized to zero).
> +	 *
> +	 * In case where CPU is already running on one of the frequencies
> +	 * present in freq-table, this would turn into a dummy call as
> +	 * __cpufreq_driver_target() would return early.
> +	 */
> +	if (has_target()) {
> +		ret = __cpufreq_driver_target(policy, policy->cur,
> +				CPUFREQ_RELATION_L);
> +		if (ret)
> +			pr_err("%s: Unable to set frequency from table: %d\n",
> +					__func__, ret);

Should we continue in that case?

> +	}
> +
>  	/* related cpus should atleast have policy->cpus */
>  	cpumask_or(policy->related_cpus, policy->related_cpus, policy->cpus);
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.