spurious bL cpufreq driver messages

DISCUSSION Rob Herring 3 participants 9 messages May 17, 2013 – May 22, 2013 CPU frequency scaling framework

Share & tools

Rob Herring

author

2013-05-17 19:36 UTC #0 lore raw

The bL cpufreq driver appears to collide with the cpufreq-cpu0 driver. There doesn't appear to be a functional problem, but just spurious prints which indicate it is trying to do something. I think it needs better checking whether the chip is actually multi-cluster. This is what I get when I hotplug cpus:

[  909.372319] CPU1: Booted secondary processor
[  909.372616] cpu cpu1: get_cluster_clk_and_freq_table: init_opp_table
failed, cpu: 1, err: -61
[  909.385411] cpu cpu1: get_cluster_clk_and_freq_table: Failed to get
data for cluster: 9
[  909.394161] CPU2: shutdown
[  909.400957] CPU2: Booted secondary processor
[  909.401253] cpu cpu2: get_cluster_clk_and_freq_table: init_opp_table
failed, cpu: 2, err: -61
[  909.414044] cpu cpu2: get_cluster_clk_and_freq_table: Failed to get

data for cluster: 9

Rob

Viresh Kumar

maintainer

2013-05-18 02:03 UTC #1 lore raw

Quoted 16 lines · Rob Herring

The bL cpufreq driver appears to collide with the cpufreq-cpu0 driver. There doesn't appear to be a functional problem, but just spurious prints which indicate it is trying to do something. I think it needs better checking whether the chip is actually multi-cluster. This is what I get when I hotplug cpus:
[  909.372319] CPU1: Booted secondary processor
[  909.372616] cpu cpu1: get_cluster_clk_and_freq_table: init_opp_table
failed, cpu: 1, err: -61
[  909.385411] cpu cpu1: get_cluster_clk_and_freq_table: Failed to get
data for cluster: 9
[  909.394161] CPU2: shutdown
[  909.400957] CPU2: Booted secondary processor
[  909.401253] cpu cpu2: get_cluster_clk_and_freq_table: init_opp_table
failed, cpu: 2, err: -61
[  909.414044] cpu cpu2: get_cluster_clk_and_freq_table: Failed to get

Quoted 1 line

data for cluster: 9

See if this fixes your issue.

https://groups.google.com/forum/?fromgroups#!topic/linux.kernel/XJF-82rad4o

Attached too.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cpufreq-arm_big_little_dt-Register-driver-only-if-DT.patch
Type: application/octet-stream
Size: 3967 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130518/88214fa6/attachment.obj>

Rob Herring

author

2013-05-18 19:51 UTC #2 lore raw

Quoted 19 lines · Viresh Kumar

Quoted 16 lines · Rob Herring
The bL cpufreq driver appears to collide with the cpufreq-cpu0 driver. There doesn't appear to be a functional problem, but just spurious prints which indicate it is trying to do something. I think it needs better checking whether the chip is actually multi-cluster. This is what I get when I hotplug cpus:
[  909.372319] CPU1: Booted secondary processor
[  909.372616] cpu cpu1: get_cluster_clk_and_freq_table: init_opp_table
failed, cpu: 1, err: -61
[  909.385411] cpu cpu1: get_cluster_clk_and_freq_table: Failed to get
data for cluster: 9
[  909.394161] CPU2: shutdown
[  909.400957] CPU2: Booted secondary processor
[  909.401253] cpu cpu2: get_cluster_clk_and_freq_table: init_opp_table
failed, cpu: 2, err: -61
[  909.414044] cpu cpu2: get_cluster_clk_and_freq_table: Failed to get
Quoted 1 line
data for cluster: 9

Quoted 5 lines

See if this fixes your issue.
https://groups.google.com/forum/?fromgroups#!topic/linux.kernel/XJF-82rad4o
Attached too.

No, it still registers the bL driver. The problem is not that DT data is missing, but it is present on a single cluster system. This is what my 1st cpu node looks like:

$ ls /proc/device-tree/cpus/cpu at 900/
clock-latency  clocks      device_type  next-level-cache  reg
clock-names    compatible  name         operating-points  transition-latency

This is the boot log:

