cpufreq: allow user to specify voltage

cpufreq: allow user to specify voltage

[Pavel Machek]

Hi!

This allows user to specify voltage manually. This gives me 40 extra
minutes (1h50m -> 2h30m) on HP omnibook which appears to have broken
bios tables. Please apply,

								Pavel 
--- clean/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-02-15 18:51:11.000000000 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-02-18 17:36:29.000000000 +0100
@@ -210,7 +210,7 @@
 }
 
 
-static void change_speed (unsigned int index)
+static void change_speed (unsigned int index, unsigned int voltage)
 {
 	u8 fid, vid;
 	struct cpufreq_freqs freqs;
@@ -226,6 +226,14 @@
 	fid = powernow_table[index].index & 0xFF;
 	vid = (powernow_table[index].index & 0xFF00) >> 8;
 
+	if (voltage) {
+		int i;
+		for (i=0; i<32; i++)
+			if (mobile_vid_table[i] == voltage)
+				vid = i;
+		printk("Voltage overriden to %d mV, index 0x%x\n", voltage, vid);
+	}
+
 	freqs.cpu = 0;
 
 	rdmsrl (MSR_K7_FID_VID_STATUS, fidvidstatus.val);
@@ -338,7 +346,7 @@
 	if (cpufreq_frequency_table_target(policy, powernow_table, target_freq, relation, &newstate))
 		return -EINVAL;
 
-	change_speed(newstate);
+	change_speed(newstate, policy->voltage);
 
 	return 0;
 }
--- clean/drivers/cpufreq/proc_intf.c	2003-02-18 12:24:32.000000000 +0100
+++ linux/drivers/cpufreq/proc_intf.c	2003-02-18 14:23:48.000000000 +0100
@@ -28,6 +28,7 @@
 	unsigned int            min = 0;
 	unsigned int            max = 0;
 	unsigned int            cpu = 0;
+	unsigned int		voltage = 0;
 	char			str_governor[16];
 	struct cpufreq_policy   current_policy;
 	unsigned int            result = -EFAULT;
@@ -37,9 +38,24 @@
 
 	policy->min = 0;
 	policy->max = 0;
+	policy->voltage = 0;
 	policy->policy = 0;
 	policy->cpu = CPUFREQ_ALL_CPUS;
 
