2013-03-26 09:52:15

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

big LITTLE is ARM's new Architecture focussing power/performance needs of modern
world. More information about big LITTLE can be found here:

http://www.arm.com/products/processors/technologies/biglittleprocessing.php
http://lwn.net/Articles/481055/

In order to keep cpufreq support for all big LITTLE platforms simple/generic,
this patch tries to add a generic cpufreq driver layer for all big LITTLE
platforms.

The driver is divided into two parts:
- Core driver: Generic and shared across all big LITTLE SoC's
- Glue drivers: Per platform drivers providing ops to the core driver

This patch adds in a generic glue driver which would extract information from
Device Tree.

Future SoC's can either reuse the DT glue or write their own depending on the
need.

Signed-off-by: Sudeep KarkadaNagesha <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
V1->V2:
- It was reviewed here earlier: https://lkml.org/lkml/2013/3/4/614
- It supports OPP library now and doesn't create a new binding for cpufreq table
- It doesn't add any dependency on cluster node in DT, rather we work with
existing cpu nodes, Documentation updated.
- cpu_dev used because of OPP library and hence dev_err/dbg/info used at
multiple places.
- Interface with glue driver updated a bit
- IS_ERR_OR_NULL replaced with IS_ERR for clk_get
- clk_get_sys used instead of clk_get and name of clk is also updated
- Few more minor cleanups done.

It is pushed here:

http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/cpufreq-biglittle

.../bindings/cpufreq/arm_big_little_dt.txt | 65 +++++
MAINTAINERS | 11 +
drivers/cpufreq/Kconfig.arm | 12 +
drivers/cpufreq/Makefile | 4 +
drivers/cpufreq/arm_big_little.c | 282 +++++++++++++++++++++
drivers/cpufreq/arm_big_little.h | 40 +++
drivers/cpufreq/arm_big_little_dt.c | 92 +++++++
7 files changed, 506 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
create mode 100644 drivers/cpufreq/arm_big_little.c
create mode 100644 drivers/cpufreq/arm_big_little.h
create mode 100644 drivers/cpufreq/arm_big_little_dt.c

diff --git a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
new file mode 100644
index 0000000..34a460d
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
@@ -0,0 +1,65 @@
+Generic ARM big LITTLE cpufreq driver's DT glue
+-----------------------------------------------
+
+This is DT specific glue layer for generic cpufreq driver for big LITTLE
+systems.
+
+Both required and optional properties listed below must be defined
+under node /cpus/[email protected] Where x is the first cpu inside a cluster.
+
+NOTE: Cpus should boot in the order specified in DT and all cpus for a cluster
+must be present contiguously. Generic DT driver will check only node 'x' for
+cpu:x.
+
+Required properties:
+- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
+ for details
+
+Optional properties:
+- clock-latency: Specify the possible maximum transition latency for clock,
+ in unit of nanoseconds.
+
+Examples:
+
+cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ [email protected] {
+ compatible = "arm,cortex-a15";
+ reg = <0>;
+ next-level-cache = <&L2>;
+ operating-points = <
+ /* kHz uV */
+ 792000 1100000
+ 396000 950000
+ 198000 850000
+ >;
+ clock-latency = <61036>; /* two CLK32 periods */
+ };
+
+ [email protected] {
+ compatible = "arm,cortex-a15";
+ reg = <1>;
+ next-level-cache = <&L2>;
+ };
+
+ [email protected] {
+ compatible = "arm,cortex-a7";
+ reg = <100>;
+ next-level-cache = <&L2>;
+ operating-points = <
+ /* kHz uV */
+ 792000 950000
+ 396000 750000
+ 198000 450000
+ >;
+ clock-latency = <61036>; /* two CLK32 periods */
+ };
+
+ [email protected] {
+ compatible = "arm,cortex-a7";
+ reg = <101>;
+ next-level-cache = <&L2>;
+ };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 4cf5fd3..4071b71 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2206,6 +2206,17 @@ S: Maintained
F: drivers/cpufreq/
F: include/linux/cpufreq.h

+CPU FREQUENCY DRIVERS - ARM BIG LITTLE
+M: Viresh Kumar <[email protected]>
+M: Sudeep KarkadaNagesha <[email protected]>
+L: [email protected]
+L: [email protected]
+W: http://www.arm.com/products/processors/technologies/biglittleprocessing.php
+S: Maintained
+F: drivers/cpufreq/arm_big_little.h
+F: drivers/cpufreq/arm_big_little.c
+F: drivers/cpufreq/arm_big_little_dt.c
+
CPUID/MSR DRIVER
M: "H. Peter Anvin" <[email protected]>
S: Maintained
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 030ddf6..87b7e48 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -2,6 +2,18 @@
# ARM CPU Frequency scaling drivers
#

