2018-05-17 09:31:27

by Taniya Das

[permalink] [raw]
Subject: [v0 0/2] Add support for QCOM cpufreq FW driver

The CPUfreq FW present in some QCOM chipsets offloads the steps necessary
for hanging the frequency of CPUs. The driver implements the cpufreq driver
interface for this firmware.

Taniya Das (2):
dt-bindings: clock: Introduce QCOM CPUFREQ FW bindings
cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

.../bindings/cpufreq/cpufreq-qcom-fw.txt | 68 +++++
drivers/cpufreq/Kconfig.arm | 9 +
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/qcom-cpufreq-fw.c | 318 +++++++++++++++++++++
4 files changed, 396 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.



2018-05-17 09:31:02

by Taniya Das

[permalink] [raw]
Subject: [v0 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

The CPUfreq FW present in some QCOM chipsets offloads the steps necessary
for hanging the frequency of CPUs. The driver implements the cpufreq driver
interface for this firmware.

Signed-off-by: Taniya Das <[email protected]>
---
drivers/cpufreq/Kconfig.arm | 9 ++
drivers/cpufreq/Makefile | 1 +
drivers/cpufreq/qcom-cpufreq-fw.c | 318 ++++++++++++++++++++++++++++++++++++++
3 files changed, 328 insertions(+)
create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 96b35b8..a50aa6e 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -301,3 +301,12 @@ config ARM_PXA2xx_CPUFREQ
This add the CPUFreq driver support for Intel PXA2xx SOCs.

If in doubt, say N.
+
+config ARM_QCOM_CPUFREQ_FW
+ tristate "QCOM CPUFreq FW driver"
+ help
+ Support for the CPUFreq FW driver.
+ The CPUfreq FW preset in some QCOM chipsets offloads the steps
+ necessary for changing the frequency of CPUs. The driver
+ implements the cpufreq driver interface for this firmware.
+ Say Y if you want to support CPUFreq FW.
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 8d24ade..a3edbce 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_FW) += qcom-cpufreq-fw.o