+	if (sscanf(input_string, "%d:%d:%d:%15s", &cpu, &min, &voltage, str_governor) == 4)
+	{
+		if (!strcmp(str_governor, "mVforce")) {
+			printk("Have request to go to %d mV\n", voltage);
+			policy->min = min;
+			policy->max = min;
+			policy->voltage = voltage;
+			policy->cpu = cpu;
+			result = 0;
+			policy->policy = CPUFREQ_POLICY_PERFORMANCE;
+			return 0;
+		}
+	}
+
 	if (sscanf(input_string, "%d:%d:%d:%15s", &cpu, &min, &max, str_governor) == 4) 
 	{
 		policy->min = min;
--- clean/include/linux/cpufreq.h	2003-02-18 12:24:38.000000000 +0100
+++ linux/include/linux/cpufreq.h	2003-02-18 12:25:10.000000000 +0100
@@ -63,6 +63,7 @@
 	unsigned int            min;    /* in kHz */
 	unsigned int            max;    /* in kHz */
         unsigned int            policy; /* see above */
+	unsigned int		voltage;/* in mV, 0 == trust bios */
 	struct cpufreq_governor *governor; /* see below */
 	struct cpufreq_cpuinfo  cpuinfo;     /* see above */
 	struct intf_data        intf;   /* interface data */
--- clean/kernel/cpufreq.c	2003-02-18 12:24:39.000000000 +0100
+++ linux/kernel/cpufreq.c	2003-02-18 17:24:04.000000000 +0100
@@ -1103,11 +1103,12 @@
 
 	cpufreq_driver->policy[policy->cpu].min    = policy->min;
 	cpufreq_driver->policy[policy->cpu].max    = policy->max;
+	cpufreq_driver->policy[policy->cpu].voltage= policy->voltage;
 
 #ifdef CONFIG_CPU_FREQ_24_API
 	cpu_cur_freq[policy->cpu] = policy->max;
 #endif
 
 	if (cpufreq_driver->setpolicy) {
 		cpufreq_driver->policy[policy->cpu].policy = policy->policy;
 		ret = cpufreq_driver->setpolicy(policy);

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: cpufreq: allow user to specify voltage

[Dominik Brodowski]

Hi Pavel, Hi Linus,

> Hi!
>
> This allows user to specify voltage manually. This gives me 40 extra
> minutes (1h50m -> 2h30m) on HP omnibook which appears to have broken
> bios tables. Please apply,
>
>								Pavel 


Please don't apply this patch -- for the following reasons:
- it only uses the deprecated, overloaded cpufreq proc_intf
- selecting the voltage within the policy (minimum and maximum frequency,
  mode of operation) is not where it should be done: you may want a
  different voltage at min-speed as at max-speed. So the frequency tables,
  or -even better- the amd-k7-specific table may be a better choice for
  this.
- selecting the voltage manually is something which is only valid for some
  very few drivers - so let's only export one sysfs file[*] for these
  drivers.

	Dominik

[*] sysfs is the _only_ recommended way to access cpufreq these days. 

Re: cpufreq: allow user to specify voltage

[Pavel Machek]

Hi!

> > This allows user to specify voltage manually. This gives me 40 extra
> > minutes (1h50m -> 2h30m) on HP omnibook which appears to have broken
> > bios tables. Please apply,
> 
> Please don't apply this patch -- for the following reasons:
> - it only uses the deprecated, overloaded cpufreq proc_intf
> - selecting the voltage within the policy (minimum and maximum frequency,
>   mode of operation) is not where it should be done: you may want a
>   different voltage at min-speed as at max-speed. So the frequency tables,
>   or -even better- the amd-k7-specific table may be a better choice for
>   this.

Well, look again; if you use mVforce, it forces *just one* frequency
(min=max=freq you specified), so it is actually okay.

But I agree that hacking proc_intf is not the right thing to do. I did
not see that sysfs access. Where is it?

> - selecting the voltage manually is something which is only valid for some
>   very few drivers - so let's only export one sysfs file[*] for these
>   drivers.

Very few drivers? It should be common for at least Intel and AMD, no?

									Pavel
-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: cpufreq: allow user to specify voltage

[Dominik Brodowski]

Hi!

On Tue, Feb 25, 2003 at 07:03:11PM +0100, Pavel Machek wrote:
> Hi!
> 
> Well, look again; if you use mVforce, it forces *just one* frequency
> (min=max=freq you specified), so it is actually okay.
But min=max=freq is awful... I know, I reccommended it sometimes, but with
the appearance of the userspace governor (already available on the cpufreq
homepage, http://www.brodo.de/cpufreq/advanced.html , will be submitted
soon) min=max=freq is deprecated.

> But I agree that hacking proc_intf is not the right thing to do. I did
> not see that sysfs access. Where is it?

kernel/cpufreq.c

> > - selecting the voltage manually is something which is only valid for some
> >   very few drivers - so let's only export one sysfs file[*] for these
> >   drivers.
> 
> Very few drivers? It should be common for at least Intel and AMD, no?
No. Intel doesn't let you select the exact voltage. IIRC, only vew 
VIA/Cyrix-longhaul-capable processors and, of course, powernow-k7 are vaild
targets for such a patch.

	Dominik

Re: cpufreq: allow user to specify voltage

[Pavel Machek]

Hi!

> > But I agree that hacking proc_intf is not the right thing to do. I did
> > not see that sysfs access. Where is it?
> 
> kernel/cpufreq.c

Okay, I see.

> > > - selecting the voltage manually is something which is only valid for some
> > >   very few drivers - so let's only export one sysfs file[*] for these
> > >   drivers.
> > 
> > Very few drivers? It should be common for at least Intel and AMD, no?
> No. Intel doesn't let you select the exact voltage. IIRC, only vew 
> VIA/Cyrix-longhaul-capable processors and, of course, powernow-k7 are vaild
> targets for such a patch.

So I guess adding /sys/bus/system/devices/cpu0/voltage? Should code to
do that be in kernel/cpufreq.c or is it possible to do sysfs from
powernow-k7 [it does not seem easy]?
								Pavel
-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: cpufreq: allow user to specify voltage

[John Clemens]

On Tue, 25 Feb 2003, Pavel Machek wrote:

> Hi!
>
> > > But I agree that hacking proc_intf is not the right thing to do. I did
> > > not see that sysfs access. Where is it?
> >
> > kernel/cpufreq.c
>
> Okay, I see.
>
> > > > - selecting the voltage manually is something which is only valid for some
> > > >   very few drivers - so let's only export one sysfs file[*] for these
> > > >   drivers.
> > >
> > > Very few drivers? It should be common for at least Intel and AMD, no?
> > No. Intel doesn't let you select the exact voltage. IIRC, only vew
> > VIA/Cyrix-longhaul-capable processors and, of course, powernow-k7 are vaild
> > targets for such a patch.
>
> So I guess adding /sys/bus/system/devices/cpu0/voltage? Should code to
> do that be in kernel/cpufreq.c or is it possible to do sysfs from
> powernow-k7 [it does not seem easy]?
 								Pavel
I agree, there shoul dbe a way to add sysfs files from a cpufreq driver
module.  I told dave I was looking into overriding the powernow tables,
but I can't seem to get enough time away from my day job right now.

for the powernow driver, and the userspace governor, I'd like to export a
file "current_setting" or something that contains:

<frequency> <voltage> <fsb? maybe for other drivers>

A write to this file of one, two, or three values would result in changing
the frequency to the closest standard table match we have.  Unless, the
user specifies an "override" flag as a module parameter.  If the override
flag is set, then writing to that file will set the speed and voltage to
exactly what you specify (within the min/max hardware limits), and
basically ignore the standard BIOS table.

simple, elegant, gives standard default behavior....but allows one to
override the tables if they -swear- they know better.  But it's really
tough to do without a way to add sysfs files from the driver itself.

john.c

-- 
John Clemens          http://www.deater.net/john
john@deater.net     ICQ: 7175925, IM: PianoManO8
      "I Hate Quotes" -- Samuel L. Clemens

Re: cpufreq: allow user to specify voltage

[Pavel Machek]

Hi!

> > So I guess adding /sys/bus/system/devices/cpu0/voltage? Should code to
> > do that be in kernel/cpufreq.c or is it possible to do sysfs from
> > powernow-k7 [it does not seem easy]?
>  								Pavel
> I agree, there shoul dbe a way to add sysfs files from a cpufreq driver
> module.  I told dave I was looking into overriding the powernow tables,
> but I can't seem to get enough time away from my day job right now.
> 
> for the powernow driver, and the userspace governor, I'd like to export a
> file "current_setting" or something that contains:
> 
> <frequency> <voltage> <fsb? maybe for other drivers>
> 
> A write to this file of one, two, or three values would result in changing
> the frequency to the closest standard table match we have.  Unless, the
> user specifies an "override" flag as a module parameter.  If the override
> flag is set, then writing to that file will set the speed and voltage to
> exactly what you specify (within the min/max hardware limits), and
> basically ignore the standard BIOS table.

Actually I think sysfs is trying to get it into one-file-per-value...

...which is going to be problem for writing because it will not be
able to atomically update different values at once...

Oh and forget module parameter :-).
									Pavel
-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

[PATCH][RFC] Re: cpufreq: allow user to specify voltage

[Dominik Brodowski]

Here's my suggestion, (partly) based on something Patrick Mochel suggested
for the passing of attribute files of cpufreq drivers to the cpufreq core:
a NULL-terminated list of device_attributes *attr is passed to the core. And
if attr itself is NULL, no attribute is passed, of course. Using this
approach, this patch against cpufreq-CVS-HEAD adds a file

/sys/devices/sys/cpu0/scaling_available_freqs (something Carl Thompson
						asked for)

for the powernow-k7.c and the p4-clockmod.c drivers (other frequency table
based drivers can be added at will -- Carl, please don't realy _solely_ on
this file: some drivers don't use frequency tables but still might want to
use your great userspace scaling program!)

And for powernow-k7.c , the file

/sys/devices/sys/cpu0/scaling_setvoltage

should show the current voltage for the current speed (scaling_setspeed).
"echoing" a different value (must be lower than the current voltage) changes
the voltage for this frequency only. However, this override is "static" so
that if you switch to a different frequency in the meantime but get back to
the one you wanted to override the voltage setting for, the new 
user-specified value is remembered.

This is untested (don't have a powernow-k7-capable notebook), so handle with
care.

Comments most welcome,

	Dominik

diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c linux/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c	2003-02-28 21:42:13.000000000 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c	2003-02-28 21:45:00.000000000 +0100
@@ -214,6 +214,7 @@
 		else
 			p4clockmod_table[i].frequency = (stock_freq * i)/8;
 	}
+	cpufreq_frequency_table_get_attr(p4clockmod_table, policy->cpu);
 	
 	/* cpuinfo and default policy values */
 	policy->policy = CPUFREQ_POLICY_PERFORMANCE;
@@ -226,6 +227,7 @@
 
 static int cpufreq_p4_cpu_exit(struct cpufreq_policy *policy)
 {
+	cpufreq_frequency_table_put_attr(policy->cpu);
 	return cpufreq_p4_setdc(policy->cpu, DC_DISABLE);
 }
 
@@ -236,6 +238,8 @@
 	.init		= cpufreq_p4_cpu_init,
 	.exit		= cpufreq_p4_cpu_exit,
 	.name		= "p4-clockmod",
+	.attr		= {&dev_attr_scaling_available_freqs,
+			   NULL},
 };
 
 
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-02-28 21:42:13.000000000 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-02-28 22:03:04.000000000 +0100
@@ -326,6 +326,7 @@
 	return -ENODEV;
 }
 