+config ARM_BIG_LITTLE_CPUFREQ
+ tristate
+ depends on ARM_CPU_TOPOLOGY
+
+config ARM_DT_BL_CPUFREQ
+ tristate "Generic ARM big LITTLE CPUfreq driver probed via DT"
+ select ARM_BIG_LITTLE_CPUFREQ
+ depends on OF && HAVE_CLK
+ help
+ This enables the Generic CPUfreq driver for ARM big.LITTLE platform.
+ This gets frequency tables from DT.
+
config ARM_OMAP2PLUS_CPUFREQ
bool "TI OMAP2+"
depends on ARCH_OMAP2PLUS
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 863fd18..d1b0832 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -44,6 +44,10 @@ obj-$(CONFIG_X86_INTEL_PSTATE) += intel_pstate.o

##################################################################################
# ARM SoC drivers
+obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ) += arm_big_little.o
+# big LITTLE per platform glues. Keep DT_BL_CPUFREQ as the last entry in all big
+# LITTLE drivers, so that it is probed last.
+obj-$(CONFIG_ARM_DT_BL_CPUFREQ) += arm_big_little_dt.o
obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o
obj-$(CONFIG_ARM_S3C64XX_CPUFREQ) += s3c64xx-cpufreq.o
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
new file mode 100644
index 0000000..1d29d1a
--- /dev/null
+++ b/drivers/cpufreq/arm_big_little.c
@@ -0,0 +1,282 @@
+/*
+ * ARM big.LITTLE Platforms CPUFreq support
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ * Sudeep KarkadaNagesha <[email protected]>
+ *
+ * Copyright (C) 2013 Linaro.
+ * Viresh Kumar <[email protected]>
+ *
+ * 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/clk.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/cpumask.h>
+#include <linux/export.h>
+#include <linux/of_platform.h>
+#include <linux/opp.h>
+#include <linux/slab.h>
+#include <linux/topology.h>
+#include <linux/types.h>
+
+#include "arm_big_little.h"
+
+/* Currently we support only two clusters */
+#define MAX_CLUSTERS 2
+
+static struct cpufreq_arm_bL_ops *arm_bL_ops;
+static struct clk *clk[MAX_CLUSTERS];
+static struct cpufreq_frequency_table *freq_table[MAX_CLUSTERS];
+static atomic_t cluster_usage[MAX_CLUSTERS] = {ATOMIC_INIT(0), ATOMIC_INIT(0)};
+
+static int cpu_to_cluster(int cpu)
+{
+ return topology_physical_package_id(cpu);
+}
+
+static unsigned int bL_cpufreq_get(unsigned int cpu)
+{
+ u32 cur_cluster = cpu_to_cluster(cpu);
+
+ return clk_get_rate(clk[cur_cluster]) / 1000;
+}
+
+/* Validate policy frequency range */
+static int bL_cpufreq_verify_policy(struct cpufreq_policy *policy)
+{
+ u32 cur_cluster = cpu_to_cluster(policy->cpu);
+
+ return cpufreq_frequency_table_verify(policy, freq_table[cur_cluster]);
+}
+
+/* Set clock frequency */
+static int bL_cpufreq_set_target(struct cpufreq_policy *policy,
+ unsigned int target_freq, unsigned int relation)
+{
+ struct cpufreq_freqs freqs;
+ u32 cpu = policy->cpu, freq_tab_idx, cur_cluster;
+ int ret = 0;
+
+ cur_cluster = cpu_to_cluster(policy->cpu);
+
+ freqs.old = bL_cpufreq_get(policy->cpu);
+
+ /* Determine valid target frequency using freq_table */
+ cpufreq_frequency_table_target(policy, freq_table[cur_cluster],
+ target_freq, relation, &freq_tab_idx);
+ freqs.new = freq_table[cur_cluster][freq_tab_idx].frequency;
+
+ freqs.cpu = policy->cpu;
+
+ pr_debug("%s: cpu: %d, cluster: %d, oldfreq: %d, target freq: %d, new freq: %d\n",
+ __func__, cpu, cur_cluster, freqs.old, target_freq,
+ freqs.new);
+
+ if (freqs.old == freqs.new)
+ return 0;
+
+ for_each_cpu(freqs.cpu, policy->cpus)
+ cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
+
+ ret = clk_set_rate(clk[cur_cluster], freqs.new * 1000);
+ if (ret) {
+ pr_err("clk_set_rate failed: %d\n", ret);
+ return ret;
+ }
+
+ policy->cur = freqs.new;
+
+ for_each_cpu(freqs.cpu, policy->cpus)
+ cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
+
+ return ret;
+}
+
+static void put_cluster_clk_and_freq_table(struct device *cpu_dev)
+{
+ u32 cluster = cpu_to_cluster(cpu_dev->id);
+
+ if (!atomic_dec_return(&cluster_usage[cluster])) {
+ clk_put(clk[cluster]);
+ opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
+ dev_dbg(cpu_dev, "%s: cluster: %d\n", __func__, cluster);
+ }
+}
+
+static int get_cluster_clk_and_freq_table(struct device *cpu_dev)
+{
+ u32 cluster = cpu_to_cluster(cpu_dev->id);
+ char name[14] = "cpu-cluster.";
+ int ret;
+
+ if (atomic_inc_return(&cluster_usage[cluster]) != 1)
+ return 0;
+
+ ret = arm_bL_ops->init_opp_table(cpu_dev);
+ if (ret) {
+ dev_err(cpu_dev, "%s: init_opp_table failed, cpu: %d, err: %d\n",
+ __func__, cpu_dev->id, ret);
+ goto atomic_dec;
+ }
+
+ ret = opp_init_cpufreq_table(cpu_dev, &freq_table[cluster]);
+ if (ret) {
+ dev_err(cpu_dev, "%s: failed to init cpufreq table, cpu: %d, err: %d\n",
+ __func__, cpu_dev->id, ret);
+ goto atomic_dec;
+ }
+
+ name[12] = cluster + '0';
+ clk[cluster] = clk_get_sys(name, NULL);
+ if (!IS_ERR(clk[cluster])) {
+ dev_dbg(cpu_dev, "%s: clk: %p & freq table: %p, cluster: %d\n",
+ __func__, clk[cluster], freq_table[cluster],
+ cluster);
+ return 0;
+ }
+
+ dev_err(cpu_dev, "%s: Failed to get clk for cpu: %d, cluster: %d\n",
+ __func__, cpu_dev->id, cluster);
+ ret = PTR_ERR(clk[cluster]);
+ opp_free_cpufreq_table(cpu_dev, &freq_table[cluster]);
+
+atomic_dec:
+ atomic_dec(&cluster_usage[cluster]);
+ dev_err(cpu_dev, "%s: Failed to get data for cluster: %d\n", __func__,
+ cluster);
+ return ret;
+}
+
+/* Per-CPU initialization */
+static int bL_cpufreq_init(struct cpufreq_policy *policy)
+{
+ u32 cur_cluster = cpu_to_cluster(policy->cpu);
+ struct device *cpu_dev;
+ int ret;
+
+ cpu_dev = get_cpu_device(policy->cpu);
+ if (!cpu_dev) {
+ pr_err("%s: failed to get cpu%d device\n", __func__,
+ policy->cpu);
+ return -ENODEV;
+ }
+
+ ret = get_cluster_clk_and_freq_table(cpu_dev);
+ if (ret)
+ return ret;
+
+ ret = cpufreq_frequency_table_cpuinfo(policy, freq_table[cur_cluster]);
+ if (ret) {
+ dev_err(cpu_dev, "CPU %d, cluster: %d invalid freq table\n",
+ policy->cpu, cur_cluster);
+ put_cluster_clk_and_freq_table(cpu_dev);
+ return ret;
+ }
+
+ cpufreq_frequency_table_get_attr(freq_table[cur_cluster], policy->cpu);
+
+ if (arm_bL_ops->get_transition_latency)
+ policy->cpuinfo.transition_latency =
+ arm_bL_ops->get_transition_latency(cpu_dev);
+ else
+ policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
+
+ policy->cur = bL_cpufreq_get(policy->cpu);
+
+ cpumask_copy(policy->cpus, topology_core_cpumask(policy->cpu));
+
+ dev_info(cpu_dev, "CPU %d initialized\n", policy->cpu);
+ return 0;
+}
+
+static int bL_cpufreq_exit(struct cpufreq_policy *policy)
+{
+ struct device *cpu_dev;
+
+ cpu_dev = get_cpu_device(policy->cpu);
+ if (!cpu_dev) {
+ pr_err("%s: failed to get cpu%d device\n", __func__,
+ policy->cpu);
+ return -ENODEV;
+ }
+
+ put_cluster_clk_and_freq_table(cpu_dev);
+ dev_dbg(cpu_dev, "%s: Exited, cpu: %d\n", __func__, policy->cpu);
+
+ return 0;
+}
+
+/* Export freq_table to sysfs */
+static struct freq_attr *bL_cpufreq_attr[] = {
+ &cpufreq_freq_attr_scaling_available_freqs,
+ NULL,
+};
+
+static struct cpufreq_driver bL_cpufreq_driver = {
+ .name = "arm-big-little",
+ .flags = CPUFREQ_STICKY,
+ .verify = bL_cpufreq_verify_policy,
+ .target = bL_cpufreq_set_target,
+ .get = bL_cpufreq_get,
+ .init = bL_cpufreq_init,
+ .exit = bL_cpufreq_exit,
+ .have_multiple_policies = true,
+ .attr = bL_cpufreq_attr,
+};
+
+int bL_cpufreq_register(struct cpufreq_arm_bL_ops *ops)
+{
+ int ret;
+
+ if (arm_bL_ops) {
+ pr_debug("%s: Already registered: %s, exiting\n", __func__,
+ arm_bL_ops->name);
+ return -EBUSY;
+ }
+
+ if (!ops || !strlen(ops->name) || !ops->init_opp_table) {
+ pr_err("%s: Invalid arm_bL_ops, exiting\n", __func__);
+ return -ENODEV;
+ }
+
+ arm_bL_ops = ops;
+
+ ret = cpufreq_register_driver(&bL_cpufreq_driver);
+ if (ret) {
+ pr_info("%s: Failed registering platform driver: %s, err: %d\n",
+ __func__, ops->name, ret);
+ arm_bL_ops = NULL;
+ } else {
+ pr_info("%s: Registered platform driver: %s\n", __func__,
+ ops->name);
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(bL_cpufreq_register);
+
+void bL_cpufreq_unregister(struct cpufreq_arm_bL_ops *ops)
+{
+ if (arm_bL_ops != ops) {
+ pr_err("%s: Registered with: %s, can't unregister, exiting\n",
+ __func__, arm_bL_ops->name);
+ return;
+ }
+
+ cpufreq_unregister_driver(&bL_cpufreq_driver);
+ pr_info("%s: Un-registered platform driver: %s\n", __func__,
+ arm_bL_ops->name);
+ arm_bL_ops = NULL;
+}
+EXPORT_SYMBOL_GPL(bL_cpufreq_unregister);
diff --git a/drivers/cpufreq/arm_big_little.h b/drivers/cpufreq/arm_big_little.h
new file mode 100644
index 0000000..70f18fc
--- /dev/null
+++ b/drivers/cpufreq/arm_big_little.h
@@ -0,0 +1,40 @@
+/*
+ * ARM big.LITTLE platform's CPUFreq header file
+ *
+ * Copyright (C) 2013 ARM Ltd.
+ * Sudeep KarkadaNagesha <[email protected]>
+ *
+ * Copyright (C) 2013 Linaro.
+ * Viresh Kumar <[email protected]>
+ *
+ * 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.
+ */
+#ifndef CPUFREQ_ARM_BIG_LITTLE_H
+#define CPUFREQ_ARM_BIG_LITTLE_H
+
+#include <linux/cpufreq.h>
+#include <linux/device.h>
+#include <linux/types.h>
+
+struct cpufreq_arm_bL_ops {
+ char name[CPUFREQ_NAME_LEN];
+ int (*get_transition_latency)(struct device *cpu_dev);
+
+ /*
+ * This must set opp table for cpu_dev in a similar way as done by
+ * of_init_opp_table().
+ */
+ int (*init_opp_table)(struct device *cpu_dev);
+};
+
+int bL_cpufreq_register(struct cpufreq_arm_bL_ops *ops);
+void bL_cpufreq_unregister(struct cpufreq_arm_bL_ops *ops);
+
+#endif /* CPUFREQ_ARM_BIG_LITTLE_H */
diff --git a/drivers/cpufreq/arm_big_little_dt.c b/drivers/cpufreq/arm_big_little_dt.c
new file mode 100644
index 0000000..452ff46
--- /dev/null
+++ b/drivers/cpufreq/arm_big_little_dt.c
@@ -0,0 +1,92 @@
+/*
+ * Generic big.LITTLE CPUFreq Interface driver
+ *
+ * It provides necessary ops to arm_big_little cpufreq driver and gets
+ * Frequency information from Device Tree. Freq table in DT must be in KHz.
+ *
+ * Copyright (C) 2013 Linaro.
+ * Viresh Kumar <[email protected]>
+ *
+ * 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/cpufreq.h>
+#include <linux/device.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/opp.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include "arm_big_little.h"
+
+static int dt_init_opp_table(struct device *cpu_dev)
+{
+ struct device_node *np = NULL;
+ int count = 0, ret;
+
+ for_each_child_of_node(of_find_node_by_path("/cpus"), np) {
+ if (count++ != cpu_dev->id)
+ continue;
+ if (!of_get_property(np, "operating-points", NULL))
+ return -ENODATA;
+
+ cpu_dev->of_node = np;
+
+ ret = of_init_opp_table(cpu_dev);
+ if (ret)
+ return ret;
+
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
+static int dt_get_transition_latency(struct device *cpu_dev)
+{
+ struct device_node *np = NULL;
+ u32 transition_latency = CPUFREQ_ETERNAL;
+ int count = 0;
+
+ for_each_child_of_node(of_find_node_by_path("/cpus"), np) {
+ if (count++ != cpu_dev->id)
+ continue;
+
+ of_property_read_u32(np, "clock-latency", &transition_latency);
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
+static struct cpufreq_arm_bL_ops dt_bL_ops = {
+ .name = "dt-bl",
+ .get_transition_latency = dt_get_transition_latency,
+ .init_opp_table = dt_init_opp_table,
+};
+
+static int generic_bL_init(void)
+{
+ return bL_cpufreq_register(&dt_bL_ops);
+}
+module_init(generic_bL_init);
+
+static void generic_bL_exit(void)
+{
+ return bL_cpufreq_unregister(&dt_bL_ops);
+}
+module_exit(generic_bL_exit);
+
+MODULE_AUTHOR("Viresh Kumar <[email protected]>");
+MODULE_DESCRIPTION("Generic ARM big LITTLE cpufreq driver via DT");
+MODULE_LICENSE("GPL");
--
1.7.12.rc2.18.g61b472e


2013-03-26 13:17:14

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

On 03/26/2013 04:51 AM, Viresh Kumar wrote:
> big LITTLE is ARM's new Architecture focussing power/performance needs of modern
> world. More information about big LITTLE can be found here:
>
> http://www.arm.com/products/processors/technologies/biglittleprocessing.php
> http://lwn.net/Articles/481055/
>
> In order to keep cpufreq support for all big LITTLE platforms simple/generic,
> this patch tries to add a generic cpufreq driver layer for all big LITTLE
> platforms.
>
> The driver is divided into two parts:
> - Core driver: Generic and shared across all big LITTLE SoC's
> - Glue drivers: Per platform drivers providing ops to the core driver
>
> This patch adds in a generic glue driver which would extract information from
> Device Tree.
>
> Future SoC's can either reuse the DT glue or write their own depending on the
> need.
>
> Signed-off-by: Sudeep KarkadaNagesha <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> V1->V2:
> - It was reviewed here earlier: https://lkml.org/lkml/2013/3/4/614
> - It supports OPP library now and doesn't create a new binding for cpufreq table
> - It doesn't add any dependency on cluster node in DT, rather we work with
> existing cpu nodes, Documentation updated.
> - cpu_dev used because of OPP library and hence dev_err/dbg/info used at
> multiple places.
> - Interface with glue driver updated a bit
> - IS_ERR_OR_NULL replaced with IS_ERR for clk_get
> - clk_get_sys used instead of clk_get and name of clk is also updated
> - Few more minor cleanups done.
>
> It is pushed here:
>
> http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/cpufreq-biglittle
>
> .../bindings/cpufreq/arm_big_little_dt.txt | 65 +++++
> MAINTAINERS | 11 +
> drivers/cpufreq/Kconfig.arm | 12 +
> drivers/cpufreq/Makefile | 4 +
> drivers/cpufreq/arm_big_little.c | 282 +++++++++++++++++++++
> drivers/cpufreq/arm_big_little.h | 40 +++
> drivers/cpufreq/arm_big_little_dt.c | 92 +++++++
> 7 files changed, 506 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
> create mode 100644 drivers/cpufreq/arm_big_little.c
> create mode 100644 drivers/cpufreq/arm_big_little.h
> create mode 100644 drivers/cpufreq/arm_big_little_dt.c
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
> new file mode 100644
> index 0000000..34a460d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/arm_big_little_dt.txt
> @@ -0,0 +1,65 @@
> +Generic ARM big LITTLE cpufreq driver's DT glue
> +-----------------------------------------------
> +
> +This is DT specific glue layer for generic cpufreq driver for big LITTLE
> +systems.

I fail to see anything bL specific here. This is just multi-cluster, but
even for that I don't see anything new other than simply allowing per
cpu or per cluster opp's. The fact that we have one opp for a cluster is
simply an implementation decision in CortexA cores.

> +
> +Both required and optional properties listed below must be defined
> +under node /cpus/[email protected] Where x is the first cpu inside a cluster.
> +
> +NOTE: Cpus should boot in the order specified in DT and all cpus for a cluster
> +must be present contiguously. Generic DT driver will check only node 'x' for
> +cpu:x.

The OS cannot necessarily control the order. The OS should be able to
boot on which ever core comes up first.

> +
> +Required properties:
> +- operating-points: Refer to Documentation/devicetree/bindings/power/opp.txt
> + for details
> +
> +Optional properties:
> +- clock-latency: Specify the possible maximum transition latency for clock,
> + in unit of nanoseconds.

Don't we already have "transition-latency" defined? Don't create
something new. Ideally, this should have had "-ns" appended, but the
binding is already defined.

> +
> +Examples:
> +
> +cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + [email protected] {
> + compatible = "arm,cortex-a15";
> + reg = <0>;
> + next-level-cache = <&L2>;
> + operating-points = <
> + /* kHz uV */
> + 792000 1100000
> + 396000 950000
> + 198000 850000
> + >;
> + clock-latency = <61036>; /* two CLK32 periods */
> + };
> +
> + [email protected] {
> + compatible = "arm,cortex-a15";
> + reg = <1>;
> + next-level-cache = <&L2>;
> + };
> +
> + [email protected] {
> + compatible = "arm,cortex-a7";
> + reg = <100>;
> + next-level-cache = <&L2>;
> + operating-points = <
> + /* kHz uV */
> + 792000 950000
> + 396000 750000
> + 198000 450000
> + >;
> + clock-latency = <61036>; /* two CLK32 periods */
> + };
> +
> + [email protected] {
> + compatible = "arm,cortex-a7";
> + reg = <101>;
> + next-level-cache = <&L2>;
> + };
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4cf5fd3..4071b71 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2206,6 +2206,17 @@ S: Maintained
> F: drivers/cpufreq/
> F: include/linux/cpufreq.h
>
> +CPU FREQUENCY DRIVERS - ARM BIG LITTLE
> +M: Viresh Kumar <[email protected]>
> +M: Sudeep KarkadaNagesha <[email protected]>
> +L: [email protected]
> +L: [email protected]
> +W: http://www.arm.com/products/processors/technologies/biglittleprocessing.php
> +S: Maintained
> +F: drivers/cpufreq/arm_big_little.h
> +F: drivers/cpufreq/arm_big_little.c
> +F: drivers/cpufreq/arm_big_little_dt.c
> +
> CPUID/MSR DRIVER
> M: "H. Peter Anvin" <[email protected]>
> S: Maintained
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 030ddf6..87b7e48 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -2,6 +2,18 @@
> # ARM CPU Frequency scaling drivers
> #
>
> +config ARM_BIG_LITTLE_CPUFREQ
> + tristate
> + depends on ARM_CPU_TOPOLOGY
> +
> +config ARM_DT_BL_CPUFREQ
> + tristate "Generic ARM big LITTLE CPUfreq driver probed via DT"
> + select ARM_BIG_LITTLE_CPUFREQ
> + depends on OF && HAVE_CLK
> + help
> + This enables the Generic CPUfreq driver for ARM big.LITTLE platform.
> + This gets frequency tables from DT.
> +
> config ARM_OMAP2PLUS_CPUFREQ
> bool "TI OMAP2+"
> depends on ARCH_OMAP2PLUS
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 863fd18..d1b0832 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -44,6 +44,10 @@ obj-$(CONFIG_X86_INTEL_PSTATE) += intel_pstate.o
>
> ##################################################################################
> # ARM SoC drivers
> +obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ) += arm_big_little.o
> +# big LITTLE per platform glues. Keep DT_BL_CPUFREQ as the last entry in all big
> +# LITTLE drivers, so that it is probed last.
> +obj-$(CONFIG_ARM_DT_BL_CPUFREQ) += arm_big_little_dt.o
> obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
> obj-$(CONFIG_ARM_S3C2416_CPUFREQ) += s3c2416-cpufreq.o
> obj-$(CONFIG_ARM_S3C64XX_CPUFREQ) += s3c64xx-cpufreq.o
> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
> new file mode 100644
> index 0000000..1d29d1a
> --- /dev/null
> +++ b/drivers/cpufreq/arm_big_little.c
> @@ -0,0 +1,282 @@
> +/*
> + * ARM big.LITTLE Platforms CPUFreq support

What is specific to bL? This looks like just a multi-cluster cpufreq
driver and should be generalized to that. Also, why can't the existing
cpufreq-cpu0 driver be extended to support this?

> + * Copyright (C) 2013 ARM Ltd.
> + * Sudeep KarkadaNagesha <[email protected]>
> + *
> + * Copyright (C) 2013 Linaro.
> + * Viresh Kumar <[email protected]>
> + *
> + * 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/clk.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/cpumask.h>
> +#include <linux/export.h>
> +#include <linux/of_platform.h>
> +#include <linux/opp.h>
> +#include <linux/slab.h>
> +#include <linux/topology.h>
> +#include <linux/types.h>
> +
> +#include "arm_big_little.h"
> +
> +/* Currently we support only two clusters */
> +#define MAX_CLUSTERS 2

