post-OLS pending cpufreq patches

post-OLS pending cpufreq patches

[Dominik Brodowski]

[-- Attachment #1.1: Type: text/plain, Size: 1866 bytes --]

Hi Dave, list,

As KS and OLS are over, here's a status update on what patches and/or what
issues (in the world outside of KS and OLS) are at what state at the moment.
Also, I'm looking forward to read Paul's paper in the OLS proceedings, and
to see what Dave has added to his TODO list :)

- [core] HT fix before CPU group support is merged
	fixes bug #3012: blind de-referencing of CPUdata independent of
	whether this CPU is actually registered with the CPUfreq core is 
	bad. The small fix proposed is really straightforward, the proper
	solution will be to merge the cpufreq-SMT patchkit, but that's much
	more invasive and not yet completely ready. A "must" for 2.6.8.

- [speedstep-smi] get_frequencies SMM call causes failures 
	fixes bug reported by Pierre Maziere. Also really straightforward.

- [speedstep-centrino] reduce Jeremey's mail [Jeremey Fitzhardinge]
	latest version looks good, I'm just not yet 100% sure whether
	initialization of table is according to new/2.6. coding stlye.
	Please consider for merging for 2.6.8, too.
	
- [speedstep-centrino] SMP support [Venkatesh Pallipadi]
	looks good, IMO, but needs to be adapted to Jeremey's patch which
	should go in first, IMO.

- [pmac] scaling_availbale_frequencies, #define cleanup [John Clemens]
x [pmac] does it keep frequency across suspend? If not, remove "equal" check
		in target and/or set_speed
	Don't really know about pmac's side of things, just wanted to
	mention	it again.

- [core] SMT patchkit
	needs a bit of work, will re-submit for 2.6.9

x [gx-suspmod] trouble
	gx-suspmod seems to be quite sick right now. Even reverting the
	latest changes leads to strange behaviour. Investigating...

- [acpi-core] disable SMM access to SpeedStep using ACPI-FADT pstate_cnt
	prepared an updated version, will submit for 2.6.9.

Thanks,
	Dominik

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@www.linux.org.uk
http://www.linux.org.uk/mailman/listinfo/cpufreq

Re: post-OLS pending cpufreq patches

[Dave Jones]

On Tue, Jul 27, 2004 at 10:51:26PM +0200, Dominik Brodowski wrote:

 > As KS and OLS are over, here's a status update on what patches and/or what
 > issues (in the world outside of KS and OLS) are at what state at the moment.
 > Also, I'm looking forward to read Paul's paper in the OLS proceedings, and
 > to see what Dave has added to his TODO list :)

I fly back today (should have been last night, but thats a long
story for another time), and expect to spend a few days doing
nothing work related at all when I get back 8-)

Notes I scribbled in my TODO during OLS include..

- rdmsr_on_cpu(int cpunr)
Paul mentioned this would make life easier in powernow-k8
It's low hanging fruit, as most of the implementation is
already in the msr driver. We can just generic-ise that.

- perform multiple scaling operations to 'step' to driver.
I've had some reports from k7 users that jumps from low->high
still dont work, but gradual increments work. This may also
solve the longhaul problems. k8 also could this I think.

- check permissons on cpufreq sysfs files
Someone mentioned that have these root r-- is a bit
pointless as they aren't really security sensitive.

- fill in gaps between low/high tables
Some powernow-k7 tables only have 2-3 entries in their PST's
though we know from the spec that other frequencies are available.
We could fill in these frequences, and for voltage take value from
next highest.

- no-table support for k7
Recently I've been getting more and more reports from folks putting
mobile K7's into desktop boards, which have no BIOS support
(no PST table, or ACPI P states).
For high speed, take what we booted at.
For low ?
Could be tricky.

- scheduler bits
"migrate everything off this CPU"
Useful when dropping CPU speed to very low speed/power, when
another CPU in the system is not-quite-idle.

- hotplug cpu support?
won't save any power today, but worth thinking about *now*,
as future CPUs could potentially have big savings here.

- longhaul ACPI support ?
Newer systems should support P-states.

- longhaul voltage scaling
I ripped this out long ago. Need to revisit it.


Thanks for scooping up the patches, I'll take a look at merging
them a few days after I get back, possibly the weekend, Monday
at the latest.

		Dave

Re: post-OLS pending cpufreq patches