+static DECLARE_MUTEX(change_speed_lock);
 
 static int powernow_target (struct cpufreq_policy *policy,
 			    unsigned int target_freq,
@@ -336,7 +337,9 @@
 	if (cpufreq_frequency_table_target(policy, powernow_table, target_freq, relation, &newstate))
 		return -EINVAL;
 
+	down(&change_speed_lock);
 	change_speed(newstate);
+	up(&change_speed_lock);
 
 	return 0;
 }
@@ -365,6 +368,7 @@
 	printk (KERN_INFO PFX "Minimum speed %d MHz. Maximum speed %d MHz.\n",
 				minimum_speed, maximum_speed);
 
+	cpufreq_frequency_table_get_attr(powernow_table, policy->cpu);
 	policy->policy = CPUFREQ_POLICY_PERFORMANCE;
 	policy->cpuinfo.transition_latency = latency;
 	policy->cur = maximum_speed;
@@ -372,11 +376,103 @@
 	return cpufreq_frequency_table_cpuinfo(policy, powernow_table);
 }
 
+static int powernow_cpu_exit(struct cpufreq_policy *policy)
+{
+	cpufreq_frequency_table_put_attr(policy->cpu);
+	return 0;
+}
+
+static ssize_t show_voltage (struct device *dev, char *buf)
+{
+	union msr_fidvidstatus fidvidstatus;
+	unsigned int freq;
+	unsigned int i;
+	u8 vid;
+
+	rdmsrl (MSR_K7_FID_VID_STATUS, fidvidstatus.val);
+	freq = fsb * fid_codes[fidvidstatus.bits.CFID] * 100;
+
+	for (i=0; (powernow_table[i].frequency != CPUFREQ_TABLE_END); i++) {
+		if (powernow_table[i].frequency == CPUFREQ_ENTRY_INVALID)
+			continue;
+		if (powernow_table[i].frequency == freq)
+			goto found;
+	}
+	return -EINVAL;
+
+ found:
+	vid = (powernow_table[i].index & 0xFF00) >> 8;
+	return sprintf(buf, "%dmV\n", mobile_vid_table[vid]);
+}
+
+static ssize_t store_voltage (struct device *dev, 
+			      const char *buf, size_t count) 
+{
+	union msr_fidvidstatus fidvidstatus;
+	unsigned int freq;
+	unsigned int i;
+	unsigned int tmp;
+	unsigned int new_voltage;
+	unsigned int ret;
+	u8 vid, new_vid;
+
+	ret = sscanf (buf, "%d", &new_voltage);
+	if (ret != 1 || !new_voltage)
+		return -EINVAL;
+
+	for (i=0; i<32; i++) {
+		if (mobile_vid_table[i] == new_voltage)
+			goto match;
+	}
+	return -EINVAL;
+
+ match:
+	new_vid = (u8) i;
+
+	rdmsrl (MSR_K7_FID_VID_STATUS, fidvidstatus.val);
+	freq = fsb * fid_codes[fidvidstatus.bits.CFID] * 100;
+
+	for (i=0; (powernow_table[i].frequency != CPUFREQ_TABLE_END); i++) {
+		if (powernow_table[i].frequency == CPUFREQ_ENTRY_INVALID)
+			continue;
+		if (powernow_table[i].frequency == freq)
+			goto found;
+	}
+	return -ENODEV;
+
+ found:
+	vid = (powernow_table[i].index & 0xFF00) >> 8;
+
+	/* safety check */
+	if (mobile_vid_table[new_vid] > mobile_vid_table[vid])
+		return -EINVAL;
+
+	printk("overwriting voltage for frequency %d MHz with %d mV (old was %d mV).\n", 
+	       (freq/1000), mobile_vid_table[new_vid], mobile_vid_table[vid]);
+
+	down(&change_speed_lock);
+	tmp = powernow_table[i].index;
+	tmp &= powernow_table[i].index & 0x00FF;
+	tmp |= (new_vid << 8); /* upper 8 bits */
+	powernow_table[i].index = tmp; /* actual over-writing */
+
+	/* we're not changing speed but voltage, but anyways... */
+	change_speed (i);
+	up(&change_speed_lock);
+
+	return count;
+}
+static DEVICE_ATTR(scaling_setvoltage, (S_IRUGO | S_IWUSR), show_voltage, store_voltage);
+
 static struct cpufreq_driver powernow_driver = {
 	.verify 	= powernow_verify,
 	.target 	= powernow_target,
 	.init		= powernow_cpu_init,
+	.exit		= powernow_cpu_exit,
 	.name		= "powernow-k7",
+	.attr		= {&dev_attr_scaling_available_freqs,
+			   &dev_attr_scaling_setvoltage,
+			   NULL},
 };
 
 static int __init powernow_init (void)