Why? Because that is what the define is or there are other limitations?
Seems like this could and should be dynamically discovered with DT.

Rob

2013-03-26 14:30:58

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

On 26 March 2013 18:47, Rob Herring <[email protected]> wrote:
> On 03/26/2013 04:51 AM, Viresh Kumar wrote:
> I fail to see anything bL specific here. This is just multi-cluster, but
> even for that I don't see anything new other than simply allowing per
> cpu or per cluster opp's. The fact that we have one opp for a cluster is
> simply an implementation decision in CortexA cores.

For now it is a very simple driver and yes probably multi-cluster only. It
will be updated with In Kernel Switcher support as soon as Linaro makes
a decision to opensource that (likely to happen soon).

Yes, there can be implementations, like krait, which can have per cpu
control of opp, but we will wait for any such platform before making this
driver complex.

>> +NOTE: Cpus should boot in the order specified in DT and all cpus for a cluster
>> +must be present contiguously. Generic DT driver will check only node 'x' for
>> +cpu:x.
>
> The OS cannot necessarily control the order. The OS should be able to
> boot on which ever core comes up first.

This should be a FIXME instead (will update it). I am waiting for
final DT bindings
from ARM guys before this can be written on stone.

For now this is the limitation and it should go away as soon as DT bindings are
finalized.

