Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1416636 > unrolled thread

[RFC 0/1] ARM: print MHz in /proc/cpuinfo

Back to article view | Back to linux.kernel

Contents

  [RFC 0/1] ARM: print MHz in /proc/cpuinfo Jon Mason <jon.mason@broadcom.com> - 2016-06-07 23:10 +0200
    [RFC 1/1] ARM: print MHz in /proc/cpuinfo Jon Mason <jon.mason@broadcom.com> - 2016-06-07 23:10 +0200
      Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo Sudeep Holla <sudeep.holla@arm.com> - 2016-06-08 10:40 +0200
        Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo Jon Mason <jon.mason@broadcom.com> - 2016-06-08 21:40 +0200
          Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo Sudeep Holla <sudeep.holla@arm.com> - 2016-06-09 11:10 +0200
            Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo Jon Mason <jon.mason@broadcom.com> - 2016-06-09 19:40 +0200
    Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-06-08 00:30 +0200
      Re: [RFC 0/1] ARM: print MHz in /proc/cpuinfo Jon Mason <jon.mason@broadcom.com> - 2016-06-08 01:00 +0200

#1416636 — [RFC 0/1] ARM: print MHz in /proc/cpuinfo

Many users (and some applications) are expecting the CPU clock speed to
be output in /proc/cpuinfo (as is done in x86, avr32, c6x, tile, parisc,
ia64, and xtensa).  This can be trivially added by simply querying the
clock described in the CPU node of the device tree.  It appears that
many of the DTSI files in arch/arm/boot/dts already have this defined.
So, this will add this desired functionality for many boards with
already existing information.  For those that do not have this defined,
it will simply not output the string in question (thus keeping
everything the same as before).

The output was modeled after x86 (based on number of significant digits
and location in the output), but is similar to other architectures.  For
example, the output on my local board looks like:

# cat /proc/cpuinfo 
processor	: 0
model name	: ARMv7 Processor rev 0 (v7l)
cpu MHz		: 1200.000
BogoMIPS	: 1200.00
Features	: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpd32 
CPU implementer	: 0x41
CPU architecture: 7
CPU variant	: 0x3
CPU part	: 0xc09
CPU revision	: 0

processor	: 1
model name	: ARMv7 Processor rev 0 (v7l)
cpu MHz		: 1200.000
BogoMIPS	: 1200.00
Features	: half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpd32 
CPU implementer	: 0x41
CPU architecture: 7
CPU variant	: 0x3
CPU part	: 0xc09
CPU revision	: 0

Hardware	: Broadcom Northstar Plus SoC
Revision	: 0000
Serial		: 0000000000000000

Thanks,
Jon



Jon Mason (1):
  ARM: print MHz in /proc/cpuinfo

 arch/arm/kernel/setup.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

-- 
1.9.1

[toc] | [next] | [standalone]

#1416639 — [RFC 1/1] ARM: print MHz in /proc/cpuinfo

Query the CPU core clock in the device tree to determine the core clock
speed.  Output this clock rate in /proc/cpuinfo to match the output
from other architectures.  The output is intentionally patterned after
the x86 output, to match existing (and possibly expected) convention.

If any errors are encountered in querying the clock (or the speed is
erroneously zero), nothing will be printed out.  Thus any existing
devices that do not have CPU clocks defined in the device tree will
work as before.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 arch/arm/kernel/setup.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 7b53500..0c3e25a 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -33,6 +33,7 @@
 #include <linux/compiler.h>
 #include <linux/sort.h>
 #include <linux/psci.h>
+#include <linux/clk.h>
 
 #include <asm/unified.h>
 #include <asm/cp15.h>
@@ -1178,10 +1179,32 @@ static const char *hwcap2_str[] = {
 	NULL
 };
 