##################################################################################
diff --git a/drivers/cpufreq/qcom-cpufreq-fw.c b/drivers/cpufreq/qcom-cpufreq-fw.c
new file mode 100644
index 0000000..67996d5
--- /dev/null
+++ b/drivers/cpufreq/qcom-cpufreq-fw.c
@@ -0,0 +1,318 @@
+// 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 INIT_RATE 300000000UL
+#define XO_RATE 19200000UL
+#define LUT_MAX_ENTRIES 40U
+#define CORE_COUNT_VAL(val) ((val & GENMASK(18, 16)) >> 16)
+#define LUT_ROW_SIZE 32
+
+struct cpufreq_qcom {
+ struct cpufreq_frequency_table *table;
+ struct device *dev;
+ void __iomem *perf_base;
+ void __iomem *lut_base;
+ cpumask_t related_cpus;
+ unsigned int max_cores;
+};
+
+static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
+
+static int
+qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy, unsigned int index)
+{
+ struct cpufreq_qcom *c = policy->driver_data;
+
+ if (index >= LUT_MAX_ENTRIES) {
+ dev_err(c->dev,
+ "Passing an index (%u) that's greater than max (%d)\n",
+ index, LUT_MAX_ENTRIES - 1);
+ return -EINVAL;
+ }
+
+ writel_relaxed(index, c->perf_base);
+
+ /* Make sure the write goes through before proceeding */
+ mb();
+ return 0;
+}
+
+static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
+{
+ struct cpufreq_qcom *c;
+ unsigned int index;
+
+ c = qcom_freq_domain_map[cpu];
+ if (!c)
+ return -ENODEV;
+
+ index = readl_relaxed(c->perf_base);
+ index = min(index, LUT_MAX_ENTRIES - 1);
+
+ return c->table[index].frequency;
+}
+
+static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
+{
+ struct cpufreq_qcom *c;
+ int ret;
+
+ 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->table = c->table;
+ policy->driver_data = c;
+
+ return ret;
+}
+
+static struct freq_attr *qcom_cpufreq_fw_attr[] = {
+ &cpufreq_freq_attr_scaling_available_freqs,
+ &cpufreq_freq_attr_scaling_boost_freqs,
+ NULL
+};
+
+static struct cpufreq_driver cpufreq_qcom_fw_driver = {
+ .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
+ CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
+ .verify = cpufreq_generic_frequency_table_verify,
+ .target_index = qcom_cpufreq_fw_target_index,
+ .get = qcom_cpufreq_fw_get,
+ .init = qcom_cpufreq_fw_cpu_init,
+ .name = "qcom-cpufreq-fw",
+ .attr = qcom_cpufreq_fw_attr,
+ .boost_enabled = true,
+};
+
+static int qcom_read_lut(struct platform_device *pdev,
+ struct cpufreq_qcom *c)
+{
+ struct device *dev = &pdev->dev;
+ u32 data, src, lval, i, core_count, prev_cc = 0;
+
+ c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
+ sizeof(*c->table), GFP_KERNEL);
+ if (!c->table)
+ return -ENOMEM;
+
+ for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+ data = readl_relaxed(c->lut_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 = INIT_RATE / 1000;
+ else
+ c->table[i].frequency = XO_RATE * lval / 1000;
+
+ c->table[i].driver_data = 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)
+ c->table[i].frequency = CPUFREQ_ENTRY_INVALID;
+
+ /*
+ * Two of the same frequencies with the same core counts means
+ * end of table.
+ */
+ if (i > 0 && c->table[i - 1].driver_data ==
+ c->table[i].driver_data
+ && prev_cc == core_count) {
+ struct cpufreq_frequency_table *prev = &c->table[i - 1];
+
+ if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
+ prev->flags = CPUFREQ_BOOST_FREQ;
+ prev->frequency = prev->driver_data;
+ }
+
+ break;
+ }
+ prev_cc = core_count;
+ }
+ c->table[i].frequency = CPUFREQ_TABLE_END;
+
+ return 0;
+}
+
+static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
+{
+ struct device_node *dev_phandle;
+ struct device *cpu_dev;
+ int cpu, i = 0, ret = -ENOENT;
+
+ dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
+ while (dev_phandle) {
+ for_each_possible_cpu(cpu) {
+ cpu_dev = get_cpu_device(cpu);
+ if (cpu_dev && cpu_dev->of_node == dev_phandle) {
+ cpumask_set_cpu(cpu, m);
+ ret = 0;
+ break;
+ }
+ }
+ dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
+ }
+
+ return ret;
+}
+
+static int qcom_cpu_resources_init(struct platform_device *pdev,
+ struct device_node *np)
+{
+ struct cpufreq_qcom *c;
+ struct resource res;
+ struct device *dev = &pdev->dev;
+ void __iomem *en_base;
+ int cpu, index = 0, ret;
+
+ c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
+
+ res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
+ if (!res) {
+ dev_err(dev, "Enable base not defined for %s\n", np->name);
+ return ret;
+ }
+
+ en_base = devm_ioremap(dev, res->start, resource_size(res));
+ if (!en_base) {
+ dev_err(dev, "Unable to map %s en-base\n", np->name);
+ return -ENOMEM;
+ }
+
+ /* FW should be enabled state to proceed */
+ if (!(readl_relaxed(en_base) & 0x1)) {
+ dev_err(dev, "%s firmware not enabled\n", np->name);
+ return -ENODEV;
+ }
+
+ devm_iounmap(&pdev->dev, en_base);
+
+ index = of_property_match_string(np, "reg-names", "perf_base");
+ if (index < 0)
+ return index;
+
+ if (of_address_to_resource(np, index, &res))
+ return -ENOMEM;
+
+ c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
+ if (!c->perf_base) {
+ dev_err(dev, "Unable to map %s perf-base\n", np->name);
+ return -ENOMEM;
+ }
+
+ index = of_property_match_string(np, "reg-names", "lut_base");
+ if (index < 0)
+ return index;
+
+ if (of_address_to_resource(np, index, &res))
+ return -ENOMEM;
+
+ c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
+ if (!c->lut_base) {
+ dev_err(dev, "Unable to map %s lut-base\n", np->name);
+ return -ENOMEM;
+ }
+
+ ret = qcom_get_related_cpus(np, &c->related_cpus);
+ if (ret) {
+ dev_err(dev, "%s failed to get core phandles\n", np->name);
+ return ret;
+ }
+
+ c->max_cores = cpumask_weight(&c->related_cpus);
+
+ ret = qcom_read_lut(pdev, c);
+ if (ret) {
+ dev_err(dev, "%s failed to read LUT\n", np->name);
+ return ret;
+ }
+
+ for_each_cpu(cpu, &c->related_cpus)
+ qcom_freq_domain_map[cpu] = c;
+
+ return 0;
+}
+
+static int qcom_resources_init(struct platform_device *pdev)
+{
+ struct device_node *np;
+ int ret = -ENODEV;
+
+ for_each_available_child_of_node(pdev->dev.of_node, np) {
+ if (of_device_is_compatible(np, "cpufreq")) {
+ ret = qcom_cpu_resources_init(pdev, np);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
+static int qcom_cpufreq_fw_driver_probe(struct platform_device *pdev)
+{
+ int rc = 0;
+
+ /* 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_fw_driver);
+ if (rc) {
+ dev_err(&pdev->dev, "CPUFreq FW driver failed to register\n");
+ return rc;
+ }
+
+ dev_info(&pdev->dev, "QCOM CPUFreq FW driver inited\n");
+
+ return 0;
+}
+
+static const struct of_device_id match_table[] = {
+ { .compatible = "qcom,cpufreq-fw" },
+ {}
+};
+
+static struct platform_driver qcom_cpufreq_fw_driver = {
+ .probe = qcom_cpufreq_fw_driver_probe,
+ .driver = {
+ .name = "qcom-cpufreq-fw",
+ .of_match_table = match_table,
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init qcom_cpufreq_fw_init(void)
+{
+ return platform_driver_register(&qcom_cpufreq_fw_driver);
+}
+subsys_initcall(qcom_cpufreq_fw_init);
+
+static void __exit qcom_cpufreq_fw_exit(void)
+{
+ platform_driver_unregister(&qcom_cpufreq_fw_driver);
+}
+module_exit(qcom_cpufreq_fw_exit);
+
+MODULE_DESCRIPTION("QCOM CPU Frequency FW");
+MODULE_LICENSE("GPL v2");
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


2018-05-17 09:31:56

by Taniya Das

[permalink] [raw]
Subject: [v0 1/2] dt-bindings: clock: Introduce QCOM CPUFREQ FW bindings

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 firmware.

Signed-off-by: Taniya Das <[email protected]>
---
.../bindings/cpufreq/cpufreq-qcom-fw.txt | 68 ++++++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
new file mode 100644
index 0000000..bc912f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
@@ -0,0 +1,68 @@
+Qualcomm Technologies, Inc. CPUFREQ Bindings
+
+CPUFREQ FW 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-fw".
+
+Note that #address-cells, #size-cells, and ranges shall be present to ensure
+the cpufreq can address a freq-domain registers.
+
+A freq-domain sub-node would be defined for the cpus with the following
+properties:
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: must be "cpufreq".
+
+- reg
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Addresses and sizes for the memory of the perf_base
+ , lut_base and en_base.
+- reg-names
+ Usage: required
+ Value type: <stringlist>
+ Definition: Address names. Must be "perf_base", "lut_base",
+ "en_base".
+ Must be specified in the same order as the
+ corresponding addresses are specified in the reg
+ property.
+
+- qcom,cpulist
+ Usage: required
+ Value type: <phandles of CPU>
+ Definition: List of related cpu handles which are under a cluster.
+
+Example:
+ qcom,cpufreq-fw {
+ compatible = "qcom,cpufreq-fw";
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ freq-domain-0 {
+ compatible = "cpufreq";
+ reg = <0x17d43920 0x4>,
+ <0x17d43110 0x500>,
+ <0x17d41000 0x4>;
+ reg-names = "perf_base", "lut_base", "en_base";
+ qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3>;
+ };
+
+ freq-domain-1 {
+ compatible = "cpufreq";
+ reg = <0x17d46120 0x4>,
+ <0x17d45910 0x500>,
+ <0x17d45800 0x4>;
+ reg-names = "perf_base", "lut_base", "en_base";
+ qcom,cpulist = <&CPU4 &CPU5 &CPU6 &CPU7>;
+ };
+ };
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.


2018-05-17 09:39:54

by Amit Kucheria

[permalink] [raw]
Subject: Re: [v0 0/2] Add support for QCOM cpufreq FW driver

On Thu, May 17, 2018 at 12:29 PM, Taniya Das <[email protected]> wrote:
> The CPUfreq FW present in some QCOM chipsets offloads the steps necessary
> for hanging the frequency of CPUs. The driver implements the cpufreq driver

s/hanging/changing :-)

> interface for this firmware.
>
> Taniya Das (2):
> dt-bindings: clock: Introduce QCOM CPUFREQ FW bindings
> cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver
>
> .../bindings/cpufreq/cpufreq-qcom-fw.txt | 68 +++++
> drivers/cpufreq/Kconfig.arm | 9 +
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/qcom-cpufreq-fw.c | 318 +++++++++++++++++++++
> 4 files changed, 396 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
> create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c
>
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
> of the Code Aurora Forum, hosted by the Linux Foundation.
>

2018-05-17 10:14:37

by Viresh Kumar

[permalink] [raw]
Subject: Re: [v0 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

On 17-05-18, 15:00, Taniya Das wrote:
> The CPUfreq FW present in some QCOM chipsets offloads the steps necessary
> for hanging the frequency of CPUs. The driver implements the cpufreq driver
> interface for this firmware.
>
> Signed-off-by: Taniya Das <[email protected]>
> ---
> drivers/cpufreq/Kconfig.arm | 9 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/qcom-cpufreq-fw.c | 318 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 328 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 96b35b8..a50aa6e 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -301,3 +301,12 @@ config ARM_PXA2xx_CPUFREQ
> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>
> If in doubt, say N.
> +
> +config ARM_QCOM_CPUFREQ_FW
> + tristate "QCOM CPUFreq FW driver"
> + help
> + Support for the CPUFreq FW driver.
> + The CPUfreq FW preset in some QCOM chipsets offloads the steps
> + necessary for changing the frequency of CPUs. The driver
> + implements the cpufreq driver interface for this firmware.
> + Say Y if you want to support CPUFreq FW.
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 8d24ade..a3edbce 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_FW) += qcom-cpufreq-fw.o
>
>
> ##################################################################################
> diff --git a/drivers/cpufreq/qcom-cpufreq-fw.c b/drivers/cpufreq/qcom-cpufreq-fw.c
> new file mode 100644
> index 0000000..67996d5
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq-fw.c
> @@ -0,0 +1,318 @@
> +// 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 INIT_RATE 300000000UL
> +#define XO_RATE 19200000UL
> +#define LUT_MAX_ENTRIES 40U
> +#define CORE_COUNT_VAL(val) ((val & GENMASK(18, 16)) >> 16)
> +#define LUT_ROW_SIZE 32
> +
> +struct cpufreq_qcom {
> + struct cpufreq_frequency_table *table;
> + struct device *dev;
> + void __iomem *perf_base;
> + void __iomem *lut_base;
> + cpumask_t related_cpus;
> + unsigned int max_cores;
> +};
> +
> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> +
> +static int
> +qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy, unsigned int index)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> +
> + if (index >= LUT_MAX_ENTRIES) {
> + dev_err(c->dev,
> + "Passing an index (%u) that's greater than max (%d)\n",

Alignment issues here. Run checkpatch --strict.

> + index, LUT_MAX_ENTRIES - 1);
> + return -EINVAL;
> + }
> +
> + writel_relaxed(index, c->perf_base);
> +
> + /* Make sure the write goes through before proceeding */
> + mb();
> + return 0;
> +}
> +
> +static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
> +{
> + struct cpufreq_qcom *c;
> + unsigned int index;
> +
> + c = qcom_freq_domain_map[cpu];
> + if (!c)
> + return -ENODEV;
> +
> + index = readl_relaxed(c->perf_base);
> + index = min(index, LUT_MAX_ENTRIES - 1);
> +
> + return c->table[index].frequency;
> +}
> +
> +static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_qcom *c;
> + int ret;
> +
> + 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->table = c->table;
> + policy->driver_data = c;
> +
> + return ret;
> +}
> +
> +static struct freq_attr *qcom_cpufreq_fw_attr[] = {
> + &cpufreq_freq_attr_scaling_available_freqs,
> + &cpufreq_freq_attr_scaling_boost_freqs,
> + NULL
> +};
> +
> +static struct cpufreq_driver cpufreq_qcom_fw_driver = {
> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target_index = qcom_cpufreq_fw_target_index,
> + .get = qcom_cpufreq_fw_get,
> + .init = qcom_cpufreq_fw_cpu_init,
> + .name = "qcom-cpufreq-fw",
> + .attr = qcom_cpufreq_fw_attr,
> + .boost_enabled = true,
> +};
> +
> +static int qcom_read_lut(struct platform_device *pdev,
> + struct cpufreq_qcom *c)
> +{
> + struct device *dev = &pdev->dev;
> + u32 data, src, lval, i, core_count, prev_cc = 0;
> +
> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
> + sizeof(*c->table), GFP_KERNEL);
> + if (!c->table)
> + return -ENOMEM;
> +
> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> + data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
> + src = ((data & GENMASK(31, 30)) >> 30);
> + lval = (data & GENMASK(7, 0));
> + core_count = CORE_COUNT_VAL(data);

Why do you need this here ? And why do below in case this doesn't
match max-cores count ?

> +
> + if (!src)
> + c->table[i].frequency = INIT_RATE / 1000;
> + else
> + c->table[i].frequency = XO_RATE * lval / 1000;
> +
> + c->table[i].driver_data = 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)
> + c->table[i].frequency = CPUFREQ_ENTRY_INVALID;
> +
> + /*
> + * Two of the same frequencies with the same core counts means
> + * end of table.
> + */
> + if (i > 0 && c->table[i - 1].driver_data ==
> + c->table[i].driver_data
> + && prev_cc == core_count) {
> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
> +
> + if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
> + prev->flags = CPUFREQ_BOOST_FREQ;
> + prev->frequency = prev->driver_data;
> + }
> +
> + break;
> + }
> + prev_cc = core_count;
> + }
> + c->table[i].frequency = CPUFREQ_TABLE_END;
> +
> + return 0;
> +}
> +
> +static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
> +{
> + struct device_node *dev_phandle;
> + struct device *cpu_dev;
> + int cpu, i = 0, ret = -ENOENT;
> +
> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);

