Skip to contentSkip to conversation

ARM: imx6: add opp table when cpufreq is enabled

PATCH New Anson Huang 3 participants 8 messages Aug 23, 2016 – Aug 29, 2016

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape
lore
Share & tools
lore

Anson Huang

author
#0 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

As cpufreq driver expects opp table is supplied by platform, so it is better to add opp table if cpufreq is enabled.

Signed-off-by Anson Huang <[email protected]>
---
 arch/arm/mach-imx/common.h      |  1 +
 arch/arm/mach-imx/mach-imx6q.c  | 31 ++++---------------------------
 arch/arm/mach-imx/mach-imx6sl.c |  4 +++-
 arch/arm/mach-imx/mach-imx6sx.c |  4 +++-
 arch/arm/mach-imx/mach-imx6ul.c |  4 +++-
 arch/arm/mach-imx/pm-imx6.c     | 25 +++++++++++++++++++++++++
 6 files changed, 39 insertions(+), 30 deletions(-)
arch/arm/mach-imx/common.h +1 -0
index bcca481..b757811 100644
@@ -124,6 +124,7 @@ static inline void imx6_suspend(void __iomem *ocram_vbase) {}
 #endif
 
 void imx6_pm_ccm_init(const char *ccm_compat);
+void imx6_pm_opp_init(void);
 void imx6q_pm_init(void);
 void imx6dl_pm_init(void);
 void imx6sl_pm_init(void);
arch/arm/mach-imx/mach-imx6q.c +4 -27
index 97fd251..09d295b 100644
@@ -338,32 +338,6 @@ put_node:
 	of_node_put(np);
 }
 
-static void __init imx6q_opp_init(void)
-{
-	struct device_node *np;
-	struct device *cpu_dev = get_cpu_device(0);
-
-	if (!cpu_dev) {
-		pr_warn("failed to get cpu0 device\n");
-		return;
-	}
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_warn("failed to find cpu0 node\n");
-		return;
-	}
-
-	if (dev_pm_opp_of_add_table(cpu_dev)) {
-		pr_warn("failed to init OPP table\n");
-		goto put_node;
-	}
-
-	imx6q_opp_check_speed_grading(cpu_dev);
-
-put_node:
-	of_node_put(np);
-}
-
 static struct platform_device imx6q_cpufreq_pdev = {
 	.name = "imx6q-cpufreq",
 };

@@ -378,7 +352,10 @@ static void __init imx6q_init_late(void)
 		imx6q_cpuidle_init();
 
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
-		imx6q_opp_init();
+		struct device *cpu_dev = get_cpu_device(0);
+
+		imx6_pm_opp_init();
+		imx6q_opp_check_speed_grading(cpu_dev);
 		platform_device_register(&imx6q_cpufreq_pdev);
 	}
 }
arch/arm/mach-imx/mach-imx6sl.c +3 -1
index 0408490..009bfa8 100644
@@ -38,8 +38,10 @@ static void __init imx6sl_fec_init(void)
 static void __init imx6sl_init_late(void)
 {
 	/* imx6sl reuses imx6q cpufreq driver */
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 
 	imx6sl_cpuidle_init();
 }