diff -ruN linux-original/drivers/cpufreq/freq_table.c linux/drivers/cpufreq/freq_table.c
--- linux-original/drivers/cpufreq/freq_table.c	2003-02-26 09:44:57.000000000 +0100
+++ linux/drivers/cpufreq/freq_table.c	2003-02-28 21:17:07.000000000 +0100
@@ -198,6 +198,53 @@
 EXPORT_SYMBOL_GPL(cpufreq_frequency_table_target);
 
 
+static struct cpufreq_frequency_table *show_table[NR_CPUS];
+/**
+ * show_scaling_governor - show the current policy for the specified CPU
+ */
+static ssize_t show_available_freqs (struct device *dev, char *buf)
+{
+	unsigned int i = 0;
+	unsigned int cpu = to_cpu_nr(dev);
+	ssize_t count = 0;
+	struct cpufreq_frequency_table *table;
+
+	if (!show_table[cpu])
+		return -ENODEV;
+
+	table = show_table[cpu];
+
+	for (i=0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
+		if (table[i].frequency == CPUFREQ_ENTRY_INVALID)
+			continue;
+		count += sprintf(&buf[count], "%d ", table[i].frequency);
+	}
+	count += sprintf(&buf[count], "\n");
+
+	return count;
+
+}
+DEVICE_ATTR(scaling_available_freqs, S_IRUGO, show_available_freqs, NULL);
+EXPORT_SYMBOL_GPL(dev_attr_scaling_available_freqs);
+
+/*
+ * if you use these, you must assure that the frequency table is valid
+ * all the time between get_attr and put_attr!
+ */
+void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table, 
+				      unsigned int cpu)
+{
+	show_table[cpu] = table;
+}
+EXPORT_SYMBOL_GPL(cpufreq_frequency_table_get_attr);
+
+void cpufreq_frequency_table_put_attr(unsigned int cpu)
+{
+	show_table[cpu] = 0;
+}
+EXPORT_SYMBOL_GPL(cpufreq_frequency_table_put_attr);
+
+
 MODULE_AUTHOR ("Dominik Brodowski <linux@brodo.de>");
 MODULE_DESCRIPTION ("CPUfreq frequency table helpers");
 MODULE_LICENSE ("GPL");
