cpufreq: qcom-hw: Add support for QCOM cpufreq HW

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0 -0

new file mode 100644
index 0000000..2b82965
@@ -0,0 +1,172 @@

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +8 -0

+       Usage:          required.
+       Definition:     Number of cells in a freqency domain specifier.
+
+* 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.
+
+

Quoting Taniya Das (2018-12-12 23:49:53)

Quoted 6 lines

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +0 -0

new file mode 100644
index 0000000..2b82965
@@ -0,0 +1,172 @@

[...]

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt line 34

Quoted 7 lines

+- reg-names

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +6 -0

+       Usage:          Optional
+       Value type:     <string>
+       Definition:     Frequency domain name i.e.
+                       "freq-domain0", "freq-domain1".
+
+- freq-domain-cells:

Should be #freq-domain-cells? Or maybe #qcom,freq-domain-cells?

Quoted 8 lines

Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt new file +8 -0

+       Usage:          required.
+       Definition:     Number of cells in a freqency domain specifier.
+
+* 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.
+
+

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 Stephen Boyd <[email protected]>

Signed-off-by Taniya Das <[email protected]>

---
 drivers/cpufreq/Kconfig.arm       |  11 ++
 drivers/cpufreq/Makefile          |   1 +
 drivers/cpufreq/qcom-cpufreq-hw.c | 305 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 317 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c

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 Stephen Boyd <[email protected]>

Signed-off-by Taniya Das <[email protected]>

---

BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error upon bringing the policy online the second time. I guess cpufreq_stats aren't able to be freed from there because they take locks in different order vs. the normal path?

Quoted 4 lines

BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error upon bringing the policy online the second time. I guess cpufreq_stats aren't able to be freed from there because they take locks in different order vs. the normal path?

Please share the lockdep report and the steps to reproduce it. I will see if I can simulate the failure forcefully..

It's on a v4.19 kernel with this cpufreq hw driver backported to it. I think all it takes is to return an error the second time the policy is initialized when cpufreq_online() calls into the cpufreq driver.

Quoted 3 lines

It's on a v4.19 kernel with this cpufreq hw driver backported to it. I think all it takes is to return an error the second time the policy is initialized when cpufreq_online() calls into the cpufreq driver.

What do you mean by "the second time the policy is initialized" ?

We call cpufreq_online() only once for each policy.

I have one policy for four CPUs. So take down all four of those CPUs by writing a 0 to the online file for each CPU, and then bring them back online. That should make cpufreq_driver->init() be called twice, once during boot when the CPUs are bound to the cpufreq devices, and second from the sysfs write when the user brings the first CPU in that policy online again by writing a 1 to the online file. If that second time it fails I suspect we hit the lockdep warning.

Quoting Viresh Kumar (2018-12-13 02:05:06)

Quoted 9 lines · Stephen Boyd

Quoted 4 lines

BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error upon bringing the policy online the second time. I guess cpufreq_stats aren't able to be freed from there because they take locks in different order vs. the normal path?

Please share the lockdep report and the steps to reproduce it. I will see if I can simulate the failure forcefully..

It's on a v4.19 kernel with this cpufreq hw driver backported to it. I think all it takes is to return an error the second time the policy is initialized when cpufreq_online() calls into the cpufreq driver.

======================================================
WARNING: possible circular locking dependency detected
4.19.8 #61 Tainted: G W
------------------------------------------------------
cpuhp/5/36 is trying to acquire lock:
000000003e901e8a (kn->count#326){++++}, at: kernfs_remove_by_name_ns+0x44/0x80

but task is already holding lock:
00000000dd7f52c3 (&policy->rwsem){++++}, at: cpufreq_policy_free+0x17c/0x1cc

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&policy->rwsem){++++}:

        down_read+0x50/0xcc
        show+0x30/0x78
        sysfs_kf_seq_show+0x17c/0x25c
        kernfs_seq_show+0xb4/0xf8
        seq_read+0x4a8/0x8f0
        kernfs_fop_read+0xe0/0x360
        __vfs_read+0x80/0x328
        vfs_read+0xd0/0x1d4
        ksys_read+0x88/0x118
        __arm64_sys_read+0x4c/0x5c
        el0_svc_common+0x124/0x1c4
        el0_svc_compat_handler+0x64/0x8c
        el0_svc_compat+0x8/0x18

Hi Stephen,

Quoted 31 lines · Stephen Boyd

Quoting Viresh Kumar (2018-12-13 02:05:06)