arch/arm/mach-imx/mach-imx6sx.c +3 -1
index 7f52d9b..2c5b78b 100644
@@ -93,8 +93,10 @@ static void __init imx6sx_init_late(void)
 {
 	imx6sx_cpuidle_init();
 
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 }
 
 static const char * const imx6sx_dt_compat[] __initconst = {
arch/arm/mach-imx/mach-imx6ul.c +3 -1
index 6bb7d9c..c2cd61c 100644
@@ -80,8 +80,10 @@ static void __init imx6ul_init_irq(void)
 
 static void __init imx6ul_init_late(void)
 {
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 }
 
 static const char * const imx6ul_dt_compat[] __initconst = {
arch/arm/mach-imx/pm-imx6.c +25 -0
index fe708e2..9a6f34c 100644
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
+#include <linux/pm_opp.h>
 #include <linux/regmap.h>
 #include <linux/suspend.h>
 #include <asm/cacheflush.h>

@@ -620,6 +621,30 @@ void __init imx6_pm_ccm_init(const char *ccm_compat)
 	writel_relaxed(val, ccm_base + CLPCR);
 }
 
+void __init imx6_pm_opp_init(void)
+{
+	struct device_node *np;
+	struct device *cpu_dev = get_cpu_device(0);
+
+	if (!cpu_dev) {
+		pr_warn("failed to get cpu0 device\n");
+		return;
+	}
+	np = of_node_get(cpu_dev->of_node);
+	if (!np) {
+		pr_warn("failed to find cpu0 node\n");
+		return;
+	}
+
+	if (dev_pm_opp_of_add_table(cpu_dev)) {
+		pr_warn("failed to init OPP table\n");
+		goto put_node;
+	}
+
+put_node:
+	of_node_put(np);
+}
+
 void __init imx6q_pm_init(void)
 {
 	imx6_pm_common_init(&imx6q_pm_data);
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

Quoted 175 lines · Anson Huang

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

As cpufreq driver expects opp table is supplied by platform, so it is better to add opp table if cpufreq is enabled.

Signed-off-by Anson Huang <[email protected]>
---
 arch/arm/mach-imx/common.h      |  1 +
 arch/arm/mach-imx/mach-imx6q.c  | 31 ++++---------------------------
 arch/arm/mach-imx/mach-imx6sl.c |  4 +++-
 arch/arm/mach-imx/mach-imx6sx.c |  4 +++-
 arch/arm/mach-imx/mach-imx6ul.c |  4 +++-
 arch/arm/mach-imx/pm-imx6.c     | 25 +++++++++++++++++++++++++
 6 files changed, 39 insertions(+), 30 deletions(-)
arch/arm/mach-imx/common.h +1 -0
index bcca481..b757811 100644
@@ -124,6 +124,7 @@ static inline void imx6_suspend(void __iomem *ocram_vbase) {}
 #endif
 
 void imx6_pm_ccm_init(const char *ccm_compat);
+void imx6_pm_opp_init(void);
 void imx6q_pm_init(void);
 void imx6dl_pm_init(void);
 void imx6sl_pm_init(void);
arch/arm/mach-imx/mach-imx6q.c +4 -27
index 97fd251..09d295b 100644
@@ -338,32 +338,6 @@ put_node:
 	of_node_put(np);
 }
 
-static void __init imx6q_opp_init(void)
-{
-	struct device_node *np;
-	struct device *cpu_dev = get_cpu_device(0);
-
-	if (!cpu_dev) {
-		pr_warn("failed to get cpu0 device\n");
-		return;
-	}
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_warn("failed to find cpu0 node\n");
-		return;
-	}
-
-	if (dev_pm_opp_of_add_table(cpu_dev)) {
-		pr_warn("failed to init OPP table\n");
-		goto put_node;
-	}
-
-	imx6q_opp_check_speed_grading(cpu_dev);
-
-put_node:
-	of_node_put(np);
-}
-
 static struct platform_device imx6q_cpufreq_pdev = {
 	.name = "imx6q-cpufreq",
 };

@@ -378,7 +352,10 @@ static void __init imx6q_init_late(void)
 		imx6q_cpuidle_init();
 
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
-		imx6q_opp_init();
+		struct device *cpu_dev = get_cpu_device(0);
+
+		imx6_pm_opp_init();
+		imx6q_opp_check_speed_grading(cpu_dev);
 		platform_device_register(&imx6q_cpufreq_pdev);
 	}
 }
arch/arm/mach-imx/mach-imx6sl.c +3 -1
index 0408490..009bfa8 100644
@@ -38,8 +38,10 @@ static void __init imx6sl_fec_init(void)
 static void __init imx6sl_init_late(void)
 {
 	/* imx6sl reuses imx6q cpufreq driver */
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 
 	imx6sl_cpuidle_init();
 }
