Re: [PATCH] CPUFreq Support for PXA255

[Dominik Brodowski <linux@dominikbrodowski.net>]

Hi,

On Tue, Jul 19, 2005 at 11:15:02AM +0100, Ian Campbell wrote:
> +static int pxa_set_target(struct cpufreq_policy *policy,
> +			  unsigned int target_freq, unsigned int relation)
> +{
> +	int idx;
> +	struct cpufreq_freqs freqs;
> +	struct pxa_freqs *pxa_freq_settings;
> +	struct cpufreq_frequency_table *pxa_freqs_table;
> +	unsigned long flags;
> +	unsigned int unused;
> +	unsigned int preset_mdrefr, postset_mdrefr;
> +	void *ramstart;
> +
> +	/* Get the current policy */
> +	pxa_select_freq_table(policy, &pxa_freq_settings, &pxa_freqs_table);

In my opinion you should only switch between the tables in verify. Only then
can we assure that the ->min and ->max values provided are OK at this
moment.

> +	/* set default policy and cpuinfo */
> +	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
> +	policy->cpuinfo.max_freq = max_frequency;
> +	policy->cpuinfo.min_freq = PXA25x_MIN_FREQ;
> +	policy->cpuinfo.transition_latency = 1000000;	/* FIXME: 1 ms, assumed */
> +	policy->cur = get_clk_frequency_khz(0);		/* current freq */
> +	policy->min = policy->max = policy->cur;

Better set policy->min to cpuinfo.min_freq and ->max to cpuinfo.max_freq.

> +static struct freq_attr* pxa_cpufreq_attr[] = {
> +        &cpufreq_freq_attr_scaling_available_freqs,

Tab, not space


Looks good, thanks!

	Dominik