Quoted 9 lines · Stephen Boyd

Quoted 4 lines

BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error upon bringing the policy online the second time. I guess cpufreq_stats aren't able to be freed from there because they take locks in different order vs. the normal path?

Please share the lockdep report and the steps to reproduce it. I will see if I can simulate the failure forcefully..

It's on a v4.19 kernel with this cpufreq hw driver backported to it. I think all it takes is to return an error the second time the policy is initialized when cpufreq_online() calls into the cpufreq driver.

======================================================
WARNING: possible circular locking dependency detected
4.19.8 #61 Tainted: G W
------------------------------------------------------
cpuhp/5/36 is trying to acquire lock:
000000003e901e8a (kn->count#326){++++}, at: kernfs_remove_by_name_ns+0x44/0x80

but task is already holding lock:
00000000dd7f52c3 (&policy->rwsem){++++}, at: cpufreq_policy_free+0x17c/0x1cc

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (&policy->rwsem){++++}:

Quoted 13 lines

        down_read+0x50/0xcc
        show+0x30/0x78
        sysfs_kf_seq_show+0x17c/0x25c
        kernfs_seq_show+0xb4/0xf8
        seq_read+0x4a8/0x8f0
        kernfs_fop_read+0xe0/0x360
        __vfs_read+0x80/0x328
        vfs_read+0xd0/0x1d4
        ksys_read+0x88/0x118
        __arm64_sys_read+0x4c/0x5c
        el0_svc_common+0x124/0x1c4
        el0_svc_compat_handler+0x64/0x8c
        el0_svc_compat+0x8/0x18

I failed to reproduce it over linux/next.

I had the following changes over linux/next:
https://pastebin.ubuntu.com/p/zkVm77PGdY/

I also did savedefconfig to show what all I changed in it. I faked multiple clusters on my hikey960 board, which is not big little..

And here is the command list from history that I ran after boot.

  501  grep . /sys/devices/system/cpu/cpufreq/*/*
  502  grep . /sys/devices/system/cpu/cpufreq/*/*/*
  503  grep . /sys/devices/system/cpu/cpufreq/*/*/*
  504  grep . /sys/devices/system/cpu/cpufreq/*/*/*
  505  grep . /sys/devices/system/cpu/cpufreq/*/*/*
  506  grep . /sys/devices/system/cpu/cpufreq/*/*
  507  grep . /sys/devices/system/cpu/cpufreq/*/*
  508  echo 0 > /sys/devices/system/cpu/cpu4/online 
  509  echo 0 > /sys/devices/system/cpu/cpu5/online 
  510  echo 0 > /sys/devices/system/cpu/cpu6/online 

  511  echo 0 > /sys/devices/system/cpu/cpu7/online 
  512  grep . /sys/devices/system/cpu/cpufreq/*/*
  513  grep . /sys/devices/system/cpu/cpufreq/*/*/*
  514  grep . /sys/devices/system/cpu/cpufreq/*/*
  515  echo 1 > /sys/devices/system/cpu/cpu4/online 
  516  grep . /sys/devices/system/cpu/cpufreq/*/*
  517  grep . /sys/devices/system/cpu/cpufreq/*/*/*
  518  dmesg 

I don't see any failure returned from cpufreq_dt's cpufreq_init() function. Maybe put a static int counter = 0 and then fail cpufreq_init() the second time that it's called for the same policy pointer? I have a system with two policies, so I made it fail and return -EINVAL when the counter == 2 and I see the lockdep splat.

I did the following:

grep . /sys/devices/system/cpu/cpufreq/*/* >/dev/null echo 0 > /sys/devices/system/cpu/cpu4/online echo 0 > /sys/devices/system/cpu/cpu5/online echo 0 > /sys/devices/system/cpu/cpu6/online echo 0 > /sys/devices/system/cpu/cpu7/online echo 1 > /sys/devices/system/cpu/cpu4/online dmesg

And boom, lockdep splat.

Quoting Taniya Das (2018-12-12 23:49:54)

Quoted 8 lines

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 Stephen Boyd <[email protected]>

Signed-off-by Taniya Das <[email protected]>

---

Reviewed-by Stephen Boyd <[email protected]>

But I noticed that we don't release the I/O region anymore so hotplug and replug of a whole clk domain fails. I guess devm_ioremap_resource() was just too much magic so how about we downgrade to devm_ioremap() instead?

BTW, Viresh, I see a lockdep splat when cpufreq_init returns an error upon bringing the policy online the second time. I guess cpufreq_stats aren't able to be freed from there because they take locks in different order vs. the normal path?

-----8<-------

drivers/cpufreq/qcom-cpufreq-hw.c +0 -0

index fce7a1162e87..0e1105151478 100644
@@ -182,9 +182,12 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
  	index = args.args[0];
 
  	res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);

drivers/cpufreq/qcom-cpufreq-hw.c +0 -0

  	/* HW should be in enabled state to proceed */
  	if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {

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 Stephen Boyd <[email protected]>

Signed-off-by Taniya Das <[email protected]>

---
 drivers/cpufreq/Kconfig.arm       |  11 ++
 drivers/cpufreq/Makefile          |   1 +
 drivers/cpufreq/qcom-cpufreq-hw.c | 305 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 317 insertions(+)
 create mode 100644 drivers/cpufreq/qcom-cpufreq-hw.c

drivers/cpufreq/Kconfig.arm +11 -0

index 4e1131e..688f102 100644
@@ -114,6 +114,17 @@ config ARM_QCOM_CPUFREQ_KRYO
 
 	  If in doubt, say N.
 
+config ARM_QCOM_CPUFREQ_HW
+	tristate "QCOM CPUFreq HW driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	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.
+
 config ARM_S3C_CPUFREQ
 	bool
 	help

drivers/cpufreq/Makefile +1 -0

index d5ee456..08c071b 100644
@@ -61,6 +61,7 @@ obj-$(CONFIG_MACH_MVEBU_V7)		+= mvebu-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)	+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)	+= pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)			+= pxa3xx-cpufreq.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW)	+= qcom-cpufreq-hw.o
 obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)	+= qcom-cpufreq-kryo.o
 obj-$(CONFIG_ARM_S3C2410_CPUFREQ)	+= s3c2410-cpufreq.o
 obj-$(CONFIG_ARM_S3C2412_CPUFREQ)	+= s3c2412-cpufreq.o