arch/arm/mach-imx/mach-imx6sx.c +3 -1
index 7f52d9b..2c5b78b 100644
@@ -93,8 +93,10 @@ static void __init imx6sx_init_late(void)
 {
 	imx6sx_cpuidle_init();
 
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 }
 
 static const char * const imx6sx_dt_compat[] __initconst = {
arch/arm/mach-imx/mach-imx6ul.c +3 -1
index 6bb7d9c..c2cd61c 100644
@@ -80,8 +80,10 @@ static void __init imx6ul_init_irq(void)
 
 static void __init imx6ul_init_late(void)
 {
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 }
 
 static const char * const imx6ul_dt_compat[] __initconst = {
arch/arm/mach-imx/pm-imx6.c +25 -0
index fe708e2..9a6f34c 100644
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
+#include <linux/pm_opp.h>
 #include <linux/regmap.h>
 #include <linux/suspend.h>
 #include <asm/cacheflush.h>

@@ -620,6 +621,30 @@ void __init imx6_pm_ccm_init(const char *ccm_compat)
 	writel_relaxed(val, ccm_base + CLPCR);
 }
 
+void __init imx6_pm_opp_init(void)
+{
+	struct device_node *np;
+	struct device *cpu_dev = get_cpu_device(0);
+
+	if (!cpu_dev) {
+		pr_warn("failed to get cpu0 device\n");
+		return;
+	}
+	np = of_node_get(cpu_dev->of_node);
+	if (!np) {
+		pr_warn("failed to find cpu0 node\n");
+		return;
+	}
+
+	if (dev_pm_opp_of_add_table(cpu_dev)) {
+		pr_warn("failed to init OPP table\n");
+		goto put_node;
+	}
+
+put_node:
+	of_node_put(np);
+}
+
 void __init imx6q_pm_init(void)
 {
 	imx6_pm_common_init(&imx6q_pm_data);
Signature · 1 line
1.9.1

After applying this patch, the error message is gone, and cpufreq works well at my imx6sx and imx6ul boards.

Reviewed-by Peter Chen <[email protected]>
Tested-by Peter Chen <[email protected]>
Signature · 2 lines
Best Regards,
Peter Chen
#2 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

In reply to Anson Huang
Quoted 5 lines · Anson Huang

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

The cpufreq driver calls dev_pm_opp_get_opp_count() to test if platform supplies an OPP table. If not, the driver will call into dev_pm_opp_of_add_table() on its own. So this is not an error message, and cpufreq driver should just work fine on i.MX6SX and i.MX6SL.

Shawn

Quoted 175 lines

As cpufreq driver expects opp table is supplied by platform, so it is better to add opp table if cpufreq is enabled.

Signed-off-by Anson Huang <[email protected]>
---
 arch/arm/mach-imx/common.h      |  1 +
 arch/arm/mach-imx/mach-imx6q.c  | 31 ++++---------------------------
 arch/arm/mach-imx/mach-imx6sl.c |  4 +++-
 arch/arm/mach-imx/mach-imx6sx.c |  4 +++-
 arch/arm/mach-imx/mach-imx6ul.c |  4 +++-
 arch/arm/mach-imx/pm-imx6.c     | 25 +++++++++++++++++++++++++
 6 files changed, 39 insertions(+), 30 deletions(-)
arch/arm/mach-imx/common.h +1 -0
index bcca481..b757811 100644
@@ -124,6 +124,7 @@ static inline void imx6_suspend(void __iomem *ocram_vbase) {}
 #endif
 
 void imx6_pm_ccm_init(const char *ccm_compat);
+void imx6_pm_opp_init(void);
 void imx6q_pm_init(void);
 void imx6dl_pm_init(void);
 void imx6sl_pm_init(void);
arch/arm/mach-imx/mach-imx6q.c +4 -27
index 97fd251..09d295b 100644
@@ -338,32 +338,6 @@ put_node:
 	of_node_put(np);
 }
 
-static void __init imx6q_opp_init(void)
-{
-	struct device_node *np;
-	struct device *cpu_dev = get_cpu_device(0);
-
-	if (!cpu_dev) {
-		pr_warn("failed to get cpu0 device\n");
-		return;
-	}
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_warn("failed to find cpu0 node\n");
-		return;
-	}
-
-	if (dev_pm_opp_of_add_table(cpu_dev)) {
-		pr_warn("failed to init OPP table\n");
-		goto put_node;
-	}
-
-	imx6q_opp_check_speed_grading(cpu_dev);
-
-put_node:
-	of_node_put(np);
-}
-
 static struct platform_device imx6q_cpufreq_pdev = {
 	.name = "imx6q-cpufreq",
 };

@@ -378,7 +352,10 @@ static void __init imx6q_init_late(void)
 		imx6q_cpuidle_init();
 
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
-		imx6q_opp_init();
+		struct device *cpu_dev = get_cpu_device(0);
+
+		imx6_pm_opp_init();
+		imx6q_opp_check_speed_grading(cpu_dev);
 		platform_device_register(&imx6q_cpufreq_pdev);
 	}
 }