TBH, I am not a great fan of the CPU phandle list you have created
here. Lets see what Rob has to say on this.

> + while (dev_phandle) {
> + for_each_possible_cpu(cpu) {
> + cpu_dev = get_cpu_device(cpu);
> + if (cpu_dev && cpu_dev->of_node == dev_phandle) {
> + cpumask_set_cpu(cpu, m);
> + ret = 0;

Maybe just remove this line ...

> + break;
> + }
> + }
> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
> + }
> +
> + return ret;

and check for empty cpumask for an error here.

> +}
> +
> +static int qcom_cpu_resources_init(struct platform_device *pdev,
> + struct device_node *np)

You may want to align these properly. Try running checkpatch with
--strict option.

> +{
> + struct cpufreq_qcom *c;
> + struct resource res;
> + struct device *dev = &pdev->dev;
> + void __iomem *en_base;
> + int cpu, index = 0, ret;

Why initialize index here ?

> +
> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);

Check for a valid 'c' here ?

> +
> + res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");

You are assigning a pointer to a structure here :(

> + if (!res) {
> + dev_err(dev, "Enable base not defined for %s\n", np->name);
> + return ret;
> + }
> +
> + en_base = devm_ioremap(dev, res->start, resource_size(res));

You don't get a build error for doing res->start here ? Looks like you
sent a driver upstream which doesn't even build.

> + if (!en_base) {
> + dev_err(dev, "Unable to map %s en-base\n", np->name);
> + return -ENOMEM;
> + }
> +
> + /* FW should be enabled state to proceed */
> + if (!(readl_relaxed(en_base) & 0x1)) {
> + dev_err(dev, "%s firmware not enabled\n", np->name);
> + return -ENODEV;
> + }
> +
> + devm_iounmap(&pdev->dev, en_base);
> +
> + index = of_property_match_string(np, "reg-names", "perf_base");
> + if (index < 0)
> + return index;
> +
> + if (of_address_to_resource(np, index, &res))
> + return -ENOMEM;
> +
> + c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
> + if (!c->perf_base) {
> + dev_err(dev, "Unable to map %s perf-base\n", np->name);
> + return -ENOMEM;
> + }
> +
> + index = of_property_match_string(np, "reg-names", "lut_base");
> + if (index < 0)
> + return index;
> +
> + if (of_address_to_resource(np, index, &res))
> + return -ENOMEM;
> +
> + c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
> + if (!c->lut_base) {
> + dev_err(dev, "Unable to map %s lut-base\n", np->name);
> + return -ENOMEM;
> + }
> +
> + ret = qcom_get_related_cpus(np, &c->related_cpus);
> + if (ret) {
> + dev_err(dev, "%s failed to get core phandles\n", np->name);
> + return ret;
> + }
> +
> + c->max_cores = cpumask_weight(&c->related_cpus);
> +
> + ret = qcom_read_lut(pdev, c);
> + if (ret) {
> + dev_err(dev, "%s failed to read LUT\n", np->name);
> + return ret;
> + }
> +
> + for_each_cpu(cpu, &c->related_cpus)
> + qcom_freq_domain_map[cpu] = c;
> +
> + return 0;
> +}
> +
> +static int qcom_resources_init(struct platform_device *pdev)
> +{
> + struct device_node *np;
> + int ret = -ENODEV;
> +
> + for_each_available_child_of_node(pdev->dev.of_node, np) {
> + if (of_device_is_compatible(np, "cpufreq")) {
> + ret = qcom_cpu_resources_init(pdev, np);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return ret;

Don't initialize ret to -ENODEV, rather return -ENODEV directly here.
That makes it more readable.

> +}
> +
> +static int qcom_cpufreq_fw_driver_probe(struct platform_device *pdev)
> +{
> + int rc = 0;

Don't need to initialize to 0 here.

> +
> + /* 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_fw_driver);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq FW driver failed to register\n");
> + return rc;
> + }
> +
> + dev_info(&pdev->dev, "QCOM CPUFreq FW driver inited\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id match_table[] = {
> + { .compatible = "qcom,cpufreq-fw" },
> + {}
> +};
> +
> +static struct platform_driver qcom_cpufreq_fw_driver = {
> + .probe = qcom_cpufreq_fw_driver_probe,
> + .driver = {
> + .name = "qcom-cpufreq-fw",
> + .of_match_table = match_table,
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init qcom_cpufreq_fw_init(void)
> +{
> + return platform_driver_register(&qcom_cpufreq_fw_driver);
> +}
> +subsys_initcall(qcom_cpufreq_fw_init);

Why this for a driver which can be built as a module ? You really want
it to be built as a module ?

> +
> +static void __exit qcom_cpufreq_fw_exit(void)
> +{
> + platform_driver_unregister(&qcom_cpufreq_fw_driver);
> +}

But you don't unregister the cpufreq driver ?

> +module_exit(qcom_cpufreq_fw_exit);
> +
> +MODULE_DESCRIPTION("QCOM CPU Frequency FW");
> +MODULE_LICENSE("GPL v2");
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
> of the Code Aurora Forum, hosted by the Linux Foundation.

--
viresh

2018-05-17 10:15:25

by Viresh Kumar

[permalink] [raw]
Subject: Re: [v0 1/2] dt-bindings: clock: Introduce QCOM CPUFREQ FW bindings

+ Rob.

On 17-05-18, 15:00, 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 firmware.
>
> Signed-off-by: Taniya Das <[email protected]>
> ---
> .../bindings/cpufreq/cpufreq-qcom-fw.txt | 68 ++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
>
> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
> new file mode 100644
> index 0000000..bc912f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
> @@ -0,0 +1,68 @@
> +Qualcomm Technologies, Inc. CPUFREQ Bindings
> +
> +CPUFREQ FW 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-fw".
> +
> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
> +the cpufreq can address a freq-domain registers.
> +
> +A freq-domain sub-node would be defined for the cpus with the following
> +properties:
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be "cpufreq".
> +
> +- reg
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: Addresses and sizes for the memory of the perf_base
> + , lut_base and en_base.
> +- reg-names
> + Usage: required
> + Value type: <stringlist>
> + Definition: Address names. Must be "perf_base", "lut_base",
> + "en_base".
> + Must be specified in the same order as the
> + corresponding addresses are specified in the reg
> + property.
> +
> +- qcom,cpulist
> + Usage: required
> + Value type: <phandles of CPU>
> + Definition: List of related cpu handles which are under a cluster.
> +
> +Example:
> + qcom,cpufreq-fw {
> + compatible = "qcom,cpufreq-fw";
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + freq-domain-0 {
> + compatible = "cpufreq";
> + reg = <0x17d43920 0x4>,
> + <0x17d43110 0x500>,
> + <0x17d41000 0x4>;
> + reg-names = "perf_base", "lut_base", "en_base";
> + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3>;
> + };
> +
> + freq-domain-1 {
> + compatible = "cpufreq";
> + reg = <0x17d46120 0x4>,
> + <0x17d45910 0x500>,
> + <0x17d45800 0x4>;
> + reg-names = "perf_base", "lut_base", "en_base";
> + qcom,cpulist = <&CPU4 &CPU5 &CPU6 &CPU7>;
> + };
> + };
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
> of the Code Aurora Forum, hosted by the Linux Foundation.

--
viresh

2018-05-17 10:28:53

by Amit Kucheria

[permalink] [raw]
Subject: Re: [v0 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

On Thu, May 17, 2018 at 12:30 PM, Taniya Das <[email protected]> wrote:
> The CPUfreq FW present in some QCOM chipsets offloads the steps necessary
> for hanging the frequency of CPUs. The driver implements the cpufreq driver

s/hanging/changing :-)

> interface for this firmware.
>
> Signed-off-by: Taniya Das <[email protected]>
> ---
> drivers/cpufreq/Kconfig.arm | 9 ++
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/qcom-cpufreq-fw.c | 318 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 328 insertions(+)
> create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 96b35b8..a50aa6e 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -301,3 +301,12 @@ config ARM_PXA2xx_CPUFREQ
> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>
> If in doubt, say N.
> +
> +config ARM_QCOM_CPUFREQ_FW
> + tristate "QCOM CPUFreq FW driver"
> + help
> + Support for the CPUFreq FW driver.
> + The CPUfreq FW preset in some QCOM chipsets offloads the steps
> + necessary for changing the frequency of CPUs. The driver
> + implements the cpufreq driver interface for this firmware.
> + Say Y if you want to support CPUFreq FW.
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 8d24ade..a3edbce 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_FW) += qcom-cpufreq-fw.o
>
>
> ##################################################################################
> diff --git a/drivers/cpufreq/qcom-cpufreq-fw.c b/drivers/cpufreq/qcom-cpufreq-fw.c
> new file mode 100644
> index 0000000..67996d5
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq-fw.c
> @@ -0,0 +1,318 @@
> +// 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 INIT_RATE 300000000UL
> +#define XO_RATE 19200000UL
> +#define LUT_MAX_ENTRIES 40U
> +#define CORE_COUNT_VAL(val) ((val & GENMASK(18, 16)) >> 16)
> +#define LUT_ROW_SIZE 32
> +
> +struct cpufreq_qcom {
> + struct cpufreq_frequency_table *table;
> + struct device *dev;
> + void __iomem *perf_base;
> + void __iomem *lut_base;
> + cpumask_t related_cpus;
> + unsigned int max_cores;
> +};
> +
> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
> +
> +static int
> +qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy, unsigned int index)
> +{
> + struct cpufreq_qcom *c = policy->driver_data;
> +
> + if (index >= LUT_MAX_ENTRIES) {
> + dev_err(c->dev,
> + "Passing an index (%u) that's greater than max (%d)\n",
> + index, LUT_MAX_ENTRIES - 1);
> + return -EINVAL;
> + }
> +
> + writel_relaxed(index, c->perf_base);
> +
> + /* Make sure the write goes through before proceeding */
> + mb();
> + return 0;
> +}
> +
> +static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
> +{
> + struct cpufreq_qcom *c;
> + unsigned int index;
> +
> + c = qcom_freq_domain_map[cpu];
> + if (!c)
> + return -ENODEV;
> +
> + index = readl_relaxed(c->perf_base);
> + index = min(index, LUT_MAX_ENTRIES - 1);
> +
> + return c->table[index].frequency;
> +}
> +
> +static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_qcom *c;
> + int ret;
> +
> + 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->table = c->table;
> + policy->driver_data = c;
> +
> + return ret;
> +}
> +
> +static struct freq_attr *qcom_cpufreq_fw_attr[] = {
> + &cpufreq_freq_attr_scaling_available_freqs,
> + &cpufreq_freq_attr_scaling_boost_freqs,
> + NULL
> +};
> +
> +static struct cpufreq_driver cpufreq_qcom_fw_driver = {
> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target_index = qcom_cpufreq_fw_target_index,
> + .get = qcom_cpufreq_fw_get,
> + .init = qcom_cpufreq_fw_cpu_init,
> + .name = "qcom-cpufreq-fw",
> + .attr = qcom_cpufreq_fw_attr,
> + .boost_enabled = true,
> +};
> +
> +static int qcom_read_lut(struct platform_device *pdev,
> + struct cpufreq_qcom *c)
> +{
> + struct device *dev = &pdev->dev;
> + u32 data, src, lval, i, core_count, prev_cc = 0;


Get rid of prev_cc variable initialisation here and in other places
through out the code

> +
> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
> + sizeof(*c->table), GFP_KERNEL);
> + if (!c->table)
> + return -ENOMEM;
> +
> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> + data = readl_relaxed(c->lut_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 = INIT_RATE / 1000;
> + else
> + c->table[i].frequency = XO_RATE * lval / 1000;
> +
> + c->table[i].driver_data = 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)
> + c->table[i].frequency = CPUFREQ_ENTRY_INVALID;
> +
> + /*
> + * Two of the same frequencies with the same core counts means
> + * end of table.
> + */
> + if (i > 0 && c->table[i - 1].driver_data ==
> + c->table[i].driver_data
> + && prev_cc == core_count) {
> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
> +
> + if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
> + prev->flags = CPUFREQ_BOOST_FREQ;
> + prev->frequency = prev->driver_data;
> + }
> +
> + break;
> + }
> + prev_cc = core_count;
> + }
> + c->table[i].frequency = CPUFREQ_TABLE_END;
> +
> + return 0;
> +}
> +
> +static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
> +{
> + struct device_node *dev_phandle;
> + struct device *cpu_dev;
> + int cpu, i = 0, ret = -ENOENT;
> +
> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
> + while (dev_phandle) {
> + for_each_possible_cpu(cpu) {
> + cpu_dev = get_cpu_device(cpu);
> + if (cpu_dev && cpu_dev->of_node == dev_phandle) {
> + cpumask_set_cpu(cpu, m);
> + ret = 0;
> + break;
> + }
> + }
> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
> + }
> +
> + return ret;
> +}
> +
> +static int qcom_cpu_resources_init(struct platform_device *pdev,
> + struct device_node *np)
> +{
> + struct cpufreq_qcom *c;
> + struct resource res;
> + struct device *dev = &pdev->dev;
> + void __iomem *en_base;
> + int cpu, index = 0, ret;
> +
> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);