[Dominik Brodowski]

On Wed, Jul 28, 2004 at 02:59:16PM +0100, Dave Jones wrote:
> On Tue, Jul 27, 2004 at 10:51:26PM +0200, Dominik Brodowski wrote:
> 
>  > As KS and OLS are over, here's a status update on what patches and/or what
>  > issues (in the world outside of KS and OLS) are at what state at the moment.
>  > Also, I'm looking forward to read Paul's paper in the OLS proceedings, and
>  > to see what Dave has added to his TODO list :)
> 
> I fly back today (should have been last night, but thats a long
> story for another time), and expect to spend a few days doing
> nothing work related at all when I get back 8-)

Enjoy that time!

> Notes I scribbled in my TODO during OLS include..
> - check permissons on cpufreq sysfs files
> Someone mentioned that have these root r-- is a bit
> pointless as they aren't really security sensitive.

-r--------  1 root root 4096 28. Jul 18:43 cpuinfo_cur_freq
-r--r--r--  1 root root 4096 28. Jul 18:43 cpuinfo_max_freq
-r--r--r--  1 root root 4096 28. Jul 18:43 cpuinfo_min_freq
-r--r--r--  1 root root 4096 28. Jul 18:43 scaling_available_frequencies
-r--r--r--  1 root root 4096 28. Jul 18:43 scaling_available_governors
-r--r--r--  1 root root 4096 28. Jul 18:43 scaling_cur_freq
-r--r--r--  1 root root 4096 28. Jul 18:43 scaling_driver
-rw-r--r--  1 root root 4096 28. Jul 18:43 scaling_governor
-rw-r--r--  1 root root 4096 28. Jul 18:43 scaling_max_freq
-rw-r--r--  1 root root 4096 28. Jul 18:43 scaling_min_freq
-rw-r--r--  1 root root 4096 28. Jul 18:43 scaling_setspeed

The only file r-- is "cpuinfo_cur_freq", and that may involve calls to
hardware which may take a bit of time... so it's less a security issue, but
a "local DoS" issue because the system might get unresponsive [though I'd
doubt any current cpufreq driver actually might be "exploitable" this
way...]

> Thanks for scooping up the patches, I'll take a look at merging
> them a few days after I get back, possibly the weekend, Monday
> at the latest.

Thanks,
	Dominik

Re: post-OLS pending cpufreq patches

[Dave Jones]

On Tue, Jul 27, 2004 at 10:51:26PM +0200, Dominik Brodowski wrote:

 > - [core] HT fix before CPU group support is merged
 > 	fixes bug #3012: blind de-referencing of CPUdata independent of
 > 	whether this CPU is actually registered with the CPUfreq core is 
 > 	bad. The small fix proposed is really straightforward, the proper
 > 	solution will be to merge the cpufreq-SMT patchkit, but that's much
 > 	more invasive and not yet completely ready. A "must" for 2.6.8.

couldnt find this in my inbox/cpufreq folder. can you bounce this please?

 > - [speedstep-smi] get_frequencies SMM call causes failures 
 > 	fixes bug reported by Pierre Maziere. Also really straightforward.

ditto (unless this is the 0xffff patch I just merged ?)

 > - [speedstep-centrino] reduce Jeremey's mail [Jeremey Fitzhardinge]
 > 	latest version looks good, I'm just not yet 100% sure whether
 > 	initialization of table is according to new/2.6. coding stlye.
 > 	Please consider for merging for 2.6.8, too.

merged.

 > - [speedstep-centrino] SMP support [Venkatesh Pallipadi]
 > 	looks good, IMO, but needs to be adapted to Jeremey's patch which
 > 	should go in first, IMO.

merged.

 > - [pmac] scaling_availbale_frequencies, #define cleanup [John Clemens]
 > x [pmac] does it keep frequency across suspend? If not, remove "equal" check
 > 		in target and/or set_speed
 > 	Don't really know about pmac's side of things, just wanted to
 > 	mention	it again.

pmac bits I'd rather go via benh, as he has a far better handle
on whats going on in that world, along with hardware with which
to test these changes.
 
 > - [core] SMT patchkit
 > 	needs a bit of work, will re-submit for 2.6.9

Hmm.

Subject: Re: [PATCH 2.6.8-rc2] speedstep SMT support
...
DB> Patch looks good, so: Dave, could you please merge it?