arch/arm/mach-imx/mach-imx6sl.c +3 -1
index 0408490..009bfa8 100644
@@ -38,8 +38,10 @@ static void __init imx6sl_fec_init(void)
 static void __init imx6sl_init_late(void)
 {
 	/* imx6sl reuses imx6q cpufreq driver */
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 
 	imx6sl_cpuidle_init();
 }
arch/arm/mach-imx/mach-imx6sx.c +3 -1
index 7f52d9b..2c5b78b 100644
@@ -93,8 +93,10 @@ static void __init imx6sx_init_late(void)
 {
 	imx6sx_cpuidle_init();
 
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 }
 
 static const char * const imx6sx_dt_compat[] __initconst = {
arch/arm/mach-imx/mach-imx6ul.c +3 -1
index 6bb7d9c..c2cd61c 100644
@@ -80,8 +80,10 @@ static void __init imx6ul_init_irq(void)
 
 static void __init imx6ul_init_late(void)
 {
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL, 0);
+	}
 }
 
 static const char * const imx6ul_dt_compat[] __initconst = {
arch/arm/mach-imx/pm-imx6.c +25 -0
index fe708e2..9a6f34c 100644
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
+#include <linux/pm_opp.h>
 #include <linux/regmap.h>
 #include <linux/suspend.h>
 #include <asm/cacheflush.h>

@@ -620,6 +621,30 @@ void __init imx6_pm_ccm_init(const char *ccm_compat)
 	writel_relaxed(val, ccm_base + CLPCR);
 }
 
+void __init imx6_pm_opp_init(void)
+{
+	struct device_node *np;
+	struct device *cpu_dev = get_cpu_device(0);
+
+	if (!cpu_dev) {
+		pr_warn("failed to get cpu0 device\n");
+		return;
+	}
+	np = of_node_get(cpu_dev->of_node);
+	if (!np) {
+		pr_warn("failed to find cpu0 node\n");
+		return;
+	}
+
+	if (dev_pm_opp_of_add_table(cpu_dev)) {
+		pr_warn("failed to init OPP table\n");
+		goto put_node;
+	}
+
+put_node:
+	of_node_put(np);
+}
+
 void __init imx6q_pm_init(void)
 {
 	imx6_pm_common_init(&imx6q_pm_data);
Signature · 7 lines
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#3 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

Quoted 11 lines · Shawn Guo
Quoted 5 lines · Anson Huang

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

The cpufreq driver calls dev_pm_opp_get_opp_count() to test if platform supplies an OPP table. If not, the driver will call into dev_pm_opp_of_add_table() on its own. So this is not an error message, and cpufreq driver should just work fine on i.MX6SX and i.MX6SL.

If you think the message might confuse people, we can add an info message in cpufreq driver in case it adds OPP table by its own, which can explain the 'error' message a bit from dev_pm_opp_get_opp_count().

Shawn

author
#4 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

Best Regards!
Anson Huang

Quoted 22 lines

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org]
Sent: 2016-08-29 2:27 PM
To: Yongcai Huang <[email protected]>
Cc: Fabio Estevam <[email protected]>; Peter Chen <[email protected]>; linux at armlinux.org.uk; linux-arm- kernel at lists.infradead.org; kernel at pengutronix.de
Subject: Re: [PATCH] ARM: imx6: add opp table when cpufreq is enabled

Quoted 11 lines · Shawn Guo
Quoted 5 lines · Anson Huang

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

The cpufreq driver calls dev_pm_opp_get_opp_count() to test if platform supplies an OPP table. If not, the driver will call into dev_pm_opp_of_add_table() on its own. So this is not an error message, and cpufreq driver should just work fine on i.MX6SX and i.MX6SL.

Quoted 5 lines

If you think the message might confuse people, we can add an info message in cpufreq driver in case it adds OPP table by its own, which can explain the 'error' message a bit from dev_pm_opp_get_opp_count().

Shawn

After rethink this, if we can avoid this "error" message printed out from common OPP framework, why NOT do it? Although the code change is kind of big, but the logic is quite simple. Adding info message in cpufreq driver is a bit better, but the "error" message is still there which may concern some people who really care about it, adding this patch would make it easier for our future kernel upgrade.