diff -ruN linux-original/include/linux/cpufreq.h linux/include/linux/cpufreq.h
--- linux-original/include/linux/cpufreq.h	2003-02-28 21:42:13.000000000 +0100
+++ linux/include/linux/cpufreq.h	2003-02-28 20:56:28.000000000 +0100
@@ -166,8 +166,11 @@
 	/* optional, for the moment */
 	int	(*init)		(struct cpufreq_policy *policy);
 	int	(*exit)		(struct cpufreq_policy *policy);
+	struct device_attribute *attr[];
 };
 
+inline int to_cpu_nr (struct device *dev);
+
 int cpufreq_register_driver(struct cpufreq_driver *driver_data);
 int cpufreq_unregister_driver(struct cpufreq_driver *driver_data);
 /* deprecated */
@@ -298,6 +301,14 @@
 				   unsigned int relation,
 				   unsigned int *index);
 
+/* the following are really really optional */
+extern struct device_attribute dev_attr_scaling_available_freqs;
+
+void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table, 
+				      unsigned int cpu);
+
+void cpufreq_frequency_table_put_attr(unsigned int cpu);
+
 #endif /* CONFIG_CPU_FREQ_TABLE */
 
 #endif /* _LINUX_CPUFREQ_H */
diff -ruN linux-original/kernel/cpufreq.c linux/kernel/cpufreq.c
--- linux-original/kernel/cpufreq.c	2003-02-28 21:42:13.000000000 +0100
+++ linux/kernel/cpufreq.c	2003-02-28 21:47:46.000000000 +0100
@@ -102,7 +102,7 @@
         .devnum = 0,
 };
 
