[PATCH v12 0/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW

[PATCH v12 0/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW

[Taniya Das]

 [v12]
   * Remove per-cpu domain global structure.

 [v11]
   * Updated the code logic as per Stephen.
   * Default boost enabled is removed.
   * Update the clock name to use "alternate" for GPLL0 source in code and
     Documentation binding.
   * Module description updated.
   * perf_base updated to perf_state_reg.

 [v10]
  * Update Documentation binding for cpufreq node.
  * Make the driver 'tristate' instead of 'bool' and update code.
  * Update the clock name to reflect the hardware name.
  * Remove support for varying offset.

 [v9]
  * Update the Documentation binding for freq-domain-cells & cpufreq node.
  * Address comments in Kconfig.arm & Makefile.
  * Remove include file & MODULE_DESCRIPTION not required.
  * Fix the code for 'of_node_put(cpu_np)'.

 [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           | 172 ++++++++++++
 drivers/cpufreq/Kconfig.arm                        |  11 +
 drivers/cpufreq/Makefile                           |   1 +
 drivers/cpufreq/qcom-cpufreq-hw.c                  | 305 +++++++++++++++++++++
 4 files changed, 489 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.

[PATCH v12 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings

[Taniya Das]

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 <tdas@codeaurora.org>
---
 .../bindings/cpufreq/cpufreq-qcom-hw.txt           | 172 +++++++++++++++++++++
 1 file changed, 172 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..2b82965
--- /dev/null
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
@@ -0,0 +1,172 @@
+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", "alternate".
+
+- 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".
+
+- freq-domain-cells:
+	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.
+
+
+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: 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", "alternate";
+
+		#freq-domain-cells = <1>;
+	};
+}
-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the  Linux Foundation.

[PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Taniya Das]

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 <skannan@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 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

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 4e1131e..688f102 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -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
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index d5ee456..08c071b 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -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
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
new file mode 100644
index 0000000..a65d0ae
--- /dev/null
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -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);
+}
+subsys_initcall(qcom_cpufreq_hw_init);
+
+static void __exit qcom_cpufreq_hw_exit(void)
+{
+	platform_driver_unregister(&qcom_cpufreq_hw_driver);
+}
+module_exit(qcom_cpufreq_hw_exit);
+
+MODULE_DESCRIPTION("QCOM CPUFREQ HW Driver");
+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.

Re: [PATCH v12 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings

[Stephen Boyd]

Quoting Taniya Das (2018-12-12 23:49:53)
> 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..2b82965
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
> @@ -0,0 +1,172 @@
[...]
> +- reg-names
> +       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?

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

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Viresh Kumar]

On 13-12-18, 13:19, 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 <skannan@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---
>  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

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Stephen Boyd]

Quoting Taniya Das (2018-12-12 23:49:54)
> 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 <skannan@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

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<-------
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index fce7a1162e87..0e1105151478 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -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);
-	base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	if (!res)
+		return -ENODEV;
+
+	base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!base)
+		return -ENOMEM;
 
 	/* HW should be in enabled state to proceed */
 	if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Viresh Kumar]

On 13-12-18, 01:58, Stephen Boyd wrote:
> 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..

-- 
viresh

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Stephen Boyd]

Quoting Viresh Kumar (2018-12-13 02:05:06)
> On 13-12-18, 01:58, Stephen Boyd wrote:
> > 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

 -> #0 (kn->count#326){++++}:
        lock_acquire+0x244/0x360
        __kernfs_remove+0x324/0x4fc
        kernfs_remove_by_name_ns+0x44/0x80
        remove_files+0x5c/0xd8
        sysfs_remove_group+0xb4/0xec
        cpufreq_stats_free_table+0x50/0x9c
        cpufreq_policy_free+0x184/0x1cc
        cpufreq_online+0xa44/0xc4c
        cpuhp_cpufreq_online+0x1c/0x2c
        cpuhp_invoke_callback+0x608/0x1328
        cpuhp_thread_fun+0x1dc/0x440
        smpboot_thread_fn+0x46c/0x7c0
        kthread+0x248/0x260
        ret_from_fork+0x10/0x18

 other info that might help us debug this:

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(&policy->rwsem);
                                lock(kn->count#326);
                                lock(&policy->rwsem);
   lock(kn->count#326);

  *** DEADLOCK ***

 2 locks held by cpuhp/5/36:
  #0: 00000000549a28c3 (cpuhp_state-up){+.+.}, at: cpuhp_lock_acquire+0x8/0x48
  #1: 00000000dd7f52c3 (&policy->rwsem){++++}, at: cpufreq_policy_free+0x17c/0x1cc

 stack backtrace:
 CPU: 5 PID: 36 Comm: cpuhp/5 Tainted: G        W         4.19.8 #61
 Call trace:
  dump_backtrace+0x0/0x2f8
  show_stack+0x20/0x2c
  __dump_stack+0x20/0x28
  dump_stack+0xcc/0x10c
  print_circular_bug+0x2c0/0x328
  __lock_acquire+0x22b0/0x2708
  lock_acquire+0x244/0x360
  __kernfs_remove+0x324/0x4fc
  kernfs_remove_by_name_ns+0x44/0x80
  remove_files+0x5c/0xd8
  sysfs_remove_group+0xb4/0xec
  cpufreq_stats_free_table+0x50/0x9c
  cpufreq_policy_free+0x184/0x1cc
  cpufreq_online+0xa44/0xc4c
  cpuhp_cpufreq_online+0x1c/0x2c
  cpuhp_invoke_callback+0x608/0x1328
  cpuhp_thread_fun+0x1dc/0x440
  smpboot_thread_fn+0x46c/0x7c0
  kthread+0x248/0x260
  ret_from_fork+0x10/0x18

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Viresh Kumar]

On 13-12-18, 02:12, Stephen Boyd wrote:
> 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.

-- 
viresh

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Stephen Boyd]