Actually, if we do NOT have speed grading fuse check need for some platforms, the opp table init can be put in cpufreq for all platforms, so I think the best way is to put all opp table init in either platform code or cpufreq driver, some platforms put it in platform code and some in cpufreq driver is NOT the best.

Thanks.
Anson.

#5 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

Quoted 35 lines · Yongcai Huang
Quoted 23 lines

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org]
Sent: 2016-08-29 2:27 PM
To: Yongcai Huang <[email protected]>
Cc: Fabio Estevam <[email protected]>; Peter Chen <[email protected]>; linux at armlinux.org.uk; linux-arm- kernel at lists.infradead.org; kernel at pengutronix.de
Subject: Re: [PATCH] ARM: imx6: add opp table when cpufreq is enabled

Quoted 11 lines · Shawn Guo
Quoted 5 lines · Anson Huang

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

The cpufreq driver calls dev_pm_opp_get_opp_count() to test if platform supplies an OPP table. If not, the driver will call into dev_pm_opp_of_add_table() on its own. So this is not an error message, and cpufreq driver should just work fine on i.MX6SX and i.MX6SL.

Quoted 5 lines

If you think the message might confuse people, we can add an info message in cpufreq driver in case it adds OPP table by its own, which can explain the 'error' message a bit from dev_pm_opp_get_opp_count().

Shawn

After rethink this, if we can avoid this "error" message printed out from common OPP framework, why NOT do it? Although the code change is kind of big, but the logic is quite simple. Adding info message in cpufreq driver is a bit better, but the "error" message is still there which may concern some people who really care about it, adding this patch would make it easier for our future kernel upgrade.

Quoted 4 lines

Actually, if we do NOT have speed grading fuse check need for some platforms, the opp table init can be put in cpufreq for all platforms, so I think the best way is to put all opp table init in either platform code or cpufreq driver, some platforms put it in platform code and some in cpufreq driver is NOT the best.

It will be the best if we can have cpufreq driver handle OPP table for all i.MX6 cases.

My problem with the platform approach is that every time we have a new platform support, we need to patch platform code to add OPP table. That's not good.

Shawn

author
#6 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

Best Regards!
Anson Huang

Quoted 55 lines

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org]
Sent: 2016-08-29 10:47 PM
To: Yongcai Huang <[email protected]>
Cc: Fabio Estevam <[email protected]>; Peter Chen <[email protected]>; linux at armlinux.org.uk; linux-arm- kernel at lists.infradead.org; kernel at pengutronix.de
Subject: Re: [PATCH] ARM: imx6: add opp table when cpufreq is enabled

Quoted 17 lines · Yongcai Huang
Quoted 17 lines

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org]
Sent: 2016-08-29 2:27 PM
To: Yongcai Huang <[email protected]>
Cc: Fabio Estevam <[email protected]>; Peter Chen <[email protected]>; linux at armlinux.org.uk; linux-arm- kernel at lists.infradead.org; kernel at pengutronix.de
Subject: Re: [PATCH] ARM: imx6: add opp table when cpufreq is enabled

Quoted 5 lines · Shawn Guo
Quoted 4 lines · Anson Huang

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 1 line

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

The cpufreq driver calls dev_pm_opp_get_opp_count() to test if platform supplies an OPP table. If not, the driver will call into dev_pm_opp_of_add_table() on its own. So this is not an error message, and cpufreq driver should just work fine on i.MX6SX and

i.MX6SL.

Quoted 3 lines
Quoted 3 lines

If you think the message might confuse people, we can add an info message in cpufreq driver in case it adds OPP table by its own, which can

explain the 'error'

Quoted 9 lines
Quoted 3 lines

message a bit from dev_pm_opp_get_opp_count().

Shawn

After rethink this, if we can avoid this "error" message printed out from common OPP framework, why NOT do it? Although the code change is kind of big, but the logic is quite simple. Adding info message in cpufreq driver is a bit better, but the "error" message is still there which may concern some people who really care about it, adding this patch

would make it easier for our future kernel upgrade.

Quoted 5 lines

Actually, if we do NOT have speed grading fuse check need for some platforms, the opp table init can be put in cpufreq for all platforms, so I think the best way is to put all opp table init in either platform code or cpufreq driver, some platforms put it in platform code and

