[v8]
* Address comments to update code to take cpufreq_hw phandle and index from
the CPU nodes.
* Updated the Documentation for the above change in DT.
* Updated logic for assigning 'qcom_freq_domain_map' for related CPUs.
* Clock input to the HW block is taken from DT which has been updated in
code and Device tree documentation.
[v7]
* Updated the logic to check for related CPUs.
[v6]
* Renamed match table 'qcom_cpufreq_hw_match'.
* Renamed 'qcom_read_lut' to 'qcom_cpufreq_hw_read_lut'.
* Updated the logic to check for related CPUs at the beginning of the
'qcom_cpu_resources_init'.
* Use devm_ioremap_resource instead of devm_ioremap.
* Update the use of of_node_put to handle error conditions.
* Use policy->cached_resolved_idx in fast switch callback.
* Keep precalculated offsets 'reg_bases'.
* XO clock is taken from Device tree.
* Update documentation binding for clocks/clock-names.
* Minor comments in Kconfig.arm.
* Comments to move dev_info to dev_dbg.
[v5]
* Remove mapping different register regions of perf/lut/enable,
instead map the entire HW region.
* Add reg_offset/cpufreq_qcom_std_offsets to be supplied as device data.
* Check of src == 0 during lut read.
* Add of_node_put(cpu_np) in qcom_get_related_cpus
* Update the qcom_cpu_resources_init for register offset data,
and cleanup the related cpus to keep a single copy of CPUfreq.
* Replace FW with HW, update Kconfig, rename filename qcom-cpufreq-hw.c
* Update the documentation binding to reflect the changes of mapping the
* entire HW region.
[v4]
* Fixed console messages as per comments.
* Return error from qcom_resources_init()
in the cases where failed to get frequency domain.
* Rename cpu_dev to cpu_np in qcom_resources_init,
qcom_get_related_cpus(). Also use temp variable freq_np in
qcom_get_related_cpus().
* Update qcom_cpufreq_fw_get() to use the policy data to incoporate
the hotplug use case.
* Update code to use of fast_switching.
* Check for !c->max_cores instead of cpumask_empty in
qcom_get_related_cpus().
* Update the logic of assigning 'c' to qcom_freq_domain_map[cpu].
[v3]
* Remove index check from 'qcom_cpufreq_fw_target_index'.
* Update the Documentation binding to add the platform specific properties in
the CPU nodes, node name "qcom,freq-domain".
* Update return value to '0' from -ENODEV from 'qcom_cpufreq_fw_get'.
* Update the logic for boost frequency to use local variables instead of
cpufreq driver data in 'qcom_read_lut'.
* Update the logic in 'qcom_get_related_cpus' to find the related cpus.
* Update the reg-names to remove "_base" and also update the binding with the
description of these registers.
* Update the logic in 'qcom_resources_init' to address the new device tree
notation of handling the frequency domain phandles.
[v2]
* Fixed the alignment issues in "qcom_cpufreq_fw_target_index" for dev_err and
also for "qcom_cpu_resources_init".
* Removed ret = 0 from qcom_get_related_cpus and added to check for
cpu_mask_empty to return -ENOENT.
* Fixes qcom_cpu_resources_init function
* Remove initialization of 'index'
* Check for valid 'c'
* Removed initialization of 'prev_cc' from 'qcom_read_lut'.
Taniya Das (2):
dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings
cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver
.../bindings/cpufreq/cpufreq-qcom-hw.txt | 169 ++++++++++
drivers/cpufreq/Kconfig.arm | 11 +
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/qcom-cpufreq-hw.c | 354 +++++++++++++++++++++
4 files changed, 535 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.
Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's
SoCs. This is required for managing the cpu frequency transitions which are
controlled by the hardware engine.
Signed-off-by: Taniya Das <[email protected]>
---
.../bindings/cpufreq/cpufreq-qcom-hw.txt | 169 +++++++++++++++++++++
1 file changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
new file mode 100644
index 0000000..c06941c
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
@@ -0,0 +1,169 @@
+Qualcomm Technologies, Inc. CPUFREQ Bindings
+
+CPUFREQ HW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
+SoCs to manage frequency in hardware. It is capable of controlling frequency
+for multiple clusters.
+
+Properties:
+- compatible
+ Usage: required
+ Value type: <string>
+ Definition: must be "qcom,cpufreq-hw".
+
+- clocks
+ Usage: required
+ Value type: <phandle> From common clock binding.
+ Definition: clock handle for XO clock and GPLL0 clock.
+
+- clock-names
+ Usage: required
+ Value type: <string> From common clock binding.
+ Definition: must be "xo", "cpu_clk".
+
+- reg
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Addresses and sizes for the memory of the HW bases in
+ each frequency domain.
+- reg-names
+ Usage: Optional
+ Value type: <string>
+ Definition: Frequency domain name i.e.
+ "freq-domain0", "freq-domain1".
+
+* Property qcom,freq-domain
+Devices supporting freq-domain must set their "qcom,freq-domain" property with
+phandle to a cpufreq_hw followed by the Domain ID(0/1) in the CPU DT node.
+
+
+Example:
+
+Example 1: Dual-cluster, Quad-core per cluster. CPUs within a cluster switch
+DCVS state together.
+
+/ {
+ cpus {
+ #address-cells = <2>;
+ #size-cells = <0>;
+
+ CPU0: cpu@0 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x0>;
+ enable-method = "psci";
+ next-level-cache = <&L2_0>;
+ qcom,freq-domain = <&cpufreq_hw 0>;
+ L2_0: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ L3_0: l3-cache {
+ compatible = "cache";
+ };
+ };
+ };
+
+ CPU1: cpu@100 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x100>;
+ enable-method = "psci";
+ next-level-cache = <&L2_100>;
+ qcom,freq-domain = <&cpufreq_hw 0>;
+ L2_100: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU2: cpu@200 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x200>;
+ enable-method = "psci";
+ next-level-cache = <&L2_200>;
+ qcom,freq-domain = <&cpufreq_hw 0>;
+ L2_200: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU3: cpu@300 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x300>;
+ enable-method = "psci";
+ next-level-cache = <&L2_300>;
+ qcom,freq-domain = <&cpufreq_hw 0>;
+ L2_300: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU4: cpu@400 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x400>;
+ enable-method = "psci";
+ next-level-cache = <&L2_400>;
+ qcom,freq-domain = <&cpufreq_hw 1>;
+ L2_400: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU5: cpu@500 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x500>;
+ enable-method = "psci";
+ next-level-cache = <&L2_500>;
+ qcom,freq-domain = <&cpufreq_hw 1>;
+ L2_500: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU6: cpu@600 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x600>;
+ enable-method = "psci";
+ next-level-cache = <&L2_600>;
+ qcom,freq-domain = <&cpufreq_hw 1>;
+ L2_600: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+
+ CPU7: cpu@700 {
+ device_type = "cpu";
+ compatible = "qcom,kryo385";
+ reg = <0x0 0x700>;
+ enable-method = "psci";
+ next-level-cache = <&L2_700>;
+ qcom,freq-domain = <&cpufreq_hw 1>;
+ L2_700: l2-cache {
+ compatible = "cache";
+ next-level-cache = <&L3_0>;
+ };
+ };
+ };
+
+ soc {
+ cpufreq_hw: qcom,cpufreq-hw {
+ compatible = "qcom,cpufreq-hw";
+ reg = <0x17d43000 0x1400>, <0x17d45800 0x1400>;
+ reg-names = "freq-domain0", "freq-domain1";
+
+ clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
+ clock-names = "xo", "cpu_clk";
+
+ #freq-domains-cells = <1>
+
+ };
+}
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.
The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
for changing the frequency of CPUs. The driver implements the cpufreq
driver interface for this hardware engine.
Signed-off-by: Saravana Kannan <[email protected]>
Signed-off-by: Taniya Das <[email protected]>
---
drivers/cpufreq/Kconfig.arm | 11 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/qcom-cpufreq-hw.c | 354 ++++++++++++++++++++++++++++++++++++++
3 files changed, 366 insertions(+)
create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 0cd8eb7..93a9d72 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ
This add the CPUFreq driver support for Intel PXA2xx SOCs.
If in doubt, say N.
+
+config ARM_QCOM_CPUFREQ_HW
+ bool "QCOM CPUFreq HW driver"
+ depends on ARCH_QCOM
+ help
+ Support for the CPUFreq HW driver.
+ Some QCOM chipsets have a HW engine to offload the steps
+ necessary for changing the frequency of the CPUs. Firmware loaded
+ in this engine exposes a programming interface to the OS.
+ The driver implements the cpufreq interface for this HW engine.
+ Say Y if you want to support CPUFreq HW.
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index c1ffeab..ca48a1d 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o
obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
##################################################################################
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
new file mode 100644
index 0000000..43107ab
--- /dev/null
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -0,0 +1,354 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/cpufreq.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+
+#define LUT_MAX_ENTRIES 40U
+#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16)
+#define LUT_ROW_SIZE 32
+#define CLK_HW_DIV 2
+
+enum {
+ REG_ENABLE,
+ REG_LUT_TABLE,
+ REG_PERF_STATE,
+
+ REG_ARRAY_SIZE,
+};
+
+struct cpufreq_qcom {
+ struct cpufreq_frequency_table *table;
+ void __iomem *reg_bases[REG_ARRAY_SIZE];
+ cpumask_t related_cpus;
+ unsigned int max_cores;
+ unsigned long xo_rate;
+ unsigned long cpu_hw_rate;
+};
+
+static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
+ [REG_ENABLE] = 0x0,
+ [REG_LUT_TABLE] = 0x110,
+ [REG_PERF_STATE] = 0x920,
+};
+
+static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
+
+static int
+qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
+ unsigned int index)
+{
+ struct cpufreq_qcom *c = policy->driver_data;
+
+ writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
+
+ return 0;
+}
+
+static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
+{
+ struct cpufreq_qcom *c;
+ struct cpufreq_policy *policy;
+ unsigned int index;
+
+ policy = cpufreq_cpu_get_raw(cpu);
+ if (!policy)
+ return 0;
+
+ c = policy->driver_data;
+
+ index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
+ index = min(index, LUT_MAX_ENTRIES - 1);
+
+ return policy->freq_table[index].frequency;
+}
+
+static unsigned int
+qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
+ unsigned int target_freq)
+{
+ struct cpufreq_qcom *c = policy->driver_data;
+ int index;
+
+ index = policy->cached_resolved_idx;
+ if (index < 0)
+ return 0;
+
+ writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
+
+ return policy->freq_table[index].frequency;
+}
+
+static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
+{
+ struct cpufreq_qcom *c;
+
+ c = qcom_freq_domain_map[policy->cpu];
+ if (!c) {
+ pr_err("No scaling support for CPU%d\n", policy->cpu);
+ return -ENODEV;
+ }
+
+ cpumask_copy(policy->cpus, &c->related_cpus);
+
+ policy->fast_switch_possible = true;
+ policy->freq_table = c->table;
+ policy->driver_data = c;
+
+ return 0;
+}
+
+static struct freq_attr *qcom_cpufreq_hw_attr[] = {
+ &cpufreq_freq_attr_scaling_available_freqs,
+ &cpufreq_freq_attr_scaling_boost_freqs,
+ NULL
+};
+
+static struct cpufreq_driver cpufreq_qcom_hw_driver = {
+ .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+ CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+ .verify = cpufreq_generic_frequency_table_verify,
+ .target_index = qcom_cpufreq_hw_target_index,
+ .get = qcom_cpufreq_hw_get,
+ .init = qcom_cpufreq_hw_cpu_init,
+ .fast_switch = qcom_cpufreq_hw_fast_switch,
+ .name = "qcom-cpufreq-hw",
+ .attr = qcom_cpufreq_hw_attr,
+ .boost_enabled = true,
+};
+
+static int qcom_cpufreq_hw_read_lut(struct platform_device *pdev,
+ struct cpufreq_qcom *c)
+{
+ struct device *dev = &pdev->dev;
+ void __iomem *base;
+ u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
+
+ c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
+ sizeof(*c->table), GFP_KERNEL);
+ if (!c->table)
+ return -ENOMEM;
+
+ base = c->reg_bases[REG_LUT_TABLE];
+
+ for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+ data = readl_relaxed(base + i * LUT_ROW_SIZE);
+ src = (data & GENMASK(31, 30)) >> 30;
+ lval = data & GENMASK(7, 0);
+ core_count = CORE_COUNT_VAL(data);
+
+ if (src)
+ c->table[i].frequency = c->xo_rate * lval / 1000;
+ else
+ c->table[i].frequency = c->cpu_hw_rate / 1000;
+
+ cur_freq = c->table[i].frequency;
+
+ dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
+ i, c->table[i].frequency, core_count);
+
+ if (core_count != c->max_cores)
+ cur_freq = CPUFREQ_ENTRY_INVALID;
+
+ /*
+ * Two of the same frequencies with the same core counts means
+ * end of table.
+ */
+ if (i > 0 && c->table[i - 1].frequency ==
+ c->table[i].frequency && prev_cc == core_count) {
+ struct cpufreq_frequency_table *prev = &c->table[i - 1];
+
+ if (prev_freq == CPUFREQ_ENTRY_INVALID)
+ prev->flags = CPUFREQ_BOOST_FREQ;
+ break;
+ }
+ prev_cc = core_count;
+ prev_freq = cur_freq;
+ }
+
+ c->table[i].frequency = CPUFREQ_TABLE_END;
+
+ return 0;
+}
+
+static int qcom_get_related_cpus(int index, struct cpumask *m)
+{
+ struct device_node *cpu_np;
+ struct of_phandle_args args;
+ int cpu, ret;
+
+ for_each_possible_cpu(cpu) {
+ cpu_np = of_cpu_device_node_get(cpu);
+ if (!cpu_np)
+ continue;
+ of_node_put(cpu_np);
+
+ ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
+ "#freq-domain-cells", 0, &args);
+ if (ret < 0)
+ continue;
+
+ if (index == args.args[0])
+ cpumask_set_cpu(cpu, m);
+ }
+
+ return 0;
+}
+
+static int qcom_cpu_resources_init(struct platform_device *pdev,
+ unsigned int cpu, int index,
+ unsigned long xo_rate,
+ unsigned long cpu_hw_rate)
+{
+ struct cpufreq_qcom *c;
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+ const u16 *offsets;
+ int ret, i, cpu_r;
+ void __iomem *base;
+
+ if (qcom_freq_domain_map[cpu])
+ return 0;
+
+ c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return -ENOMEM;
+
+ offsets = of_device_get_match_data(&pdev->dev);
+ if (!offsets)
+ return -EINVAL;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, index);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++)
+ c->reg_bases[i] = base + offsets[i];
+
+ /* HW should be in enabled state to proceed */
+ if (!(readl_relaxed(c->reg_bases[REG_ENABLE]) & 0x1)) {
+ dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
+ return -ENODEV;
+ }
+
+ ret = qcom_get_related_cpus(index, &c->related_cpus);
+ if (ret) {
+ dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
+ return ret;
+ }
+
+ c->max_cores = cpumask_weight(&c->related_cpus);
+ if (!c->max_cores)
+ return -ENOENT;
+
+ c->xo_rate = xo_rate;
+ c->cpu_hw_rate = cpu_hw_rate;
+
+ ret = qcom_cpufreq_hw_read_lut(pdev, c);
+ if (ret) {
+ dev_err(dev, "Domain-%d failed to read LUT\n", index);
+ return ret;
+ }
+
+ for_each_cpu(cpu_r, &c->related_cpus)
+ qcom_freq_domain_map[cpu_r] = c;
+
+ return 0;
+}
+
+static int qcom_resources_init(struct platform_device *pdev)
+{
+ struct device_node *cpu_np;
+ struct of_phandle_args args;
+ struct clk *clk;
+ unsigned int cpu;
+ unsigned long xo_rate, cpu_hw_rate;
+ int ret;
+
+ clk = devm_clk_get(&pdev->dev, "xo");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ xo_rate = clk_get_rate(clk);
+
+ devm_clk_put(&pdev->dev, clk);
+
+ clk = devm_clk_get(&pdev->dev, "cpu_clk");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
+
+ devm_clk_put(&pdev->dev, clk);
+
+ for_each_possible_cpu(cpu) {
+ cpu_np = of_cpu_device_node_get(cpu);
+ if (!cpu_np) {
+ dev_dbg(&pdev->dev, "Failed to get cpu %d device\n",
+ cpu);
+ continue;
+ }
+
+ ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
+ "#freq-domain-cells", 0, &args);
+ if (ret < 0)
+ return ret;
+
+ ret = qcom_cpu_resources_init(pdev, cpu, args.args[0],
+ xo_rate, cpu_hw_rate);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
+{
+ int rc;
+
+ /* Get the bases of cpufreq for domains */
+ rc = qcom_resources_init(pdev);
+ if (rc) {
+ dev_err(&pdev->dev, "CPUFreq resource init failed\n");
+ return rc;
+ }
+
+ rc = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
+ if (rc) {
+ dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
+ return rc;
+ }
+
+ dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n");
+
+ return 0;
+}
+
+static const struct of_device_id qcom_cpufreq_hw_match[] = {
+ { .compatible = "qcom,cpufreq-hw", .data = &cpufreq_qcom_std_offsets },
+ {}
+};
+
+static struct platform_driver qcom_cpufreq_hw_driver = {
+ .probe = qcom_cpufreq_hw_driver_probe,
+ .driver = {
+ .name = "qcom-cpufreq-hw",
+ .of_match_table = qcom_cpufreq_hw_match,
+ },
+};
+
+static int __init qcom_cpufreq_hw_init(void)
+{
+ return platform_driver_register(&qcom_cpufreq_hw_driver);
+}
+subsys_initcall(qcom_cpufreq_hw_init);
+
+MODULE_DESCRIPTION("QCOM firmware-based CPU Frequency driver");
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.
On Sun, Sep 23, 2018 at 04:03:12PM +0530, Taniya Das wrote:
> Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's
> SoCs. This is required for managing the cpu frequency transitions which are
> controlled by the hardware engine.
>
> Signed-off-by: Taniya Das <[email protected]>
> ---
> .../bindings/cpufreq/cpufreq-qcom-hw.txt | 169 +++++++++++++++++++++
> 1 file changed, 169 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
> new file mode 100644
> index 0000000..c06941c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
> @@ -0,0 +1,169 @@
> +Qualcomm Technologies, Inc. CPUFREQ Bindings
> +
> +CPUFREQ HW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
> +SoCs to manage frequency in hardware. It is capable of controlling frequency
> +for multiple clusters.
> +
> +Properties:
> +- compatible
> + Usage: required
> + Value type: <string>
> + Definition: must be "qcom,cpufreq-hw".
> +
> +- clocks
> + Usage: required
> + Value type: <phandle> From common clock binding.
> + Definition: clock handle for XO clock and GPLL0 clock.
> +
> +- clock-names
> + Usage: required
> + Value type: <string> From common clock binding.
> + Definition: must be "xo", "cpu_clk".
> +
> +- reg
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: Addresses and sizes for the memory of the HW bases in
> + each frequency domain.
> +- reg-names
> + Usage: Optional
> + Value type: <string>
> + Definition: Frequency domain name i.e.
> + "freq-domain0", "freq-domain1".
> +
> +* Property qcom,freq-domain
> +Devices supporting freq-domain must set their "qcom,freq-domain" property with
> +phandle to a cpufreq_hw followed by the Domain ID(0/1) in the CPU DT node.
> +
> +
> +Example:
> +
> +Example 1: Dual-cluster, Quad-core per cluster. CPUs within a cluster switch
> +DCVS state together.
> +
> +/ {
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@0 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + next-level-cache = <&L2_0>;
> + qcom,freq-domain = <&cpufreq_hw 0>;
> + L2_0: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + L3_0: l3-cache {
> + compatible = "cache";
> + };
> + };
> + };
> +
> + CPU1: cpu@100 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x100>;
> + enable-method = "psci";
> + next-level-cache = <&L2_100>;
> + qcom,freq-domain = <&cpufreq_hw 0>;
> + L2_100: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU2: cpu@200 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x200>;
> + enable-method = "psci";
> + next-level-cache = <&L2_200>;
> + qcom,freq-domain = <&cpufreq_hw 0>;
> + L2_200: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU3: cpu@300 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x300>;
> + enable-method = "psci";
> + next-level-cache = <&L2_300>;
> + qcom,freq-domain = <&cpufreq_hw 0>;
> + L2_300: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU4: cpu@400 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x400>;
> + enable-method = "psci";
> + next-level-cache = <&L2_400>;
> + qcom,freq-domain = <&cpufreq_hw 1>;
> + L2_400: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU5: cpu@500 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x500>;
> + enable-method = "psci";
> + next-level-cache = <&L2_500>;
> + qcom,freq-domain = <&cpufreq_hw 1>;
> + L2_500: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU6: cpu@600 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x600>;
> + enable-method = "psci";
> + next-level-cache = <&L2_600>;
> + qcom,freq-domain = <&cpufreq_hw 1>;
> + L2_600: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU7: cpu@700 {
> + device_type = "cpu";
> + compatible = "qcom,kryo385";
> + reg = <0x0 0x700>;
> + enable-method = "psci";
> + next-level-cache = <&L2_700>;
> + qcom,freq-domain = <&cpufreq_hw 1>;
> + L2_700: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> + };
> +
> + soc {
> + cpufreq_hw: qcom,cpufreq-hw {
Should have a unit-address and node names should be generic. We don't
really have one for this, but let's go with 'cpufreq', so:
cpufreq@17d43000
> + compatible = "qcom,cpufreq-hw";
> + reg = <0x17d43000 0x1400>, <0x17d45800 0x1400>;
> + reg-names = "freq-domain0", "freq-domain1";
> +
> + clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
> + clock-names = "xo", "cpu_clk";
> +
> + #freq-domains-cells = <1>
Not documented, but I don't think you need this if it is always 1 and
because this is not a common binding.
Rob
On Sun, Sep 23, 2018 at 04:03:13PM +0530, Taniya Das wrote:
> The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
> for changing the frequency of CPUs. The driver implements the cpufreq
> driver interface for this hardware engine.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> Signed-off-by: Taniya Das <[email protected]>
> ---
> drivers/cpufreq/Kconfig.arm | 11 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/qcom-cpufreq-hw.c | 354 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 366 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 0cd8eb7..93a9d72 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ
> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>
> If in doubt, say N.
> +
> +config ARM_QCOM_CPUFREQ_HW
> + bool "QCOM CPUFreq HW driver"
> + depends on ARCH_QCOM
Hello Taniya,
It might be a good idea to do depends on ARCH_QCOM || COMPILE_TEST
to get more build testing.
> + help
> + Support for the CPUFreq HW driver.
> + Some QCOM chipsets have a HW engine to offload the steps
> + necessary for changing the frequency of the CPUs. Firmware loaded
> + in this engine exposes a programming interface to the OS.
> + The driver implements the cpufreq interface for this HW engine.
> + Say Y if you want to support CPUFreq HW.
All other help text has a tab and two spaces, you only have one.
Also, why not add this Kconfig right after the existing ARCH_QCOM_CPUFREQ
driver?
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index c1ffeab..ca48a1d 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
> obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o
> obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
> obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
> +obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
Why not add this right after the existing ARCH_QCOM_CPUFREQ driver?
>
>
> ##################################################################################
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> new file mode 100644
> index 0000000..43107ab
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -0,0 +1,354 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/cpufreq.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
Since this can't be built as a module, drop module.h, init.h should suffice.
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +
> +#define LUT_MAX_ENTRIES 40U
> +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16)
> +#define LUT_ROW_SIZE 32
> +#define CLK_HW_DIV 2
> +
> +enum {
> + REG_ENABLE,
> + REG_LUT_TABLE,
> + REG_PERF_STATE,
> +
> + REG_ARRAY_SIZE,
> +};
> +
> +struct cpufreq_qcom {
> + struct cpufreq_frequency_table *table;
> + void __iomem *reg_bases[REG_ARRAY_SIZE];
> + cpumask_t related_cpus;
> + unsigned int max_cores;
> + unsigned long xo_rate;
> + unsigned long cpu_hw_rate;
> +};
> +
> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
> + [REG_ENABLE] = 0x0,
> + [REG_LUT_TABLE] = 0x110,
> + [REG_PERF_STATE] = 0x920,
> +};
> +
> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> +
> +static int
> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
> + unsigned int index)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> +
> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> +
> + return 0;
> +}
> +
> +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
> +{
> + struct cpufreq_qcom *c;
> + struct cpufreq_policy *policy;
> + unsigned int index;
> +
> + policy = cpufreq_cpu_get_raw(cpu);
> + if (!policy)
> + return 0;
> +
> + c = policy->driver_data;
> +
> + index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
> + index = min(index, LUT_MAX_ENTRIES - 1);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static unsigned int
> +qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
> + unsigned int target_freq)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> + int index;
> +
> + index = policy->cached_resolved_idx;
> + if (index < 0)
> + return 0;
> +
> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_qcom *c;
> +
> + c = qcom_freq_domain_map[policy->cpu];
> + if (!c) {
> + pr_err("No scaling support for CPU%d\n", policy->cpu);
> + return -ENODEV;
> + }
> +
> + cpumask_copy(policy->cpus, &c->related_cpus);
> +
> + policy->fast_switch_possible = true;
> + policy->freq_table = c->table;
> + policy->driver_data = c;
> +
> + return 0;
> +}
> +
> +static struct freq_attr *qcom_cpufreq_hw_attr[] = {
> + &cpufreq_freq_attr_scaling_available_freqs,
> + &cpufreq_freq_attr_scaling_boost_freqs,
> + NULL
> +};
> +
> +static struct cpufreq_driver cpufreq_qcom_hw_driver = {
> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target_index = qcom_cpufreq_hw_target_index,
> + .get = qcom_cpufreq_hw_get,
> + .init = qcom_cpufreq_hw_cpu_init,
> + .fast_switch = qcom_cpufreq_hw_fast_switch,
> + .name = "qcom-cpufreq-hw",
> + .attr = qcom_cpufreq_hw_attr,
> + .boost_enabled = true,
> +};
> +
> +static int qcom_cpufreq_hw_read_lut(struct platform_device *pdev,
> + struct cpufreq_qcom *c)
> +{
> + struct device *dev = &pdev->dev;
> + void __iomem *base;
> + u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
> +
> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
> + sizeof(*c->table), GFP_KERNEL);
> + if (!c->table)
> + return -ENOMEM;
> +
> + base = c->reg_bases[REG_LUT_TABLE];
> +
> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> + data = readl_relaxed(base + i * LUT_ROW_SIZE);
> + src = (data & GENMASK(31, 30)) >> 30;
> + lval = data & GENMASK(7, 0);
> + core_count = CORE_COUNT_VAL(data);
> +
> + if (src)
> + c->table[i].frequency = c->xo_rate * lval / 1000;
> + else
> + c->table[i].frequency = c->cpu_hw_rate / 1000;
> +
> + cur_freq = c->table[i].frequency;
> +
> + dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
> + i, c->table[i].frequency, core_count);
> +
> + if (core_count != c->max_cores)
> + cur_freq = CPUFREQ_ENTRY_INVALID;
> +
> + /*
> + * Two of the same frequencies with the same core counts means
> + * end of table.
> + */
> + if (i > 0 && c->table[i - 1].frequency ==
> + c->table[i].frequency && prev_cc == core_count) {
> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
> +
> + if (prev_freq == CPUFREQ_ENTRY_INVALID)
> + prev->flags = CPUFREQ_BOOST_FREQ;
> + break;
> + }
> + prev_cc = core_count;
> + prev_freq = cur_freq;
> + }
> +
> + c->table[i].frequency = CPUFREQ_TABLE_END;
> +
> + return 0;
> +}
> +
> +static int qcom_get_related_cpus(int index, struct cpumask *m)
> +{
> + struct device_node *cpu_np;
> + struct of_phandle_args args;
> + int cpu, ret;
> +
> + for_each_possible_cpu(cpu) {
> + cpu_np = of_cpu_device_node_get(cpu);
> + if (!cpu_np)
> + continue;
> + of_node_put(cpu_np);
Shouldn't of_node_put() be called after you have used the pointer?
> +
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> + "#freq-domain-cells", 0, &args);
> + if (ret < 0)
> + continue;
> +
> + if (index == args.args[0])
> + cpumask_set_cpu(cpu, m);
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_cpu_resources_init(struct platform_device *pdev,
> + unsigned int cpu, int index,
> + unsigned long xo_rate,
> + unsigned long cpu_hw_rate)
> +{
> + struct cpufreq_qcom *c;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + const u16 *offsets;
> + int ret, i, cpu_r;
> + void __iomem *base;
> +
> + if (qcom_freq_domain_map[cpu])
> + return 0;
> +
> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return -ENOMEM;
> +
> + offsets = of_device_get_match_data(&pdev->dev);
> + if (!offsets)
> + return -EINVAL;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++)
> + c->reg_bases[i] = base + offsets[i];
> +
> + /* HW should be in enabled state to proceed */
> + if (!(readl_relaxed(c->reg_bases[REG_ENABLE]) & 0x1)) {
> + dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
> + return -ENODEV;
> + }
> +
> + ret = qcom_get_related_cpus(index, &c->related_cpus);
> + if (ret) {
> + dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
> + return ret;
> + }
> +
> + c->max_cores = cpumask_weight(&c->related_cpus);
> + if (!c->max_cores)
> + return -ENOENT;
> +
> + c->xo_rate = xo_rate;
> + c->cpu_hw_rate = cpu_hw_rate;
> +
> + ret = qcom_cpufreq_hw_read_lut(pdev, c);
> + if (ret) {
> + dev_err(dev, "Domain-%d failed to read LUT\n", index);
> + return ret;
> + }
> +
> + for_each_cpu(cpu_r, &c->related_cpus)
> + qcom_freq_domain_map[cpu_r] = c;
> +
> + return 0;
> +}
> +
> +static int qcom_resources_init(struct platform_device *pdev)
> +{
> + struct device_node *cpu_np;
> + struct of_phandle_args args;
> + struct clk *clk;
> + unsigned int cpu;
> + unsigned long xo_rate, cpu_hw_rate;
> + int ret;
> +
> + clk = devm_clk_get(&pdev->dev, "xo");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + xo_rate = clk_get_rate(clk);
> +
> + devm_clk_put(&pdev->dev, clk);
> +
> + clk = devm_clk_get(&pdev->dev, "cpu_clk");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> +
> + devm_clk_put(&pdev->dev, clk);
> +
> + for_each_possible_cpu(cpu) {
> + cpu_np = of_cpu_device_node_get(cpu);
Where is the matching of_node_put?
> + if (!cpu_np) {
> + dev_dbg(&pdev->dev, "Failed to get cpu %d device\n",
> + cpu);
> + continue;
> + }
> +
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> + "#freq-domain-cells", 0, &args);
> + if (ret < 0)
> + return ret;
> +
> + ret = qcom_cpu_resources_init(pdev, cpu, args.args[0],
> + xo_rate, cpu_hw_rate);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
> +{
> + int rc;
> +
> + /* Get the bases of cpufreq for domains */
> + rc = qcom_resources_init(pdev);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq resource init failed\n");
> + return rc;
> + }
> +
> + rc = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
> + return rc;
> + }
> +
> + dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qcom_cpufreq_hw_match[] = {
> + { .compatible = "qcom,cpufreq-hw", .data = &cpufreq_qcom_std_offsets },
> + {}
> +};
> +
> +static struct platform_driver qcom_cpufreq_hw_driver = {
> + .probe = qcom_cpufreq_hw_driver_probe,
> + .driver = {
> + .name = "qcom-cpufreq-hw",
> + .of_match_table = qcom_cpufreq_hw_match,
> + },
> +};
> +
> +static int __init qcom_cpufreq_hw_init(void)
> +{
> + return platform_driver_register(&qcom_cpufreq_hw_driver);
> +}
> +subsys_initcall(qcom_cpufreq_hw_init);
> +
> +MODULE_DESCRIPTION("QCOM firmware-based CPU Frequency driver");
Since this can't be built as a module, you should drop MODULE_DESCRIPTION.
Also, checkpatch --strict gives the following warnings:
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#103:
new file mode 100644
cpufreq drivers usually have an entry in MAINTAINERS file.
WARNING: Duplicate signature
#370:
Signed-off-by: Taniya Das <[email protected]>
If you are more than one author, you can use the Co-Developed-by tag.
CHECK: Alignment should match open parenthesis
#611: FILE: drivers/cpufreq/qcom-cpufreq-hw.c:193:
+ ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
+ "#freq-domain-cells", 0, &args);
CHECK: Alignment should match open parenthesis
#718: FILE: drivers/cpufreq/qcom-cpufreq-hw.c:300:
+ ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
+ "#freq-domain-cells", 0, &args);
Kind regards,
Niklas
On 23-09-18, 16:03, Taniya Das wrote:
> The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
> for changing the frequency of CPUs. The driver implements the cpufreq
> driver interface for this hardware engine.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> Signed-off-by: Taniya Das <[email protected]>
> ---
> drivers/cpufreq/Kconfig.arm | 11 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/qcom-cpufreq-hw.c | 354 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 366 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 0cd8eb7..93a9d72 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ
> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>
> If in doubt, say N.
> +
> +config ARM_QCOM_CPUFREQ_HW
> + bool "QCOM CPUFreq HW driver"
> + depends on ARCH_QCOM
> + help
> + Support for the CPUFreq HW driver.
> + Some QCOM chipsets have a HW engine to offload the steps
> + necessary for changing the frequency of the CPUs. Firmware loaded
> + in this engine exposes a programming interface to the OS.
> + The driver implements the cpufreq interface for this HW engine.
> + Say Y if you want to support CPUFreq HW.
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index c1ffeab..ca48a1d 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
> obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o
> obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
> obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
> +obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
>
>
> ##################################################################################
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> new file mode 100644
> index 0000000..43107ab
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -0,0 +1,354 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/cpufreq.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +
> +#define LUT_MAX_ENTRIES 40U
> +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16)
> +#define LUT_ROW_SIZE 32
> +#define CLK_HW_DIV 2
> +
> +enum {
> + REG_ENABLE,
> + REG_LUT_TABLE,
> + REG_PERF_STATE,
> +
> + REG_ARRAY_SIZE,
> +};
> +
> +struct cpufreq_qcom {
> + struct cpufreq_frequency_table *table;
> + void __iomem *reg_bases[REG_ARRAY_SIZE];
> + cpumask_t related_cpus;
> + unsigned int max_cores;
> + unsigned long xo_rate;
> + unsigned long cpu_hw_rate;
> +};
> +
> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
> + [REG_ENABLE] = 0x0,
> + [REG_LUT_TABLE] = 0x110,
> + [REG_PERF_STATE] = 0x920,
> +};
> +
> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> +
> +static int
> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
> + unsigned int index)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> +
> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> +
> + return 0;
> +}
> +
> +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
> +{
> + struct cpufreq_qcom *c;
> + struct cpufreq_policy *policy;
> + unsigned int index;
> +
> + policy = cpufreq_cpu_get_raw(cpu);
> + if (!policy)
> + return 0;
> +
> + c = policy->driver_data;
> +
> + index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
> + index = min(index, LUT_MAX_ENTRIES - 1);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static unsigned int
> +qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
> + unsigned int target_freq)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> + int index;
> +
> + index = policy->cached_resolved_idx;
> + if (index < 0)
> + return 0;
> +
> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_qcom *c;
> +
> + c = qcom_freq_domain_map[policy->cpu];
> + if (!c) {
> + pr_err("No scaling support for CPU%d\n", policy->cpu);
> + return -ENODEV;
> + }
> +
> + cpumask_copy(policy->cpus, &c->related_cpus);
> +
> + policy->fast_switch_possible = true;
> + policy->freq_table = c->table;
> + policy->driver_data = c;
> +
> + return 0;
> +}
> +
> +static struct freq_attr *qcom_cpufreq_hw_attr[] = {
> + &cpufreq_freq_attr_scaling_available_freqs,
> + &cpufreq_freq_attr_scaling_boost_freqs,
> + NULL
> +};
> +
> +static struct cpufreq_driver cpufreq_qcom_hw_driver = {
> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target_index = qcom_cpufreq_hw_target_index,
> + .get = qcom_cpufreq_hw_get,
> + .init = qcom_cpufreq_hw_cpu_init,
> + .fast_switch = qcom_cpufreq_hw_fast_switch,
> + .name = "qcom-cpufreq-hw",
> + .attr = qcom_cpufreq_hw_attr,
> + .boost_enabled = true,
> +};
> +
> +static int qcom_cpufreq_hw_read_lut(struct platform_device *pdev,
> + struct cpufreq_qcom *c)
> +{
> + struct device *dev = &pdev->dev;
> + void __iomem *base;
> + u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
> +
> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
> + sizeof(*c->table), GFP_KERNEL);
> + if (!c->table)
> + return -ENOMEM;
> +
> + base = c->reg_bases[REG_LUT_TABLE];
> +
> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> + data = readl_relaxed(base + i * LUT_ROW_SIZE);
> + src = (data & GENMASK(31, 30)) >> 30;
> + lval = data & GENMASK(7, 0);
> + core_count = CORE_COUNT_VAL(data);
> +
> + if (src)
> + c->table[i].frequency = c->xo_rate * lval / 1000;
> + else
> + c->table[i].frequency = c->cpu_hw_rate / 1000;
> +
> + cur_freq = c->table[i].frequency;
> +
> + dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
> + i, c->table[i].frequency, core_count);
> +
> + if (core_count != c->max_cores)
> + cur_freq = CPUFREQ_ENTRY_INVALID;
> +
> + /*
> + * Two of the same frequencies with the same core counts means
> + * end of table.
> + */
> + if (i > 0 && c->table[i - 1].frequency ==
> + c->table[i].frequency && prev_cc == core_count) {
> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
> +
> + if (prev_freq == CPUFREQ_ENTRY_INVALID)
> + prev->flags = CPUFREQ_BOOST_FREQ;
> + break;
> + }
> + prev_cc = core_count;
> + prev_freq = cur_freq;
> + }
The logic here always surprises me, why does the firmware keeps the
table this way instead of simplifying it ?
> +
> + c->table[i].frequency = CPUFREQ_TABLE_END;
> +
> + return 0;
> +}
> +
> +static int qcom_get_related_cpus(int index, struct cpumask *m)
> +{
> + struct device_node *cpu_np;
> + struct of_phandle_args args;
> + int cpu, ret;
> +
> + for_each_possible_cpu(cpu) {
> + cpu_np = of_cpu_device_node_get(cpu);
> + if (!cpu_np)
> + continue;
> + of_node_put(cpu_np);
Heh, you can't do that here :)
> +
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
As this code still needs to use the same pointer. It is possible that
the memory associated with cpu_np gets freed as soon as you call
of_node_put() on that. You should call of_node_put() anytime after
this routine is called.
> + "#freq-domain-cells", 0, &args);
> + if (ret < 0)
> + continue;
> +
> + if (index == args.args[0])
> + cpumask_set_cpu(cpu, m);
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_cpu_resources_init(struct platform_device *pdev,
> + unsigned int cpu, int index,
> + unsigned long xo_rate,
> + unsigned long cpu_hw_rate)
> +{
> + struct cpufreq_qcom *c;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + const u16 *offsets;
> + int ret, i, cpu_r;
> + void __iomem *base;
> +
> + if (qcom_freq_domain_map[cpu])
> + return 0;
> +
> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return -ENOMEM;
> +
> + offsets = of_device_get_match_data(&pdev->dev);
> + if (!offsets)
> + return -EINVAL;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++)
> + c->reg_bases[i] = base + offsets[i];
> +
> + /* HW should be in enabled state to proceed */
> + if (!(readl_relaxed(c->reg_bases[REG_ENABLE]) & 0x1)) {
> + dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
> + return -ENODEV;
> + }
> +
> + ret = qcom_get_related_cpus(index, &c->related_cpus);
> + if (ret) {
> + dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
> + return ret;
> + }
> +
> + c->max_cores = cpumask_weight(&c->related_cpus);
> + if (!c->max_cores)
> + return -ENOENT;
> +
> + c->xo_rate = xo_rate;
> + c->cpu_hw_rate = cpu_hw_rate;
> +
> + ret = qcom_cpufreq_hw_read_lut(pdev, c);
> + if (ret) {
> + dev_err(dev, "Domain-%d failed to read LUT\n", index);
> + return ret;
> + }
> +
> + for_each_cpu(cpu_r, &c->related_cpus)
> + qcom_freq_domain_map[cpu_r] = c;
> +
> + return 0;
> +}
> +
> +static int qcom_resources_init(struct platform_device *pdev)
> +{
> + struct device_node *cpu_np;
> + struct of_phandle_args args;
> + struct clk *clk;
> + unsigned int cpu;
> + unsigned long xo_rate, cpu_hw_rate;
> + int ret;
> +
> + clk = devm_clk_get(&pdev->dev, "xo");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + xo_rate = clk_get_rate(clk);
> +
> + devm_clk_put(&pdev->dev, clk);
Why use devm_ variants if you want to put the clock anyway ?
> + clk = devm_clk_get(&pdev->dev, "cpu_clk");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> +
> + devm_clk_put(&pdev->dev, clk);
same here.
> +
> + for_each_possible_cpu(cpu) {
> + cpu_np = of_cpu_device_node_get(cpu);
Where is the of_node_put() corresponding to this ?
> + if (!cpu_np) {
> + dev_dbg(&pdev->dev, "Failed to get cpu %d device\n",
> + cpu);
> + continue;
> + }
> +
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> + "#freq-domain-cells", 0, &args);
> + if (ret < 0)
> + return ret;
> +
> + ret = qcom_cpu_resources_init(pdev, cpu, args.args[0],
> + xo_rate, cpu_hw_rate);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
> +{
> + int rc;
> +
> + /* Get the bases of cpufreq for domains */
> + rc = qcom_resources_init(pdev);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq resource init failed\n");
> + return rc;
> + }
> +
> + rc = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
> + return rc;
> + }
> +
> + dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qcom_cpufreq_hw_match[] = {
> + { .compatible = "qcom,cpufreq-hw", .data = &cpufreq_qcom_std_offsets },
> + {}
> +};
> +
> +static struct platform_driver qcom_cpufreq_hw_driver = {
> + .probe = qcom_cpufreq_hw_driver_probe,
> + .driver = {
> + .name = "qcom-cpufreq-hw",
> + .of_match_table = qcom_cpufreq_hw_match,
> + },
> +};
> +
> +static int __init qcom_cpufreq_hw_init(void)
> +{
> + return platform_driver_register(&qcom_cpufreq_hw_driver);
> +}
> +subsys_initcall(qcom_cpufreq_hw_init);
> +
> +MODULE_DESCRIPTION("QCOM firmware-based CPU Frequency driver");
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
> of the Code Aurora Forum, hosted by the Linux Foundation.
--
viresh
On Sun, Sep 23, 2018 at 04:03:13PM +0530, Taniya Das wrote:
> The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
> for changing the frequency of CPUs. The driver implements the cpufreq
> driver interface for this hardware engine.
>
> Signed-off-by: Saravana Kannan <[email protected]>
> Signed-off-by: Taniya Das <[email protected]>
> ---
> drivers/cpufreq/Kconfig.arm | 11 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/qcom-cpufreq-hw.c | 354 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 366 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 0cd8eb7..93a9d72 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ
> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>
> If in doubt, say N.
> +
> +config ARM_QCOM_CPUFREQ_HW
> + bool "QCOM CPUFreq HW driver"
> + depends on ARCH_QCOM
> + help
> + Support for the CPUFreq HW driver.
> + Some QCOM chipsets have a HW engine to offload the steps
> + necessary for changing the frequency of the CPUs. Firmware loaded
> + in this engine exposes a programming interface to the OS.
> + The driver implements the cpufreq interface for this HW engine.
> + Say Y if you want to support CPUFreq HW.
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index c1ffeab..ca48a1d 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
> obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o
> obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
> obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
> +obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
>
>
> ##################################################################################
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> new file mode 100644
> index 0000000..43107ab
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -0,0 +1,354 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/cpufreq.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +
> +#define LUT_MAX_ENTRIES 40U
> +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16)
> +#define LUT_ROW_SIZE 32
> +#define CLK_HW_DIV 2
> +
> +enum {
> + REG_ENABLE,
> + REG_LUT_TABLE,
> + REG_PERF_STATE,
> +
> + REG_ARRAY_SIZE,
> +};
> +
> +struct cpufreq_qcom {
> + struct cpufreq_frequency_table *table;
> + void __iomem *reg_bases[REG_ARRAY_SIZE];
> + cpumask_t related_cpus;
> + unsigned int max_cores;
> + unsigned long xo_rate;
> + unsigned long cpu_hw_rate;
> +};
> +
> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
> + [REG_ENABLE] = 0x0,
> + [REG_LUT_TABLE] = 0x110,
> + [REG_PERF_STATE] = 0x920,
> +};
> +
> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> +
> +static int
> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
> + unsigned int index)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> +
> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> +
> + return 0;
> +}
> +
> +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
> +{
> + struct cpufreq_qcom *c;
> + struct cpufreq_policy *policy;
> + unsigned int index;
> +
> + policy = cpufreq_cpu_get_raw(cpu);
> + if (!policy)
> + return 0;
> +
> + c = policy->driver_data;
> +
> + index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
> + index = min(index, LUT_MAX_ENTRIES - 1);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static unsigned int
> +qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
> + unsigned int target_freq)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> + int index;
> +
> + index = policy->cached_resolved_idx;
> + if (index < 0)
> + return 0;
> +
> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
> +
> + return policy->freq_table[index].frequency;
> +}
> +
> +static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_qcom *c;
> +
> + c = qcom_freq_domain_map[policy->cpu];
> + if (!c) {
> + pr_err("No scaling support for CPU%d\n", policy->cpu);
> + return -ENODEV;
> + }
> +
> + cpumask_copy(policy->cpus, &c->related_cpus);
> +
> + policy->fast_switch_possible = true;
> + policy->freq_table = c->table;
> + policy->driver_data = c;
> +
> + return 0;
> +}
> +
> +static struct freq_attr *qcom_cpufreq_hw_attr[] = {
> + &cpufreq_freq_attr_scaling_available_freqs,
> + &cpufreq_freq_attr_scaling_boost_freqs,
> + NULL
> +};
> +
> +static struct cpufreq_driver cpufreq_qcom_hw_driver = {
> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target_index = qcom_cpufreq_hw_target_index,
> + .get = qcom_cpufreq_hw_get,
> + .init = qcom_cpufreq_hw_cpu_init,
> + .fast_switch = qcom_cpufreq_hw_fast_switch,
> + .name = "qcom-cpufreq-hw",
> + .attr = qcom_cpufreq_hw_attr,
> + .boost_enabled = true,
> +};
> +
> +static int qcom_cpufreq_hw_read_lut(struct platform_device *pdev,
> + struct cpufreq_qcom *c)
> +{
> + struct device *dev = &pdev->dev;
> + void __iomem *base;
> + u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
> +
> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
> + sizeof(*c->table), GFP_KERNEL);
> + if (!c->table)
> + return -ENOMEM;
> +
> + base = c->reg_bases[REG_LUT_TABLE];
> +
> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> + data = readl_relaxed(base + i * LUT_ROW_SIZE);
> + src = (data & GENMASK(31, 30)) >> 30;
> + lval = data & GENMASK(7, 0);
> + core_count = CORE_COUNT_VAL(data);
> +
> + if (src)
> + c->table[i].frequency = c->xo_rate * lval / 1000;
> + else
> + c->table[i].frequency = c->cpu_hw_rate / 1000;
> +
> + cur_freq = c->table[i].frequency;
> +
> + dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
> + i, c->table[i].frequency, core_count);
> +
> + if (core_count != c->max_cores)
> + cur_freq = CPUFREQ_ENTRY_INVALID;
> +
> + /*
> + * Two of the same frequencies with the same core counts means
> + * end of table.
> + */
> + if (i > 0 && c->table[i - 1].frequency ==
> + c->table[i].frequency && prev_cc == core_count) {
> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
> +
> + if (prev_freq == CPUFREQ_ENTRY_INVALID)
> + prev->flags = CPUFREQ_BOOST_FREQ;
> + break;
> + }
> + prev_cc = core_count;
> + prev_freq = cur_freq;
> + }
> +
> + c->table[i].frequency = CPUFREQ_TABLE_END;
> +
> + return 0;
> +}
> +
> +static int qcom_get_related_cpus(int index, struct cpumask *m)
> +{
> + struct device_node *cpu_np;
> + struct of_phandle_args args;
> + int cpu, ret;
> +
> + for_each_possible_cpu(cpu) {
> + cpu_np = of_cpu_device_node_get(cpu);
> + if (!cpu_np)
> + continue;
> + of_node_put(cpu_np);
> +
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> + "#freq-domain-cells", 0, &args);
Hello Taniya,
in your DT binding, you use
#freq-domains-cells = <1>
Yet here you parse for "#freq-domain-cells".
Kind regards,
Niklas
> + if (ret < 0)
> + continue;
> +
> + if (index == args.args[0])
> + cpumask_set_cpu(cpu, m);
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_cpu_resources_init(struct platform_device *pdev,
> + unsigned int cpu, int index,
> + unsigned long xo_rate,
> + unsigned long cpu_hw_rate)
> +{
> + struct cpufreq_qcom *c;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + const u16 *offsets;
> + int ret, i, cpu_r;
> + void __iomem *base;
> +
> + if (qcom_freq_domain_map[cpu])
> + return 0;
> +
> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return -ENOMEM;
> +
> + offsets = of_device_get_match_data(&pdev->dev);
> + if (!offsets)
> + return -EINVAL;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, index);
> + base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++)
> + c->reg_bases[i] = base + offsets[i];
> +
> + /* HW should be in enabled state to proceed */
> + if (!(readl_relaxed(c->reg_bases[REG_ENABLE]) & 0x1)) {
> + dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
> + return -ENODEV;
> + }
> +
> + ret = qcom_get_related_cpus(index, &c->related_cpus);
> + if (ret) {
> + dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
> + return ret;
> + }
> +
> + c->max_cores = cpumask_weight(&c->related_cpus);
> + if (!c->max_cores)
> + return -ENOENT;
> +
> + c->xo_rate = xo_rate;
> + c->cpu_hw_rate = cpu_hw_rate;
> +
> + ret = qcom_cpufreq_hw_read_lut(pdev, c);
> + if (ret) {
> + dev_err(dev, "Domain-%d failed to read LUT\n", index);
> + return ret;
> + }
> +
> + for_each_cpu(cpu_r, &c->related_cpus)
> + qcom_freq_domain_map[cpu_r] = c;
> +
> + return 0;
> +}
> +
> +static int qcom_resources_init(struct platform_device *pdev)
> +{
> + struct device_node *cpu_np;
> + struct of_phandle_args args;
> + struct clk *clk;
> + unsigned int cpu;
> + unsigned long xo_rate, cpu_hw_rate;
> + int ret;
> +
> + clk = devm_clk_get(&pdev->dev, "xo");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + xo_rate = clk_get_rate(clk);
> +
> + devm_clk_put(&pdev->dev, clk);
> +
> + clk = devm_clk_get(&pdev->dev, "cpu_clk");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> +
> + devm_clk_put(&pdev->dev, clk);
> +
> + for_each_possible_cpu(cpu) {
> + cpu_np = of_cpu_device_node_get(cpu);
> + if (!cpu_np) {
> + dev_dbg(&pdev->dev, "Failed to get cpu %d device\n",
> + cpu);
> + continue;
> + }
> +
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> + "#freq-domain-cells", 0, &args);
> + if (ret < 0)
> + return ret;
> +
> + ret = qcom_cpu_resources_init(pdev, cpu, args.args[0],
> + xo_rate, cpu_hw_rate);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
> +{
> + int rc;
> +
> + /* Get the bases of cpufreq for domains */
> + rc = qcom_resources_init(pdev);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq resource init failed\n");
> + return rc;
> + }
> +
> + rc = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
> + return rc;
> + }
> +
> + dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id qcom_cpufreq_hw_match[] = {
> + { .compatible = "qcom,cpufreq-hw", .data = &cpufreq_qcom_std_offsets },
> + {}
> +};
> +
> +static struct platform_driver qcom_cpufreq_hw_driver = {
> + .probe = qcom_cpufreq_hw_driver_probe,
> + .driver = {
> + .name = "qcom-cpufreq-hw",
> + .of_match_table = qcom_cpufreq_hw_match,
> + },
> +};
> +
> +static int __init qcom_cpufreq_hw_init(void)
> +{
> + return platform_driver_register(&qcom_cpufreq_hw_driver);
> +}
> +subsys_initcall(qcom_cpufreq_hw_init);
> +
> +MODULE_DESCRIPTION("QCOM firmware-based CPU Frequency driver");
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
> of the Code Aurora Forum, hosted by the Linux Foundation.
>
On 08-10-18, 10:40, Niklas Cassel wrote:
> > + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> > + "#freq-domain-cells", 0, &args);
>
> Hello Taniya,
>
> in your DT binding, you use
>
> #freq-domains-cells = <1>
>
> Yet here you parse for "#freq-domain-cells".
And what is the problem with that ? Or are you suggesting her to call
of_parse_phandle_with_fixed_args() ?
--
viresh
On Mon, Oct 08, 2018 at 03:29:52PM +0530, Viresh Kumar wrote:
> On 08-10-18, 10:40, Niklas Cassel wrote:
> > > + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> > > + "#freq-domain-cells", 0, &args);
> >
> > Hello Taniya,
> >
> > in your DT binding, you use
> >
> > #freq-domains-cells = <1>
> >
> > Yet here you parse for "#freq-domain-cells".
>
> And what is the problem with that ? Or are you suggesting her to call
> of_parse_phandle_with_fixed_args() ?
Hello Viresh,
#freq-domain-cells != #freq-domains-cells
The code should parse for the exact same property name as stated in the
DT binding.
Kind regards,
Niklas
On 08-10-18, 12:38, Niklas Cassel wrote:
> On Mon, Oct 08, 2018 at 03:29:52PM +0530, Viresh Kumar wrote:
> > On 08-10-18, 10:40, Niklas Cassel wrote:
> > > > + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> > > > + "#freq-domain-cells", 0, &args);
> > >
> > > Hello Taniya,
> > >
> > > in your DT binding, you use
> > >
> > > #freq-domains-cells = <1>
> > >
> > > Yet here you parse for "#freq-domain-cells".
> >
> > And what is the problem with that ? Or are you suggesting her to call
> > of_parse_phandle_with_fixed_args() ?
>
> Hello Viresh,
>
> #freq-domain-cells != #freq-domains-cells
>
> The code should parse for the exact same property name as stated in the
> DT binding.
Oh, I missed that both the names are different here. It makes sense
now :)
--
viresh
On 10/8/2018 4:08 PM, Niklas Cassel wrote:
> On Mon, Oct 08, 2018 at 03:29:52PM +0530, Viresh Kumar wrote:
>> On 08-10-18, 10:40, Niklas Cassel wrote:
>>>> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
>>>> + "#freq-domain-cells", 0, &args);
>>>
>>> Hello Taniya,
>>>
>>> in your DT binding, you use
>>>
>>> #freq-domains-cells = <1>
>>>
>>> Yet here you parse for "#freq-domain-cells".
>>
>> And what is the problem with that ? Or are you suggesting her to call
>> of_parse_phandle_with_fixed_args() ?
>
> Hello Viresh,
>
> #freq-domain-cells != #freq-domains-cells
>
> The code should parse for the exact same property name as stated in the
> DT binding.
>
> Kind regards,
> Niklas
>
The extra 's' was a typo, would fix it in the next patch.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
Hello Rob,
Thanks for your review comments.
On 9/27/2018 9:01 PM, Rob Herring wrote:
> On Sun, Sep 23, 2018 at 04:03:12PM +0530, Taniya Das wrote:
>> Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's
>> SoCs. This is required for managing the cpu frequency transitions which are
>> controlled by the hardware engine.
>>
>> Signed-off-by: Taniya Das <[email protected]>
>> ---
>> .../bindings/cpufreq/cpufreq-qcom-hw.txt | 169 +++++++++++++++++++++
>> 1 file changed, 169 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
>> new file mode 100644
>> index 0000000..c06941c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
>> @@ -0,0 +1,169 @@
>> +Qualcomm Technologies, Inc. CPUFREQ Bindings
>> +
>> +CPUFREQ HW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
>> +SoCs to manage frequency in hardware. It is capable of controlling frequency
>> +for multiple clusters.
>> +
>> +Properties:
>> +- compatible
>> + Usage: required
>> + Value type: <string>
>> + Definition: must be "qcom,cpufreq-hw".
>> +
>> +- clocks
>> + Usage: required
>> + Value type: <phandle> From common clock binding.
>> + Definition: clock handle for XO clock and GPLL0 clock.
>> +
>> +- clock-names
>> + Usage: required
>> + Value type: <string> From common clock binding.
>> + Definition: must be "xo", "cpu_clk".
>> +
>> +- reg
>> + Usage: required
>> + Value type: <prop-encoded-array>
>> + Definition: Addresses and sizes for the memory of the HW bases in
>> + each frequency domain.
>> +- reg-names
>> + Usage: Optional
>> + Value type: <string>
>> + Definition: Frequency domain name i.e.
>> + "freq-domain0", "freq-domain1".
>> +
>> +* Property qcom,freq-domain
>> +Devices supporting freq-domain must set their "qcom,freq-domain" property with
>> +phandle to a cpufreq_hw followed by the Domain ID(0/1) in the CPU DT node.
>> +
>> +
>> +Example:
>> +
>> +Example 1: Dual-cluster, Quad-core per cluster. CPUs within a cluster switch
>> +DCVS state together.
>> +
>> +/ {
>> + cpus {
>> + #address-cells = <2>;
>> + #size-cells = <0>;
>> +
>> + CPU0: cpu@0 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x0>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_0>;
>> + qcom,freq-domain = <&cpufreq_hw 0>;
>> + L2_0: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + L3_0: l3-cache {
>> + compatible = "cache";
>> + };
>> + };
>> + };
>> +
>> + CPU1: cpu@100 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x100>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_100>;
>> + qcom,freq-domain = <&cpufreq_hw 0>;
>> + L2_100: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + };
>> + };
>> +
>> + CPU2: cpu@200 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x200>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_200>;
>> + qcom,freq-domain = <&cpufreq_hw 0>;
>> + L2_200: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + };
>> + };
>> +
>> + CPU3: cpu@300 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x300>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_300>;
>> + qcom,freq-domain = <&cpufreq_hw 0>;
>> + L2_300: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + };
>> + };
>> +
>> + CPU4: cpu@400 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x400>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_400>;
>> + qcom,freq-domain = <&cpufreq_hw 1>;
>> + L2_400: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + };
>> + };
>> +
>> + CPU5: cpu@500 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x500>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_500>;
>> + qcom,freq-domain = <&cpufreq_hw 1>;
>> + L2_500: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + };
>> + };
>> +
>> + CPU6: cpu@600 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x600>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_600>;
>> + qcom,freq-domain = <&cpufreq_hw 1>;
>> + L2_600: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + };
>> + };
>> +
>> + CPU7: cpu@700 {
>> + device_type = "cpu";
>> + compatible = "qcom,kryo385";
>> + reg = <0x0 0x700>;
>> + enable-method = "psci";
>> + next-level-cache = <&L2_700>;
>> + qcom,freq-domain = <&cpufreq_hw 1>;
>> + L2_700: l2-cache {
>> + compatible = "cache";
>> + next-level-cache = <&L3_0>;
>> + };
>> + };
>> + };
>> +
>> + soc {
>> + cpufreq_hw: qcom,cpufreq-hw {
>
> Should have a unit-address and node names should be generic. We don't
> really have one for this, but let's go with 'cpufreq', so:
>
> cpufreq@17d43000
>
Thanks Rob, will update in the next patch.
>> + compatible = "qcom,cpufreq-hw";
>> + reg = <0x17d43000 0x1400>, <0x17d45800 0x1400>;
>> + reg-names = "freq-domain0", "freq-domain1";
>> +
>> + clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
>> + clock-names = "xo", "cpu_clk";
>> +
>> + #freq-domains-cells = <1>
>
> Not documented, but I don't think you need this if it is always 1 and
> because this is not a common binding.
>
I would add it in the next patch just to keep it documented.
> Rob
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
Hello Niklas,
Thanks for your review comments.
On 10/5/2018 2:34 AM, Niklas Cassel wrote:
> On Sun, Sep 23, 2018 at 04:03:13PM +0530, Taniya Das wrote:
>> The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
>> for changing the frequency of CPUs. The driver implements the cpufreq
>> driver interface for this hardware engine.
>>
>> Signed-off-by: Saravana Kannan <[email protected]>
>> Signed-off-by: Taniya Das <[email protected]>
>> ---
>> drivers/cpufreq/Kconfig.arm | 11 ++
>> drivers/cpufreq/Makefile | 1 +
>> drivers/cpufreq/qcom-cpufreq-hw.c | 354 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 366 insertions(+)
>> create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 0cd8eb7..93a9d72 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ
>> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>>
>> If in doubt, say N.
>> +
>> +config ARM_QCOM_CPUFREQ_HW
>> + bool "QCOM CPUFreq HW driver"
>> + depends on ARCH_QCOM
>
> Hello Taniya,
>
> It might be a good idea to do depends on ARCH_QCOM || COMPILE_TEST
> to get more build testing.
>
Would add it in the next patch.
>> + help
>> + Support for the CPUFreq HW driver.
>> + Some QCOM chipsets have a HW engine to offload the steps
>> + necessary for changing the frequency of the CPUs. Firmware loaded
>> + in this engine exposes a programming interface to the OS.
>> + The driver implements the cpufreq interface for this HW engine.
>> + Say Y if you want to support CPUFreq HW.
>
> All other help text has a tab and two spaces, you only have one.
>
> Also, why not add this Kconfig right after the existing ARCH_QCOM_CPUFREQ
> driver?
>
Would update the same in the next patch.
>> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
>> index c1ffeab..ca48a1d 100644
>> --- a/drivers/cpufreq/Makefile
>> +++ b/drivers/cpufreq/Makefile
>> @@ -85,6 +85,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
>> obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o
>> obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
>> obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
>> +obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
>
> Why not add this right after the existing ARCH_QCOM_CPUFREQ driver?
>
Would update in the next patch.
>>
>>
>> ##################################################################################
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>> new file mode 100644
>> index 0000000..43107ab
>> --- /dev/null
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>> @@ -0,0 +1,354 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/cpufreq.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>
> Since this can't be built as a module, drop module.h, init.h should suffice.
>
Would clean it up.
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +
>> +#define LUT_MAX_ENTRIES 40U
>> +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16)
>> +#define LUT_ROW_SIZE 32
>> +#define CLK_HW_DIV 2
>> +
>> +enum {
>> + REG_ENABLE,
>> + REG_LUT_TABLE,
>> + REG_PERF_STATE,
>> +
>> + REG_ARRAY_SIZE,
>> +};
>> +
>> +struct cpufreq_qcom {
>> + struct cpufreq_frequency_table *table;
>> + void __iomem *reg_bases[REG_ARRAY_SIZE];
>> + cpumask_t related_cpus;
>> + unsigned int max_cores;
>> + unsigned long xo_rate;
>> + unsigned long cpu_hw_rate;
>> +};
>> +
>> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
>> + [REG_ENABLE] = 0x0,
>> + [REG_LUT_TABLE] = 0x110,
>> + [REG_PERF_STATE] = 0x920,
>> +};
>> +
>> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
>> +
>> +static int
>> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
>> + unsigned int index)
>> +{
>> + struct cpufreq_qcom *c = policy->driver_data;
>> +
>> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
>> +{
>> + struct cpufreq_qcom *c;
>> + struct cpufreq_policy *policy;
>> + unsigned int index;
>> +
>> + policy = cpufreq_cpu_get_raw(cpu);
>> + if (!policy)
>> + return 0;
>> +
>> + c = policy->driver_data;
>> +
>> + index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
>> + index = min(index, LUT_MAX_ENTRIES - 1);
>> +
>> + return policy->freq_table[index].frequency;
>> +}
>> +
>> +static unsigned int
>> +qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
>> + unsigned int target_freq)
>> +{
>> + struct cpufreq_qcom *c = policy->driver_data;
>> + int index;
>> +
>> + index = policy->cached_resolved_idx;
>> + if (index < 0)
>> + return 0;
>> +
>> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
>> +
>> + return policy->freq_table[index].frequency;
>> +}
>> +
>> +static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>> +{
>> + struct cpufreq_qcom *c;
>> +
>> + c = qcom_freq_domain_map[policy->cpu];
>> + if (!c) {
>> + pr_err("No scaling support for CPU%d\n", policy->cpu);
>> + return -ENODEV;
>> + }
>> +
>> + cpumask_copy(policy->cpus, &c->related_cpus);
>> +
>> + policy->fast_switch_possible = true;
>> + policy->freq_table = c->table;
>> + policy->driver_data = c;
>> +
>> + return 0;
>> +}
>> +
>> +static struct freq_attr *qcom_cpufreq_hw_attr[] = {
>> + &cpufreq_freq_attr_scaling_available_freqs,
>> + &cpufreq_freq_attr_scaling_boost_freqs,
>> + NULL
>> +};
>> +
>> +static struct cpufreq_driver cpufreq_qcom_hw_driver = {
>> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
>> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
>> + .verify = cpufreq_generic_frequency_table_verify,
>> + .target_index = qcom_cpufreq_hw_target_index,
>> + .get = qcom_cpufreq_hw_get,
>> + .init = qcom_cpufreq_hw_cpu_init,
>> + .fast_switch = qcom_cpufreq_hw_fast_switch,
>> + .name = "qcom-cpufreq-hw",
>> + .attr = qcom_cpufreq_hw_attr,
>> + .boost_enabled = true,
>> +};
>> +
>> +static int qcom_cpufreq_hw_read_lut(struct platform_device *pdev,
>> + struct cpufreq_qcom *c)
>> +{
>> + struct device *dev = &pdev->dev;
>> + void __iomem *base;
>> + u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
>> +
>> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
>> + sizeof(*c->table), GFP_KERNEL);
>> + if (!c->table)
>> + return -ENOMEM;
>> +
>> + base = c->reg_bases[REG_LUT_TABLE];
>> +
>> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>> + data = readl_relaxed(base + i * LUT_ROW_SIZE);
>> + src = (data & GENMASK(31, 30)) >> 30;
>> + lval = data & GENMASK(7, 0);
>> + core_count = CORE_COUNT_VAL(data);
>> +
>> + if (src)
>> + c->table[i].frequency = c->xo_rate * lval / 1000;
>> + else
>> + c->table[i].frequency = c->cpu_hw_rate / 1000;
>> +
>> + cur_freq = c->table[i].frequency;
>> +
>> + dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
>> + i, c->table[i].frequency, core_count);
>> +
>> + if (core_count != c->max_cores)
>> + cur_freq = CPUFREQ_ENTRY_INVALID;
>> +
>> + /*
>> + * Two of the same frequencies with the same core counts means
>> + * end of table.
>> + */
>> + if (i > 0 && c->table[i - 1].frequency ==
>> + c->table[i].frequency && prev_cc == core_count) {
>> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
>> +
>> + if (prev_freq == CPUFREQ_ENTRY_INVALID)
>> + prev->flags = CPUFREQ_BOOST_FREQ;
>> + break;
>> + }
>> + prev_cc = core_count;
>> + prev_freq = cur_freq;
>> + }
>> +
>> + c->table[i].frequency = CPUFREQ_TABLE_END;
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_get_related_cpus(int index, struct cpumask *m)
>> +{
>> + struct device_node *cpu_np;
>> + struct of_phandle_args args;
>> + int cpu, ret;
>> +
>> + for_each_possible_cpu(cpu) {
>> + cpu_np = of_cpu_device_node_get(cpu);
>> + if (!cpu_np)
>> + continue;
>> + of_node_put(cpu_np);
>
> Shouldn't of_node_put() be called after you have used the pointer?
>
Yes, it would be moved.
>> +
>> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
>> + "#freq-domain-cells", 0, &args);
>> + if (ret < 0)
>> + continue;
>> +
>> + if (index == args.args[0])
>> + cpumask_set_cpu(cpu, m);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_cpu_resources_init(struct platform_device *pdev,
>> + unsigned int cpu, int index,
>> + unsigned long xo_rate,
>> + unsigned long cpu_hw_rate)
>> +{
>> + struct cpufreq_qcom *c;
>> + struct resource *res;
>> + struct device *dev = &pdev->dev;
>> + const u16 *offsets;
>> + int ret, i, cpu_r;
>> + void __iomem *base;
>> +
>> + if (qcom_freq_domain_map[cpu])
>> + return 0;
>> +
>> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
>> + if (!c)
>> + return -ENOMEM;
>> +
>> + offsets = of_device_get_match_data(&pdev->dev);
>> + if (!offsets)
>> + return -EINVAL;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, index);
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++)
>> + c->reg_bases[i] = base + offsets[i];
>> +
>> + /* HW should be in enabled state to proceed */
>> + if (!(readl_relaxed(c->reg_bases[REG_ENABLE]) & 0x1)) {
>> + dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
>> + return -ENODEV;
>> + }
>> +
>> + ret = qcom_get_related_cpus(index, &c->related_cpus);
>> + if (ret) {
>> + dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
>> + return ret;
>> + }
>> +
>> + c->max_cores = cpumask_weight(&c->related_cpus);
>> + if (!c->max_cores)
>> + return -ENOENT;
>> +
>> + c->xo_rate = xo_rate;
>> + c->cpu_hw_rate = cpu_hw_rate;
>> +
>> + ret = qcom_cpufreq_hw_read_lut(pdev, c);
>> + if (ret) {
>> + dev_err(dev, "Domain-%d failed to read LUT\n", index);
>> + return ret;
>> + }
>> +
>> + for_each_cpu(cpu_r, &c->related_cpus)
>> + qcom_freq_domain_map[cpu_r] = c;
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_resources_init(struct platform_device *pdev)
>> +{
>> + struct device_node *cpu_np;
>> + struct of_phandle_args args;
>> + struct clk *clk;
>> + unsigned int cpu;
>> + unsigned long xo_rate, cpu_hw_rate;
>> + int ret;
>> +
>> + clk = devm_clk_get(&pdev->dev, "xo");
>> + if (IS_ERR(clk))
>> + return PTR_ERR(clk);
>> +
>> + xo_rate = clk_get_rate(clk);
>> +
>> + devm_clk_put(&pdev->dev, clk);
>> +
>> + clk = devm_clk_get(&pdev->dev, "cpu_clk");
>> + if (IS_ERR(clk))
>> + return PTR_ERR(clk);
>> +
>> + cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
>> +
>> + devm_clk_put(&pdev->dev, clk);
>> +
>> + for_each_possible_cpu(cpu) {
>> + cpu_np = of_cpu_device_node_get(cpu);
>
> Where is the matching of_node_put?
>
Yes, missed adding it.
>> + if (!cpu_np) {
>> + dev_dbg(&pdev->dev, "Failed to get cpu %d device\n",
>> + cpu);
>> + continue;
>> + }
>> +
>> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
>> + "#freq-domain-cells", 0, &args);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = qcom_cpu_resources_init(pdev, cpu, args.args[0],
>> + xo_rate, cpu_hw_rate);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>> +{
>> + int rc;
>> +
>> + /* Get the bases of cpufreq for domains */
>> + rc = qcom_resources_init(pdev);
>> + if (rc) {
>> + dev_err(&pdev->dev, "CPUFreq resource init failed\n");
>> + return rc;
>> + }
>> +
>> + rc = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
>> + if (rc) {
>> + dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
>> + return rc;
>> + }
>> +
>> + dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n");
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id qcom_cpufreq_hw_match[] = {
>> + { .compatible = "qcom,cpufreq-hw", .data = &cpufreq_qcom_std_offsets },
>> + {}
>> +};
>> +
>> +static struct platform_driver qcom_cpufreq_hw_driver = {
>> + .probe = qcom_cpufreq_hw_driver_probe,
>> + .driver = {
>> + .name = "qcom-cpufreq-hw",
>> + .of_match_table = qcom_cpufreq_hw_match,
>> + },
>> +};
>> +
>> +static int __init qcom_cpufreq_hw_init(void)
>> +{
>> + return platform_driver_register(&qcom_cpufreq_hw_driver);
>> +}
>> +subsys_initcall(qcom_cpufreq_hw_init);
>> +
>> +MODULE_DESCRIPTION("QCOM firmware-based CPU Frequency driver");
>
> Since this can't be built as a module, you should drop MODULE_DESCRIPTION.
>
>
> Also, checkpatch --strict gives the following warnings:
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #103:
> new file mode 100644
>
> cpufreq drivers usually have an entry in MAINTAINERS file.
>
>
> WARNING: Duplicate signature
> #370:
> Signed-off-by: Taniya Das <[email protected]>
>
> If you are more than one author, you can use the Co-Developed-by tag.
>
Hmm, I do see multiple SOBs being used and merged.
>
> CHECK: Alignment should match open parenthesis
> #611: FILE: drivers/cpufreq/qcom-cpufreq-hw.c:193:
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> + "#freq-domain-cells", 0, &args);
>
> CHECK: Alignment should match open parenthesis
> #718: FILE: drivers/cpufreq/qcom-cpufreq-hw.c:300:
> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
> + "#freq-domain-cells", 0, &args);
>
>
Would fix the same.
>
>
> Kind regards,
> Niklas
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
Hello Viresh,
Thanks for your review comments.
On 10/5/2018 4:25 PM, Viresh Kumar wrote:
> On 23-09-18, 16:03, Taniya Das wrote:
>> The CPUfreq HW present in some QCOM chipsets offloads the steps necessary
>> for changing the frequency of CPUs. The driver implements the cpufreq
>> driver interface for this hardware engine.
>>
>> Signed-off-by: Saravana Kannan <[email protected]>
>> Signed-off-by: Taniya Das <[email protected]>
>> ---
>> drivers/cpufreq/Kconfig.arm | 11 ++
>> drivers/cpufreq/Makefile | 1 +
>> drivers/cpufreq/qcom-cpufreq-hw.c | 354 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 366 insertions(+)
>> create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 0cd8eb7..93a9d72 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -298,3 +298,14 @@ config ARM_PXA2xx_CPUFREQ
>> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>>
>> If in doubt, say N.
>> +
>> +config ARM_QCOM_CPUFREQ_HW
>> + bool "QCOM CPUFreq HW driver"
>> + depends on ARCH_QCOM
>> + help
>> + Support for the CPUFreq HW driver.
>> + Some QCOM chipsets have a HW engine to offload the steps
>> + necessary for changing the frequency of the CPUs. Firmware loaded
>> + in this engine exposes a programming interface to the OS.
>> + The driver implements the cpufreq interface for this HW engine.
>> + Say Y if you want to support CPUFreq HW.
>> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
>> index c1ffeab..ca48a1d 100644
>> --- a/drivers/cpufreq/Makefile
>> +++ b/drivers/cpufreq/Makefile
>> @@ -85,6 +85,7 @@ obj-$(CONFIG_ARM_TEGRA124_CPUFREQ) += tegra124-cpufreq.o
>> obj-$(CONFIG_ARM_TEGRA186_CPUFREQ) += tegra186-cpufreq.o
>> obj-$(CONFIG_ARM_TI_CPUFREQ) += ti-cpufreq.o
>> obj-$(CONFIG_ARM_VEXPRESS_SPC_CPUFREQ) += vexpress-spc-cpufreq.o
>> +obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
>>
>>
>> ##################################################################################
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
>> new file mode 100644
>> index 0000000..43107ab
>> --- /dev/null
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>> @@ -0,0 +1,354 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/cpufreq.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +
>> +#define LUT_MAX_ENTRIES 40U
>> +#define CORE_COUNT_VAL(val) (((val) & (GENMASK(18, 16))) >> 16)
>> +#define LUT_ROW_SIZE 32
>> +#define CLK_HW_DIV 2
>> +
>> +enum {
>> + REG_ENABLE,
>> + REG_LUT_TABLE,
>> + REG_PERF_STATE,
>> +
>> + REG_ARRAY_SIZE,
>> +};
>> +
>> +struct cpufreq_qcom {
>> + struct cpufreq_frequency_table *table;
>> + void __iomem *reg_bases[REG_ARRAY_SIZE];
>> + cpumask_t related_cpus;
>> + unsigned int max_cores;
>> + unsigned long xo_rate;
>> + unsigned long cpu_hw_rate;
>> +};
>> +
>> +static const u16 cpufreq_qcom_std_offsets[REG_ARRAY_SIZE] = {
>> + [REG_ENABLE] = 0x0,
>> + [REG_LUT_TABLE] = 0x110,
>> + [REG_PERF_STATE] = 0x920,
>> +};
>> +
>> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
>> +
>> +static int
>> +qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
>> + unsigned int index)
>> +{
>> + struct cpufreq_qcom *c = policy->driver_data;
>> +
>> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
>> +{
>> + struct cpufreq_qcom *c;
>> + struct cpufreq_policy *policy;
>> + unsigned int index;
>> +
>> + policy = cpufreq_cpu_get_raw(cpu);
>> + if (!policy)
>> + return 0;
>> +
>> + c = policy->driver_data;
>> +
>> + index = readl_relaxed(c->reg_bases[REG_PERF_STATE]);
>> + index = min(index, LUT_MAX_ENTRIES - 1);
>> +
>> + return policy->freq_table[index].frequency;
>> +}
>> +
>> +static unsigned int
>> +qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
>> + unsigned int target_freq)
>> +{
>> + struct cpufreq_qcom *c = policy->driver_data;
>> + int index;
>> +
>> + index = policy->cached_resolved_idx;
>> + if (index < 0)
>> + return 0;
>> +
>> + writel_relaxed(index, c->reg_bases[REG_PERF_STATE]);
>> +
>> + return policy->freq_table[index].frequency;
>> +}
>> +
>> +static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>> +{
>> + struct cpufreq_qcom *c;
>> +
>> + c = qcom_freq_domain_map[policy->cpu];
>> + if (!c) {
>> + pr_err("No scaling support for CPU%d\n", policy->cpu);
>> + return -ENODEV;
>> + }
>> +
>> + cpumask_copy(policy->cpus, &c->related_cpus);
>> +
>> + policy->fast_switch_possible = true;
>> + policy->freq_table = c->table;
>> + policy->driver_data = c;
>> +
>> + return 0;
>> +}
>> +
>> +static struct freq_attr *qcom_cpufreq_hw_attr[] = {
>> + &cpufreq_freq_attr_scaling_available_freqs,
>> + &cpufreq_freq_attr_scaling_boost_freqs,
>> + NULL
>> +};
>> +
>> +static struct cpufreq_driver cpufreq_qcom_hw_driver = {
>> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
>> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
>> + .verify = cpufreq_generic_frequency_table_verify,
>> + .target_index = qcom_cpufreq_hw_target_index,
>> + .get = qcom_cpufreq_hw_get,
>> + .init = qcom_cpufreq_hw_cpu_init,
>> + .fast_switch = qcom_cpufreq_hw_fast_switch,
>> + .name = "qcom-cpufreq-hw",
>> + .attr = qcom_cpufreq_hw_attr,
>> + .boost_enabled = true,
>> +};
>> +
>> +static int qcom_cpufreq_hw_read_lut(struct platform_device *pdev,
>> + struct cpufreq_qcom *c)
>> +{
>> + struct device *dev = &pdev->dev;
>> + void __iomem *base;
>> + u32 data, src, lval, i, core_count, prev_cc, prev_freq, cur_freq;
>> +
>> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
>> + sizeof(*c->table), GFP_KERNEL);
>> + if (!c->table)
>> + return -ENOMEM;
>> +
>> + base = c->reg_bases[REG_LUT_TABLE];
>> +
>> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>> + data = readl_relaxed(base + i * LUT_ROW_SIZE);
>> + src = (data & GENMASK(31, 30)) >> 30;
>> + lval = data & GENMASK(7, 0);
>> + core_count = CORE_COUNT_VAL(data);
>> +
>> + if (src)
>> + c->table[i].frequency = c->xo_rate * lval / 1000;
>> + else
>> + c->table[i].frequency = c->cpu_hw_rate / 1000;
>> +
>> + cur_freq = c->table[i].frequency;
>> +
>> + dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
>> + i, c->table[i].frequency, core_count);
>> +
>> + if (core_count != c->max_cores)
>> + cur_freq = CPUFREQ_ENTRY_INVALID;
>> +
>> + /*
>> + * Two of the same frequencies with the same core counts means
>> + * end of table.
>> + */
>> + if (i > 0 && c->table[i - 1].frequency ==
>> + c->table[i].frequency && prev_cc == core_count) {
>> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
>> +
>> + if (prev_freq == CPUFREQ_ENTRY_INVALID)
>> + prev->flags = CPUFREQ_BOOST_FREQ;
>> + break;
>> + }
>> + prev_cc = core_count;
>> + prev_freq = cur_freq;
>> + }
>
> The logic here always surprises me, why does the firmware keeps the
> table this way instead of simplifying it ?
>
Unfortunately that is the only way to identify.
>> +
>> + c->table[i].frequency = CPUFREQ_TABLE_END;
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_get_related_cpus(int index, struct cpumask *m)
>> +{
>> + struct device_node *cpu_np;
>> + struct of_phandle_args args;
>> + int cpu, ret;
>> +
>> + for_each_possible_cpu(cpu) {
>> + cpu_np = of_cpu_device_node_get(cpu);
>> + if (!cpu_np)
>> + continue;
>> + of_node_put(cpu_np);
>
> Heh, you can't do that here :)
>
Yes, I would take care to put it in the right place.
>> +
>> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
>
> As this code still needs to use the same pointer. It is possible that
> the memory associated with cpu_np gets freed as soon as you call
> of_node_put() on that. You should call of_node_put() anytime after
> this routine is called.
>
>> + "#freq-domain-cells", 0, &args);
>> + if (ret < 0)
>> + continue;
>> +
>> + if (index == args.args[0])
>> + cpumask_set_cpu(cpu, m);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_cpu_resources_init(struct platform_device *pdev,
>> + unsigned int cpu, int index,
>> + unsigned long xo_rate,
>> + unsigned long cpu_hw_rate)
>> +{
>> + struct cpufreq_qcom *c;
>> + struct resource *res;
>> + struct device *dev = &pdev->dev;
>> + const u16 *offsets;
>> + int ret, i, cpu_r;
>> + void __iomem *base;
>> +
>> + if (qcom_freq_domain_map[cpu])
>> + return 0;
>> +
>> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
>> + if (!c)
>> + return -ENOMEM;
>> +
>> + offsets = of_device_get_match_data(&pdev->dev);
>> + if (!offsets)
>> + return -EINVAL;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, index);
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + for (i = REG_ENABLE; i < REG_ARRAY_SIZE; i++)
>> + c->reg_bases[i] = base + offsets[i];
>> +
>> + /* HW should be in enabled state to proceed */
>> + if (!(readl_relaxed(c->reg_bases[REG_ENABLE]) & 0x1)) {
>> + dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
>> + return -ENODEV;
>> + }
>> +
>> + ret = qcom_get_related_cpus(index, &c->related_cpus);
>> + if (ret) {
>> + dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
>> + return ret;
>> + }
>> +
>> + c->max_cores = cpumask_weight(&c->related_cpus);
>> + if (!c->max_cores)
>> + return -ENOENT;
>> +
>> + c->xo_rate = xo_rate;
>> + c->cpu_hw_rate = cpu_hw_rate;
>> +
>> + ret = qcom_cpufreq_hw_read_lut(pdev, c);
>> + if (ret) {
>> + dev_err(dev, "Domain-%d failed to read LUT\n", index);
>> + return ret;
>> + }
>> +
>> + for_each_cpu(cpu_r, &c->related_cpus)
>> + qcom_freq_domain_map[cpu_r] = c;
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_resources_init(struct platform_device *pdev)
>> +{
>> + struct device_node *cpu_np;
>> + struct of_phandle_args args;
>> + struct clk *clk;
>> + unsigned int cpu;
>> + unsigned long xo_rate, cpu_hw_rate;
>> + int ret;
>> +
>> + clk = devm_clk_get(&pdev->dev, "xo");
>> + if (IS_ERR(clk))
>> + return PTR_ERR(clk);
>> +
>> + xo_rate = clk_get_rate(clk);
>> +
>> + devm_clk_put(&pdev->dev, clk);
>
> Why use devm_ variants if you want to put the clock anyway ?
>
Would move to non devm_ variants.
>> + clk = devm_clk_get(&pdev->dev, "cpu_clk");
>> + if (IS_ERR(clk))
>> + return PTR_ERR(clk);
>> +
>> + cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
>> +
>> + devm_clk_put(&pdev->dev, clk);
>
> same here.
>
>> +
>> + for_each_possible_cpu(cpu) {
>> + cpu_np = of_cpu_device_node_get(cpu);
>
> Where is the of_node_put() corresponding to this ?
>
>> + if (!cpu_np) {
>> + dev_dbg(&pdev->dev, "Failed to get cpu %d device\n",
>> + cpu);
>> + continue;
>> + }
>> +
>> + ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
>> + "#freq-domain-cells", 0, &args);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = qcom_cpu_resources_init(pdev, cpu, args.args[0],
>> + xo_rate, cpu_hw_rate);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
>> +{
>> + int rc;
>> +
>> + /* Get the bases of cpufreq for domains */
>> + rc = qcom_resources_init(pdev);
>> + if (rc) {
>> + dev_err(&pdev->dev, "CPUFreq resource init failed\n");
>> + return rc;
>> + }
>> +
>> + rc = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
>> + if (rc) {
>> + dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
>> + return rc;
>> + }
>> +
>> + dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n");
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id qcom_cpufreq_hw_match[] = {
>> + { .compatible = "qcom,cpufreq-hw", .data = &cpufreq_qcom_std_offsets },
>> + {}
>> +};
>> +
>> +static struct platform_driver qcom_cpufreq_hw_driver = {
>> + .probe = qcom_cpufreq_hw_driver_probe,
>> + .driver = {
>> + .name = "qcom-cpufreq-hw",
>> + .of_match_table = qcom_cpufreq_hw_match,
>> + },
>> +};
>> +
>> +static int __init qcom_cpufreq_hw_init(void)
>> +{
>> + return platform_driver_register(&qcom_cpufreq_hw_driver);
>> +}
>> +subsys_initcall(qcom_cpufreq_hw_init);
>> +
>> +MODULE_DESCRIPTION("QCOM firmware-based CPU Frequency driver");
>> --
>> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
>> of the Code Aurora Forum, hosted by the Linux Foundation.
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
On Thu, Oct 11, 2018 at 04:45:25PM +0530, Taniya Das wrote:
> > WARNING: Duplicate signature
> > #370:
> > Signed-off-by: Taniya Das <[email protected]>
> >
> > If you are more than one author, you can use the Co-Developed-by tag.
> >
>
> Hmm, I do see multiple SOBs being used and merged.
>
From Documentation/process/submitting-patches.rst:
"""
A Co-Developed-by: states that the patch was also created by another developer
along with the original author. This is useful at times when multiple people
work on a single patch. Note, this person also needs to have a Signed-off-by:
line in the patch as well.
"""
So it is not wrong to have two Signed-off-bys, just that
having the Co-Developed-by tag clearly shows that there
was more than one author.
Without the Co-Developed-by tag, the second Signed-off-by tag
is ambiguous, since the seconds Signed-off-by tag might just
have meant that that person was in the delivery chain of the
patch.
I think that the Co-Developed-by tag is optional, so if the
co-developer doesn't care about taking credit, it can probably
be skipped.
Kind regards,
Niklas
On Sun 23 Sep 03:33 PDT 2018, Taniya Das wrote:
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
> new file mode 100644
> index 0000000..c06941c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
> @@ -0,0 +1,169 @@
> +Qualcomm Technologies, Inc. CPUFREQ Bindings
> +
> +CPUFREQ HW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
> +SoCs to manage frequency in hardware. It is capable of controlling frequency
> +for multiple clusters.
> +
> +Properties:
> +- compatible
> + Usage: required
> + Value type: <string>
> + Definition: must be "qcom,cpufreq-hw".
As this binding (and driver) operates actual hardware we should probably
have a more specific compatible. Or is this the one and only "hardware
cpufreq" block that will ever come out of Qualcomm?
Regards,
Bjorn