Merged.

 > x [gx-suspmod] trouble
 > 	gx-suspmod seems to be quite sick right now. Even reverting the
 > 	latest changes leads to strange behaviour. Investigating...
 > - [acpi-core] disable SMM access to SpeedStep using ACPI-FADT pstate_cnt
 > 	prepared an updated version, will submit for 2.6.9.

Ok.

Thanks for the summary 8-)

		Dave

Re: post-OLS pending cpufreq patches

[Dominik Brodowski]

On Mon, Aug 02, 2004 at 08:33:50PM +0100, Dave Jones wrote:
> On Tue, Jul 27, 2004 at 10:51:26PM +0200, Dominik Brodowski wrote:
> 
>  > - [core] HT fix before CPU group support is merged
>  > 	fixes bug #3012: blind de-referencing of CPUdata independent of
>  > 	whether this CPU is actually registered with the CPUfreq core is 
>  > 	bad. The small fix proposed is really straightforward, the proper
>  > 	solution will be to merge the cpufreq-SMT patchkit, but that's much
>  > 	more invasive and not yet completely ready. A "must" for 2.6.8.
> 
> couldnt find this in my inbox/cpufreq folder. can you bounce this please?

Done in separate mail.

>  > - [speedstep-smi] get_frequencies SMM call causes failures 
>  > 	fixes bug reported by Pierre Maziere. Also really straightforward.
> 
> ditto (unless this is the 0xffff patch I just merged ?)

No, it's this patch you merged:
http://linux-dj.bkbits.net:8080/cpufreq/gnupatch@410e9165OpfEFE684FCd4kzghQsn5g

>  > - [pmac] scaling_availbale_frequencies, #define cleanup [John Clemens]
>  > x [pmac] does it keep frequency across suspend? If not, remove "equal" check
>  > 		in target and/or set_speed
>  > 	Don't really know about pmac's side of things, just wanted to
>  > 	mention	it again.
> 
> pmac bits I'd rather go via benh, as he has a far better handle
> on whats going on in that world, along with hardware with which
> to test these changes.

Agreed; as I said I don't know about pmac's side of things, just wanted to
keep benh awake :)

>  > - [core] SMT patchkit
>  > 	needs a bit of work, will re-submit for 2.6.9
> 
> Hmm.
> 
> Subject: Re: [PATCH 2.6.8-rc2] speedstep SMT support
> ...
> DB> Patch looks good, so: Dave, could you please merge it?
> 
> Merged.

Acutally, I meant something else...

[RFC][PATCH] (1/4) cpufreq SMT awareness: keep affected_cpu_mask
[RFC][PATCH] (2/4) cpufreq SMT awareness: save CPU's sys_device
[RFC][PATCH] (3/4) cpufreq SMT awareness: only register SMT group once
[RFC][PATCH] (4/4) cpufreq SMT awareness: code reduction

... but they need some work. Will do that against cpufreq-bk in a few days.

> Thanks for the summary 8-)
You're welcome. Thanks for removing items from the summary  by merging them :)

	Dominik

Re: post-OLS pending cpufreq patches

[Dominik Brodowski]

On Mon, Aug 02, 2004 at 08:33:50PM +0100, Dave Jones wrote:
>  > - [speedstep-centrino] SMP support [Venkatesh Pallipadi]
>  > 	looks good, IMO, but needs to be adapted to Jeremey's patch which
>  > 	should go in first, IMO.
> 
> merged.

Actually, I can't see that in either bk or snapshot... However, I guess it
needs a bit of re-work after all the changes done in the past few hours.

	Dominik

Re: post-OLS pending cpufreq patches

[Dave Jones]

On Mon, Aug 02, 2004 at 10:42:31PM +0200, Dominik Brodowski wrote:
 > On Mon, Aug 02, 2004 at 08:33:50PM +0100, Dave Jones wrote:
 > >  > - [speedstep-centrino] SMP support [Venkatesh Pallipadi]
 > >  > 	looks good, IMO, but needs to be adapted to Jeremey's patch which
 > >  > 	should go in first, IMO.
 > > 
 > > merged.
 > 
 > Actually, I can't see that in either bk or snapshot... However, I guess it
 > needs a bit of re-work after all the changes done in the past few hours.

Ok, I'm done merging bits for now, and I've just refreshed the patch
at http://www.codemonkey.org.uk/projects/bitkeeper/cpufreq/cpufreq-2004-08-02.diff
if you have anything else, please send incrementals against that.

