[PATCH] powerpc: Show current speed in /proc/cpuinfo

[PATCH] powerpc: Show current speed in /proc/cpuinfo

[Olof Johansson]

On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
changes. It makes sense to do the same on powerpc to keep users from
getting confused. Fall back to old ppc_proc_freq for non-cpufreq systems.

Also change the format to three decimals, having full Hz granularity is
silly these days.


Signed-off-by: Olof Johansson <olof@lixom.net>


Index: powerpc/arch/powerpc/kernel/setup-common.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/setup-common.c
+++ powerpc/arch/powerpc/kernel/setup-common.c
@@ -29,6 +29,7 @@
 #include <linux/root_dev.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
+#include <linux/cpufreq.h>
 #include <linux/unistd.h>
 #include <linux/serial.h>
 #include <linux/serial_8250.h>
@@ -160,6 +161,7 @@ static int show_cpuinfo(struct seq_file 
 	unsigned int pvr;
 	unsigned short maj;
 	unsigned short min;
+	unsigned int freq;
 
 	if (cpu_id == NR_CPUS) {
 #if defined(CONFIG_SMP) && defined(CONFIG_PPC32)
@@ -226,13 +228,15 @@ static int show_cpuinfo(struct seq_file 
 	}
 #endif /* CONFIG_TAU */
 
-	/*
-	 * Assume here that all clock rates are the same in a
-	 * smp system.  -- Cort
-	 */
-	if (ppc_proc_freq)
-		seq_printf(m, "clock\t\t: %lu.%06luMHz\n",
-			   ppc_proc_freq / 1000000, ppc_proc_freq % 1000000);
+	/* cpufreq_speed_get() is in kHz */
+	freq = cpufreq_quick_get(cpu_id);
+
+	if (!freq)
+		/* ppc_proc_freq is in Hz */
+		freq = ppc_proc_freq / 1000;
+
+	if (freq)
+		seq_printf(m, "clock\t\t: %u.%03uMHz\n", freq / 1000, freq % 1000);
 
 	if (ppc_md.show_percpuinfo != NULL)
 		ppc_md.show_percpuinfo(m, cpu_id);

Re: [PATCH] powerpc: Show current speed in /proc/cpuinfo

[Matt Sealey]

Olof Johansson wrote:
> On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
> changes. It makes sense to do the same on powerpc to keep users from
> getting confused. Fall back to old ppc_proc_freq for non-cpufreq systems.
> 
> Also change the format to three decimals, having full Hz granularity is
> silly these days.
> 
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> 
> 
> Index: powerpc/arch/powerpc/kernel/setup-common.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/kernel/setup-common.c
> +++ powerpc/arch/powerpc/kernel/setup-common.c
> @@ -29,6 +29,7 @@
>  #include <linux/root_dev.h>
>  #include <linux/notifier.h>
>  #include <linux/cpu.h>
> +#include <linux/cpufreq.h>
>  #include <linux/unistd.h>
>  #include <linux/serial.h>
>  #include <linux/serial_8250.h>
> @@ -160,6 +161,7 @@ static int show_cpuinfo(struct seq_file 
>  	unsigned int pvr;
>  	unsigned short maj;
>  	unsigned short min;
> +	unsigned int freq;
>  
>  	if (cpu_id == NR_CPUS) {
>  #if defined(CONFIG_SMP) && defined(CONFIG_PPC32)
> @@ -226,13 +228,15 @@ static int show_cpuinfo(struct seq_file 
>  	}
>  #endif /* CONFIG_TAU */
>  
> -	/*
> -	 * Assume here that all clock rates are the same in a
> -	 * smp system.  -- Cort
> -	 */
> -	if (ppc_proc_freq)
> -		seq_printf(m, "clock\t\t: %lu.%06luMHz\n",
> -			   ppc_proc_freq / 1000000, ppc_proc_freq % 1000000);

Would it not be relevant to keep listing the maximum CPU speed as well
as the real, current CPU speed?

Intel encode this into their CPUID string and when I bring up Windows' System
Properties dialog I get this;

Intel(R) Pentium(R) M processor 1.70GHz
593MHz, 512MB of RAM

So you can always, always find out very easily what spec your processor
is, and not just what cpufreq has done with it?

> +	/* cpufreq_speed_get() is in kHz */
> +	freq = cpufreq_quick_get(cpu_id);

Is it speed or quick? This and the comment below confused me, KHz or Hz? :D

> +
> +	if (!freq)
> +		/* ppc_proc_freq is in Hz */
> +		freq = ppc_proc_freq / 1000;
> +
> +	if (freq)
> +		seq_printf(m, "clock\t\t: %u.%03uMHz\n", freq / 1000, freq % 1000);

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: [PATCH] powerpc: Show current speed in /proc/cpuinfo

[Benjamin Herrenschmidt]

> Would it not be relevant to keep listing the maximum CPU speed as well
> as the real, current CPU speed?

Probably not in cpuinfo... there's already a place for that in sysfs.

Ben.

Re: [PATCH] powerpc: Show current speed in /proc/cpuinfo

[Olof Johansson]

On Fri, Apr 27, 2007 at 10:00:33AM +0100, Matt Sealey wrote:

> Would it not be relevant to keep listing the maximum CPU speed as well
> as the real, current CPU speed?
> 
> Intel encode this into their CPUID string and when I bring up Windows' System
> Properties dialog I get this;
> 
> Intel(R) Pentium(R) M processor 1.70GHz
> 593MHz, 512MB of RAM
> 
> So you can always, always find out very easily what spec your processor
> is, and not just what cpufreq has done with it?

On a system running cpufreq, you might not even have booted
the system at full speed, so the only way to know for sure
what the max speed is, is to look at the one in cpufreq
(/sys/devices/system/cpu/cpu<x>/cpufreq/scaling_max_freq).

For non-cpufreq platforms, the speed reported will be the same as before,
which on most platforms is whatever firmware tells us the speed of the
cpu is.

> > +	/* cpufreq_speed_get() is in kHz */
> > +	freq = cpufreq_quick_get(cpu_id);
> 
> Is it speed or quick? This and the comment below confused me, KHz or Hz? :D

It's quick, I'll fix that. ppc_proc_freq is in Hz, freq in kHz, so you
need to divide.


-Olof

Re: [PATCH] powerpc: Show current speed in /proc/cpuinfo

[Benjamin Herrenschmidt]

On Fri, 2007-04-27 at 03:41 -0500, Olof Johansson wrote:
> On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
> changes. It makes sense to do the same on powerpc to keep users from
> getting confused. Fall back to old ppc_proc_freq for non-cpufreq systems.
> 
> Also change the format to three decimals, having full Hz granularity is
> silly these days.

That doesn't work if cpufreq is a module... on mac, I used to have
exactly that hack, though at one point I changed to just updating
ppc_proc_freq :-) But then, it's easy because I only change the freq on
all CPUs at the same time. Might be different for you.

Ben.

Re: [PATCH] powerpc: Show current speed in /proc/cpuinfo

[Olof Johansson]

On Fri, Apr 27, 2007 at 07:24:38PM +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2007-04-27 at 03:41 -0500, Olof Johansson wrote:
> > On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
> > changes. It makes sense to do the same on powerpc to keep users from
> > getting confused. Fall back to old ppc_proc_freq for non-cpufreq systems.
> > 
> > Also change the format to three decimals, having full Hz granularity is
> > silly these days.
> 
> That doesn't work if cpufreq is a module... on mac, I used to have
> exactly that hack, though at one point I changed to just updating
> ppc_proc_freq :-) But then, it's easy because I only change the freq on
> all CPUs at the same time. Might be different for you.

CONFIG_CPU_FREQ is a bool. do you mean the cpufreq driver?

I could change the output to be:

clock	:	xxxx.000MHz  (current speed)

and

clock	:	xxxx.000MHz  (reported by firmware)


depending on how I get the info.


-Olof

[PATCH v2] powerpc: Show current speed in /proc/cpuinfo

[Olof Johansson]

On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
changes. It makes sense to do the same on powerpc to keep users from
getting confused.

Change the format to three decimals, having full Hz granularity is silly
these days. Also specify in output if it's cpufreq or firmware-reported:

clock           : 1250.000MHz (from firmware)
or:
clock           : 2500.000MHz (actual speed)


Signed-off-by: Olof Johansson <olof@lixom.net>


Index: powerpc/arch/powerpc/kernel/setup-common.c
===================================================================
--- powerpc.orig/arch/powerpc/kernel/setup-common.c
+++ powerpc/arch/powerpc/kernel/setup-common.c
@@ -29,6 +29,7 @@
 #include <linux/root_dev.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
+#include <linux/cpufreq.h>
 #include <linux/unistd.h>
 #include <linux/serial.h>
 #include <linux/serial_8250.h>
@@ -160,6 +161,7 @@ static int show_cpuinfo(struct seq_file 
 	unsigned int pvr;
 	unsigned short maj;
 	unsigned short min;
+	unsigned int freq;
 
 	if (cpu_id == NR_CPUS) {
 #if defined(CONFIG_SMP) && defined(CONFIG_PPC32)
@@ -226,13 +228,16 @@ static int show_cpuinfo(struct seq_file 
 	}
 #endif /* CONFIG_TAU */
 
-	/*
-	 * Assume here that all clock rates are the same in a
-	 * smp system.  -- Cort
-	 */
-	if (ppc_proc_freq)
-		seq_printf(m, "clock\t\t: %lu.%06luMHz\n",
-			   ppc_proc_freq / 1000000, ppc_proc_freq % 1000000);
+	/* cpufreq_quick_get() is in kHz */
+	freq = cpufreq_quick_get(cpu_id);
+
+	if (freq)
+		seq_printf(m, "clock\t\t: %u.%03uMHz (actual speed)\n",
+			   freq / 1000, freq % 1000);
+	else if (ppc_proc_freq)
+		seq_printf(m, "clock\t\t: %lu.%03luMHz (from firmware)\n",
+			   ppc_proc_freq / 1000000,
+			   (ppc_proc_freq / 1000) % 1000);
 
 	if (ppc_md.show_percpuinfo != NULL)
 		ppc_md.show_percpuinfo(m, cpu_id);

Re: [PATCH v2] powerpc: Show current speed in /proc/cpuinfo

[Arnd Bergmann]

On Monday 30 April 2007, Olof Johansson wrote:
> On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
> changes. It makes sense to do the same on powerpc to keep users from
> getting confused.
>=20
> Change the format to three decimals, having full Hz granularity is silly
> these days. Also specify in output if it's cpufreq or firmware-reported:
>=20
> clock =A0 =A0 =A0 =A0 =A0 : 1250.000MHz (from firmware)
> or:
> clock =A0 =A0 =A0 =A0 =A0 : 2500.000MHz (actual speed)


Good idea, but I'd be _very_ careful with changing any field in
/proc/cpuinfo. Having the extra text in there can easily confuse
parsers in poorly written user applications, maybe even in good ones ;-)

How about rather adding a new line in there for one of the fields, either

clock             : 1250.000MHz
max-clock         : 2500.000MHz

or

clock             : 2500.000MHz
current-clock     : 1250.000MHz

	Arnd <><

Re: [PATCH v2] powerpc: Show current speed in /proc/cpuinfo

[Olof Johansson]

On Wed, May 02, 2007 at 01:48:07AM +0200, Arnd Bergmann wrote:
> On Monday 30 April 2007, Olof Johansson wrote:
> > On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
> > changes. It makes sense to do the same on powerpc to keep users from
> > getting confused.
> > 
> > Change the format to three decimals, having full Hz granularity is silly
> > these days. Also specify in output if it's cpufreq or firmware-reported:
> > 
> > clock ? ? ? ? ? : 1250.000MHz (from firmware)
> > or:
> > clock ? ? ? ? ? : 2500.000MHz (actual speed)
> 
> 
> Good idea, but I'd be _very_ careful with changing any field in
> /proc/cpuinfo. Having the extra text in there can easily confuse
> parsers in poorly written user applications, maybe even in good ones ;-)
> 
> How about rather adding a new line in there for one of the fields, either

Yeah, good point. Actually, taking away the extra decimals is maybe a
bad idea as well.

I'll settle for clock/current-clock. See patch posted separately.


Thanks,

Olof

[PATCH v3] powerpc: Show current speed in /proc/cpuinfo

[Olof Johansson]

On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
changes. It makes sense to do the same on powerpc to keep users from
getting confused.

Change the format to three decimals, having full Hz granularity is silly
these days. Also specify in output if it's cpufreq or firmware-reported:

clock           : 1250.000MHz (from firmware)
or:
clock           : 2500.000MHz (actual speed)


Signed-off-by: Olof Johansson <olof@lixom.net>


Index: 2.6.21/arch/powerpc/kernel/setup-common.c
===================================================================
--- 2.6.21.orig/arch/powerpc/kernel/setup-common.c
+++ 2.6.21/arch/powerpc/kernel/setup-common.c
@@ -29,6 +29,7 @@
 #include <linux/root_dev.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
+#include <linux/cpufreq.h>
 #include <linux/unistd.h>
 #include <linux/serial.h>
 #include <linux/serial_8250.h>
@@ -160,6 +161,7 @@ static int show_cpuinfo(struct seq_file 
 	unsigned int pvr;
 	unsigned short maj;
 	unsigned short min;
+	unsigned int freq;
 
 	if (cpu_id == NR_CPUS) {
 #if defined(CONFIG_SMP) && defined(CONFIG_PPC32)
@@ -226,14 +228,18 @@ static int show_cpuinfo(struct seq_file 
 	}
 #endif /* CONFIG_TAU */
 
-	/*
-	 * Assume here that all clock rates are the same in a
-	 * smp system.  -- Cort
-	 */
+	/* ppc_proc_freq is in MHz */
 	if (ppc_proc_freq)
 		seq_printf(m, "clock\t\t: %lu.%06luMHz\n",
 			   ppc_proc_freq / 1000000, ppc_proc_freq % 1000000);
 
+	/* cpufreq_quick_get() is in kHz */
+	freq = cpufreq_quick_get(cpu_id);
+
+	if (freq)
+		seq_printf(m, "current-clock\t: %u.%03u000MHz\n",
+			   freq / 1000, freq % 1000);
+
 	if (ppc_md.show_percpuinfo != NULL)
 		ppc_md.show_percpuinfo(m, cpu_id);
 

Re: [PATCH v3] powerpc: Show current speed in /proc/cpuinfo

[Stephen Rothwell]

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

On Thu, 3 May 2007 22:16:12 -0500 olof@lixom.net (Olof Johansson) wrote:
>
> +	/* ppc_proc_freq is in MHz */
                               ^^^
Actually Hz :-)

--
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: [PATCH v3] powerpc: Show current speed in /proc/cpuinfo

[Olof Johansson]

On Fri, May 04, 2007 at 01:39:57PM +1000, Stephen Rothwell wrote:
> On Thu, 3 May 2007 22:16:12 -0500 olof@lixom.net (Olof Johansson) wrote:
> >
> > +	/* ppc_proc_freq is in MHz */
>                                ^^^
> Actually Hz :-)

Gaaah, yes. Last time I'm trying to comment my code. 2 mess-ups in
two submissions. :)

Thanks for spotting it.


-Olof

Re: [PATCH v3] powerpc: Show current speed in /proc/cpuinfo

[Paul Mackerras]

Olof Johansson writes:

> On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
> changes. It makes sense to do the same on powerpc to keep users from
> getting confused.

Well, it does on powermac already, because ppc_proc_freq gets
adjusted...  So at the very least, your patch description needs to be
changed.

Do you have a situation where the CPUs in a system can have different
clock frequencies?  If not, why not just set ppc_proc_freq when the
frequency changes?

Paul.

[PATCH] pasemi: update ppc_proc_freq from cpufreq driver

[Olof Johansson]

Update the global cpu speed variable according to current cpufreq speed,
/proc/cpuinfo reports the actual speed.


Signed-off-by: Olof Johansson <olof@lixom.net>

---

On Fri, May 04, 2007 at 02:00:36PM +1000, Paul Mackerras wrote:
> Olof Johansson writes:
> 
> > On other architectures, the frequency in /proc/cpuinfo moves with cpufreq
> > changes. It makes sense to do the same on powerpc to keep users from
> > getting confused.
> 
> Well, it does on powermac already, because ppc_proc_freq gets
> adjusted...  So at the very least, your patch description needs to be
> changed.
> 
> Do you have a situation where the CPUs in a system can have different
> clock frequencies?  If not, why not just set ppc_proc_freq when the
> frequency changes?

Sure. I'll let the cell guys do the equivalent change on their own.


Index: 2.6.21/arch/powerpc/platforms/pasemi/cpufreq.c
===================================================================
--- 2.6.21.orig/arch/powerpc/platforms/pasemi/cpufreq.c
+++ 2.6.21/arch/powerpc/platforms/pasemi/cpufreq.c
@@ -31,6 +31,7 @@
 #include <asm/hw_irq.h>
 #include <asm/io.h>
 #include <asm/prom.h>
+#include <asm/time.h>
 
 #define SDCASR_REG		0x0100
 #define SDCASR_REG_STRIDE	0x1000
@@ -204,6 +205,8 @@ static int pas_cpufreq_cpu_init(struct c
 	policy->cur = pas_freqs[cur_astate].frequency;
 	policy->cpus = cpu_online_map;
 
+	ppc_proc_freq = policy->cur * 1000ul;
+
 	cpufreq_frequency_table_get_attr(pas_freqs, policy->cpu);
 
 	/* this ensures that policy->cpuinfo_min and policy->cpuinfo_max
@@ -270,6 +273,7 @@ static int pas_cpufreq_target(struct cpu
 	cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
 	mutex_unlock(&pas_switch_mutex);
 
+	ppc_proc_freq = freqs.new * 1000ul;
 	return 0;
 }