some in cpufreq driver is NOT the best.

Quoted 8 lines

It will be the best if we can have cpufreq driver handle OPP table for all i.MX6 cases.

My problem with the platform approach is that every time we have a new platform support, we need to patch platform code to add OPP table. That's not good.

Shawn

Understood, then I think I can move the speed grading fuse check into cpufreq driver, then put the opp table init in cpufreq too, that will make the platform code much more easy for adding new platform support. Will send out a new patch set later for this change.

Regards!
Anson.

author
#7 lore raw

PATCH 1/1 ARM: imx6: add opp table when cpufreq is enabled

In reply to Shawn Guo

Best Regards!
Anson Huang

Quoted 14 lines

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org]
Sent: 2016-08-29 2:19 PM
To: Yongcai Huang <[email protected]>
Cc: linux-arm-kernel at lists.infradead.org; Fabio Estevam <[email protected]>; Peter Chen <[email protected]>; linux at armlinux.org.uk; kernel at pengutronix.de
Subject: Re: [PATCH] ARM: imx6: add opp table when cpufreq is enabled

Quoted 4 lines · Anson Huang

On those i.MX6 platforms which have no speed grading check, opp table will NOT be created in platform code, so cpufreq driver will have below error message:

Quoted 8 lines
Quoted 1 line

cpu cpu0: dev_pm_opp_get_opp_count: OPP table not found (-19)

The cpufreq driver calls dev_pm_opp_get_opp_count() to test if platform supplies an OPP table. If not, the driver will call into dev_pm_opp_of_add_table() on its own. So this is not an error message, and cpufreq driver should just work fine on i.MX6SX and i.MX6SL.

Shawn

Yes, function is OK, it is just because someone reported that the message is output in dev_pm_opp_get_opp_count function as a result of dev_err, so they suggest us to improve it.

Any way, this patch is just to avoid the message, as I think the opp table should be initialized onceno matter in platform or in cpufreq driver, if you think it is unnecessary, we can keep it in our internal tree only, thanks.

334 dev_err(dev, "%s: OPP table not found (%d)\n",
335 __func__, count);

Anson.

Quoted 174 lines
Quoted 80 lines

As cpufreq driver expects opp table is supplied by platform, so it is better to add opp table if cpufreq is enabled.

Signed-off-by Anson Huang <[email protected]>
---
 arch/arm/mach-imx/common.h      |  1 +
 arch/arm/mach-imx/mach-imx6q.c  | 31 ++++---------------------------
arch/arm/mach-imx/mach-imx6sl.c |  4 +++-
arch/arm/mach-imx/mach-imx6sx.c |  4 +++-
arch/arm/mach-imx/mach-imx6ul.c |  4 +++-
 arch/arm/mach-imx/pm-imx6.c     | 25 +++++++++++++++++++++++++
 6 files changed, 39 insertions(+), 30 deletions(-)
arch/arm/mach-imx/common.h +1 -0
index bcca481..b757811 100644
@@ -124,6 +124,7 @@ static inline void imx6_suspend(void __iomem
 *ocram_vbase) {}  #endif
 
 void imx6_pm_ccm_init(const char *ccm_compat);
+void imx6_pm_opp_init(void);
 void imx6q_pm_init(void);
 void imx6dl_pm_init(void);
 void imx6sl_pm_init(void);
(quoted diff) +0 -0

b/arch/arm/mach-imx/mach-imx6q.c index 97fd251..09d295b 100644

arch/arm/mach-imx/mach-imx6q.c +4 -26
@@ -338,32 +338,6 @@ put_node:
 	of_node_put(np);
 }
 
-static void __init imx6q_opp_init(void) -{
-	struct device_node *np;
-	struct device *cpu_dev = get_cpu_device(0);
-
-	if (!cpu_dev) {
-		pr_warn("failed to get cpu0 device\n");
-		return;
-	}
-	np = of_node_get(cpu_dev->of_node);
-	if (!np) {
-		pr_warn("failed to find cpu0 node\n");
-		return;
-	}
-
-	if (dev_pm_opp_of_add_table(cpu_dev)) {
-		pr_warn("failed to init OPP table\n");
-		goto put_node;
-	}
-
-	imx6q_opp_check_speed_grading(cpu_dev);
-
-put_node:
-	of_node_put(np);
-}
-
 static struct platform_device imx6q_cpufreq_pdev = {
 	.name = "imx6q-cpufreq",
 };