-static inline int to_cpu_nr (struct device *dev)
+inline int to_cpu_nr (struct device *dev)
 {
 	struct sys_device * cpu_sys_dev = container_of(dev, struct sys_device, dev);
 	return (cpu_sys_dev->id);
@@ -312,6 +312,7 @@
 	unsigned int cpu = to_cpu_nr(dev);
 	int ret = 0;
 	struct cpufreq_policy policy;
+	struct device_attribute ** drv_attr;
 
 	down(&cpufreq_driver_sem);
 	if (!cpufreq_driver) {
@@ -369,6 +370,12 @@
 	device_create_file (dev, &dev_attr_scaling_driver);
 	device_create_file (dev, &dev_attr_available_scaling_governors);
 
+	drv_attr = cpufreq_driver->attr;
+	while ((drv_attr) && (*drv_attr)) {
+		device_create_file(dev, *drv_attr);
+		drv_attr++;
+	}
+
 	up(&cpufreq_driver_sem);
 	return ret;
 }
@@ -384,6 +391,7 @@
 {
 	struct device * dev = intf->dev;
 	unsigned int cpu = to_cpu_nr(dev);
+	struct device_attribute ** drv_attr;
 
 	if (cpufreq_driver->target)
 		cpufreq_governor(cpu, CPUFREQ_GOV_STOP);
@@ -399,6 +407,12 @@
 	device_remove_file (dev, &dev_attr_scaling_driver);
 	device_remove_file (dev, &dev_attr_available_scaling_governors);
 
+	drv_attr = cpufreq_driver->attr;
+	while ((drv_attr) && (*drv_attr)) {
+		device_remove_file(dev, *drv_attr);
+		drv_attr++;
+	}
+
 	return 0;
 }
 

Re: [PATCH][RFC] Re: cpufreq: allow user to specify voltage

[Pavel Machek]

Hi!

> Here's my suggestion, (partly) based on something Patrick Mochel suggested
> for the passing of attribute files of cpufreq drivers to the cpufreq core:
> a NULL-terminated list of device_attributes *attr is passed to the core. And
> if attr itself is NULL, no attribute is passed, of course. Using this
> approach, this patch against cpufreq-CVS-HEAD adds a file
> 
> /sys/devices/sys/cpu0/scaling_available_freqs (something Carl Thompson
> 						asked for)

Strange, I need 

> @@ -166,8 +166,11 @@
>  	/* optional, for the moment */
>  	int	(*init)		(struct cpufreq_policy *policy);
>  	int	(*exit)		(struct cpufreq_policy *policy);
> +	struct device_attribute *attr[];

to change this to *attr[99] if I want it to compile...
								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH][RFC] Re: cpufreq: allow user to specify voltage

