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 + >; + }; + + ... + + }; + + ... + +}

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 */

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 */

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;

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");