[    1.192186] cpu cpu0: get_cluster_clk_and_freq_table: Failed to get
clk for cpu: 0, cluster: 9
[    1.200804] cpu cpu0: get_cluster_clk_and_freq_table: Failed to get
data for cluster: 9
[    1.208815] cpu cpu1: get_cluster_clk_and_freq_table:
init_opp_table failed, cpu: 1, err: -61
[    1.217345] cpu cpu1: get_cluster_clk_and_freq_table: Failed to get
data for cluster: 9
[    1.225354] cpu cpu2: get_cluster_clk_and_freq_table:
init_opp_table failed, cpu: 2, err: -61
[    1.233879] cpu cpu2: get_cluster_clk_and_freq_table: Failed to get
data for cluster: 9
[    1.241880] cpu cpu3: get_cluster_clk_and_freq_table:
init_opp_table failed, cpu: 3, err: -61
[    1.250403] cpu cpu3: get_cluster_clk_and_freq_table: Failed to get
data for cluster: 9
[    1.258406] arm_big_little: bL_cpufreq_register: Registered
platform driver: dt-bl
[    1.266123] cpufreq_cpu0: failed to get cpu0 regulator: -19
[    1.271704] cpufreq_cpu0: failed register driver: -16
[    1.276764] cpufreq-cpu0: probe of cpufreq-cpu0.0 failed with error -16

Rob

Viresh Kumar

maintainer

2013-05-20 04:36 UTC #3 lore raw

Quoted 2 lines · Rob Herring

No, it still registers the bL driver. The problem is not that DT data is missing, but it is present on a single cluster system.

I misread your mail.. I got the problem now.. Below should fix your issue (It is rebased on the patch I pointed to you earlier):

Attached too for testing as gmail copy-paste will break it.

@Rafael: Once Rob gives his Tested-by, please queue it for 3.10-rcs

------------x-----------------x-----------------

From: Viresh Kumar <[email protected]>
Date: Mon, 20 May 2013 09:57:17 +0530
Subject: [PATCH] cpufreq: arm_big_little_dt: Instantiate as platform_driver

As multiplatform build is being adopted by more and more ARM platforms, initcall function should be used very carefully. For example, when both arm_big_little_dt and cpufreq-cpu0 drivers are compiled in, arm_big_little_dt driver may try to register even if we had platform device for cpufreq-cpu0 registered.

To eliminate this undesired the effect, the patch changes arm_big_little_dt driver to have it instantiated as a platform_driver. Then it will only run on platforms that create the platform_device "arm-bL-cpufreq-dt".

Reported-by Rob Herring <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 drivers/cpufreq/arm_big_little_dt.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

(quoted diff) +0 -0

b/drivers/cpufreq/arm_big_little_dt.c
index 27e2f45..fd9e3ea 100644

drivers/cpufreq/arm_big_little_dt.c +15 -5

@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/opp.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include "arm_big_little.h"

@@ -95,7 +96,7 @@ static struct cpufreq_arm_bL_ops dt_bL_ops = {
 	.init_opp_table = dt_init_opp_table,
 };
 
-static int generic_bL_init(void)
+static int generic_bL_probe(struct platform_device *pdev)
 {
 	struct device_node *np;

@@ -106,13 +107,22 @@ static int generic_bL_init(void)
 	of_node_put(np);
 	return bL_cpufreq_register(&dt_bL_ops);
 }
-module_init(generic_bL_init);
 
-static void generic_bL_exit(void)
+static int generic_bL_remove(struct platform_device *pdev)
 {
-	return bL_cpufreq_unregister(&dt_bL_ops);
+	bL_cpufreq_unregister(&dt_bL_ops);
+	return 0;
 }