Hmm, actually, there's going to be one more fix needed.
Something I just merged broke things. (Or I missed a dependancy).

  CC      arch/i386/kernel/cpu/cpufreq/speedstep-ich.o
  arch/i386/kernel/cpu/cpufreq/speedstep-ich.c: In function `speedstep_target':
  arch/i386/kernel/cpu/cpufreq/speedstep-ich.c:250: warning: implicit declaration of function `speedstep_get'
  arch/i386/kernel/cpu/cpufreq/speedstep-ich.c: At top level:
  arch/i386/kernel/cpu/cpufreq/speedstep-ich.c:363: error: conflicting types  2


		Dave

RE: post-OLS pending cpufreq patches

[Pallipadi, Venkatesh]

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]


 

>-----Original Message-----
>From: Dave Jones [mailto:davej@redhat.com] 
>Sent: Monday, August 02, 2004 1:52 PM
>To: Pallipadi, Venkatesh; cpufreq@www.linux.org.uk
>Subject: Re: post-OLS pending cpufreq patches
>
>On Mon, Aug 02, 2004 at 10:42:31PM +0200, Dominik Brodowski wrote:
> > On Mon, Aug 02, 2004 at 08:33:50PM +0100, Dave Jones wrote:
> > >  > - [speedstep-centrino] SMP support [Venkatesh Pallipadi]
> > >  > 	looks good, IMO, but needs to be adapted to 
>Jeremey's patch which
> > >  > 	should go in first, IMO.
> > > 
> > > merged.
> > 
> > Actually, I can't see that in either bk or snapshot... 
>However, I guess it
> > needs a bit of re-work after all the changes done in the 
>past few hours.
>
>Ok, I'm done merging bits for now, and I've just refreshed the patch
>at 
>http://www.codemonkey.org.uk/projects/bitkeeper/cpufreq/cpufreq
>-2004-08-02.diff
>if you have anything else, please send incrementals against that.
>

This patch was completely missing in 2004-08-02.diff. 
And it needed some rework too. Here is the updated incremental patch.

Thanks,
Venki