>> +Optional properties:
>> +- clock-latency: Specify the possible maximum transition latency for clock,
>> + in unit of nanoseconds.
>
> Don't we already have "transition-latency" defined?

No, its clock-latency.

$ git grep "clock-latency" Documentation/devicetree/bindings/
Documentation/devicetree/bindings/cpufreq/cpufreq-cpu0.txt:-
clock-latency: Specify the possible maximum transition latency for
clock,
Documentation/devicetree/bindings/cpufreq/cpufreq-spear.txt:-
clock-latency: Specify the possible maximum transition latency for
clock, in

Though transition-latency is mentioned by the example of cpufreq-cpu0.
I will fix it.

> Don't create
> something new. Ideally, this should have had "-ns" appended, but the
> binding is already defined.

I had the same intention.

>> diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
> What is specific to bL? This looks like just a multi-cluster cpufreq
> driver and should be generalized to that. Also, why can't the existing
> cpufreq-cpu0 driver be extended to support this?

With IKS code in, it will be a complete big LITTLE driver.

>> +/* Currently we support only two clusters */
>> +#define MAX_CLUSTERS 2
>
> Why? Because that is what the define is or there are other limitations?
> Seems like this could and should be dynamically discovered with DT.

To keep it simple, as for now as all early systems (atleast which are announced)
have dual cluster systems only.