-module_exit(generic_bL_exit);
+
+static struct platform_driver generic_bL_platdrv = {
+	.driver = {
+		.name	= "arm-bL-cpufreq-dt",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= generic_bL_probe,
+	.remove		= generic_bL_remove,
+};
+module_platform_driver(generic_bL_platdrv);
 
 MODULE_AUTHOR("Viresh Kumar <[email protected]>");
 MODULE_DESCRIPTION("Generic ARM big LITTLE cpufreq driver via DT");

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cpufreq-arm_big_little_dt-Instantiate-as-platform_dr.patch
Type: application/octet-stream
Size: 2494 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130520/e3af1072/attachment.obj>

Rob Herring

author Tested-by

2013-05-21 16:42 UTC #4 lore raw

Quoted 22 lines · Viresh Kumar

Quoted 2 lines · Rob Herring
No, it still registers the bL driver. The problem is not that DT data is missing, but it is present on a single cluster system.
I misread your mail.. I got the problem now.. Below should fix your issue (It is rebased on the patch I pointed to you earlier):
Attached too for testing as gmail copy-paste will break it.
@Rafael: Once Rob gives his Tested-by, please queue it for 3.10-rcs
------------x-----------------x-----------------
From: Viresh Kumar <[email protected]>
Date: Mon, 20 May 2013 09:57:17 +0530
Subject: [PATCH] cpufreq: arm_big_little_dt: Instantiate as platform_driver
As multiplatform build is being adopted by more and more ARM platforms, initcall function should be used very carefully. For example, when both arm_big_little_dt and cpufreq-cpu0 drivers are compiled in, arm_big_little_dt driver may try to register even if we had platform device for cpufreq-cpu0 registered.

Quoted 6 lines

To eliminate this undesired the effect, the patch changes arm_big_little_dt driver to have it instantiated as a platform_driver. Then it will only run on platforms that create the platform_device "arm-bL-cpufreq-dt".
Reported-by Rob Herring <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>

That works. Thanks.

Tested-by Rob Herring <[email protected]>

Rafael J. Wysocki

2013-05-21 23:37 UTC #5 lore raw

In reply to Viresh Kumar

Quoted 22 lines · Viresh Kumar

Quoted 2 lines · Rob Herring
No, it still registers the bL driver. The problem is not that DT data is missing, but it is present on a single cluster system.
I misread your mail.. I got the problem now.. Below should fix your issue (It is rebased on the patch I pointed to you earlier):
Attached too for testing as gmail copy-paste will break it.
@Rafael: Once Rob gives his Tested-by, please queue it for 3.10-rcs
------------x-----------------x-----------------
From: Viresh Kumar <[email protected]>
Date: Mon, 20 May 2013 09:57:17 +0530
Subject: [PATCH] cpufreq: arm_big_little_dt: Instantiate as platform_driver
As multiplatform build is being adopted by more and more ARM platforms, initcall function should be used very carefully. For example, when both arm_big_little_dt and cpufreq-cpu0 drivers are compiled in, arm_big_little_dt driver may try to register even if we had platform device for cpufreq-cpu0 registered.

Quoted 6 lines

To eliminate this undesired the effect, the patch changes arm_big_little_dt driver to have it instantiated as a platform_driver. Then it will only run on platforms that create the platform_device "arm-bL-cpufreq-dt".
Reported-by Rob Herring <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>

The patch (from the attachment) didn't apply for me. Please check the bleeding-edge branch.

Thanks,
Rafael

Quoted 53 lines

---
 drivers/cpufreq/arm_big_little_dt.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

drivers/cpufreq/arm_big_little_dt.c +15 -5

index 27e2f45..fd9e3ea 100644
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/opp.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include "arm_big_little.h"

@@ -95,7 +96,7 @@ static struct cpufreq_arm_bL_ops dt_bL_ops = {
 	.init_opp_table = dt_init_opp_table,
 };
 
-static int generic_bL_init(void)
+static int generic_bL_probe(struct platform_device *pdev)
 {
 	struct device_node *np;

@@ -106,13 +107,22 @@ static int generic_bL_init(void)
 	of_node_put(np);
 	return bL_cpufreq_register(&dt_bL_ops);
 }
-module_init(generic_bL_init);
 
-static void generic_bL_exit(void)
+static int generic_bL_remove(struct platform_device *pdev)
 {
-	return bL_cpufreq_unregister(&dt_bL_ops);
+	bL_cpufreq_unregister(&dt_bL_ops);
+	return 0;
 }
-module_exit(generic_bL_exit);
+
+static struct platform_driver generic_bL_platdrv = {
+	.driver = {
+		.name	= "arm-bL-cpufreq-dt",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= generic_bL_probe,
+	.remove		= generic_bL_remove,
+};
+module_platform_driver(generic_bL_platdrv);
 
 MODULE_AUTHOR("Viresh Kumar <[email protected]>");
 MODULE_DESCRIPTION("Generic ARM big LITTLE cpufreq driver via DT");

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Viresh Kumar

maintainer

2013-05-22 01:14 UTC #6 lore raw

Quoted 27 lines · Rafael J. Wysocki

Quoted 26 lines · Viresh Kumar
Quoted 2 lines · Rob Herring
No, it still registers the bL driver. The problem is not that DT data is missing, but it is present on a single cluster system.
I misread your mail.. I got the problem now.. Below should fix your issue (It is rebased on the patch I pointed to you earlier):
Attached too for testing as gmail copy-paste will break it.
@Rafael: Once Rob gives his Tested-by, please queue it for 3.10-rcs
------------x-----------------x-----------------
From: Viresh Kumar <[email protected]>
Date: Mon, 20 May 2013 09:57:17 +0530
Subject: [PATCH] cpufreq: arm_big_little_dt: Instantiate as platform_driver
As multiplatform build is being adopted by more and more ARM platforms, initcall function should be used very carefully. For example, when both arm_big_little_dt and cpufreq-cpu0 drivers are compiled in, arm_big_little_dt driver may try to register even if we had platform device for cpufreq-cpu0 registered.
To eliminate this undesired the effect, the patch changes arm_big_little_dt driver to have it instantiated as a platform_driver. Then it will only run on platforms that create the platform_device "arm-bL-cpufreq-dt".

Quoted 5 lines

Quoted 2 lines
Reported-by Rob Herring <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
The patch (from the attachment) didn't apply for me. Please check the bleeding-edge branch.

Have you tried applying this before it (As written in my last mail)?

https://groups.google.com/forum/?fromgroups#!topic/linux.kernel/XJF-82rad4o

Above is also required fix for 3.10-rc3

Rafael J. Wysocki

2013-05-22 10:45 UTC #7 lore raw

Quoted 28 lines · Viresh Kumar

Quoted 27 lines · Rafael J. Wysocki
Quoted 26 lines · Viresh Kumar
Quoted 2 lines · Rob Herring
No, it still registers the bL driver. The problem is not that DT data is missing, but it is present on a single cluster system.
I misread your mail.. I got the problem now.. Below should fix your issue (It is rebased on the patch I pointed to you earlier):
Attached too for testing as gmail copy-paste will break it.
@Rafael: Once Rob gives his Tested-by, please queue it for 3.10-rcs
------------x-----------------x-----------------
From: Viresh Kumar <[email protected]>
Date: Mon, 20 May 2013 09:57:17 +0530
Subject: [PATCH] cpufreq: arm_big_little_dt: Instantiate as platform_driver
As multiplatform build is being adopted by more and more ARM platforms, initcall function should be used very carefully. For example, when both arm_big_little_dt and cpufreq-cpu0 drivers are compiled in, arm_big_little_dt driver may try to register even if we had platform device for cpufreq-cpu0 registered.
To eliminate this undesired the effect, the patch changes arm_big_little_dt driver to have it instantiated as a platform_driver. Then it will only run on platforms that create the platform_device "arm-bL-cpufreq-dt".

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
Reported-by Rob Herring <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
The patch (from the attachment) didn't apply for me. Please check the bleeding-edge branch.
Have you tried applying this before it (As written in my last mail)?

No and I'm not sure which mail you're referring to. There's been quite a number of them recently. :-)

Quoted 3 lines

https://groups.google.com/forum/?fromgroups#!topic/linux.kernel/XJF-82rad4o
Above is also required fix for 3.10-rc3

OK, that worked.

Please check the bleeding-edge branch now and see if anything for-3.10-rc is still missing.

Thanks,
Rafael

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Viresh Kumar

maintainer

2013-05-22 10:52 UTC #8 lore raw

Quoted 2 lines · Rafael J. Wysocki

Please check the bleeding-edge branch now and see if anything for-3.10-rc is still missing.

You have picked all my pending patches for current rc release.. All pending are targeted for 3.11 or linux-next. So all good.

thanks.