[-- Attachment #2: speedstep_centrino.patch --]
[-- Type: application/octet-stream, Size: 7271 bytes --]

--- linux-2.6.8-rc2/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c.org	2004-08-02 17:04:11.000000000 -0700
+++ linux-2.6.8-rc2/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2004-08-02 18:00:44.000000000 -0700
@@ -70,6 +70,7 @@ static int centrino_verify_cpu_id(const 
 
 /* Operating points for current CPU */
 static struct cpu_model *centrino_model;
+static int 		centrino_cpu;
 
 #ifdef CONFIG_X86_SPEEDSTEP_CENTRINO_TABLE
 
@@ -270,21 +271,47 @@ static int centrino_verify_cpu_id(const 
 	return 0;
 }
 
-/* Extract clock in kHz from PERF_CTL value */
+/* To be called only after centrino_model is initialized */
 static unsigned extract_clock(unsigned msr)
 {
-	msr = (msr >> 8) & 0xff;
-	return msr * 100000;
+	int i;
+
+	/* 
+	 * Extract clock in kHz from PERF_CTL value
+	 * for centrino, as some DSDTs are buggy.
+	 * Ideally, this can be done using the acpi_data structure.
+	 */
+	if (centrino_cpu) {
+		msr = (msr >> 8) & 0xff;
+		return msr * 100000;
+	}
+
+	if ((!centrino_model) || (!centrino_model->op_points))
+		return 0;
+
+	msr &= 0xffff;
+	for (i = 0; 
+	     centrino_model->op_points[i].frequency != CPUFREQ_TABLE_END; 
+	     i++) {
+		if (msr == centrino_model->op_points[i].index)
+		return centrino_model->op_points[i].frequency;
+	}
+	return 0;
 }
 
 /* Return the current CPU frequency in kHz */
 static unsigned int get_cur_freq(unsigned int cpu)
 {
 	unsigned l, h;
-	if (cpu)
+	cpumask_t saved_mask;
+
+	saved_mask = current->cpus_allowed;
+	set_cpus_allowed(current, cpumask_of_cpu(cpu));
+	if (smp_processor_id() != cpu)
 		return 0;
 
 	rdmsr(MSR_IA32_PERF_STATUS, l, h);
+	set_cpus_allowed(current, saved_mask);
 	return extract_clock(l);
 }
 
@@ -296,8 +323,6 @@ static struct acpi_processor_performance
 #include <linux/acpi.h>
 #include <acpi/processor.h>
 
-#define ACPI_PDC_CAPABILITY_ENHANCED_SPEEDSTEP 0x1
-
 /*
  * centrino_cpu_init_acpi - register with ACPI P-States library 
  *
@@ -318,12 +343,12 @@ static int centrino_cpu_init_acpi(struct
         arg0.buffer.pointer = (u8 *) arg0_buf;
         arg0_buf[0] = ACPI_PDC_REVISION_ID;
         arg0_buf[1] = 1;
-        arg0_buf[2] = ACPI_PDC_CAPABILITY_ENHANCED_SPEEDSTEP;
+        arg0_buf[2] = ACPI_PDC_EST_CAPABILITY_SMP | ACPI_PDC_EST_CAPABILITY_MSR;
 
 	p.pdc = &arg_list;
 
 	/* register with ACPI core */
-        if (acpi_processor_register_performance(&p, 0))
+        if (acpi_processor_register_performance(&p, policy->cpu))
                 return -EIO;
 
 	/* verify the acpi_data */
@@ -358,13 +383,6 @@ static int centrino_cpu_init_acpi(struct
 			p.states[i].core_frequency = 0;
 			continue;
 		}
-
-		if (extract_clock(p.states[i].control) != 
-		    (p.states[i].core_frequency * 1000)) {
-			printk(KERN_DEBUG "Invalid encoded frequency\n");
-			result = -EINVAL;
-			goto err_unreg;
-		}
 	}
 
 	centrino_model = kmalloc(sizeof(struct cpu_model), GFP_KERNEL);
@@ -383,24 +401,36 @@ static int centrino_cpu_init_acpi(struct
                 goto err_kfree;
         }
 
-	cur_freq = get_cur_freq(0);
-
         for (i=0; i<p.state_count; i++) {
 		centrino_model->op_points[i].index = p.states[i].control;
 		centrino_model->op_points[i].frequency = p.states[i].core_frequency * 1000;
+	}
+	centrino_model->op_points[p.state_count].frequency = CPUFREQ_TABLE_END;
+
+	cur_freq = get_cur_freq(policy->cpu);
+
+	for (i=0; i<p.state_count; i++) {
+		if (extract_clock(centrino_model->op_points[i].index) !=
+		    (centrino_model->op_points[i].frequency)) {
+			printk(KERN_DEBUG "Invalid encoded frequency\n");
+			result = -EINVAL;
+			goto err_kfree_all;
+		}
+
 		if (cur_freq == centrino_model->op_points[i].frequency)
 			p.state = i;
 		if (!p.states[i].core_frequency)
 			centrino_model->op_points[i].frequency = CPUFREQ_ENTRY_INVALID;
 	}
-	centrino_model->op_points[p.state_count].frequency = CPUFREQ_TABLE_END;
 
 	return 0;
-
+	
+ err_kfree_all:
+	kfree(centrino_model->op_points);
  err_kfree:
 	kfree(centrino_model);
  err_unreg:
-	acpi_processor_unregister_performance(&p, 0);
+	acpi_processor_unregister_performance(&p, policy->cpu);
 	return (result);
 }
 #else