+static unsigned long cpu_freq(unsigned int core)
+{
+	struct device_node *np;
+	struct clk *c;
+	unsigned long rate = 0;
+
+	np = of_get_cpu_node(core, NULL);
+	if (!np)
+		goto err;
+
+	c = of_clk_get_by_name(np, NULL);
+	if (IS_ERR(c))
+		goto err;
+
+	rate = clk_get_rate(c);
+
+	clk_put(c);
+err:
+	return rate;
+}
+
 static int c_show(struct seq_file *m, void *v)
 {
 	int i, j;
 	u32 cpuid;
+	unsigned long rate;
 
 	for_each_online_cpu(i) {
 		/*
@@ -1194,6 +1217,12 @@ static int c_show(struct seq_file *m, void *v)
 		seq_printf(m, "model name\t: %s rev %d (%s)\n",
 			   cpu_name, cpuid & 15, elf_platform);
 
+		rate = cpu_freq(i);
+		if (rate)
+			/* Change from Hz into MHz */
+			seq_printf(m, "cpu MHz\t\t: %lu.%03lu\n",
+				   rate / 1000000, rate / 1000 % 1000);
+
 #if defined(CONFIG_SMP)
 		seq_printf(m, "BogoMIPS\t: %lu.%02lu\n",
 			   per_cpu(cpu_data, i).loops_per_jiffy / (500000UL/HZ),
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1417051 — Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

On 07/06/16 22:08, Jon Mason wrote:
> Query the CPU core clock in the device tree to determine the core clock
> speed.

How do guarantee that it's the current frequency of the CPU ?
It doesn't even represent the mix or max frequency, so it's incorrect.
Some DTs have boot frequency in that entry.

> Output this clock rate in /proc/cpuinfo to match the output
> from other architectures.  The output is intentionally patterned after
> the x86 output, to match existing (and possibly expected) convention.
>
> If any errors are encountered in querying the clock (or the speed is
> erroneously zero), nothing will be printed out.  Thus any existing
> devices that do not have CPU clocks defined in the device tree will
> work as before.
>

What if they just don't have in DT but have DVFS support ?

Also whey do we need this support when the user-space can query the
CPUFreq sysfs which is more accurate and maintains the current running
frequency ?

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1417791 — Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

On Wed, Jun 08, 2016 at 09:34:06AM +0100, Sudeep Holla wrote:
> 
> 
> On 07/06/16 22:08, Jon Mason wrote:
> >Query the CPU core clock in the device tree to determine the core clock
> >speed.
> 
> How do guarantee that it's the current frequency of the CPU ?

I am basing it on the assumption (perhaps incorrect) that the clock in
the CPU DT corresponds to the one determining the CPU clock rate.  And,
that this clock rate is accurate in describing the speed at which the
CPU is currently running.

> It doesn't even represent the mix or max frequency, so it's incorrect.
> Some DTs have boot frequency in that entry.
> 
> >Output this clock rate in /proc/cpuinfo to match the output
> >from other architectures.  The output is intentionally patterned after
> >the x86 output, to match existing (and possibly expected) convention.
> >
> >If any errors are encountered in querying the clock (or the speed is
> >erroneously zero), nothing will be printed out.  Thus any existing
> >devices that do not have CPU clocks defined in the device tree will
> >work as before.
> >
> 
> What if they just don't have in DT but have DVFS support ?

This can be extended to cover DVFS or SMC calls or anything else.
This was simply a first step to cover what appeared to be the most
prevalent case.

> Also whey do we need this support when the user-space can query the
> CPUFreq sysfs which is more accurate and maintains the current running
> frequency ?

This is exactly what x86 is doing to provide its value in
/proc/cpuinfo.  I could easily augment this patch to call
cpufreq_quick_get(), if it returns 0, then call clk_get_rate().  If
both return 0, then simply not print out anything (which would cover
all of the possibilities).  Or, I could have it just call
cpufreq_quick_get() to get the value.

Thanks,
Jon

> 
> -- 
> Regards,
> Sudeep

[toc] | [prev] | [next] | [standalone]

#1418185 — Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

On 08/06/16 20:31, Jon Mason wrote:
> On Wed, Jun 08, 2016 at 09:34:06AM +0100, Sudeep Holla wrote:
>>
>>
>> On 07/06/16 22:08, Jon Mason wrote:
>>> Query the CPU core clock in the device tree to determine the core clock
>>> speed.
>>
>> How do guarantee that it's the current frequency of the CPU ?
>
> I am basing it on the assumption (perhaps incorrect) that the clock in
> the CPU DT corresponds to the one determining the CPU clock rate.  And,
> that this clock rate is accurate in describing the speed at which the
> CPU is currently running.
>

As you already noticed, it's not always correct.

[..]

>>
>> What if they just don't have in DT but have DVFS support ?
>
> This can be extended to cover DVFS or SMC calls or anything else.
> This was simply a first step to cover what appeared to be the most
> prevalent case.
>

Using DVFS/CPUFreq makes this DT based approach irrelevant.

>> Also whey do we need this support when the user-space can query the
>> CPUFreq sysfs which is more accurate and maintains the current running
>> frequency ?
>
> This is exactly what x86 is doing to provide its value in
> /proc/cpuinfo.  I could easily augment this patch to call
> cpufreq_quick_get(), if it returns 0, then call clk_get_rate().  If
> both return 0, then simply not print out anything (which would cover
> all of the possibilities).  Or, I could have it just call
> cpufreq_quick_get() to get the value.
>

Agree x86 has, may be for legacy reasons. It even has CPUFreq sysfs
entries which is architecture agnostic while /proc/cpuinfo is more
architecture based. So applications that want to be portable across
architectures must choose the generic CPUFreq sysfs path rather than
some x86 based cpuinfo.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1418529 — Re: [RFC 1/1] ARM: print MHz in /proc/cpuinfo

On Thu, Jun 9, 2016 at 5:09 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 08/06/16 20:31, Jon Mason wrote:
>>
>> On Wed, Jun 08, 2016 at 09:34:06AM +0100, Sudeep Holla wrote:
>>>
>>>
>>>
>>> On 07/06/16 22:08, Jon Mason wrote:
>>>>
>>>> Query the CPU core clock in the device tree to determine the core clock
>>>> speed.
>>>
>>>
>>> How do guarantee that it's the current frequency of the CPU ?
>>
>>
>> I am basing it on the assumption (perhaps incorrect) that the clock in
>> the CPU DT corresponds to the one determining the CPU clock rate.  And,
>> that this clock rate is accurate in describing the speed at which the
>> CPU is currently running.
>>
>
> As you already noticed, it's not always correct.
>
> [..]
>
>>>
>>> What if they just don't have in DT but have DVFS support ?
>>
>>
>> This can be extended to cover DVFS or SMC calls or anything else.
>> This was simply a first step to cover what appeared to be the most
>> prevalent case.
>>
>
> Using DVFS/CPUFreq makes this DT based approach irrelevant.
>
>>> Also whey do we need this support when the user-space can query the
>>> CPUFreq sysfs which is more accurate and maintains the current running
>>> frequency ?
>>
>>
>> This is exactly what x86 is doing to provide its value in
>> /proc/cpuinfo.  I could easily augment this patch to call
>> cpufreq_quick_get(), if it returns 0, then call clk_get_rate().  If
>> both return 0, then simply not print out anything (which would cover
>> all of the possibilities).  Or, I could have it just call
>> cpufreq_quick_get() to get the value.
>>
>
> Agree x86 has, may be for legacy reasons. It even has CPUFreq sysfs
> entries which is architecture agnostic while /proc/cpuinfo is more
> architecture based. So applications that want to be portable across
> architectures must choose the generic CPUFreq sysfs path rather than
> some x86 based cpuinfo.

Thank you for educating me.  I am taking this (and RMK's comment) as
any modification to add CPU speed to /proc/cpuinfo is not welcomed,
anyone who wants to query this should instead look at cpufreq in
sysfs, and any dev who wants to add such a thing should look into
writing a driver in drivers/cpufreq/.

Thanks,
Jon

>
> --
> Regards,
> Sudeep

[toc] | [prev] | [next] | [standalone]

#1416685

On Tue, Jun 07, 2016 at 05:08:32PM -0400, Jon Mason wrote:
> Many users (and some applications) are expecting the CPU clock speed to
> be output in /proc/cpuinfo (as is done in x86, avr32, c6x, tile, parisc,
> ia64, and xtensa).

Such as what applications?  This is just another meaningless number,
which is just as meaningless as the bogomips number.  It tells you
nothing really about the CPU which should remotely be used for
anything other than user display.  It certainly can't be used for
algorithmic selection.

We have resisted publishing this information for years because not
every ARM CPU is capable of providing this information - for many, we
don't know what the CPU clock rate even is.  I believe it is a mistake
to publish this information.  If userspace wants to select an algorithm,
that needs to be done according to much more information than just the
CPU speed - it needs knowledge of the instruction timings as well, cache
behaviour, etc, and you might as well benchmark an implementation and
select at run time, caching the result.

Since we've never exported this information, it's not a regression
and it's not part of the kernels standard API.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1416705

On Tue, Jun 07, 2016 at 11:18:10PM +0100, Russell King - ARM Linux wrote:
> On Tue, Jun 07, 2016 at 05:08:32PM -0400, Jon Mason wrote:
> > Many users (and some applications) are expecting the CPU clock speed to
> > be output in /proc/cpuinfo (as is done in x86, avr32, c6x, tile, parisc,
> > ia64, and xtensa).
> 
> Such as what applications?  This is just another meaningless number,

To be honest, I don't have any direct knowledge of this.  I saw it in
passing while googling to see if anyone had pushed a patch like this
before.  Lots of people complaining and asking for help on message
boards.  I think it has been mostly "fixed" by those apps now using
bogomips, but per your comment below, that is not optimal.

> which is just as meaningless as the bogomips number.  It tells you
> nothing really about the CPU which should remotely be used for
> anything other than user display.  It certainly can't be used for
> algorithmic selection.

As far as being something useful, it does have some benefits.  It can
tell you the speed the core is currently running at, which is
beneficial when trying to determine if the power management is
stepping up/down the core based on load, etc.  This could be queried
via the clk_summary in debugfs, but this is not always enabled.

Also, Linux developers/users and (more important to me) customers
coming to ARM from x86 are expecting this to be there.  While it is
completely fair to tell them "this is ARM, it is different, get used
to it", it will not stop them from asking.

> We have resisted publishing this information for years because not
> every ARM CPU is capable of providing this information - for many, we
> don't know what the CPU clock rate even is.  I believe it is a mistake
> to publish this information.  If userspace wants to select an algorithm,
> that needs to be done according to much more information than just the
> CPU speed - it needs knowledge of the instruction timings as well, cache
> behaviour, etc, and you might as well benchmark an implementation and
> select at run time, caching the result.
> 
> Since we've never exported this information, it's not a regression
> and it's not part of the kernels standard API.

I do not think it is a regression, and I'm sorry if I implied it.  

For many boards, the information is already there.  From a technical
perspective, it is no big feat to query and print it (and make people
happy).  For the boards that do not have it (or it is not relevant),
we can say "this is ARM, it is different, get used to it".

Thanks,
Jon

> 
> -- 
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web