Re: [PATCH] 3/3 Dynamic cpufreq governor and updates to ACPI P-state driver

[Dominik Brodowski <linux@brodo.de>]

A few comments on this patch.

> + *  Copyright (C)  2001 Russell King
> + *            (C)  2002 - 2003 Dominik Brodowski <linux@brodo.de>

As nothing in this driver is from me, please remove my copyright

> + * $Id:$

No need for an ID tag -- you can remove that as well.

> +#define DBS_RATE		(HZ/4)	/* timer rate is 250ms */
> +#define LOW_LATENCY_FREQUENCY_CHANGE_LIMIT 	100 /* in uS */
That's really low. latency_timer is in 10^(-9) s == ns [see cpufreq.h], so I 
think you're missing a factor of 1000 here -- but maybe some drivers miss a 
factor of 1000, too...

> +static int cpufreq_governor_dbs(struct cpufreq_policy *policy,
> +				   unsigned int event)
> +{
> +	unsigned int cpu = policy->cpu;
> +
> +	switch (event) {
> +	case CPUFREQ_GOV_START:
> +		if ((!cpu_online(cpu)) || (!try_module_get(THIS_MODULE)) || !policy->cur)
> +			return -EINVAL;

You needn't worry about try_module_get()/put_module() and friends. That's handled by
the cpufreq core already.


	Dominik