We will update it once we have some real systems breaking this law.

--
viresh

2013-04-13 20:13:38

by Francesco Lavra

[permalink] [raw]
Subject: Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

On 03/26/2013 10:51 AM, Viresh Kumar wrote:
> big LITTLE is ARM's new Architecture focussing power/performance needs of modern
> world. More information about big LITTLE can be found here:
>
> http://www.arm.com/products/processors/technologies/biglittleprocessing.php
> http://lwn.net/Articles/481055/
>
> In order to keep cpufreq support for all big LITTLE platforms simple/generic,
> this patch tries to add a generic cpufreq driver layer for all big LITTLE
> platforms.
>
> The driver is divided into two parts:
> - Core driver: Generic and shared across all big LITTLE SoC's
> - Glue drivers: Per platform drivers providing ops to the core driver
>
> This patch adds in a generic glue driver which would extract information from
> Device Tree.
>
> Future SoC's can either reuse the DT glue or write their own depending on the
> need.
>
> Signed-off-by: Sudeep KarkadaNagesha <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>
[...]
> diff --git a/drivers/cpufreq/arm_big_little_dt.c b/drivers/cpufreq/arm_big_little_dt.c
> new file mode 100644
> index 0000000..452ff46
> --- /dev/null
> +++ b/drivers/cpufreq/arm_big_little_dt.c
> @@ -0,0 +1,92 @@
> +/*
> + * Generic big.LITTLE CPUFreq Interface driver
> + *
> + * It provides necessary ops to arm_big_little cpufreq driver and gets
> + * Frequency information from Device Tree. Freq table in DT must be in KHz.
> + *
> + * Copyright (C) 2013 Linaro.
> + * Viresh Kumar <[email protected]>
> + *
> + * 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/cpufreq.h>
> +#include <linux/device.h>
> +#include <linux/export.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/opp.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include "arm_big_little.h"
> +
> +static int dt_init_opp_table(struct device *cpu_dev)
> +{
> + struct device_node *np = NULL;
> + int count = 0, ret;
> +
> + for_each_child_of_node(of_find_node_by_path("/cpus"), np) {

If of_find_node_by_path() returns NULL, there will be a NULL pointer
dereference.

> + if (count++ != cpu_dev->id)
> + continue;
> + if (!of_get_property(np, "operating-points", NULL))
> + return -ENODATA;
> +
> + cpu_dev->of_node = np;
> +
> + ret = of_init_opp_table(cpu_dev);
> + if (ret)
> + return ret;
> +
> + return 0;

of_node_put() should be called on np before returning.
Also, the reference count of the parent node should be decremented as well.

These comments apply to the below function dt_get_transition_latency() too.

> + }
> +
> + return -ENODEV;
> +}
> +
> +static int dt_get_transition_latency(struct device *cpu_dev)
> +{
> + struct device_node *np = NULL;
> + u32 transition_latency = CPUFREQ_ETERNAL;
> + int count = 0;
> +
> + for_each_child_of_node(of_find_node_by_path("/cpus"), np) {
> + if (count++ != cpu_dev->id)
> + continue;
> +
> + of_property_read_u32(np, "clock-latency", &transition_latency);
> + return 0;
> + }
> +
> + return -ENODEV;
> +}

--
Francesco

2013-04-15 07:03:03

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V2] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue

On 14 April 2013 01:43, Francesco Lavra <[email protected]> wrote:

>> + for_each_child_of_node(of_find_node_by_path("/cpus"), np) {
>
> If of_find_node_by_path() returns NULL, there will be a NULL pointer
> dereference.
>
>> + if (count++ != cpu_dev->id)
>> + continue;
>> + if (!of_get_property(np, "operating-points", NULL))
>> + return -ENODATA;
>> +
>> + cpu_dev->of_node = np;
>> +
>> + ret = of_init_opp_table(cpu_dev);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>
> of_node_put() should be called on np before returning.
> Also, the reference count of the parent node should be decremented as well.
>
> These comments apply to the below function dt_get_transition_latency() too.

Thanks Francesco.

Below fixes this (I will send it separately to Rafael):

Subject: [PATCH 1/2] cpufreq: ARM big LITTLE: put DT nodes after using them

DT nodes should be put using of_node_put() to balance their usage counts. This
is not done properly in ARM's big LITTLE driver. Fix it.

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/cpufreq/arm_big_little_dt.c | 43 +++++++++++++++++++++++++------------
1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/cpufreq/arm_big_little_dt.c
b/drivers/cpufreq/arm_big_little_dt.c
index 452ff46..44be311 100644
--- a/drivers/cpufreq/arm_big_little_dt.c
+++ b/drivers/cpufreq/arm_big_little_dt.c
@@ -31,22 +31,28 @@

static int dt_init_opp_table(struct device *cpu_dev)
{
- struct device_node *np = NULL;
+ struct device_node *np, *parent;
int count = 0, ret;

- for_each_child_of_node(of_find_node_by_path("/cpus"), np) {
+ parent = of_find_node_by_path("/cpus");
+ if (!parent) {
+ pr_err("failed to find OF /cpus\n");
+ return -ENOENT;
+ }
+
+ for_each_child_of_node(parent, np) {
if (count++ != cpu_dev->id)
continue;
- if (!of_get_property(np, "operating-points", NULL))
- return -ENODATA;
-
- cpu_dev->of_node = np;
-
- ret = of_init_opp_table(cpu_dev);
- if (ret)
- return ret;
-
- return 0;
+ if (!of_get_property(np, "operating-points", NULL)) {
+ ret = -ENODATA;
+ } else {
+ cpu_dev->of_node = np;
+ ret = of_init_opp_table(cpu_dev);
+ }
+ of_node_put(np);
+ of_node_put(parent);
+
+ return ret;
}

return -ENODEV;
@@ -54,15 +60,24 @@ static int dt_init_opp_table(struct device *cpu_dev)

static int dt_get_transition_latency(struct device *cpu_dev)
{
- struct device_node *np = NULL;
+ struct device_node *np, *parent;
u32 transition_latency = CPUFREQ_ETERNAL;
int count = 0;

- for_each_child_of_node(of_find_node_by_path("/cpus"), np) {
+ parent = of_find_node_by_path("/cpus");
+ if (!parent) {
+ pr_err("failed to find OF /cpus\n");
+ return -ENOENT;
+ }
+
+ for_each_child_of_node(parent, np) {
if (count++ != cpu_dev->id)
continue;

of_property_read_u32(np, "clock-latency", &transition_latency);
+ of_node_put(np);
+ of_node_put(parent);
+
return 0;
}