[Pavel Machek]

Hi!

> Here's my suggestion, (partly) based on something Patrick Mochel suggested
> for the passing of attribute files of cpufreq drivers to the cpufreq core:
> a NULL-terminated list of device_attributes *attr is passed to the core. And
> if attr itself is NULL, no attribute is passed, of course. Using this
> approach, this patch against cpufreq-CVS-HEAD adds a file
> 
> /sys/devices/sys/cpu0/scaling_available_freqs (something Carl Thompson
> 						asked for)
> 
> for the powernow-k7.c and the p4-clockmod.c drivers (other frequency table
> based drivers can be added at will -- Carl, please don't realy _solely_ on
> this file: some drivers don't use frequency tables but still might want to
> use your great userspace scaling program!)
> 
> And for powernow-k7.c , the file
> 
> /sys/devices/sys/cpu0/scaling_setvoltage
> 
> should show the current voltage for the current speed (scaling_setspeed).
> "echoing" a different value (must be lower than the current voltage) changes
> the voltage for this frequency only. However, this override is "static" so
> that if you switch to a different frequency in the meantime but get back to
> the one you wanted to override the voltage setting for, the new 
> user-specified value is remembered.
> 
> This is untested (don't have a powernow-k7-capable notebook), so handle with
> care.

It seems to work. I'll crash my machine now (and go to sleep ;-). So
if you don't get another mail within 30 minutes of this, it worked.

									Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH][RFC] Re: cpufreq: allow user to specify voltage

[Pavel Machek]

Hi!

> And for powernow-k7.c , the file
> 
> /sys/devices/sys/cpu0/scaling_setvoltage
> 
> should show the current voltage for the current speed (scaling_setspeed).
> "echoing" a different value (must be lower than the current voltage) changes
> the voltage for this frequency only. However, this override is "static" so
> that if you switch to a different frequency in the meantime but get back to
> the one you wanted to override the voltage setting for, the new 
> user-specified value is remembered.
> 
> This is untested (don't have a powernow-k7-capable notebook), so handle with
> care.

Thanx, it works. I'd drop "can't set higher voltage" limitation
[hardware protects you, anyway, and if you make it so low your system
is unstable you can no longer fix it without reboot], but that's
minor.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]