drivers/cpufreq/qcom-cpufreq-hw.c new file +295 -0

new file mode 100644
index 0000000..a65d0ae
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/bitfield.h>
+#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>
+#include <linux/slab.h>
+
+#define LUT_MAX_ENTRIES			40U
+#define LUT_SRC				GENMASK(31, 30)
+#define LUT_L_VAL			GENMASK(7, 0)
+#define LUT_CORE_COUNT			GENMASK(18, 16)
+#define LUT_ROW_SIZE			32
+#define CLK_HW_DIV			2
+
+/* Register offsets */
+#define REG_ENABLE			0x0
+#define REG_LUT_TABLE			0x110
+#define REG_PERF_STATE			0x920
+
+static unsigned long cpu_hw_rate, xo_rate;
+static struct platform_device *global_pdev;
+
+static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
+					unsigned int index)
+{
+	void __iomem *perf_state_reg = policy->driver_data;
+
+	writel_relaxed(index, perf_state_reg);
+
+	return 0;
+}
+
+static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
+{
+	void __iomem *perf_state_reg;
+	struct cpufreq_policy *policy;
+	unsigned int index;
+
+	policy = cpufreq_cpu_get_raw(cpu);
+	if (!policy)
+		return 0;
+
+	perf_state_reg = policy->driver_data;
+
+	index = readl_relaxed(perf_state_reg);
+	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)
+{
+	void __iomem *perf_state_reg = policy->driver_data;
+	int index;
+
+	index = policy->cached_resolved_idx;
+	if (index < 0)
+		return 0;
+
+	writel_relaxed(index, perf_state_reg);
+
+	return policy->freq_table[index].frequency;
+}
+
+static int qcom_cpufreq_hw_read_lut(struct device *dev,
+				    struct cpufreq_policy *policy,
+				    void __iomem *base)
+{
+	u32 data, src, lval, i, core_count, prev_cc = 0, prev_freq = 0, freq;
+	unsigned int max_cores = cpumask_weight(policy->cpus);
+	struct cpufreq_frequency_table	*table;
+
+	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		data = readl_relaxed(base + REG_LUT_TABLE + i * LUT_ROW_SIZE);
+		src = FIELD_GET(LUT_SRC, data);
+		lval = FIELD_GET(LUT_L_VAL, data);
+		core_count = FIELD_GET(LUT_CORE_COUNT, data);
+
+		if (src)
+			freq = xo_rate * lval / 1000;
+		else
+			freq = cpu_hw_rate / 1000;
+
+		/* Ignore boosts in the middle of the table */
+		if (core_count != max_cores) {
+			table[i].frequency = CPUFREQ_ENTRY_INVALID;
+		} else {
+			table[i].frequency = freq;
+			dev_dbg(dev, "index=%d freq=%d, core_count %d\n", i,
+				freq, core_count);
+		}
+
+		/*
+		 * Two of the same frequencies with the same core counts means
+		 * end of table
+		 */
+		if (i > 0 && prev_freq == freq && prev_cc == core_count) {
+			struct cpufreq_frequency_table *prev = &table[i - 1];
+
+			/*
+			 * Only treat the last frequency that might be a boost
+			 * as the boost frequency
+			 */
+			if (prev_cc != max_cores) {
+				prev->frequency = prev_freq;
+				prev->flags = CPUFREQ_BOOST_FREQ;
+			}
+
+			break;
+		}
+
+		prev_cc = core_count;
+		prev_freq = freq;
+	}
+
+	table[i].frequency = CPUFREQ_TABLE_END;
+	policy->freq_table = table;
+
+	return 0;
+}
+
+static void 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;
+
+		ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
+						 "#freq-domain-cells", 0,
+						 &args);
+		of_node_put(cpu_np);
+		if (ret < 0)
+			continue;
+
+		if (index == args.args[0])
+			cpumask_set_cpu(cpu, m);
+	}
+}
+
+static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
+{
+	struct device *dev = &global_pdev->dev;
+	struct of_phandle_args args;
+	struct device_node *cpu_np;
+	struct resource *res;
+	void __iomem *base;
+	int ret, index;
+
+	cpu_np = of_cpu_device_node_get(policy->cpu);
+	if (!cpu_np)
+		return -EINVAL;
+
+	ret = of_parse_phandle_with_args(cpu_np, "qcom,freq-domain",
+					 "#freq-domain-cells", 0, &args);
+	of_node_put(cpu_np);
+	if (ret)
+		return ret;
+
+	index = args.args[0];
+
+	res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	/* HW should be in enabled state to proceed */
+	if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {
+		dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
+		ret = -ENODEV;
+		goto error;
+	}
+
+	qcom_get_related_cpus(index, policy->cpus);
+	if (!cpumask_weight(policy->cpus)) {
+		dev_err(dev, "Domain-%d failed to get related CPUs\n", index);
+		ret = -ENOENT;
+		goto error;
+	}
+
+	policy->driver_data = base + REG_PERF_STATE;
+
+	ret = qcom_cpufreq_hw_read_lut(dev, policy, base);
+	if (ret) {
+		dev_err(dev, "Domain-%d failed to read LUT\n", index);
+		goto error;
+	}
+
+	policy->fast_switch_possible = true;
+
+	return 0;
+error:
+	devm_iounmap(dev, base);
+	return ret;
+}
+
+static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
+{
+	void __iomem *base = policy->driver_data - REG_PERF_STATE;
+
+	kfree(policy->freq_table);
+	devm_iounmap(&global_pdev->dev, base);
+
+	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,
+	.exit		= qcom_cpufreq_hw_cpu_exit,
+	.fast_switch    = qcom_cpufreq_hw_fast_switch,
+	.name		= "qcom-cpufreq-hw",
+	.attr		= qcom_cpufreq_hw_attr,
+};
+
+static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
+{
+	struct clk *clk;
+	int ret;
+
+	clk = clk_get(&pdev->dev, "xo");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	xo_rate = clk_get_rate(clk);
+	clk_put(clk);
+
+	clk = clk_get(&pdev->dev, "alternate");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	cpu_hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
+	clk_put(clk);
+
+	global_pdev = pdev;
+
+	ret = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
+	if (ret)
+		dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
+	else
+		dev_dbg(&pdev->dev, "QCOM CPUFreq HW driver initialized\n");
+
+	return ret;
+}
+
+static int qcom_cpufreq_hw_driver_remove(struct platform_device *pdev)
+{
+	return cpufreq_unregister_driver(&cpufreq_qcom_hw_driver);
+}
+
+static const struct of_device_id qcom_cpufreq_hw_match[] = {
+	{ .compatible = "qcom,cpufreq-hw" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_cpufreq_hw_match);
+
+static struct platform_driver qcom_cpufreq_hw_driver = {
+	.probe = qcom_cpufreq_hw_driver_probe,
+	.remove = qcom_cpufreq_hw_driver_remove,
+	.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);
+}