@@ -378,7 +352,10 @@ static void __init imx6q_init_late(void)
 		imx6q_cpuidle_init();
 
 	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
-		imx6q_opp_init();
+		struct device *cpu_dev = get_cpu_device(0);
+
+		imx6_pm_opp_init();
+		imx6q_opp_check_speed_grading(cpu_dev);
 		platform_device_register(&imx6q_cpufreq_pdev);
 	}
 }
(quoted diff) +0 -0

b/arch/arm/mach-imx/mach-imx6sl.c index 0408490..009bfa8 100644

arch/arm/mach-imx/mach-imx6sl.c static void __init imx6sl_fec_init(void) static · line 42

Quoted 6 lines
arch/arm/mach-imx/mach-imx6sl.c +2 -1
@@ -38,8 +38,10 @@ static void __init imx6sl_fec_init(void)  static
 void __init imx6sl_init_late(void)  {
 	/* imx6sl reuses imx6q cpufreq driver */
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL,

0);

Quoted 9 lines
arch/arm/mach-imx/mach-imx6sl.c +1 -0
+	}
 
 	imx6sl_cpuidle_init();
 }
(quoted diff) +0 -0

b/arch/arm/mach-imx/mach-imx6sx.c index 7f52d9b..2c5b78b 100644

arch/arm/mach-imx/mach-imx6sx.c static void __init imx6sx_init_late(void) { · line 97

Quoted 6 lines
arch/arm/mach-imx/mach-imx6sx.c +2 -1
@@ -93,8 +93,10 @@ static void __init imx6sx_init_late(void)  {
 	imx6sx_cpuidle_init();
 
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL,

0);

Quoted 9 lines
arch/arm/mach-imx/mach-imx6sx.c +1 -1
+	}
 }
 
 static const char * const imx6sx_dt_compat[] __initconst = { diff
--git a/arch/arm/mach-imx/mach-imx6ul.c

b/arch/arm/mach-imx/mach-imx6ul.c index 6bb7d9c..c2cd61c 100644

arch/arm/mach-imx/mach-imx6ul.c static void __init imx6ul_init_irq(void) · line 84

Quoted 6 lines
arch/arm/mach-imx/mach-imx6ul.c +2 -1
@@ -80,8 +80,10 @@ static void __init imx6ul_init_irq(void)
 
 static void __init imx6ul_init_late(void)  {
-	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ))
+	if (IS_ENABLED(CONFIG_ARM_IMX6Q_CPUFREQ)) {
+		imx6_pm_opp_init();
 		platform_device_register_simple("imx6q-cpufreq", -1, NULL,

0);

arch/arm/mach-imx/pm-imx6.c void __init imx6_pm_ccm_init(const char

Quoted 17 lines
arch/arm/mach-imx/mach-imx6ul.c +1 -1
+	}
 }
 
 static const char * const imx6ul_dt_compat[] __initconst = { diff
--git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c

index fe708e2..9a6f34c 100644

arch/arm/mach-imx/pm-imx6.c +1 -0
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
+#include <linux/pm_opp.h>
 #include <linux/regmap.h>
 #include <linux/suspend.h>
 #include <asm/cacheflush.h>

@@ -620,6 +621,30 @@ void __init imx6_pm_ccm_init(const char

*ccm_compat)

Quoted 37 lines
arch/arm/mach-imx/pm-imx6.c +24 -1
 	writel_relaxed(val, ccm_base + CLPCR);  }
 
+void __init imx6_pm_opp_init(void)
+{
+	struct device_node *np;
+	struct device *cpu_dev = get_cpu_device(0);
+
+	if (!cpu_dev) {
+		pr_warn("failed to get cpu0 device\n");
+		return;
+	}
+	np = of_node_get(cpu_dev->of_node);
+	if (!np) {
+		pr_warn("failed to find cpu0 node\n");
+		return;
+	}
+
+	if (dev_pm_opp_of_add_table(cpu_dev)) {
+		pr_warn("failed to init OPP table\n");
+		goto put_node;
+	}
+
+put_node:
+	of_node_put(np);
+}
+
 void __init imx6q_pm_init(void)
 {
 	imx6_pm_common_init(&imx6q_pm_data);
--

1.9.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel