[RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Viresh Kumar]

Hi Guys,

DT based cpufreq drivers doesn't require much support from platform code now a
days as most of the stuff is moved behind generic APIs. Like clk APIs for
changing clock rates, regulator APIs for changing voltages, etc.

One of the bottleneck still left was how to select which cpufreq driver to probe
for a given platform as there might be multiple drivers available.

Traditionally, we used to create platform devices from machine specific code
which binds with a cpufreq driver. And while we moved towards DT based device
creation, these devices stayed as is.

The problem is getting worse now as we have architectures now with Zero platform
specific code. Forcefully these platforms have to create a new file in
drivers/cpufreq/ to just add these platform devices in order to use the generic
drivers like cpufreq-dt.c.

This has been discussed again and again, but with no solution yet. Last it was
discussed here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/256154.html

This patch is an attempt towards getting the bindings.

We only need to have cpufreq drivers name string present in "compatible"
property for the root node.. If a cpufreq driver with DT support exists with
that name, then cpufreq core will create a platform device for that.

The first patch presents the new binding, second one creates another file
responsible for creating platform devices for all DT based cpufreq drivers.

And later patches update platforms to migrate to it one by one.

A BLACKLIST of platforms already supported by these drivers is also created for
backward compatibility of newer kernels with older DTs. And so for such
platforms DT files aren't updated.

An initial RFC that presented the idea was discussed here:
https://www.mail-archive.com/devicetree@vger.kernel.org/msg52509.html

Tested-ON: Exynos5250. (The last patch for exynos depends on some commits to be
upstreamed in 3.19, presented here just for testing).

Viresh Kumar (8):
  cpufreq: Reuse "compatible" binding to probe cpufreq drivers
  cpufreq: Create cpufreq platform-device based on "compatible" from DT
  cpufreq: imx: reuse dt_device.c to create cpufreq platform device
  cpufreq: mvebu: reuse dt_device.c to create cpufreq platform device
  cpufreq: shmobile: reuse dt_device.c to create cpufreq platform device
  cpufreq: zynq: reuse dt_device.c to create cpufreq platform device
  cpufreq: calxeda: reuse dt_device.c to create cpufreq platform device
  cpufreq: exynos: reuse dt_device.c to create cpufreq platform device

 .../devicetree/bindings/cpufreq/drivers.txt        | 46 +++++++++++++++
 arch/arm/mach-exynos/exynos.c                      | 27 +++------
 arch/arm/mach-imx/imx27-dt.c                       |  4 --
 arch/arm/mach-imx/mach-imx51.c                     |  3 -
 arch/arm/mach-mvebu/pmsu.c                         |  1 -
 arch/arm/mach-shmobile/Makefile                    |  1 -
 arch/arm/mach-shmobile/common.h                    |  7 ---
 arch/arm/mach-shmobile/cpufreq.c                   | 17 ------
 arch/arm/mach-zynq/common.c                        |  2 -
 drivers/cpufreq/Makefile                           |  2 +-
 drivers/cpufreq/dt_device.c                        | 68 ++++++++++++++++++++++
 drivers/cpufreq/highbank-cpufreq.c                 |  5 --
 12 files changed, 123 insertions(+), 60 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/drivers.txt
 delete mode 100644 arch/arm/mach-shmobile/cpufreq.c
 create mode 100644 drivers/cpufreq/dt_device.c

-- 
2.0.3.693.g996b0fd

[RFC V1 1/8] cpufreq: Reuse "compatible" binding to probe cpufreq drivers

[Viresh Kumar]

DT based cpufreq drivers doesn't require much support from platform code now a
days as most of the stuff is moved behind generic APIs. Like clk APIs for
changing clock rates, regulator APIs for changing voltages, etc.

One of the bottleneck still left was how to select which cpufreq driver to probe
for a given platform as there might be multiple drivers available.

Traditionally, we used to create platform devices from machine specific code
which binds with a cpufreq driver. And while we moved towards DT based device
creation, these devices stayed as is.

The problem is getting worse now as we have architectures now with Zero platform
specific code. Forcefully these platforms have to create a new file in
drivers/cpufreq/ to just add these platform devices in order to use the generic
drivers like cpufreq-dt.c.

This has been discussed again and again, but with no solution yet. Last it was
discussed here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/256154.html

This patch is an attempt towards getting the bindings.

We only need to have cpufreq drivers name string present in "compatible"
property for the root node.. If a cpufreq driver with DT support exists with
that name, then cpufreq core will create a platform device for that.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/cpufreq/drivers.txt        | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cpufreq/drivers.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/drivers.txt b/Documentation/devicetree/bindings/cpufreq/drivers.txt
new file mode 100644
index 0000000..6a33150
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/drivers.txt
@@ -0,0 +1,46 @@
+Binding to select which cpufreq driver to register
+--------------------------------------------------
+
+This presents generic DT binding for selecting which cpufreq-driver to probe for
+platforms.
+
+The property listed below must be defined in root nodes compatible list. We
+don't support multiple CPUFreq driver currently for different cluster and so
+this information isn't required to be present in cpu nodes.
+
+Required properties:
+- None
+
+Optional properties:
+- compatible: CPUFreq driver to probe. For example: "arm-bL-cpufreq-dt",
+  "cpufreq-dt", etc. A platform device will be created with this name by cpufreq
+  core.
+
+Examples:
+
+/ {
+	compatible = "samsung,exynos5250", "cpufreq-dt";
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+			next-level-cache = <&L2>;
+			operating-points = <
+				/* kHz    uV */
+				792000  1100000
+				396000  950000
+				198000  850000
+				>;
+		};
+
+		...
+
+	};
+
+	...
+
+}
-- 
2.0.3.693.g996b0fd

[RFC V1 2/8] cpufreq: Create cpufreq platform-device based on "compatible" from DT

[Viresh Kumar]

DT based cpufreq drivers doesn't require much support from platform code now a
days as most of the stuff is moved behind generic APIs. Like clk APIs for
changing clock rates, regulator APIs for changing voltages, etc.

One of the bottleneck still left was how to select which cpufreq driver to probe
for a given platform as there might be multiple drivers available.

Traditionally, we used to create platform devices from machine specific code
which binds with a cpufreq driver. And while we moved towards DT based device
creation, these devices stayed as is.

The problem is getting worse now as we have architectures now with Zero platform
specific code. Forcefully these platforms have to create a new file in
drivers/cpufreq/ to just add these platform devices in order to use the generic
drivers like cpufreq-dt.c.

This has been discussed again and again, but with no solution yet. Last it was
discussed here:

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/256154.html

This patch creates a separate file which will be responsible for creating
cpufreq platform device based on the match with "compatible" property from DT.

A Blacklist is also provided for backward compatibility with older DTs.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/Makefile    |  2 +-
 drivers/cpufreq/dt_device.c | 52 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 drivers/cpufreq/dt_device.c

diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index b3ca7b0..d6feb0b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -1,6 +1,6 @@
 # CPUfreq core
 obj-$(CONFIG_CPU_FREQ)			+= cpufreq.o freq_table.o
-obj-$(CONFIG_PM_OPP)			+= cpufreq_opp.o
+obj-$(CONFIG_PM_OPP)			+= cpufreq_opp.o dt_device.o
 
 # CPUfreq stats
 obj-$(CONFIG_CPU_FREQ_STAT)             += cpufreq_stats.o
diff --git a/drivers/cpufreq/dt_device.c b/drivers/cpufreq/dt_device.c
new file mode 100644
index 0000000..7800968
--- /dev/null
+++ b/drivers/cpufreq/dt_device.c
@@ -0,0 +1,52 @@
+/*
+ * Creates platform device for probing cpufreq driver
+ *
+ * Copyright (C) 2014 Linaro.
+ * Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/of_device.h>
+
+static const struct of_device_id compatible_machine_match[] = {
+	/* All new machines must have the below compatible to use this driver */
+	{ .compatible = "cpufreq-dt",		.data = "cpufreq-dt" },
+	{ .compatible = "arm-bL-cpufreq-dt",	.data = "arm-bL-cpufreq-dt" },
+
+	/* BLACKLIST of existing users of cpufreq-dt below */
+
+	/* BLACKLIST of existing users of arm-bL-cpufreq-dt below */
+
+	/* BLACKLIST of existing users of other drivers below */
+
+	{},
+};
+
+static int cpufreq_dt_create_platform_device(void)
+{
+	struct device_node *root = of_find_node_by_path("/");
+	const struct of_device_id *match;
+
+	match = of_match_node(compatible_machine_match, root);
+	if (!match) {
+		pr_debug("%s: Couldn't find a match\n", __func__);
+		return -ENODEV;
+	}
+
+	pr_debug("%s: Create device for compatible:%s, driver:%s\n", __func__,
+		 match->compatible, (char *)match->data);
+	platform_device_register_simple(match->data, -1, NULL, 0);
+
+	return 0;
+}
+late_initcall(cpufreq_dt_create_platform_device);
-- 
2.0.3.693.g996b0fd

[RFC V1 3/8] cpufreq: imx: reuse dt_device.c to create cpufreq platform device

[Viresh Kumar]

We now have a common interface for create platform device required to probe
cpufreq-dt driver (and others as well). Lets create devices from dt_device.c
instead of platform specific code.

For imx, we are updating the blacklist instead of DT because the newer kernel
should be backwards compatible with older DT as well. We can update the
"compatible" property in DT but it wouldn't make a difference as we already have
imx in the blacklist.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-imx/imx27-dt.c   | 4 ----
 arch/arm/mach-imx/mach-imx51.c | 3 ---
 drivers/cpufreq/dt_device.c    | 2 ++
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-imx/imx27-dt.c b/arch/arm/mach-imx/imx27-dt.c
index dc8f1a6..f12fe83 100644
--- a/arch/arm/mach-imx/imx27-dt.c
+++ b/arch/arm/mach-imx/imx27-dt.c
@@ -20,13 +20,9 @@
 
 static void __init imx27_dt_init(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-dt", };
-
 	mxc_arch_reset_init_dt();
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-
-	platform_device_register_full(&devinfo);
 }
 
 static const char * const imx27_dt_board_compat[] __initconst = {
diff --git a/arch/arm/mach-imx/mach-imx51.c b/arch/arm/mach-imx/mach-imx51.c
index 2c5fcaf..17d16a6 100644
--- a/arch/arm/mach-imx/mach-imx51.c
+++ b/arch/arm/mach-imx/mach-imx51.c
@@ -51,14 +51,11 @@ static void __init imx51_ipu_mipi_setup(void)
 
 static void __init imx51_dt_init(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-dt", };
-
 	mxc_arch_reset_init_dt();
 	imx51_ipu_mipi_setup();
 	imx_src_init();
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-	platform_device_register_full(&devinfo);
 }
 
 static void __init imx51_init_late(void)
diff --git a/drivers/cpufreq/dt_device.c b/drivers/cpufreq/dt_device.c
index 7800968..cf01bed 100644
--- a/drivers/cpufreq/dt_device.c
+++ b/drivers/cpufreq/dt_device.c
@@ -24,6 +24,8 @@ static const struct of_device_id compatible_machine_match[] = {
 	{ .compatible = "arm-bL-cpufreq-dt",	.data = "arm-bL-cpufreq-dt" },
 
 	/* BLACKLIST of existing users of cpufreq-dt below */
+	{ .compatible = "fsl,imx27",		.data = "cpufreq-dt" },
+	{ .compatible = "fsl,imx51",		.data = "cpufreq-dt" },
 
 	/* BLACKLIST of existing users of arm-bL-cpufreq-dt below */
 
-- 
2.0.3.693.g996b0fd

[RFC V1 4/8] cpufreq: mvebu: reuse dt_device.c to create cpufreq platform device

[Viresh Kumar]

We now have a common interface for create platform device required to probe
cpufreq-dt driver (and others as well). Lets create devices from dt_device.c
instead of platform specific code.

For marvell, we are updating the blacklist instead of DT because the newer
kernel should be backwards compatible with older DT as well. We can update the
"compatible" property in DT but it wouldn't make a difference as we already have
imx in the blacklist.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-mvebu/pmsu.c  | 1 -
 drivers/cpufreq/dt_device.c | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c
index bbd8664..9d02618 100644
--- a/arch/arm/mach-mvebu/pmsu.c
+++ b/arch/arm/mach-mvebu/pmsu.c
@@ -644,7 +644,6 @@ static int __init armada_xp_pmsu_cpufreq_init(void)
 		}
 	}
 
-	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
 	return 0;
 }
 
diff --git a/drivers/cpufreq/dt_device.c b/drivers/cpufreq/dt_device.c
index cf01bed..33fc046 100644
--- a/drivers/cpufreq/dt_device.c
+++ b/drivers/cpufreq/dt_device.c
@@ -27,6 +27,8 @@ static const struct of_device_id compatible_machine_match[] = {
 	{ .compatible = "fsl,imx27",		.data = "cpufreq-dt" },
 	{ .compatible = "fsl,imx51",		.data = "cpufreq-dt" },
 
+	{ .compatible = "marvell,armadaxp",	.data = "cpufreq-dt" },
+
 	/* BLACKLIST of existing users of arm-bL-cpufreq-dt below */
 
 	/* BLACKLIST of existing users of other drivers below */
-- 
2.0.3.693.g996b0fd

[RFC V1 5/8] cpufreq: shmobile: reuse dt_device.c to create cpufreq platform device

[Viresh Kumar]

We now have a common interface for create platform device required to probe
cpufreq-dt driver (and others as well). Lets create devices from dt_device.c
instead of platform specific code.

For shmobile, we are updating the blacklist instead of DT because the newer
kernel should be backwards compatible with older DT as well. We can update the
"compatible" property in DT but it wouldn't make a difference as we already have
imx in the blacklist.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-shmobile/Makefile  |  1 -
 arch/arm/mach-shmobile/common.h  |  7 -------
 arch/arm/mach-shmobile/cpufreq.c | 17 -----------------
 drivers/cpufreq/dt_device.c      |  3 +++
 4 files changed, 3 insertions(+), 25 deletions(-)
 delete mode 100644 arch/arm/mach-shmobile/cpufreq.c

diff --git a/arch/arm/mach-shmobile/Makefile b/arch/arm/mach-shmobile/Makefile
index e20f278..6780401 100644
--- a/arch/arm/mach-shmobile/Makefile
+++ b/arch/arm/mach-shmobile/Makefile
@@ -48,7 +48,6 @@ smp-$(CONFIG_ARCH_EMEV2)	+= smp-emev2.o headsmp-scu.o platsmp-scu.o
 # PM objects
 obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
-obj-$(CONFIG_CPU_FREQ)		+= cpufreq.o
 obj-$(CONFIG_PM_RCAR)		+= pm-rcar.o
 obj-$(CONFIG_PM_RMOBILE)	+= pm-rmobile.o
 
diff --git a/arch/arm/mach-shmobile/common.h b/arch/arm/mach-shmobile/common.h
index 72087c7..97ddd5f 100644
--- a/arch/arm/mach-shmobile/common.h
+++ b/arch/arm/mach-shmobile/common.h
@@ -45,19 +45,12 @@ int shmobile_cpuidle_init(void);
 static inline int shmobile_cpuidle_init(void) { return 0; }
 #endif
 
-#ifdef CONFIG_CPU_FREQ
-int shmobile_cpufreq_init(void);
-#else
-static inline int shmobile_cpufreq_init(void) { return 0; }
-#endif
-
 extern void __iomem *shmobile_scu_base;
 
 static inline void __init shmobile_init_late(void)
 {
 	shmobile_suspend_init();
 	shmobile_cpuidle_init();
-	shmobile_cpufreq_init();
 }
 
 #endif /* __ARCH_MACH_COMMON_H */
diff --git a/arch/arm/mach-shmobile/cpufreq.c b/arch/arm/mach-shmobile/cpufreq.c
deleted file mode 100644
index 57fbff0..0000000
--- a/arch/arm/mach-shmobile/cpufreq.c
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * CPUFreq support code for SH-Mobile ARM
- *
- *  Copyright (C) 2014 Gaku Inami
- *
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
- */
-
-#include <linux/platform_device.h>
-
-int __init shmobile_cpufreq_init(void)
-{
-	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
-	return 0;
-}
diff --git a/drivers/cpufreq/dt_device.c b/drivers/cpufreq/dt_device.c
index 33fc046..85dc002 100644
--- a/drivers/cpufreq/dt_device.c
+++ b/drivers/cpufreq/dt_device.c
@@ -29,6 +29,9 @@ static const struct of_device_id compatible_machine_match[] = {
 
 	{ .compatible = "marvell,armadaxp",	.data = "cpufreq-dt" },
 
+	{ .compatible = "renesas,sh7372",	.data = "cpufreq-dt" },
+	{ .compatible = "renesas,sh73a0",	.data = "cpufreq-dt" },
+
 	/* BLACKLIST of existing users of arm-bL-cpufreq-dt below */
 
 	/* BLACKLIST of existing users of other drivers below */
-- 
2.0.3.693.g996b0fd

[RFC V1 6/8] cpufreq: zynq: reuse dt_device.c to create cpufreq platform device

[Viresh Kumar]

We now have a common interface for create platform device required to probe
cpufreq-dt driver (and others as well). Lets create devices from dt_device.c
instead of platform specific code.

For zynq, we are updating the blacklist instead of DT because the newer kernel
should be backwards compatible with older DT as well. We can update the
"compatible" property in DT but it wouldn't make a difference as we already have
imx in the blacklist.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-zynq/common.c | 2 --
 drivers/cpufreq/dt_device.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 26f92c2..add3647 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -110,7 +110,6 @@ static void __init zynq_init_late(void)
  */
 static void __init zynq_init_machine(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-dt", };
 	struct soc_device_attribute *soc_dev_attr;
 	struct soc_device *soc_dev;
 	struct device *parent = NULL;
@@ -145,7 +144,6 @@ static void __init zynq_init_machine(void)
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
 
 	platform_device_register(&zynq_cpuidle_device);
-	platform_device_register_full(&devinfo);
 
 	zynq_slcr_init();
 }
diff --git a/drivers/cpufreq/dt_device.c b/drivers/cpufreq/dt_device.c
index 85dc002..2075228 100644
--- a/drivers/cpufreq/dt_device.c
+++ b/drivers/cpufreq/dt_device.c
@@ -32,6 +32,8 @@ static const struct of_device_id compatible_machine_match[] = {
 	{ .compatible = "renesas,sh7372",	.data = "cpufreq-dt" },
 	{ .compatible = "renesas,sh73a0",	.data = "cpufreq-dt" },
 
+	{ .compatible = "xlnx,zynq-7000",	.data = "cpufreq-dt" },
+
 	/* BLACKLIST of existing users of arm-bL-cpufreq-dt below */
 
 	/* BLACKLIST of existing users of other drivers below */
-- 
2.0.3.693.g996b0fd

[RFC V1 7/8] cpufreq: calxeda: reuse dt_device.c to create cpufreq platform device

[Viresh Kumar]

We now have a common interface for create platform device required to probe
cpufreq-dt driver (and others as well). Lets create devices from dt_device.c
instead of platform specific code.

For calxeda, we are updating the blacklist instead of DT because the newer kernel
should be backwards compatible with older DT as well. We can update the
"compatible" property in DT but it wouldn't make a difference as we already have
imx in the blacklist.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/dt_device.c        | 3 +++
 drivers/cpufreq/highbank-cpufreq.c | 5 -----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/dt_device.c b/drivers/cpufreq/dt_device.c
index 2075228..8b5ac05 100644
--- a/drivers/cpufreq/dt_device.c
+++ b/drivers/cpufreq/dt_device.c
@@ -34,6 +34,9 @@ static const struct of_device_id compatible_machine_match[] = {
 
 	{ .compatible = "xlnx,zynq-7000",	.data = "cpufreq-dt" },
 
+	{ .compatible = "calxeda,highbank",	.data = "cpufreq-dt" },
+	{ .compatible = "calxeda,ecx-2000",	.data = "cpufreq-dt" },
+
 	/* BLACKLIST of existing users of arm-bL-cpufreq-dt below */
 
 	/* BLACKLIST of existing users of other drivers below */
diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
index 1608f71..faab680 100644
--- a/drivers/cpufreq/highbank-cpufreq.c
+++ b/drivers/cpufreq/highbank-cpufreq.c
@@ -20,7 +20,6 @@
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/pl320-ipc.h>
-#include <linux/platform_device.h>
 
 #define HB_CPUFREQ_CHANGE_NOTE	0x80000001
 #define HB_CPUFREQ_IPC_LEN	7
@@ -60,7 +59,6 @@ static struct notifier_block hb_cpufreq_clk_nb = {
 
 static int hb_cpufreq_driver_init(void)
 {
-	struct platform_device_info devinfo = { .name = "cpufreq-dt", };
 	struct device *cpu_dev;
 	struct clk *cpu_clk;
 	struct device_node *np;
@@ -95,9 +93,6 @@ static int hb_cpufreq_driver_init(void)
 		goto out_put_node;
 	}
 
-	/* Instantiate cpufreq-dt */
-	platform_device_register_full(&devinfo);
-
 out_put_node:
 	of_node_put(np);
 	return ret;
-- 
2.0.3.693.g996b0fd

[RFC V1 8/8] cpufreq: exynos: reuse dt_device.c to create cpufreq platform device

[Viresh Kumar]

We now have a common interface for create platform device required to probe
cpufreq-dt driver (and others as well). Lets create devices from dt_device.c
instead of platform specific code.

For exynos, we are updating the blacklist instead of DT because the newer kernel
should be backwards compatible with older DT as well. We can update the
"compatible" property in DT but it wouldn't make a difference as we already have
imx in the blacklist.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm/mach-exynos/exynos.c | 27 ++++++++-------------------
 drivers/cpufreq/dt_device.c   |  6 +++++-
 2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index a1be294..15a4bbd 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -283,30 +283,17 @@ static void __init exynos_init_irq(void)
 }
 
 static const struct of_device_id exynos_cpufreq_matches[] = {
-	{ .compatible = "samsung,exynos5420", .data = "arm-bL-cpufreq-dt" },
-	{ .compatible = "samsung,exynos5250", .data = "cpufreq-dt" },
-	{ .compatible = "samsung,exynos4210", .data = "cpufreq-dt" },
-	{ .compatible = "samsung,exynos5440", .data = "exynos5440-cpufreq" },
+	{ .compatible = "samsung,exynos5420" },
+	{ .compatible = "samsung,exynos5250" },
+	{ .compatible = "samsung,exynos4210" },
+	{ .compatible = "samsung,exynos5440" },
 	{ /* sentinel */ }
 };
 
-static void __init exynos_cpufreq_init(void)
-{
-	struct device_node *root = of_find_node_by_path("/");
-	const struct of_device_id *match;
-
-	match = of_match_node(exynos_cpufreq_matches, root);
-	if (!match) {
-		platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
-		return;
-	}
-
-	platform_device_register_simple(match->data, -1, NULL, 0);
-}
-
 static void __init exynos_dt_machine_init(void)
 {
 	struct device_node *i2c_np;
+	struct device_node *root = of_find_node_by_path("/");
 	const char *i2c_compat = "samsung,s3c2440-i2c";
 	unsigned int tmp;
 	int id;
@@ -343,7 +330,9 @@ static void __init exynos_dt_machine_init(void)
 			of_machine_is_compatible("samsung,exynos5250"))
 		platform_device_register(&exynos_cpuidle);
 
-	exynos_cpufreq_init();
+	/* Other devices are created by dt_device.c */
+	if (!of_match_node(exynos_cpufreq_matches, root))
+		platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
 }
diff --git a/drivers/cpufreq/dt_device.c b/drivers/cpufreq/dt_device.c
index 8b5ac05..bc4bc97 100644
--- a/drivers/cpufreq/dt_device.c
+++ b/drivers/cpufreq/dt_device.c
@@ -24,6 +24,9 @@ static const struct of_device_id compatible_machine_match[] = {
 	{ .compatible = "arm-bL-cpufreq-dt",	.data = "arm-bL-cpufreq-dt" },
 
 	/* BLACKLIST of existing users of cpufreq-dt below */
+	{ .compatible = "samsung,exynos5250",	.data = "cpufreq-dt" },
+	{ .compatible = "samsung,exynos4210",	.data = "cpufreq-dt" },
+
 	{ .compatible = "fsl,imx27",		.data = "cpufreq-dt" },
 	{ .compatible = "fsl,imx51",		.data = "cpufreq-dt" },
 
@@ -38,9 +41,10 @@ static const struct of_device_id compatible_machine_match[] = {
 	{ .compatible = "calxeda,ecx-2000",	.data = "cpufreq-dt" },
 
 	/* BLACKLIST of existing users of arm-bL-cpufreq-dt below */
+	{ .compatible = "samsung,exynos5420",	.data = "arm-bL-cpufreq-dt" },
 
 	/* BLACKLIST of existing users of other drivers below */
-
+	{ .compatible = "samsung,exynos5440",	.data = "exynos5440-cpufreq" },
 	{},
 };
 
-- 
2.0.3.693.g996b0fd

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Arnd Bergmann]

On Monday 01 December 2014 17:11:21 Viresh Kumar wrote:
> 
> DT based cpufreq drivers doesn't require much support from platform code now a
> days as most of the stuff is moved behind generic APIs. Like clk APIs for
> changing clock rates, regulator APIs for changing voltages, etc.
> 
> One of the bottleneck still left was how to select which cpufreq driver to probe
> for a given platform as there might be multiple drivers available.
> 
> Traditionally, we used to create platform devices from machine specific code
> which binds with a cpufreq driver. And while we moved towards DT based device
> creation, these devices stayed as is.
> 
> The problem is getting worse now as we have architectures now with Zero platform
> specific code. Forcefully these platforms have to create a new file in
> drivers/cpufreq/ to just add these platform devices in order to use the generic
> drivers like cpufreq-dt.c.
> 
> This has been discussed again and again, but with no solution yet. Last it was
> discussed here:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/256154.html
> 
> This patch is an attempt towards getting the bindings.
> 
> We only need to have cpufreq drivers name string present in "compatible"
> property for the root node.. If a cpufreq driver with DT support exists with
> that name, then cpufreq core will create a platform device for that.
> 
> The first patch presents the new binding, second one creates another file
> responsible for creating platform devices for all DT based cpufreq drivers.
> 
> And later patches update platforms to migrate to it one by one.
> 
> A BLACKLIST of platforms already supported by these drivers is also created for
> backward compatibility of newer kernels with older DTs. And so for such
> platforms DT files aren't updated.
> 
> An initial RFC that presented the idea was discussed here:
> https://www.mail-archive.com/devicetree@vger.kernel.org/msg52509.html
> 
> Tested-ON: Exynos5250. (The last patch for exynos depends on some commits to be
> upstreamed in 3.19, presented here just for testing).

Thanks a lot for working on this, we really need to figure it out one day!

Your patches seem well-implemented, so if everybody thinks the general
approach is the best solution, we should do that. From my point of view,
there are two things I would do differently:

- In the DT binding, I would strongly prefer anything but the root compatible
  property as the key for the new platforms. Clearly we have to keep using
  it for the backwards-compatibility case, as you do, but I think there
  are more appropriate places to put it. Sorting from most favorite to least
  favorite, my list would be:
	1. a new property in /cpus/
	2. a new property each /cpus/cpu@... node.
	3. the compatible property of the /cpus node
	4. a top-level device node that gets turned into a platform device
	5. a new property in the / node
	6. a new property in the /chosen node
	7. the compatible property in the / node

- Implementation-wise, I don't think it's helpful to have a global function
  that registers a platform device to be consumed by the device driver. I'd
  rather just see a module_init function in each driver that rather than
  registering a platform_driver scans the DT properties. This is only really
  necessary when not using DT (omap2, omap3, davinci, loongson) or when
  passing additional resources or platform_data (kirkwood, but that can
  look up the "marvell,orion-system-controller" node if necessary).
  My preferred solution would be to eventually remove the platform_device
  registration for all DT users. If a driver needs a platform device pointer
  internally, it can use platform_create_bundle(), but that's probably not
  even necessary.

In short, I would suggest something along the lines of the patch below.

	Arnd
8<-----
[PATCH, RFC] cpufreq: dt: simplify and generalize probing

We should not have to create a platform device from platform specific code
in order to use the completely generic cpufreq-dt driver. This adds
a simpler method by creating a new standard property of the "/cpus" node
to look for, with a fallback for existing users. The list of existing
users needs to be completed, and the same change done for the other
DT based drivers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 6f5f5615fbf1..697b4dc47715 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -345,13 +345,50 @@ static struct cpufreq_driver dt_cpufreq_driver = {
 	.attr = cpufreq_generic_attr,
 };
 
-static int dt_cpufreq_probe(struct platform_device *pdev)
+/*
+ * these machines are using the driver but provide no standard
+ * probing method, only for old machines with existing dtbs.
+ */
+static struct of_device_id legacy_machines = {
+	{ .compatible = "calxeda,highbank" },
+	{ .compatible = "renesas,sh7372" },
+	{ .compatible = "renesas,sh73a0"  },
+	{ .compatible = "samsung,exynos5250" },
+	{ .compatible = "samsung,exynos4210" },
+	{ .compatible = "xlnx,zynq-7000" },
+};
+
+static bool dt_cpufreq_available(void)
+{
+	struct device_node *node;
+	bool ret;
+
+	node = of_find_node_by_path("/cpus");
+	if (!node)
+		return 0;
+
+	/* the specific property needs to be debated */
+	ret = of_property_read_bool("linux,cpu-frequency-scaling");
+	of_node_put(node);
+	if (ret)
+		return 1;
+
+	node = of_find_node_by_path("/");
+	ret = of_match_device(&legacy_machines, node);
+	of_node_put(node);
+
+	return ret;
+}
+
+static int __init dt_cpufreq_probe(void)
 {
 	struct device *cpu_dev;
 	struct regulator *cpu_reg;
 	struct clk *cpu_clk;
 	int ret;
 
+	if (!dt_cpufreq_available())
+		return -ENXIO;
 	/*
 	 * All per-cluster (CPUs sharing clock/voltages) initialization is done
 	 * from ->init(). In probe(), we just need to make sure that clk and
@@ -367,29 +404,19 @@ static int dt_cpufreq_probe(struct platform_device *pdev)
 	if (!IS_ERR(cpu_reg))
 		regulator_put(cpu_reg);
 
-	dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
-
 	ret = cpufreq_register_driver(&dt_cpufreq_driver);
 	if (ret)
 		dev_err(cpu_dev, "failed register driver: %d\n", ret);
 
 	return ret;
 }
+module_init(dt_cpufreq_probe);
 
-static int dt_cpufreq_remove(struct platform_device *pdev)
+static void __exit dt_cpufreq_remove(void)
 {
 	cpufreq_unregister_driver(&dt_cpufreq_driver);
-	return 0;
 }
-
-static struct platform_driver dt_cpufreq_platdrv = {
-	.driver = {
-		.name	= "cpufreq-dt",
-	},
-	.probe		= dt_cpufreq_probe,
-	.remove		= dt_cpufreq_remove,
-};
-module_platform_driver(dt_cpufreq_platdrv);
+module_exit(dt_cpufreq_remove);
 
 MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>");
 MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Viresh Kumar]

On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
> Thanks a lot for working on this, we really need to figure it out one day!

:)

> Your patches seem well-implemented, so if everybody thinks the general
> approach is the best solution, we should do that. From my point of view,
> there are two things I would do differently:
>
> - In the DT binding, I would strongly prefer anything but the root compatible
>   property as the key for the new platforms. Clearly we have to keep using
>   it for the backwards-compatibility case, as you do, but I think there
>   are more appropriate places to put it. Sorting from most favorite to least
>   favorite, my list would be:
>         1. a new property in /cpus/
>         2. a new property each /cpus/cpu@... node.

I did it this way earlier and named it dvfs-method but probably putting this
into the /cpus/ node is far better. But then Sudeep asked to utilize
compatible property only..

Are you fine with the name here? "dvfs-method"

>         3. the compatible property of the /cpus node
>         4. a top-level device node that gets turned into a platform device
>         5. a new property in the / node
>         6. a new property in the /chosen node
>         7. the compatible property in the / node
>
> - Implementation-wise, I don't think it's helpful to have a global function
>   that registers a platform device to be consumed by the device driver. I'd
>   rather just see a module_init function in each driver that rather than

Okay, this might work better in longer run. I am fine with it.

> [PATCH, RFC] cpufreq: dt: simplify and generalize probing
>
> We should not have to create a platform device from platform specific code
> in order to use the completely generic cpufreq-dt driver. This adds
> a simpler method by creating a new standard property of the "/cpus" node
> to look for, with a fallback for existing users. The list of existing
> users needs to be completed, and the same change done for the other
> DT based drivers.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index 6f5f5615fbf1..697b4dc47715 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -345,13 +345,50 @@ static struct cpufreq_driver dt_cpufreq_driver = {
>         .attr = cpufreq_generic_attr,
>  };
>
> -static int dt_cpufreq_probe(struct platform_device *pdev)
> +/*
> + * these machines are using the driver but provide no standard
> + * probing method, only for old machines with existing dtbs.
> + */
> +static struct of_device_id legacy_machines = {
> +       { .compatible = "calxeda,highbank" },
> +       { .compatible = "renesas,sh7372" },
> +       { .compatible = "renesas,sh73a0"  },
> +       { .compatible = "samsung,exynos5250" },
> +       { .compatible = "samsung,exynos4210" },
> +       { .compatible = "xlnx,zynq-7000" },
> +};
> +
> +static bool dt_cpufreq_available(void)
> +{
> +       struct device_node *node;
> +       bool ret;
> +
> +       node = of_find_node_by_path("/cpus");
> +       if (!node)
> +               return 0;
> +
> +       /* the specific property needs to be debated */
> +       ret = of_property_read_bool("linux,cpu-frequency-scaling");

How can this be a bool? We need to differentiate on which binding
wants to go for arm-bl or cupfreq-dt or any other driver. So we surely
need a string ?

> +       of_node_put(node);
> +       if (ret)
> +               return 1;
> +
> +       node = of_find_node_by_path("/");
> +       ret = of_match_device(&legacy_machines, node);
> +       of_node_put(node);
> +
> +       return ret;
> +}
> +
> +static int __init dt_cpufreq_probe(void)
>  {
>         struct device *cpu_dev;
>         struct regulator *cpu_reg;
>         struct clk *cpu_clk;
>         int ret;
>
> +       if (!dt_cpufreq_available())
> +               return -ENXIO;
>         /*
>          * All per-cluster (CPUs sharing clock/voltages) initialization is done
>          * from ->init(). In probe(), we just need to make sure that clk and
> @@ -367,29 +404,19 @@ static int dt_cpufreq_probe(struct platform_device *pdev)
>         if (!IS_ERR(cpu_reg))
>                 regulator_put(cpu_reg);
>
> -       dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
> -

We still need this, and its about how clocks are shared between CPUs.

>         ret = cpufreq_register_driver(&dt_cpufreq_driver);
>         if (ret)
>                 dev_err(cpu_dev, "failed register driver: %d\n", ret);
>
>         return ret;
>  }
> +module_init(dt_cpufreq_probe);
>
> -static int dt_cpufreq_remove(struct platform_device *pdev)
> +static void __exit dt_cpufreq_remove(void)
>  {
>         cpufreq_unregister_driver(&dt_cpufreq_driver);
> -       return 0;
>  }
> -
> -static struct platform_driver dt_cpufreq_platdrv = {
> -       .driver = {
> -               .name   = "cpufreq-dt",
> -       },
> -       .probe          = dt_cpufreq_probe,
> -       .remove         = dt_cpufreq_remove,
> -};
> -module_platform_driver(dt_cpufreq_platdrv);
> +module_exit(dt_cpufreq_remove);
>
>  MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>");
>  MODULE_AUTHOR("Shawn Guo <shawn.guo@linaro.org>");
>

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Sudeep Holla]

On 01/12/14 13:29, Viresh Kumar wrote:
> On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
>> Thanks a lot for working on this, we really need to figure it out one day!
>
> :)
>
>> Your patches seem well-implemented, so if everybody thinks the general
>> approach is the best solution, we should do that. From my point of view,
>> there are two things I would do differently:
>>
>> - In the DT binding, I would strongly prefer anything but the root compatible
>>    property as the key for the new platforms. Clearly we have to keep using
>>    it for the backwards-compatibility case, as you do, but I think there
>>    are more appropriate places to put it. Sorting from most favorite to least
>>    favorite, my list would be:
>>          1. a new property in /cpus/
>>          2. a new property each /cpus/cpu@... node.
>
> I did it this way earlier and named it dvfs-method but probably putting this
> into the /cpus/ node is far better. But then Sudeep asked to utilize
> compatible property only..
>
> Are you fine with the name here? "dvfs-method"
>

That's right, I don't like driver specific method in the cpu node as you
initially did. But if it's a property in the chosen node (where we
usually put the Linux specific properties), I am fine with
that as Arnd has illustrated in his patch.

Regards,
Sudeep

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Arnd Bergmann]

On Monday 01 December 2014 18:59:20 Viresh Kumar wrote:
> On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
> > Thanks a lot for working on this, we really need to figure it out one day!
> 
> :)
> 
> > Your patches seem well-implemented, so if everybody thinks the general
> > approach is the best solution, we should do that. From my point of view,
> > there are two things I would do differently:
> >
> > - In the DT binding, I would strongly prefer anything but the root compatible
> >   property as the key for the new platforms. Clearly we have to keep using
> >   it for the backwards-compatibility case, as you do, but I think there
> >   are more appropriate places to put it. Sorting from most favorite to least
> >   favorite, my list would be:
> >         1. a new property in /cpus/
> >         2. a new property each /cpus/cpu@... node.
> 
> I did it this way earlier and named it dvfs-method but probably putting this
> into the /cpus/ node is far better. But then Sudeep asked to utilize
> compatible property only..
> 
> Are you fine with the name here? "dvfs-method"

No objection here, whatever makes sense to you.

> > +static bool dt_cpufreq_available(void)
> > +{
> > +       struct device_node *node;
> > +       bool ret;
> > +
> > +       node = of_find_node_by_path("/cpus");
> > +       if (!node)
> > +               return 0;
> > +
> > +       /* the specific property needs to be debated */
> > +       ret = of_property_read_bool("linux,cpu-frequency-scaling");
> 
> How can this be a bool? We need to differentiate on which binding
> wants to go for arm-bl or cupfreq-dt or any other driver. So we surely
> need a string ?

I guess a string would be better here, the idea here was to
have a different bool property per driver, which would also
work.

> > @@ -367,29 +404,19 @@ static int dt_cpufreq_probe(struct platform_device *pdev)
> >         if (!IS_ERR(cpu_reg))
> >                 regulator_put(cpu_reg);
> >
> > -       dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
> > -
> 
> We still need this, and its about how clocks are shared between CPUs.

I didn't see where this comes from. Who is setting up this platform
data?

	Arnd

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Arnd Bergmann]

On Monday 01 December 2014 13:35:25 Sudeep Holla wrote:
> On 01/12/14 13:29, Viresh Kumar wrote:
> > On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
> >> Thanks a lot for working on this, we really need to figure it out one day!
> >
> > 
> >
> >> Your patches seem well-implemented, so if everybody thinks the general
> >> approach is the best solution, we should do that. From my point of view,
> >> there are two things I would do differently:
> >>
> >> - In the DT binding, I would strongly prefer anything but the root compatible
> >>    property as the key for the new platforms. Clearly we have to keep using
> >>    it for the backwards-compatibility case, as you do, but I think there
> >>    are more appropriate places to put it. Sorting from most favorite to least
> >>    favorite, my list would be:
> >>          1. a new property in /cpus/
> >>          2. a new property each /cpus/cpu@... node.
> >
> > I did it this way earlier and named it dvfs-method but probably putting this
> > into the /cpus/ node is far better. But then Sudeep asked to utilize
> > compatible property only..
> >
> > Are you fine with the name here? "dvfs-method"
> >
> 
> That's right, I don't like driver specific method in the cpu node as you
> initially did. But if it's a property in the chosen node (where we
> usually put the Linux specific properties), I am fine with
> that as Arnd has illustrated in his patch.

I would prefer the /cpus node over the /chosen node because the former
describes the hardware while the latter is supposed to be user-settable
(on real open-firmware at least). But I think either one is better than
using the / node compatible string.

How about a "linux,cpu-dvfs-method" property in the root node? Would
that work better for you than a "linux,dvfs-method" in the "/cpus"
node?

	Arnd

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Viresh Kumar]

On 1 December 2014 at 19:35, Arnd Bergmann <arnd@arndb.de> wrote:
> I guess a string would be better here, the idea here was to
> have a different bool property per driver, which would also
> work.

Hmm, I will prefer string as we don't need to define any more bindings then
for new drivers.

>> > @@ -367,29 +404,19 @@ static int dt_cpufreq_probe(struct platform_device *pdev)
>> >         if (!IS_ERR(cpu_reg))
>> >                 regulator_put(cpu_reg);
>> >
>> > -       dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
>> > -
>>
>> We still need this, and its about how clocks are shared between CPUs.
>
> I didn't see where this comes from. Who is setting up this platform
> data?

Mvebu is using it to communicate that all CPUs have separate
clock lines.

--
viresh

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Viresh Kumar]

On 1 December 2014 at 19:41, Arnd Bergmann <arnd@arndb.de> wrote:
> I would prefer the /cpus node over the /chosen node because the former
> describes the hardware while the latter is supposed to be user-settable
> (on real open-firmware at least). But I think either one is better than
> using the / node compatible string.
>
> How about a "linux,cpu-dvfs-method" property in the root node? Would
> that work better for you than a "linux,dvfs-method" in the "/cpus"
> node?

I will choose "linux,dvfs-method" in the "/cpus" node.

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Arnd Bergmann]

On Monday 01 December 2014 20:18:10 Viresh Kumar wrote:
> On 1 December 2014 at 19:35, Arnd Bergmann <arnd@arndb.de> wrote:
> > I guess a string would be better here, the idea here was to
> > have a different bool property per driver, which would also
> > work.
> 
> Hmm, I will prefer string as we don't need to define any more bindings then
> for new drivers.

Right. You'd still need to define the known values though, so in
effect it's not much of a difference. I have no problem with 
a string property though.

> >> > @@ -367,29 +404,19 @@ static int dt_cpufreq_probe(struct platform_device *pdev)
> >> >         if (!IS_ERR(cpu_reg))
> >> >                 regulator_put(cpu_reg);
> >> >
> >> > -       dt_cpufreq_driver.driver_data = dev_get_platdata(&pdev->dev);
> >> > -
> >>
> >> We still need this, and its about how clocks are shared between CPUs.
> >
> > I didn't see where this comes from. Who is setting up this platform
> > data?
> 
> Mvebu is using it to communicate that all CPUs have separate
> clock lines.

I still don't see where it does that. All I see for mvebu is

	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);

without any platform data. I see this patch
http://lists.linaro.org/pipermail/linaro-kernel/2014-September/017693.html
on the mailing list, but it's not in linux-next, and it obviously
would not work any more with the patch I proposed. Instead I suppose
you would use a different string to match against for the case of
separate clocks.

	Arnd

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Sudeep Holla]

On 01/12/14 14:11, Arnd Bergmann wrote:
> On Monday 01 December 2014 13:35:25 Sudeep Holla wrote:
>> On 01/12/14 13:29, Viresh Kumar wrote:
>>> On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
>>>> Thanks a lot for working on this, we really need to figure it out one day!
>>>
>>>
>>>
>>>> Your patches seem well-implemented, so if everybody thinks the general
>>>> approach is the best solution, we should do that. From my point of view,
>>>> there are two things I would do differently:
>>>>
>>>> - In the DT binding, I would strongly prefer anything but the root compatible
>>>>     property as the key for the new platforms. Clearly we have to keep using
>>>>     it for the backwards-compatibility case, as you do, but I think there
>>>>     are more appropriate places to put it. Sorting from most favorite to least
>>>>     favorite, my list would be:
>>>>           1. a new property in /cpus/
>>>>           2. a new property each /cpus/cpu@... node.
>>>
>>> I did it this way earlier and named it dvfs-method but probably putting this
>>> into the /cpus/ node is far better. But then Sudeep asked to utilize
>>> compatible property only..
>>>
>>> Are you fine with the name here? "dvfs-method"
>>>
>>
>> That's right, I don't like driver specific method in the cpu node as you
>> initially did. But if it's a property in the chosen node (where we
>> usually put the Linux specific properties), I am fine with
>> that as Arnd has illustrated in his patch.
>
> I would prefer the /cpus node over the /chosen node because the former
> describes the hardware while the latter is supposed to be user-settable
> (on real open-firmware at least). But I think either one is better than
> using the / node compatible string.
>

Agreed, the main reason for objection was that in the initial proposal
it was more a Linux configuration rather than hardware property.

<dvfs-method> = "cpufreq-dt";

I don't see anything hardware feature presented with that. On the other
hand, we could represent the some thing like whether the cpu share
clock or are they independently clocked as a hardware property in the
cpu nodes.

> How about a "linux,cpu-dvfs-method" property in the root node? Would
> that work better for you than a "linux,dvfs-method" in the "/cpus"
> node?
>

I will leave that to the DT maintainers for specific details though
my preference is still chosen node as it's more related to Linux and
it's driver choice and unlikely to be used by other OSes. IMO we just
need single entry in the DT, so I am fine with either of your choice above.

Regards,
Sudeep

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Arnd Bergmann]

On Monday 01 December 2014 15:07:15 Sudeep Holla wrote:
> On 01/12/14 14:11, Arnd Bergmann wrote:
> > On Monday 01 December 2014 13:35:25 Sudeep Holla wrote:
> >> On 01/12/14 13:29, Viresh Kumar wrote:
> >>> On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
> >>>> Thanks a lot for working on this, we really need to figure it out one day!
> >>>
> >>>
> >>>
> >>>> Your patches seem well-implemented, so if everybody thinks the general
> >>>> approach is the best solution, we should do that. From my point of view,
> >>>> there are two things I would do differently:
> >>>>
> >>>> - In the DT binding, I would strongly prefer anything but the root compatible
> >>>>     property as the key for the new platforms. Clearly we have to keep using
> >>>>     it for the backwards-compatibility case, as you do, but I think there
> >>>>     are more appropriate places to put it. Sorting from most favorite to least
> >>>>     favorite, my list would be:
> >>>>           1. a new property in /cpus/
> >>>>           2. a new property each /cpus/cpu@... node.
> >>>
> >>> I did it this way earlier and named it dvfs-method but probably putting this
> >>> into the /cpus/ node is far better. But then Sudeep asked to utilize
> >>> compatible property only..
> >>>
> >>> Are you fine with the name here? "dvfs-method"
> >>>
> >>
> >> That's right, I don't like driver specific method in the cpu node as you
> >> initially did. But if it's a property in the chosen node (where we
> >> usually put the Linux specific properties), I am fine with
> >> that as Arnd has illustrated in his patch.
> >
> > I would prefer the /cpus node over the /chosen node because the former
> > describes the hardware while the latter is supposed to be user-settable
> > (on real open-firmware at least). But I think either one is better than
> > using the / node compatible string.
> >
> 
> Agreed, the main reason for objection was that in the initial proposal
> it was more a Linux configuration rather than hardware property.
> 
> <dvfs-method> = "cpufreq-dt";
> 
> I don't see anything hardware feature presented with that. On the other
> hand, we could represent the some thing like whether the cpu share
> clock or are they independently clocked as a hardware property in the
> cpu nodes.

My interpretation of the dvfs-method property is that the string states
how dvfs works. In particular, it would be used to tell the difference
between machines that just rely on the clocks and regulators properties
of the CPU nodes as defined in bindings/cpufreq/cpufreq-dt.txt compared
to those that need platform-specific properties beyond that. The value
of the string should indeed not be "cpufreq-dt", as that would be a linux
implementation detail and inappropriate here. 

For the strings, we could use a set like

	linux,dvfs-method = "generic"; /* bindings/cpufreq/cpufreq-dt.txt */
	linux,dvfs-method = "arm,big-little"; /* bindings/cpufreq/arm_big_little_dt.txt */
	linux,dvfs-method = "samsung,exynos4210"; /* legacy exynos, all four */
	linux,dvfs-method = "samsung,exynos4212";
	linux,dvfs-method = "samsung,exynos4412";
	linux,dvfs-method = "samsung,exynos5250";

Note that the "linux," prefix here does not mean that the property would
not be interpreted by another OS or that it doesn't describe the hardware.
Instead, it means that it is defined by linux first and not specific to
some other vendor. We could also drop the prefix, but that has the danger
of conflicting with another property that someone already defined, while
the "linux," namespace is managed through our upstream git and we know that
nobody is using this property name.

> > How about a "linux,cpu-dvfs-method" property in the root node? Would
> > that work better for you than a "linux,dvfs-method" in the "/cpus"
> > node?
> >
> 
> I will leave that to the DT maintainers for specific details though
> my preference is still chosen node as it's more related to Linux and
> it's driver choice and unlikely to be used by other OSes. IMO we just
> need single entry in the DT, so I am fine with either of your choice above.

Ok.

	Arnd

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Sudeep Holla]

On 01/12/14 16:03, Arnd Bergmann wrote:
> On Monday 01 December 2014 15:07:15 Sudeep Holla wrote:
>> On 01/12/14 14:11, Arnd Bergmann wrote:
>>> On Monday 01 December 2014 13:35:25 Sudeep Holla wrote:
>>>> On 01/12/14 13:29, Viresh Kumar wrote:
>>>>> On 1 December 2014 at 18:24, Arnd Bergmann <arnd@arndb.de> wrote:
>>>>>> Thanks a lot for working on this, we really need to figure it out one day!
>>>>>
>>>>>
>>>>>
>>>>>> Your patches seem well-implemented, so if everybody thinks the general
>>>>>> approach is the best solution, we should do that. From my point of view,
>>>>>> there are two things I would do differently:
>>>>>>
>>>>>> - In the DT binding, I would strongly prefer anything but the root compatible
>>>>>>      property as the key for the new platforms. Clearly we have to keep using
>>>>>>      it for the backwards-compatibility case, as you do, but I think there
>>>>>>      are more appropriate places to put it. Sorting from most favorite to least
>>>>>>      favorite, my list would be:
>>>>>>            1. a new property in /cpus/
>>>>>>            2. a new property each /cpus/cpu@... node.
>>>>>
>>>>> I did it this way earlier and named it dvfs-method but probably putting this
>>>>> into the /cpus/ node is far better. But then Sudeep asked to utilize
>>>>> compatible property only..
>>>>>
>>>>> Are you fine with the name here? "dvfs-method"
>>>>>
>>>>
>>>> That's right, I don't like driver specific method in the cpu node as you
>>>> initially did. But if it's a property in the chosen node (where we
>>>> usually put the Linux specific properties), I am fine with
>>>> that as Arnd has illustrated in his patch.
>>>
>>> I would prefer the /cpus node over the /chosen node because the former
>>> describes the hardware while the latter is supposed to be user-settable
>>> (on real open-firmware at least). But I think either one is better than
>>> using the / node compatible string.
>>>
>>
>> Agreed, the main reason for objection was that in the initial proposal
>> it was more a Linux configuration rather than hardware property.
>>
>> <dvfs-method> = "cpufreq-dt";
>>
>> I don't see anything hardware feature presented with that. On the other
>> hand, we could represent the some thing like whether the cpu share
>> clock or are they independently clocked as a hardware property in the
>> cpu nodes.
>
> My interpretation of the dvfs-method property is that the string states
> how dvfs works. In particular, it would be used to tell the difference
> between machines that just rely on the clocks and regulators properties
> of the CPU nodes as defined in bindings/cpufreq/cpufreq-dt.txt compared
> to those that need platform-specific properties beyond that. The value
> of the string should indeed not be "cpufreq-dt", as that would be a linux
> implementation detail and inappropriate here.
>

May be I misunderstood, but from Viresh's initial proposal my
understanding was that the value of the property would indicate that it
should use the cpufreq-dt driver and that sounded like Linux specific.

If it's not going to be used in that manner and is what have explained
above, I am fine with that as that's exactly what I am trying to convey
in this thread(though I now realize that I have not been so clear :( )

Regards,
Sudeep

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Rob Herring]

On Mon, Dec 1, 2014 at 6:54 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 01 December 2014 17:11:21 Viresh Kumar wrote:
>>
>> DT based cpufreq drivers doesn't require much support from platform code now a
>> days as most of the stuff is moved behind generic APIs. Like clk APIs for
>> changing clock rates, regulator APIs for changing voltages, etc.
>>
>> One of the bottleneck still left was how to select which cpufreq driver to probe
>> for a given platform as there might be multiple drivers available.
>>
>> Traditionally, we used to create platform devices from machine specific code
>> which binds with a cpufreq driver. And while we moved towards DT based device
>> creation, these devices stayed as is.
>>
>> The problem is getting worse now as we have architectures now with Zero platform
>> specific code. Forcefully these platforms have to create a new file in
>> drivers/cpufreq/ to just add these platform devices in order to use the generic
>> drivers like cpufreq-dt.c.
>>
>> This has been discussed again and again, but with no solution yet. Last it was
>> discussed here:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-May/256154.html
>>
>> This patch is an attempt towards getting the bindings.
>>
>> We only need to have cpufreq drivers name string present in "compatible"
>> property for the root node.. If a cpufreq driver with DT support exists with
>> that name, then cpufreq core will create a platform device for that.
>>
>> The first patch presents the new binding, second one creates another file
>> responsible for creating platform devices for all DT based cpufreq drivers.
>>
>> And later patches update platforms to migrate to it one by one.
>>
>> A BLACKLIST of platforms already supported by these drivers is also created for
>> backward compatibility of newer kernels with older DTs. And so for such
>> platforms DT files aren't updated.
>>
>> An initial RFC that presented the idea was discussed here:
>> https://www.mail-archive.com/devicetree@vger.kernel.org/msg52509.html
>>
>> Tested-ON: Exynos5250. (The last patch for exynos depends on some commits to be
>> upstreamed in 3.19, presented here just for testing).
>
> Thanks a lot for working on this, we really need to figure it out one day!
>
> Your patches seem well-implemented, so if everybody thinks the general
> approach is the best solution, we should do that. From my point of view,
> there are two things I would do differently:

I think the changes here for the "legacy" DTs are fine, but they
should be separated from the DT binding changes.

> - In the DT binding, I would strongly prefer anything but the root compatible
>   property as the key for the new platforms. Clearly we have to keep using
>   it for the backwards-compatibility case, as you do, but I think there
>   are more appropriate places to put it. Sorting from most favorite to least
>   favorite, my list would be:
>         1. a new property in /cpus/
>         2. a new property each /cpus/cpu@... node.
>         3. the compatible property of the /cpus node
>         4. a top-level device node that gets turned into a platform device
>         5. a new property in the / node
>         6. a new property in the /chosen node
>         7. the compatible property in the / node

We already have some properties such as clocks and OPP in the cpu
nodes. Granted, those are probably not sufficient to bind against. The
current OPP binding has shown to be insufficient based on some of the
past proposals on how to handle various different scenarios:

- Different topologies of OPPs: single shared clock vs. independent
clock per core vs. shared clock per cluster; different OPP per cluster
- Support for turbo modes
- Other per OPP settings: transition latencies, disabled status, etc.?

I don't want to see band-aids that only fix one problem here and this
series is included. We need to define a better way to define OPPs and
deprecate the existing binding. I think we probably need something
with a node per OPP so we can add new properties. These can have a
compatible property including something generic for purposes of
matching. So something like this:

opp@?? {
  compatible = "highbank-opp", "generic-cpu-opp";
  clocks = <&clk-controller 0>;
  clock-frequency = <1000000000>;
  opp-supply = <&cpu-supply>;
  voltage-uV = <1000000>;
  turbo-mode;
  status = "disabled";
};

I've left how to map cpus and clusters with OPPs as an exercise for
the reader. :)

> - Implementation-wise, I don't think it's helpful to have a global function
>   that registers a platform device to be consumed by the device driver. I'd
>   rather just see a module_init function in each driver that rather than
>   registering a platform_driver scans the DT properties. This is only really
>   necessary when not using DT (omap2, omap3, davinci, loongson) or when
>   passing additional resources or platform_data (kirkwood, but that can
>   look up the "marvell,orion-system-controller" node if necessary).
>   My preferred solution would be to eventually remove the platform_device
>   registration for all DT users. If a driver needs a platform device pointer
>   internally, it can use platform_create_bundle(), but that's probably not
>   even necessary.

This is essentially undoing what has been the general direction for
cpufreq drivers. Not saying that is wrong, but we should have
consistent direction here.

Rob

Re: [RFC V1 0/8] CPUFreq: create platform-dev for DT based cpufreq drivers

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Mon, 01 Dec 2014 15:59:14 +0100, Arnd Bergmann wrote:

> I still don't see where it does that. All I see for mvebu is
> 
> 	platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
> 
> without any platform data. I see this patch
> http://lists.linaro.org/pipermail/linaro-kernel/2014-September/017693.html
> on the mailing list, but it's not in linux-next, and it obviously
> would not work any more with the patch I proposed. Instead I suppose
> you would use a different string to match against for the case of
> separate clocks.

Hum, right. Actually, only the cpufreq driver part has been taken, and
I forgot to resubmit the mach-mvebu part of the solution. I'll do so
today.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com