Quoting Viresh Kumar (2018-12-13 02:14:17)
> On 13-12-18, 02:12, Stephen Boyd wrote:
> > 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.

I may have also catted the policy*/stat/* files before offlining any
CPUs. If you can't trigger it I can try to reproduce again with a static
counter in cpufreq_online() that fails when set to 1 or something.

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Viresh Kumar]

On 13-12-18, 02:32, Stephen Boyd wrote:
> 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.

Okay, that is a special sequence. I will try to reproduce that
locally. Thanks.

-- 
viresh

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Matthias Kaehlcke]

On Thu, Dec 13, 2018 at 01:19:54PM +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 <skannan@codeaurora.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---
>  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
> 
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index 4e1131e..688f102 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -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
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index d5ee456..08c071b 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -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
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> new file mode 100644
> index 0000000..a65d0ae
> --- /dev/null
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -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);
> +}
> +subsys_initcall(qcom_cpufreq_hw_init);
> +
> +static void __exit qcom_cpufreq_hw_exit(void)
> +{
> +	platform_driver_unregister(&qcom_cpufreq_hw_driver);
> +}
> +module_exit(qcom_cpufreq_hw_exit);
> +
> +MODULE_DESCRIPTION("QCOM CPUFREQ HW Driver");
> +MODULE_LICENSE("GPL v2");

FWIW:

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [PATCH v12 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ Firmware bindings

[Taniya Das]

Hello Stephen,

On 12/13/2018 1:58 PM, Stephen Boyd wrote:
> Quoting Taniya Das (2018-12-12 23:49:53)
>> 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..2b82965
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-hw.txt
>> @@ -0,0 +1,172 @@
> [...]
>> +- reg-names
>> +       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?
> 

Updated in the next series.

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

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

--

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Taniya Das]

Hello Stephen, Viresh,

On 12/13/2018 3:28 PM, Stephen Boyd wrote:
> Quoting Taniya Das (2018-12-12 23:49:54)
>> 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 <skannan@codeaurora.org>
>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
>> ---
> 
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> 
> 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<-------
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index fce7a1162e87..0e1105151478 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -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);
> -	base = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(base))
> -		return PTR_ERR(base);
> +	if (!res)
> +		return -ENODEV;
> +
> +	base = devm_ioremap(dev, res->start, resource_size(res));
> +	if (!base)
> +		return -ENOMEM;
>   

Updated the above in the next series.

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

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

--

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Viresh Kumar]

Hi Stephen,

On 13-12-18, 02:12, Stephen Boyd wrote:
> Quoting Viresh Kumar (2018-12-13 02:05:06)
> > On 13-12-18, 01:58, Stephen Boyd wrote:
> > > 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

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 

-- 
viresh

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Stephen Boyd]

Quoting Viresh Kumar (2018-12-17 21:45:45)
> Hi Stephen,
> 
> On 13-12-18, 02:12, Stephen Boyd wrote:
> > Quoting Viresh Kumar (2018-12-13 02:05:06)
> > > On 13-12-18, 01:58, Stephen Boyd wrote:
> > > > 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
> 
> I failed to reproduce it over linux/next.
> 
> I had the following changes over linux/next:
> https://pastebin.ubuntu.com/p/zkVm77PGdY/

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

Re: [PATCH v12 2/2] cpufreq: qcom-hw: Add support for QCOM cpufreq HW driver

[Viresh Kumar]

On 18-12-18, 11:13, Stephen Boyd wrote:
> 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.

Yuck. You were so clear on this earlier, how can I forget it. :(

Here is the additional diff. And I do get this print message on my screen while
I try to online CPU4.

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 639feca22d27..b836e93fd87d 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -151,6 +151,11 @@ static int cpufreq_init(struct cpufreq_policy *policy)
        const char *name;
        int ret;
 
+       if (policy->count++) {
+               pr_info("%s: %d\n", __func__, __LINE__);
+               return -EINVAL;
+       }
+
        cpu_dev = get_cpu_device(policy->cpu);
        if (!cpu_dev) {
                pr_err("failed to get cpu%d device\n", policy->cpu);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 882a9b9e34bc..643141a2013f 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -151,6 +151,7 @@ struct cpufreq_policy {
 
        /* For cpufreq driver's internal use */
        void                    *driver_data;
+       int                     count;
 };

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

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


But still no lockdep :(

Can you try that on mainline as well ?

-- 
viresh