Check if memory allocated successfully?

> +
> + res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
> + if (!res) {
> + dev_err(dev, "Enable base not defined for %s\n", np->name);
> + return ret;
> + }


This is not going to build. Did you build-test this on a 4.17-rc kenel ?

> +
> + en_base = devm_ioremap(dev, res->start, resource_size(res));
> + if (!en_base) {
> + dev_err(dev, "Unable to map %s en-base\n", np->name);
> + return -ENOMEM;
> + }
> +
> + /* FW should be enabled state to proceed */
> + if (!(readl_relaxed(en_base) & 0x1)) {
> + dev_err(dev, "%s firmware not enabled\n", np->name);
> + return -ENODEV;
> + }
> +
> + devm_iounmap(&pdev->dev, en_base);
> +
> + index = of_property_match_string(np, "reg-names", "perf_base");
> + if (index < 0)
> + return index;
> +
> + if (of_address_to_resource(np, index, &res))
> + return -ENOMEM;
> +
> + c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
> + if (!c->perf_base) {
> + dev_err(dev, "Unable to map %s perf-base\n", np->name);
> + return -ENOMEM;
> + }
> +
> + index = of_property_match_string(np, "reg-names", "lut_base");
> + if (index < 0)
> + return index;
> +
> + if (of_address_to_resource(np, index, &res))
> + return -ENOMEM;
> +
> + c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
> + if (!c->lut_base) {
> + dev_err(dev, "Unable to map %s lut-base\n", np->name);
> + return -ENOMEM;
> + }
> +
> + ret = qcom_get_related_cpus(np, &c->related_cpus);
> + if (ret) {
> + dev_err(dev, "%s failed to get core phandles\n", np->name);
> + return ret;
> + }
> +
> + c->max_cores = cpumask_weight(&c->related_cpus);
> +
> + ret = qcom_read_lut(pdev, c);
> + if (ret) {
> + dev_err(dev, "%s failed to read LUT\n", np->name);
> + return ret;
> + }
> +
> + for_each_cpu(cpu, &c->related_cpus)
> + qcom_freq_domain_map[cpu] = c;
> +
> + return 0;
> +}
> +
> +static int qcom_resources_init(struct platform_device *pdev)
> +{
> + struct device_node *np;
> + int ret = -ENODEV;
> +
> + for_each_available_child_of_node(pdev->dev.of_node, np) {
> + if (of_device_is_compatible(np, "cpufreq")) {
> + ret = qcom_cpu_resources_init(pdev, np);
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int qcom_cpufreq_fw_driver_probe(struct platform_device *pdev)
> +{
> + int rc = 0;
> +
> + /* 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_fw_driver);
> + if (rc) {
> + dev_err(&pdev->dev, "CPUFreq FW driver failed to register\n");
> + return rc;
> + }
> +
> + dev_info(&pdev->dev, "QCOM CPUFreq FW driver inited\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id match_table[] = {
> + { .compatible = "qcom,cpufreq-fw" },
> + {}
> +};
> +
> +static struct platform_driver qcom_cpufreq_fw_driver = {
> + .probe = qcom_cpufreq_fw_driver_probe,
> + .driver = {
> + .name = "qcom-cpufreq-fw",
> + .of_match_table = match_table,
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +static int __init qcom_cpufreq_fw_init(void)
> +{
> + return platform_driver_register(&qcom_cpufreq_fw_driver);
> +}
> +subsys_initcall(qcom_cpufreq_fw_init);
> +
> +static void __exit qcom_cpufreq_fw_exit(void)
> +{
> + platform_driver_unregister(&qcom_cpufreq_fw_driver);
> +}
> +module_exit(qcom_cpufreq_fw_exit);
> +
> +MODULE_DESCRIPTION("QCOM CPU Frequency FW");
> +MODULE_LICENSE("GPL v2");
> --
> Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
> of the Code Aurora Forum, hosted by the Linux Foundation.
>

2018-05-17 17:15:58

by Taniya Das

[permalink] [raw]
Subject: Re: [v0 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

Hi Viresh,

Thanks for the review comments, I have already fixed the resource
comment in the v1 series which I sent across. I will fix the rest of the
comments and send it for review.

On 5/17/2018 3:44 PM, Viresh Kumar wrote:
> On 17-05-18, 15:00, Taniya Das wrote:
>> The CPUfreq FW present in some QCOM chipsets offloads the steps necessary
>> for hanging the frequency of CPUs. The driver implements the cpufreq driver
>> interface for this firmware.
>>
>> Signed-off-by: Taniya Das <[email protected]>
>> ---
>> drivers/cpufreq/Kconfig.arm | 9 ++
>> drivers/cpufreq/Makefile | 1 +
>> drivers/cpufreq/qcom-cpufreq-fw.c | 318 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 328 insertions(+)
>> create mode 100644 drivers/cpufreq/qcom-cpufreq-fw.c
>>
>> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
>> index 96b35b8..a50aa6e 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -301,3 +301,12 @@ config ARM_PXA2xx_CPUFREQ
>> This add the CPUFreq driver support for Intel PXA2xx SOCs.
>>
>> If in doubt, say N.
>> +
>> +config ARM_QCOM_CPUFREQ_FW
>> + tristate "QCOM CPUFreq FW driver"
>> + help
>> + Support for the CPUFreq FW driver.
>> + The CPUfreq FW preset in some QCOM chipsets offloads the steps
>> + necessary for changing the frequency of CPUs. The driver
>> + implements the cpufreq driver interface for this firmware.
>> + Say Y if you want to support CPUFreq FW.
>> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
>> index 8d24ade..a3edbce 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_FW) += qcom-cpufreq-fw.o
>>
>>
>> ##################################################################################
>> diff --git a/drivers/cpufreq/qcom-cpufreq-fw.c b/drivers/cpufreq/qcom-cpufreq-fw.c
>> new file mode 100644
>> index 0000000..67996d5
>> --- /dev/null
>> +++ b/drivers/cpufreq/qcom-cpufreq-fw.c
>> @@ -0,0 +1,318 @@
>> +// 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 INIT_RATE 300000000UL
>> +#define XO_RATE 19200000UL
>> +#define LUT_MAX_ENTRIES 40U
>> +#define CORE_COUNT_VAL(val) ((val & GENMASK(18, 16)) >> 16)
>> +#define LUT_ROW_SIZE 32
>> +
>> +struct cpufreq_qcom {
>> + struct cpufreq_frequency_table *table;
>> + struct device *dev;
>> + void __iomem *perf_base;
>> + void __iomem *lut_base;
>> + cpumask_t related_cpus;
>> + unsigned int max_cores;
>> +};
>> +
>> +static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
>> +
>> +static int
>> +qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy, unsigned int index)
>> +{
>> + struct cpufreq_qcom *c = policy->driver_data;
>> +
>> + if (index >= LUT_MAX_ENTRIES) {
>> + dev_err(c->dev,
>> + "Passing an index (%u) that's greater than max (%d)\n",
>
> Alignment issues here. Run checkpatch --strict.
>
>> + index, LUT_MAX_ENTRIES - 1);
>> + return -EINVAL;
>> + }
>> +
>> + writel_relaxed(index, c->perf_base);
>> +
>> + /* Make sure the write goes through before proceeding */
>> + mb();
>> + return 0;
>> +}
>> +
>> +static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
>> +{
>> + struct cpufreq_qcom *c;
>> + unsigned int index;
>> +
>> + c = qcom_freq_domain_map[cpu];
>> + if (!c)
>> + return -ENODEV;
>> +
>> + index = readl_relaxed(c->perf_base);
>> + index = min(index, LUT_MAX_ENTRIES - 1);
>> +
>> + return c->table[index].frequency;
>> +}
>> +
>> +static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
>> +{
>> + struct cpufreq_qcom *c;
>> + int ret;
>> +
>> + 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->table = c->table;
>> + policy->driver_data = c;
>> +
>> + return ret;
>> +}
>> +
>> +static struct freq_attr *qcom_cpufreq_fw_attr[] = {
>> + &cpufreq_freq_attr_scaling_available_freqs,
>> + &cpufreq_freq_attr_scaling_boost_freqs,
>> + NULL
>> +};
>> +
>> +static struct cpufreq_driver cpufreq_qcom_fw_driver = {
>> + .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
>> + CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
>> + .verify = cpufreq_generic_frequency_table_verify,
>> + .target_index = qcom_cpufreq_fw_target_index,
>> + .get = qcom_cpufreq_fw_get,
>> + .init = qcom_cpufreq_fw_cpu_init,
>> + .name = "qcom-cpufreq-fw",
>> + .attr = qcom_cpufreq_fw_attr,
>> + .boost_enabled = true,
>> +};
>> +
>> +static int qcom_read_lut(struct platform_device *pdev,
>> + struct cpufreq_qcom *c)
>> +{
>> + struct device *dev = &pdev->dev;
>> + u32 data, src, lval, i, core_count, prev_cc = 0;
>> +
>> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
>> + sizeof(*c->table), GFP_KERNEL);
>> + if (!c->table)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>> + data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
>> + src = ((data & GENMASK(31, 30)) >> 30);
>> + lval = (data & GENMASK(7, 0));
>> + core_count = CORE_COUNT_VAL(data);
>
> Why do you need this here ? And why do below in case this doesn't
> match max-cores count ?
>
>> +
>> + if (!src)
>> + c->table[i].frequency = INIT_RATE / 1000;
>> + else
>> + c->table[i].frequency = XO_RATE * lval / 1000;
>> +
>> + c->table[i].driver_data = 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)
>> + c->table[i].frequency = CPUFREQ_ENTRY_INVALID;
>> +
>> + /*
>> + * Two of the same frequencies with the same core counts means
>> + * end of table.
>> + */
>> + if (i > 0 && c->table[i - 1].driver_data ==
>> + c->table[i].driver_data
>> + && prev_cc == core_count) {
>> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
>> +
>> + if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
>> + prev->flags = CPUFREQ_BOOST_FREQ;
>> + prev->frequency = prev->driver_data;
>> + }
>> +
>> + break;
>> + }
>> + prev_cc = core_count;
>> + }
>> + c->table[i].frequency = CPUFREQ_TABLE_END;
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
>> +{
>> + struct device_node *dev_phandle;
>> + struct device *cpu_dev;
>> + int cpu, i = 0, ret = -ENOENT;
>> +
>> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
>
> TBH, I am not a great fan of the CPU phandle list you have created
> here. Lets see what Rob has to say on this.
>
>> + while (dev_phandle) {
>> + for_each_possible_cpu(cpu) {
>> + cpu_dev = get_cpu_device(cpu);
>> + if (cpu_dev && cpu_dev->of_node == dev_phandle) {
>> + cpumask_set_cpu(cpu, m);
>> + ret = 0;
>
> Maybe just remove this line ...
>
>> + break;
>> + }
>> + }
>> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
>> + }
>> +
>> + return ret;
>
> and check for empty cpumask for an error here.
>
>> +}
>> +
>> +static int qcom_cpu_resources_init(struct platform_device *pdev,
>> + struct device_node *np)
>
> You may want to align these properly. Try running checkpatch with
> --strict option.
>
>> +{
>> + struct cpufreq_qcom *c;
>> + struct resource res;
>> + struct device *dev = &pdev->dev;
>> + void __iomem *en_base;
>> + int cpu, index = 0, ret;
>
> Why initialize index here ?
>
>> +
>> + c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
>
> Check for a valid 'c' here ?
>
>> +
>> + res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
>
> You are assigning a pointer to a structure here :(
>
>> + if (!res) {
>> + dev_err(dev, "Enable base not defined for %s\n", np->name);
>> + return ret;
>> + }
>> +
>> + en_base = devm_ioremap(dev, res->start, resource_size(res));
>
> You don't get a build error for doing res->start here ? Looks like you
> sent a driver upstream which doesn't even build.
>
>> + if (!en_base) {
>> + dev_err(dev, "Unable to map %s en-base\n", np->name);
>> + return -ENOMEM;
>> + }
>> +
>> + /* FW should be enabled state to proceed */
>> + if (!(readl_relaxed(en_base) & 0x1)) {
>> + dev_err(dev, "%s firmware not enabled\n", np->name);
>> + return -ENODEV;
>> + }
>> +
>> + devm_iounmap(&pdev->dev, en_base);
>> +
>> + index = of_property_match_string(np, "reg-names", "perf_base");
>> + if (index < 0)
>> + return index;
>> +
>> + if (of_address_to_resource(np, index, &res))
>> + return -ENOMEM;
>> +
>> + c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
>> + if (!c->perf_base) {
>> + dev_err(dev, "Unable to map %s perf-base\n", np->name);
>> + return -ENOMEM;
>> + }
>> +
>> + index = of_property_match_string(np, "reg-names", "lut_base");
>> + if (index < 0)
>> + return index;
>> +
>> + if (of_address_to_resource(np, index, &res))
>> + return -ENOMEM;
>> +
>> + c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
>> + if (!c->lut_base) {
>> + dev_err(dev, "Unable to map %s lut-base\n", np->name);
>> + return -ENOMEM;
>> + }
>> +
>> + ret = qcom_get_related_cpus(np, &c->related_cpus);
>> + if (ret) {
>> + dev_err(dev, "%s failed to get core phandles\n", np->name);
>> + return ret;
>> + }
>> +
>> + c->max_cores = cpumask_weight(&c->related_cpus);
>> +
>> + ret = qcom_read_lut(pdev, c);
>> + if (ret) {
>> + dev_err(dev, "%s failed to read LUT\n", np->name);
>> + return ret;
>> + }
>> +
>> + for_each_cpu(cpu, &c->related_cpus)
>> + qcom_freq_domain_map[cpu] = c;
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_resources_init(struct platform_device *pdev)
>> +{
>> + struct device_node *np;
>> + int ret = -ENODEV;
>> +
>> + for_each_available_child_of_node(pdev->dev.of_node, np) {
>> + if (of_device_is_compatible(np, "cpufreq")) {
>> + ret = qcom_cpu_resources_init(pdev, np);
>> + if (ret)
>> + return ret;
>> + }
>> + }
>> +
>> + return ret;
>
> Don't initialize ret to -ENODEV, rather return -ENODEV directly here.
> That makes it more readable.
>
>> +}
>> +
>> +static int qcom_cpufreq_fw_driver_probe(struct platform_device *pdev)
>> +{
>> + int rc = 0;
>
> Don't need to initialize to 0 here.
>
>> +
>> + /* 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_fw_driver);
>> + if (rc) {
>> + dev_err(&pdev->dev, "CPUFreq FW driver failed to register\n");
>> + return rc;
>> + }
>> +
>> + dev_info(&pdev->dev, "QCOM CPUFreq FW driver inited\n");
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id match_table[] = {
>> + { .compatible = "qcom,cpufreq-fw" },
>> + {}
>> +};
>> +
>> +static struct platform_driver qcom_cpufreq_fw_driver = {
>> + .probe = qcom_cpufreq_fw_driver_probe,
>> + .driver = {
>> + .name = "qcom-cpufreq-fw",
>> + .of_match_table = match_table,
>> + .owner = THIS_MODULE,
>> + },
>> +};
>> +
>> +static int __init qcom_cpufreq_fw_init(void)
>> +{
>> + return platform_driver_register(&qcom_cpufreq_fw_driver);
>> +}
>> +subsys_initcall(qcom_cpufreq_fw_init);
>
> Why this for a driver which can be built as a module ? You really want
> it to be built as a module ?
>
>> +
>> +static void __exit qcom_cpufreq_fw_exit(void)
>> +{
>> + platform_driver_unregister(&qcom_cpufreq_fw_driver);
>> +}
>
> But you don't unregister the cpufreq driver ?
>
>> +module_exit(qcom_cpufreq_fw_exit);
>> +
>> +MODULE_DESCRIPTION("QCOM CPU Frequency FW");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 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.

--

2018-05-17 19:17:26

by Saravana Kannan

[permalink] [raw]
Subject: Re: [v0 1/2] dt-bindings: clock: Introduce QCOM CPUFREQ FW bindings

On 05/17/2018 03:14 AM, Viresh Kumar wrote:
> + Rob.
>
> On 17-05-18, 15:00, 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 firmware.
>>
>> Signed-off-by: Taniya Das <[email protected]>
>> ---
>> .../bindings/cpufreq/cpufreq-qcom-fw.txt | 68 ++++++++++++++++++++++
>> 1 file changed, 68 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt

Isn't the prefix wrong? Shouldn't it be dt-bindings: cpufreq:?

-Saravana


--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2018-05-17 19:26:32

by Saravana Kannan

[permalink] [raw]
Subject: Re: [v0 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

On 05/17/2018 03:14 AM, Viresh Kumar wrote:
> On 17-05-18, 15:00, Taniya Das wrote:
>> The CPUfreq FW present in some QCOM chipsets offloads the steps necessary
>> for hanging the frequency of CPUs. The driver implements the cpufreq driver
>> interface for this firmware.
>>
>> Signed-off-by: Taniya Das <[email protected]>
>> ---
>>
>> ##################################################################################
>> diff --git a/drivers/cpufreq/qcom-cpufreq-fw.c b/drivers/cpufreq/qcom-cpufreq-fw.c

>> +
>> +static int qcom_read_lut(struct platform_device *pdev,
>> + struct cpufreq_qcom *c)
>> +{
>> + struct device *dev = &pdev->dev;
>> + u32 data, src, lval, i, core_count, prev_cc = 0;
>> +
>> + c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
>> + sizeof(*c->table), GFP_KERNEL);
>> + if (!c->table)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>> + data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
>> + src = ((data & GENMASK(31, 30)) >> 30);
>> + lval = (data & GENMASK(7, 0));
>> + core_count = CORE_COUNT_VAL(data);
>
> Why do you need this here ? And why do below in case this doesn't
> match max-cores count ?

This is how we detect boost frequencies.

>> +
>> + if (!src)
>> + c->table[i].frequency = INIT_RATE / 1000;
>> + else
>> + c->table[i].frequency = XO_RATE * lval / 1000;
>> +
>> + c->table[i].driver_data = 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)
>> + c->table[i].frequency = CPUFREQ_ENTRY_INVALID;

The FW might has some frequencies marked as "boost frequencies" when
there are higher non-boost frequencies. So, we mark them as invalid.

>> +
>> + /*
>> + * Two of the same frequencies with the same core counts means
>> + * end of table.
>> + */
>> + if (i > 0 && c->table[i - 1].driver_data ==
>> + c->table[i].driver_data
>> + && prev_cc == core_count) {
>> + struct cpufreq_frequency_table *prev = &c->table[i - 1];
>> +
>> + if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
>> + prev->flags = CPUFREQ_BOOST_FREQ;
>> + prev->frequency = prev->driver_data;
>> + }
>> +
>> + break;
>> + }
>> + prev_cc = core_count;
>> + }
>> + c->table[i].frequency = CPUFREQ_TABLE_END;
>> +
>> + return 0;
>> +}
>> +
>> +static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
>> +{
>> + struct device_node *dev_phandle;
>> + struct device *cpu_dev;
>> + int cpu, i = 0, ret = -ENOENT;
>> +
>> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
>
> TBH, I am not a great fan of the CPU phandle list you have created
> here. Lets see what Rob has to say on this.
>

Neither do we, but this is the only real way of mapping the logical CPU
numbers to the real CPUs in HW that belong to the same freq domain.
Because boot CPU is always going to be CPU0 if I'm not mistaken.

>> + while (dev_phandle) {
>> + for_each_possible_cpu(cpu) {
>> + cpu_dev = get_cpu_device(cpu);
>> + if (cpu_dev && cpu_dev->of_node == dev_phandle) {
>> + cpumask_set_cpu(cpu, m);
>> + ret = 0;
>
> Maybe just remove this line ...
>
>> + break;
>> + }
>> + }
>> + dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
>> + }
>> +
>> + return ret;


--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2018-05-19 01:24:57

by kernel test robot

[permalink] [raw]
Subject: Re: [v0 2/2] cpufreq: qcom-fw: Add support for QCOM cpufreq FW driver

Hi Taniya,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Taniya-Das/Add-support-for-QCOM-cpufreq-FW-driver/20180519-050902
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm

All errors (new ones prefixed by >>):

drivers/cpufreq/qcom-cpufreq-fw.c: In function 'qcom_cpufreq_fw_cpu_init':
>> drivers/cpufreq/qcom-cpufreq-fw.c:77:8: error: 'struct cpufreq_policy' has no member named 'table'
policy->table = c->table;
^~
drivers/cpufreq/qcom-cpufreq-fw.c: In function 'qcom_cpu_resources_init':
>> drivers/cpufreq/qcom-cpufreq-fw.c:187:37: error: passing argument 1 of 'platform_get_resource_byname' from incompatible pointer type [-Werror=incompatible-pointer-types]
res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
^~~
In file included from include/linux/of_device.h:6:0,
from include/linux/of_platform.h:12,
from drivers/cpufreq/qcom-cpufreq-fw.c:11:
include/linux/platform_device.h:56:25: note: expected 'struct platform_device *' but argument is of type 'struct device *'
extern struct resource *platform_get_resource_byname(struct platform_device *,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/cpufreq/qcom-cpufreq-fw.c:187:6: error: incompatible types when assigning to type 'struct resource' from type 'struct resource *'
res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
^
>> drivers/cpufreq/qcom-cpufreq-fw.c:188:6: error: wrong type argument to unary exclamation mark
if (!res) {
^
>> drivers/cpufreq/qcom-cpufreq-fw.c:193:33: error: invalid type argument of '->' (have 'struct resource')
en_base = devm_ioremap(dev, res->start, resource_size(res));
^~
>> drivers/cpufreq/qcom-cpufreq-fw.c:193:56: error: incompatible type for argument 1 of 'resource_size'
en_base = devm_ioremap(dev, res->start, resource_size(res));
^~~
In file included from include/linux/of_address.h:4:0,
from drivers/cpufreq/qcom-cpufreq-fw.c:10:
include/linux/ioport.h:196:31: note: expected 'const struct resource *' but argument is of type 'struct resource'
static inline resource_size_t resource_size(const struct resource *res)
^~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +77 drivers/cpufreq/qcom-cpufreq-fw.c

> 11 #include <linux/of_platform.h>
12
13 #define INIT_RATE 300000000UL
14 #define XO_RATE 19200000UL
15 #define LUT_MAX_ENTRIES 40U
16 #define CORE_COUNT_VAL(val) ((val & GENMASK(18, 16)) >> 16)
17 #define LUT_ROW_SIZE 32
18
19 struct cpufreq_qcom {
20 struct cpufreq_frequency_table *table;
21 struct device *dev;
22 void __iomem *perf_base;
23 void __iomem *lut_base;
24 cpumask_t related_cpus;
25 unsigned int max_cores;
26 };
27
28 static struct cpufreq_qcom *qcom_freq_domain_map[NR_CPUS];
29
30 static int
31 qcom_cpufreq_fw_target_index(struct cpufreq_policy *policy, unsigned int index)
32 {
33 struct cpufreq_qcom *c = policy->driver_data;
34
35 if (index >= LUT_MAX_ENTRIES) {
36 dev_err(c->dev,
37 "Passing an index (%u) that's greater than max (%d)\n",
38 index, LUT_MAX_ENTRIES - 1);
39 return -EINVAL;
40 }
41
42 writel_relaxed(index, c->perf_base);
43
44 /* Make sure the write goes through before proceeding */
45 mb();
46 return 0;
47 }
48
49 static unsigned int qcom_cpufreq_fw_get(unsigned int cpu)
50 {
51 struct cpufreq_qcom *c;
52 unsigned int index;
53
54 c = qcom_freq_domain_map[cpu];
55 if (!c)
56 return -ENODEV;
57
58 index = readl_relaxed(c->perf_base);
59 index = min(index, LUT_MAX_ENTRIES - 1);
60
61 return c->table[index].frequency;
62 }
63
64 static int qcom_cpufreq_fw_cpu_init(struct cpufreq_policy *policy)
65 {
66 struct cpufreq_qcom *c;
67 int ret;
68
69 c = qcom_freq_domain_map[policy->cpu];
70 if (!c) {
71 pr_err("No scaling support for CPU%d\n", policy->cpu);
72 return -ENODEV;
73 }
74
75 cpumask_copy(policy->cpus, &c->related_cpus);
76
> 77 policy->table = c->table;
78 policy->driver_data = c;
79
80 return ret;
81 }
82
83 static struct freq_attr *qcom_cpufreq_fw_attr[] = {
84 &cpufreq_freq_attr_scaling_available_freqs,
85 &cpufreq_freq_attr_scaling_boost_freqs,
86 NULL
87 };
88
89 static struct cpufreq_driver cpufreq_qcom_fw_driver = {
90 .flags = CPUFREQ_STICKY | CPUFREQ_NEED_INITIAL_FREQ_CHECK |
91 CPUFREQ_HAVE_GOVERNOR_PER_POLICY,
92 .verify = cpufreq_generic_frequency_table_verify,
93 .target_index = qcom_cpufreq_fw_target_index,
94 .get = qcom_cpufreq_fw_get,
95 .init = qcom_cpufreq_fw_cpu_init,
96 .name = "qcom-cpufreq-fw",
97 .attr = qcom_cpufreq_fw_attr,
98 .boost_enabled = true,
99 };
100
101 static int qcom_read_lut(struct platform_device *pdev,
102 struct cpufreq_qcom *c)
103 {
104 struct device *dev = &pdev->dev;
105 u32 data, src, lval, i, core_count, prev_cc = 0;
106
107 c->table = devm_kcalloc(dev, LUT_MAX_ENTRIES + 1,
108 sizeof(*c->table), GFP_KERNEL);
109 if (!c->table)
110 return -ENOMEM;
111
112 for (i = 0; i < LUT_MAX_ENTRIES; i++) {
113 data = readl_relaxed(c->lut_base + i * LUT_ROW_SIZE);
114 src = ((data & GENMASK(31, 30)) >> 30);
115 lval = (data & GENMASK(7, 0));
116 core_count = CORE_COUNT_VAL(data);
117
118 if (!src)
119 c->table[i].frequency = INIT_RATE / 1000;
120 else
121 c->table[i].frequency = XO_RATE * lval / 1000;
122
123 c->table[i].driver_data = c->table[i].frequency;
124
125 dev_dbg(dev, "index=%d freq=%d, core_count %d\n",
126 i, c->table[i].frequency, core_count);
127
128 if (core_count != c->max_cores)
129 c->table[i].frequency = CPUFREQ_ENTRY_INVALID;
130
131 /*
132 * Two of the same frequencies with the same core counts means
133 * end of table.
134 */
135 if (i > 0 && c->table[i - 1].driver_data ==
136 c->table[i].driver_data
137 && prev_cc == core_count) {
138 struct cpufreq_frequency_table *prev = &c->table[i - 1];
139
140 if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
141 prev->flags = CPUFREQ_BOOST_FREQ;
142 prev->frequency = prev->driver_data;
143 }
144
145 break;
146 }
147 prev_cc = core_count;
148 }
149 c->table[i].frequency = CPUFREQ_TABLE_END;
150
151 return 0;
152 }
153
154 static int qcom_get_related_cpus(struct device_node *np, struct cpumask *m)
155 {
156 struct device_node *dev_phandle;
157 struct device *cpu_dev;
158 int cpu, i = 0, ret = -ENOENT;
159
160 dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
161 while (dev_phandle) {
162 for_each_possible_cpu(cpu) {
163 cpu_dev = get_cpu_device(cpu);
164 if (cpu_dev && cpu_dev->of_node == dev_phandle) {
165 cpumask_set_cpu(cpu, m);
166 ret = 0;
167 break;
168 }
169 }
170 dev_phandle = of_parse_phandle(np, "qcom,cpulist", i++);
171 }
172
173 return ret;
174 }
175
176 static int qcom_cpu_resources_init(struct platform_device *pdev,
177 struct device_node *np)
178 {
179 struct cpufreq_qcom *c;
180 struct resource res;
181 struct device *dev = &pdev->dev;
182 void __iomem *en_base;
183 int cpu, index = 0, ret;
184
185 c = devm_kzalloc(dev, sizeof(*c), GFP_KERNEL);
186
> 187 res = platform_get_resource_byname(dev, IORESOURCE_MEM, "en_base");
> 188 if (!res) {
189 dev_err(dev, "Enable base not defined for %s\n", np->name);
190 return ret;
191 }
192
> 193 en_base = devm_ioremap(dev, res->start, resource_size(res));
194 if (!en_base) {
195 dev_err(dev, "Unable to map %s en-base\n", np->name);
196 return -ENOMEM;
197 }
198
199 /* FW should be enabled state to proceed */
200 if (!(readl_relaxed(en_base) & 0x1)) {
201 dev_err(dev, "%s firmware not enabled\n", np->name);
202 return -ENODEV;
203 }
204
205 devm_iounmap(&pdev->dev, en_base);
206
207 index = of_property_match_string(np, "reg-names", "perf_base");
208 if (index < 0)
209 return index;
210
211 if (of_address_to_resource(np, index, &res))
212 return -ENOMEM;
213
214 c->perf_base = devm_ioremap(dev, res.start, resource_size(&res));
215 if (!c->perf_base) {
216 dev_err(dev, "Unable to map %s perf-base\n", np->name);
217 return -ENOMEM;
218 }
219
220 index = of_property_match_string(np, "reg-names", "lut_base");
221 if (index < 0)
222 return index;
223
224 if (of_address_to_resource(np, index, &res))
225 return -ENOMEM;
226
227 c->lut_base = devm_ioremap(dev, res.start, resource_size(&res));
228 if (!c->lut_base) {
229 dev_err(dev, "Unable to map %s lut-base\n", np->name);
230 return -ENOMEM;
231 }
232
233 ret = qcom_get_related_cpus(np, &c->related_cpus);
234 if (ret) {
235 dev_err(dev, "%s failed to get core phandles\n", np->name);
236 return ret;
237 }
238
239 c->max_cores = cpumask_weight(&c->related_cpus);
240
241 ret = qcom_read_lut(pdev, c);
242 if (ret) {
243 dev_err(dev, "%s failed to read LUT\n", np->name);
244 return ret;
245 }
246
247 for_each_cpu(cpu, &c->related_cpus)
248 qcom_freq_domain_map[cpu] = c;
249
250 return 0;
251 }
252

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (10.80 kB)
.config.gz (63.69 kB)
Download all attachments