Re: Cpufreq for opteron
From: Dominik Brodowski (linux_at_brodo.de)
Date: 08/27/03
- Previous message: Chris Rankin: "Looking for tunable hardware parameters in 2.4.22"
- In reply to: Pavel Machek: "Re: Cpufreq for opteron"
- Next in thread: Christoph Hellwig: "Re: Cpufreq for opteron"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Wed, 27 Aug 2003 01:02:32 +0200 To: Pavel Machek <pavel@suse.cz>
On Fri, Aug 22, 2003 at 10:05:49PM +0200, Pavel Machek wrote:
> Hi!
>
> Here's an updated version. It is quite a bit shorter, and thats
> good. Does it look good enough for inclusion?
> +#define VERSION "version 1.00.06 - August 13, 2003"
If AMD wants it in, let's put it into the /* comment header at the top */ ?
> +#ifdef CONFIG_SMP
> +#error cpufreq support is disabled for config_smp
> +#endif
> +#ifdef CONFIG_PREEMPT
> +#warning noone tested this on preempt system, beware
> +#endif
Actually I doubt an #error and a #warning are necessary here. I'd like to
hear more about the CONFIG_SMP && CONFIG_CPU_FREQ issues; please discuss
this on cpufreq@www.linux.org.uk . The #warning is more of an assumption and
can go away, IMHO.
> +/*
> +The PSB table supplied by BIOS allows for the definition of the number of
> +p-states that can be used when running on a/c, and the number of p-states
> +that can be used when running on battery. This allows laptop manufacturers
> +to force the system to save power when running from battery. The relationship
> +is :
> + 1 <= number_of_battery_p_states <= maximum_number_of_p_states
> +
> +This driver does NOT have the support in it to detect transitions from
> +a/c power to battery power, and thus trigger the transition to a lower
> +p-state if required. This is because I need ACPI and the 2.6 kernel to do
> +this, and this is a 2.4 kernel driver. Check back for a new improved driver
> +for the 2.6 kernel soon.
> +
> +This code therefore assumes it is on battery at all times, and thus
> +restricts performance to number_of_battery_p_states. For desktops,
> + number_of_battery_p_states == maximum_number_of_pstates,
> +so this is not actually a restriction.
> +*/
> +
> +static u32 batterypstates; /* limit on the number of p states when on battery */
> + /* - set by BIOS in the PSB/PST */
> +
> +static int onbattery = 1; /* Set if running on battery, reset otherwise. */
> + /* Of no relevance unless batterypstates < */
> + /* numpstates, as defined in the PSB/PST. */
I dislike this interaction between ACPI and cpufreq -- it should be done
more generally. Let's discuss that seperately, though. And if the code is
dead code at the moment.
> + if (num_online_cpus() != 1) {
> + printk(KERN_ERR PFX "multiprocessor systems not currently supported\n");
> + return 0;
> + }
> +
> + if (c->x86_vendor != X86_VENDOR_AMD) {
> + printk(KERN_INFO PFX "Not an AMD processor\n");
Please don't be so verbose if it's such a clear failing reason. dprintk
would be enough.
> + printk(KERN_ERR PFX "AMD Athlon 64 or AMD Opteron processor required\n");
same here.
> +static int
> +drv_verify(struct cpufreq_policy *pol)
> +{
<snip>
> + dprintk(KERN_DEBUG PFX
> + "ver: cpu%d, min %d, max %d, cur %d, pol %d (%s)\n", pol->cpu,
> + pol->min, pol->max, pol->cur, pol->policy,
> + pol->policy ==
> + CPUFREQ_POLICY_POWERSAVE ? "psave" : pol->policy ==
> + CPUFREQ_POLICY_PERFORMANCE ? "perf" : "unk");
> +
> + if (pol->cpu != 0) {
> + printk(KERN_ERR PFX "verify - cpu not 0\n");
> + return -ENODEV;
> + }
> +
> + res = find_match(&targ, &min, &max,
> + pol->policy ==
> + CPUFREQ_POLICY_POWERSAVE ? SEARCH_DOWN : SEARCH_UP, 0,
> + 0);
Why do you check for CPUFREQ_POLICY here??? In a ->target class cpufreq
driver[*] you must not worry about the policy, only about min and max
frequency.
[*] ->target class cpufreq drivers [cpufreq_driver->target is used] have
specific operating frequencies.
->setpolicy class cpufreq drivers have operating frequency ranges
[currently only available on Transmeta Crusoe processors]
Also, it'd be great if this driver were converted to use the CPUfreq
freuqency table helpers -- check drivers/cpufreq/freq_table.c in
linux-2.6.0-test4. Using it makes the code much cleaner, easier to
understand, and less prone to errors.
Dominik
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Previous message: Chris Rankin: "Looking for tunable hardware parameters in 2.4.22"
- In reply to: Pavel Machek: "Re: Cpufreq for opteron"
- Next in thread: Christoph Hellwig: "Re: Cpufreq for opteron"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
|