@@ -415,9 +445,6 @@ static int centrino_cpu_init(struct cpuf
 	int ret;
 	int i;
 
-	if (policy->cpu != 0)
-		return -ENODEV;
-
 	/* Only Intel makes Enhanced Speedstep-capable CPUs */
 	if (cpu->x86_vendor != X86_VENDOR_INTEL || !cpu_has(cpu, X86_FEATURE_EST))
 		return -ENODEV;
@@ -426,13 +453,20 @@ static int centrino_cpu_init(struct cpuf
 		if (centrino_verify_cpu_id(cpu, &cpu_ids[i]))
 			break;
 
-	if (i == N_IDS) {
-		printk(KERN_INFO PFX "found unsupported CPU with Enhanced SpeedStep: "
-		       "send /proc/cpuinfo to " MAINTAINER "\n");
-		return -ENODEV;
-	}
+	if (i != N_IDS)
+		centrino_cpu = 1;
 
 	if (centrino_cpu_init_acpi(policy)) {
+		if (policy->cpu != 0)
+			return -ENODEV;
+
+		if (!centrino_cpu) {
+			printk(KERN_INFO PFX "found unsupported CPU with "
+			"Enhanced SpeedStep: send /proc/cpuinfo to " 
+			MAINTAINER "\n");
+			return -ENODEV;
+		}
+
 		if (centrino_cpu_init_table(policy)) {
 			return -ENODEV;
 		}
@@ -454,7 +488,7 @@ static int centrino_cpu_init(struct cpuf
 		}
 	}
 
-	freq = get_cur_freq(0);
+	freq = get_cur_freq(policy->cpu);
 
 	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
 	policy->cpuinfo.transition_latency = 10000; /* 10uS transition latency */
@@ -481,7 +515,7 @@ static int centrino_cpu_exit(struct cpuf
 
 #ifdef CONFIG_X86_SPEEDSTEP_CENTRINO_ACPI
 	if (!centrino_model->model_name) {
-		acpi_processor_unregister_performance(&p, 0);
+		acpi_processor_unregister_performance(&p, policy->cpu);
 		kfree(centrino_model->op_points);
 		kfree(centrino_model);
 	}
@@ -519,19 +553,35 @@ static int centrino_target (struct cpufr
 	unsigned int    newstate = 0;
 	unsigned int	msr, oldmsr, h;
 	struct cpufreq_freqs	freqs;
+	cpumask_t		saved_mask;
+	int			retval;
 
 	if (centrino_model == NULL)
 		return -ENODEV;
 
+	/*
+	 * Support for SMP systems.
+	 * Make sure we are running on the CPU that wants to change frequency
+	 */
+	saved_mask = current->cpus_allowed;
+	set_cpus_allowed(current, cpumask_of_cpu(policy->cpu));
+	if (smp_processor_id() != policy->cpu) {
+		return(-EAGAIN);
+	}
+
 	if (cpufreq_frequency_table_target(policy, centrino_model->op_points, target_freq,
-					   relation, &newstate))
-		return -EINVAL;
+					   relation, &newstate)) {
+		retval = -EINVAL;
+		goto migrate_end;
+	}
 
 	msr = centrino_model->op_points[newstate].index;
 	rdmsr(MSR_IA32_PERF_CTL, oldmsr, h);
 
-	if (msr == (oldmsr & 0xffff))
-		return 0;
+	if (msr == (oldmsr & 0xffff)) {
+		retval = 0;
+		goto migrate_end;
+	}
 
 	/* Hm, old frequency can either be the last value we put in
 	   PERF_CTL, or whatever it is now. The trouble is that TM2
@@ -545,7 +595,7 @@ static int centrino_target (struct cpufr
 	   TODO: work out how the TCC interrupts work, and try to
 	   catch the CPU changing things under us.
 	*/
-	freqs.cpu = 0;
+	freqs.cpu = policy->cpu;
 	freqs.old = extract_clock(oldmsr);
 	freqs.new = extract_clock(msr);
 	
@@ -564,7 +614,10 @@ static int centrino_target (struct cpufr
 
 	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 
-	return 0;
+	retval = 0;
+migrate_end:
+	set_cpus_allowed(current, saved_mask);
+	return (retval);
 }
 
 static struct freq_attr* centrino_attr[] = {

[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Cpufreq mailing list
Cpufreq@www.linux.org.uk
http://www.linux.org.uk/mailman/listinfo/cpufreq

Re: post-OLS pending cpufreq patches

[Dominik Brodowski]

On Mon, Aug 02, 2004 at 03:45:14PM -0700, Pallipadi, Venkatesh wrote:
> This patch was completely missing in 2004-08-02.diff. 
> And it needed some rework too. Here is the updated incremental patch.

Could you send the diff to include/acpi/processor.h as well, please, else
the build will fail.

Thanks,
	Dominik

Re: post-OLS pending cpufreq patches

[Dave Jones]

On Tue, Aug 03, 2004 at 02:42:23PM +0200, Dominik Brodowski wrote:
 > On Mon, Aug 02, 2004 at 03:45:14PM -0700, Pallipadi, Venkatesh wrote:
 > > This patch was completely missing in 2004-08-02.diff. 
 > > And it needed some rework too. Here is the updated incremental patch.
 > 
 > Could you send the diff to include/acpi/processor.h as well, please, else
 > the build will fail.

Even with that, it still fails the build, with quite a few warnings.

  CC      arch/i386/kernel/cpu/cpufreq/speedstep-centrino.o
In file included from arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:25:
include/acpi/processor.h:44: error: `ACPI_C_STATE_COUNT' undeclared here (not in a function)
include/acpi/processor.h:60: error: syntax error before "acpi_integer"
include/acpi/processor.h:60: warning: no semicolon at end of struct or union
include/acpi/processor.h:61: warning: type defaults to `int' in declaration of `power'
include/acpi/processor.h:61: warning: data definition has no type or storage class
include/acpi/processor.h:62: error: syntax error before "transition_latency"
include/acpi/processor.h:62: warning: type defaults to `int' in declaration of `transition_latency'
include/acpi/processor.h:62: warning: data definition has no type or storage class
include/acpi/processor.h:63: error: syntax error before "bus_master_latency"
include/acpi/processor.h:63: warning: type defaults to `int' in declaration of `bus_master_latency'
include/acpi/processor.h:63: warning: data definition has no type or storage class
include/acpi/processor.h:64: error: syntax error before "control"
include/acpi/processor.h:64: warning: type defaults to `int' in declaration of `control'
include/acpi/processor.h:64: warning: data definition has no type or storage class
include/acpi/processor.h:65: error: syntax error before "status"
include/acpi/processor.h:65: warning: type defaults to `int' in declaration of `status'
include/acpi/processor.h:65: warning: data definition has no type or storage class
include/acpi/processor.h:125: error: syntax error before "acpi_handle"
include/acpi/processor.h:125: warning: no semicolon at end of struct or union
include/acpi/processor.h:130: error: conflicting types for 'power'
include/acpi/processor.h:61: error: previous declaration of 'power' was here
include/acpi/processor.h:134: error: syntax error before '}' token
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c: In function `centrino_cpu_init_acpi':
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:347: error: `ACPI_PDC_EST_CAPABILITY_SMP' undeclared (first use in this function)
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:347: error: (Each undeclared identifier is reported only once
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:347: error: for each function it appears in.)
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:347: error: `ACPI_PDC_EST_CAPABILITY_MSR' undeclared (first use in this function)
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:370: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:370: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:370: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:370: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:376: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:376: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:382: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:382: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:382: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:382: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:384: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:384: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:397: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:397: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:406: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:406: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:407: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:407: error: dereferencing pointer to incomplete type
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:423: error: invalid use of undefined type `struct acpi_processor_px'
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:423: error: dereferencing pointer to incomplete type
{standard input}: Assembler messages:
{standard input}:341: Error: symbol `power' is already defined
make[1]: *** [arch/i386/kernel/cpu/cpufreq/speedstep-centrino.o] Error 1
make: *** [arch/i386/kernel/cpu/cpufreq/speedstep-centrino.o] Error 2


		Dave

Re: post-OLS pending cpufreq patches

[Dominik Brodowski]

On Tue, Aug 03, 2004 at 02:17:29PM +0100, Dave Jones wrote:
> In file included from arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:25:

 #include <acpi/processor.h>

is only included around ~300, AFAICS [even in Venki's patch]... and if you 
move that above to line ~25 [but please only #ifdef'ed], you need to

 #include <linux/acpi.h>

first.

Could you try that out, please?

	Dominik

Re: post-OLS pending cpufreq patches

[Dave Jones]

On Tue, Aug 03, 2004 at 03:30:10PM +0200, Dominik Brodowski wrote:
 > On Tue, Aug 03, 2004 at 02:17:29PM +0100, Dave Jones wrote:
 > > In file included from arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:25:
 > 
 >  #include <acpi/processor.h>
 > 
 > is only included around ~300, AFAICS [even in Venki's patch]... and if you 
 > move that above to line ~25 [but please only #ifdef'ed], you need to
 > 
 >  #include <linux/acpi.h>
 > 
 > first.
 > 
 > Could you try that out, please?

getting closer. just some missing defs introduced by this patch now.

  CC      arch/i386/kernel/cpu/cpufreq/speedstep-centrino.o
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c: In function `centrino_cpu_init_acpi':
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:348: error: `ACPI_PDC_EST_CAPABILITY_SMP' undeclared (first use in this function)
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:348: error: (Each undeclared identifier is reported only once
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:348: error: for each function it appears in.)
arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:348: error: `ACPI_PDC_EST_CAPABILITY_MSR' undeclared (first use in this function)
make[1]: *** [arch/i386/kernel/cpu/cpufreq/speedstep-centrino.o] Error 1
make: *** [arch/i386/kernel/cpu/cpufreq/speedstep-centrino.o] Error 2

		Dave

Re: post-OLS pending cpufreq patches

[Dominik Brodowski]

On Tue, Aug 03, 2004 at 02:37:24PM +0100, Dave Jones wrote:
> On Tue, Aug 03, 2004 at 03:30:10PM +0200, Dominik Brodowski wrote:
>  > On Tue, Aug 03, 2004 at 02:17:29PM +0100, Dave Jones wrote:
>  > > In file included from arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c:25:
>  > 
>  >  #include <acpi/processor.h>
>  > 
>  > is only included around ~300, AFAICS [even in Venki's patch]... and if you 
>  > move that above to line ~25 [but please only #ifdef'ed], you need to
>  > 
>  >  #include <linux/acpi.h>
>  > 
>  > first.
>  > 
>  > Could you try that out, please?
> 
> getting closer. just some missing defs introduced by this patch now.

That's the patch to include/acpi/processor.h I was talking about...

but that is actually include/asm/acpi.h, as can be seen here:
http://bugzilla.kernel.org/attachment.cgi?id=2919&action=view

Can you try out applying _only_ the hunk for include/asm-i386/acpi.h,
please?

Thanks,
	Dominik

Re: post-OLS pending cpufreq patches

[Dave Jones]

On Tue, Aug 03, 2004 at 04:07:50PM +0200, Dominik Brodowski wrote:

 > >  >  #include <acpi/processor.h>
 > >  > 
 > >  > is only included around ~300, AFAICS [even in Venki's patch]... and if you 
 > >  > move that above to line ~25 [but please only #ifdef'ed], you need to
 > >  > 
 > >  >  #include <linux/acpi.h>
 > >  > 
 > >  > first.
 > >  > 
 > >  > Could you try that out, please?
 > > 
 > > getting closer. just some missing defs introduced by this patch now.
 > 
 > That's the patch to include/acpi/processor.h I was talking about...
 > 
 > but that is actually include/asm/acpi.h, as can be seen here:
 > http://bugzilla.kernel.org/attachment.cgi?id=2919&action=view
 > 
 > Can you try out applying _only_ the hunk for include/asm-i386/acpi.h,
 > please?

Yeah, that works.  Or compiles at least, I don't have centrino hardware
to test this.  I'll check it in, and see who screams.

		Dave

RE: post-OLS pending cpufreq patches

[Pallipadi, Venkatesh]

Sorry about all the missing parts and the confusions about this one.
I was using cpufreq patch over latest mm tree, which has all of these 
missing changes/includes. So, everything worked fine in my local 
tests yesterday.

Thanks,
Venki  

>-----Original Message-----
>From: Dave Jones [mailto:davej@redhat.com] 
>Sent: Tuesday, August 03, 2004 7:20 AM
>To: Pallipadi, Venkatesh; cpufreq@www.linux.org.uk
>Subject: Re: post-OLS pending cpufreq patches
>
>On Tue, Aug 03, 2004 at 04:07:50PM +0200, Dominik Brodowski wrote:
>
> > >  >  #include <acpi/processor.h>
> > >  > 
> > >  > is only included around ~300, AFAICS [even in Venki's 
>patch]... and if you 
> > >  > move that above to line ~25 [but please only 
>#ifdef'ed], you need to
> > >  > 
> > >  >  #include <linux/acpi.h>
> > >  > 
> > >  > first.
> > >  > 
> > >  > Could you try that out, please?
> > > 
> > > getting closer. just some missing defs introduced by this 
>patch now.
> > 
> > That's the patch to include/acpi/processor.h I was talking about...
> > 
> > but that is actually include/asm/acpi.h, as can be seen here:
> > http://bugzilla.kernel.org/attachment.cgi?id=2919&action=view
> > 
> > Can you try out applying _only_ the hunk for 
>include/asm-i386/acpi.h,
> > please?
>
>Yeah, that works.  Or compiles at least, I don't have centrino hardware
>to test this.  I'll check it in, and see who screams.
>
>		Dave
>
>